797002 - Uninitialised value use in nsLayoutUtils::InflationMinFontSizeFor

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Julian Seward [:jseward]]

When loading

http://edition.cnn.com/2012/10/01/travel/american-airlines-customers-complaining/index.html?hpt=us_t3

Various variants of:

Conditional jump or move depends on uninitialised value(s)
   at 0x48799E8: floorf (/home/sewardj/AND4-xxray/bionic/libm/src/s_floorf.c:41)
   by 0x2FFC62A7: nsLayoutUtils::InflationMinFontSizeFor(nsIFrame const*) (ff-opt/layout/base/../../dist/include/nsCoord.h:64)
   by 0x2FFC62DF: nsLayoutUtils::FontSizeInflationFor(nsIFrame const*) (layout/base/nsLayoutUtils.cpp:4935)
   by 0x30021F87: nsCSSOffsetState::ComputeMargin(int) (layout/generic/nsHTMLReflowState.cpp:103)
   by 0x3002266B: nsCSSOffsetState::InitOffsets(int, nsIAtom*, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:2127)
   by 0x30023DB9: nsHTMLReflowState::InitConstraints(nsPresContext*, int, int, nsMargin const*, nsMargin const*, nsIAtom*) (layout/generic/nsHTMLReflowState.cpp:1889)
   by 0x300243B7: nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:316)
   by 0x30024781: nsHTMLReflowState::nsHTMLReflowState(nsPresContext*, nsHTMLReflowState const&, nsIFrame*, nsSize const&, int, int, bool) (layout/generic/nsHTMLReflowState.cpp:187)
   by 0x30006581: nsFontInflationData::UpdateWidth(nsHTMLReflowState const&) (layout/generic/nsFontInflationData.cpp:165)
   by 0x30006697: nsFontInflationData::UpdateFontInflationDataWidthFor(nsHTMLReflowState const&) (layout/generic/nsFontInflationData.cpp:62)
   by 0x300236F5: nsHTMLReflowState::InitResizeFlags(nsPresContext*, nsIAtom*) (layout/generic/nsHTMLReflowState.cpp:442)
   by 0x300243C1: nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:318)
   by 0x30024839: nsHTMLReflowState::nsHTMLReflowState(nsPresContext*, nsIFrame*, nsRenderingContext*, nsSize const&, unsigned int) (layout/generic/nsHTMLReflowState.cpp:79)
   by 0x2FFDAADD: PresShell::DoReflow(nsIFrame*, bool) (layout/base/nsPresShell.cpp:7386)
   by 0x2FFDB0B3: PresShell::ProcessReflowCommands(bool) (layout/base/nsPresShell.cpp:7570)
   by 0x2FFDE095: PresShell::FlushPendingNotifications(mozFlushType) (layout/base/nsPresShell.cpp:3872)
   by 0x2FFDF5B9: nsRefreshDriver::Notify(nsITimer*) (layout/base/nsRefreshDriver.cpp:403)
   by 0x308E114B: nsTimerImpl::Fire() (xpcom/threads/nsTimerImpl.cpp:476)
   by 0x308E12FB: nsTimerEvent::Run() (xpcom/threads/nsTimerImpl.cpp:556)
   by 0x308DEB67: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:612)


and a bit later, various variants of:

Conditional jump or move depends on uninitialised value(s)
   at 0x2FFC3D72: nsLayoutUtils::FontSizeInflationInner(nsIFrame const*, int) (layout/base/nsLayoutUtils.cpp:4859)
   by 0x2FFC62E7: nsLayoutUtils::FontSizeInflationFor(nsIFrame const*) (layout/base/nsLayoutUtils.cpp:4935)
   by 0x30021F87: nsCSSOffsetState::ComputeMargin(int) (layout/generic/nsHTMLReflowState.cpp:103)
   by 0x3002266B: nsCSSOffsetState::InitOffsets(int, nsIAtom*, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:2127)
   by 0x30023DB9: nsHTMLReflowState::InitConstraints(nsPresContext*, int, int, nsMargin const*, nsMargin const*, nsIAtom*) (layout/generic/nsHTMLReflowState.cpp:1889)
   by 0x300243B7: nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:316)
   by 0x30024781: nsHTMLReflowState::nsHTMLReflowState(nsPresContext*, nsHTMLReflowState const&, nsIFrame*, nsSize const&, int, int, bool) (layout/generic/nsHTMLReflowState.cpp:187)
   by 0x30006581: nsFontInflationData::UpdateWidth(nsHTMLReflowState const&) (layout/generic/nsFontInflationData.cpp:165)
   by 0x30006697: nsFontInflationData::UpdateFontInflationDataWidthFor(nsHTMLReflowState const&) (layout/generic/nsFontInflationData.cpp:62)
   by 0x300236F5: nsHTMLReflowState::InitResizeFlags(nsPresContext*, nsIAtom*) (layout/generic/nsHTMLReflowState.cpp:442)
   by 0x300243C1: nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:318)
   by 0x30024839: nsHTMLReflowState::nsHTMLReflowState(nsPresContext*, nsIFrame*, nsRenderingContext*, nsSize const&, unsigned int) (layout/generic/nsHTMLReflowState.cpp:79)
   by 0x2FFDAADD: PresShell::DoReflow(nsIFrame*, bool) (layout/base/nsPresShell.cpp:7386)
   by 0x2FFDB0B3: PresShell::ProcessReflowCommands(bool) (layout/base/nsPresShell.cpp:7570)
   by 0x2FFDE095: PresShell::FlushPendingNotifications(mozFlushType) (layout/base/nsPresShell.cpp:3872)
   by 0x2FFDF5B9: nsRefreshDriver::Notify(nsITimer*) (layout/base/nsRefreshDriver.cpp:403)
   by 0x308E114B: nsTimerImpl::Fire() (xpcom/threads/nsTimerImpl.cpp:476)
   by 0x308E12FB: nsTimerEvent::Run() (xpcom/threads/nsTimerImpl.cpp:556)
   by 0x308DEB67: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:612)
   by 0x308B4F45: NS_ProcessNextEvent_P(nsIThread*, bool) (ff-opt/xpcom/build/nsThreadUtils.cpp:220)

Comment 1

[Joe Drew (not getting mail)]

Font inflation, hooray

Comment 3

[Julian Seward [:jseward]]

I'm not sure I believe that (the dup marking), because this is a
build from m-c of about 8 hours ago, and bug 795313's fixed claims
to have been pushed to m-c on 28th Sept.

Comment 4

[Scott Johnson (:jwir3)]

(In reply to Julian Seward from comment #3)
> I'm not sure I believe that (the dup marking), because this is a
> build from m-c of about 8 hours ago, and bug 795313's fixed claims
> to have been pushed to m-c on 28th Sept.

Ok, I'll reopen it if you think that it's still a bug. Could you double-check the "Built from" line in your about:buildconfig on this device? (Just to be sure we're talking about a build from later than bug 795313).

Comment 5

[Scott Johnson (:jwir3)]

Ah, ok. I think I might see what's happening here. InitConstraints(), where this is happening, is called just before InitResizeFlags() in nsHTMLReflowState::Init(). InitResizeFlags() is where the UpdateFontInflationDataWidthFor() is calculated, which is what initializes the mNCAWidth property.

Yeah, this probably different than bug 795313.

Comment 6

[Joe Drew (not getting mail)]

Scott, is this something that can be fixed easily/quickly?

Comment 7

[Scott Johnson (:jwir3)]

(In reply to Joe Drew (:JOEDREW! \o/) from comment #6)
> Scott, is this something that can be fixed easily/quickly?

Possibly. I think I know what's happening, but one of the blockers is that I can't reproduce this issue. I meant to post a patch for Julian to test, but then I got sidetracked. I'll see if I can get this done today.

Comment 8

[Scott Johnson (:jwir3)]

Attachment: b797002

This is a patch that should fix the issue, albeit in a workaround sort-of way. It simply makes the nsFontInflationData structure have a flag that indicates whether or not it's had its width calculated for the first time.

Julian, if you could test this patch, I'd greatly appreciate it.

Comment 9

[Julian Seward [:jseward]]

(In reply to Scott Johnson (:jwir3) from comment #8)
> Julian, if you could test this patch, I'd greatly appreciate it.

The patch doesn't appear to fix the complaints -- they are still there
for cnn.com.  

I noticed two things.  Firstly, I never got you a stack trace that
contains origin data for the uninitialised values, which makes your
analysis unnecessarily difficult.

Secondly, I discovered recently that Valgrind/Memcheck can sometimes
be confused by optimised Thumb code and incorrectly report
uninitialised value errors on correct code.  I will make
investigations to see whether that is indeed the case.

It would be useful to know (w.r.t. "Secondly" above) whether you could
see a likely scenario where undefined values got used, or whether you
couldn't, and hence the patch is mostly defensive.

Comment 10

[Julian Seward [:jseward]]

Here's a representative of one of the two groups of errors messages I
see, that I think are related here.  After some analysis of the
machine code involved I believe this is more likely than not, a real
error.  This time it is with origin data.  Note that this is with 
the patch from comment 8 in place.

Conditional jump or move depends on uninitialised value(s)
   at 0x48799E8: floorf (/home/sewardj/AND4-xxray/bionic/libm/src/s_floorf.c:41)
   by 0x34FA933F: nsLayoutUtils::InflationMinFontSizeFor(nsIFrame const*) (ff-opt/layout/base/../../dist/include/nsCoord.h:64)
   by 0x34FA9373: nsLayoutUtils::FontSizeInflationFor(nsIFrame const*) (layout/base/nsLayoutUtils.cpp:5085)
   by 0x34FFFE1D: nsCSSOffsetState::ComputeMargin(int) (layout/generic/nsHTMLReflowState.cpp:103)
   by 0x35000029: nsCSSOffsetState::InitOffsets(int, nsIAtom*, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:2127)
   by 0x350008B3: nsHTMLReflowState::InitConstraints(nsPresContext*, int, int, nsMargin const*, nsMargin const*, nsIAtom*) (layout/generic/nsHTMLReflowState.cpp:1842)
   by 0x35001109: nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:316)
   by 0x350015C1: nsHTMLReflowState::nsHTMLReflowState(nsPresContext*, nsIFrame*, nsRenderingContext*, nsSize const&, unsigned int) (layout/generic/nsHTMLReflowState.cpp
:79)
   by 0x34FBA929: PresShell::DoReflow(nsIFrame*, bool) (layout/base/nsPresShell.cpp:7385)
   by 0x34FBBDFF: PresShell::ProcessReflowCommands(bool) (layout/base/nsPresShell.cpp:7564)
   by 0x34FBC17B: PresShell::FlushPendingNotifications(mozFlushType) (layout/base/nsPresShell.cpp:3867)
   by 0x34FC093B: nsRefreshDriver::Notify(nsITimer*) (layout/base/nsRefreshDriver.cpp:403)
   by 0x35878051: nsTimerImpl::Fire() (xpcom/threads/nsTimerImpl.cpp:475)
   by 0x358781DF: nsTimerEvent::Run() (xpcom/threads/nsTimerImpl.cpp:555)
   by 0x35875693: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:612)
   by 0x3584E637: NS_ProcessNextEvent_P(nsIThread*, bool) (ff-opt/xpcom/build/nsThreadUtils.cpp:220)

 Uninitialised value was created by a heap allocation
   at 0x480687C: malloc (/home/sewardj/Vg38BRANCH/branch38droid2/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
   by 0x2292071F: moz_xmalloc (memory/mozalloc/mozalloc.cpp:57)
   by 0x34FE5551: nsFontInflationData::UpdateFontInflationDataWidthFor(nsHTMLReflowState const&) (ff-opt/layout/generic/../../dist/include/mozilla/mozalloc.h:200)
   by 0x35000D15: nsHTMLReflowState::InitResizeFlags(nsPresContext*, nsIAtom*) (layout/generic/nsHTMLReflowState.cpp:437)
   by 0x35001113: nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) (layout/generic/nsHTMLReflowState.cpp:318)
   by 0x350015C1: nsHTMLReflowState::nsHTMLReflowState(nsPresContext*, nsIFrame*, nsRenderingContext*, nsSize const&, unsigned int) (layout/generic/nsHTMLReflowState.cpp
:79)
   by 0x34FBA929: PresShell::DoReflow(nsIFrame*, bool) (layout/base/nsPresShell.cpp:7385)
   by 0x34FBBDFF: PresShell::ProcessReflowCommands(bool) (layout/base/nsPresShell.cpp:7564)
   by 0x34FBC17B: PresShell::FlushPendingNotifications(mozFlushType) (layout/base/nsPresShell.cpp:3867)
   by 0x34FC093B: nsRefreshDriver::Notify(nsITimer*) (layout/base/nsRefreshDriver.cpp:403)
   by 0x35878051: nsTimerImpl::Fire() (xpcom/threads/nsTimerImpl.cpp:475)
   by 0x358781DF: nsTimerEvent::Run() (xpcom/threads/nsTimerImpl.cpp:555)
   by 0x35875693: nsThread::ProcessNextEvent(bool, bool*) (xpcom/threads/nsThread.cpp:612)
   by 0x3584E637: NS_ProcessNextEvent_P(nsIThread*, bool) (ff-opt/xpcom/build/nsThreadUtils.cpp:220)
   by 0x3573ADA3: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (ipc/glue/MessagePump.cpp:82)
   by 0x3589FCE7: MessageLoop::RunInternal() (ipc/chromium/src/base/message_loop.cc:215)

Comment 11

[Scott Johnson (:jwir3)]

Julian:

Compiled and ran the current valgrind source (3.9.SVN I think) with my copy of mozilla-central, and I still can't get it to show the same errors that you see.

Perhaps someone else can reproduce and we can sanity check this? I can see where this could happen, but I don't see why it would happen on your machine and not mine...

Comment 12

[Julian Seward [:jseward]]

I think this is likely to have been fixed now.  I can't reproduce it
on desktop linux, at least.  Any objections if I close it?

Comment 13

[Julian Seward [:jseward]]

Closing.