1213711 - Conditional jump or move depends on uninitialised value(s) in nsDisplayTransform::GetBounds

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: full valgrind error

While running reftests under a recent svn trunk build of valgrind on Fedora 22 I found one instance of Conditional jump or move depends on uninitialised value(s)
at nsDisplayTransform::GetBounds(nsDisplayListBuilder*, bool*)

vncserver :1

vncviewer :1

(DISPLAY=:1.0 make -C firefox-debug reftest EXTRA_TEST_ARGS='--timeout=86400   --debugger=/usr/local/bin/valgrind --debugger-args=" --fair-sched=yes --smc-check=all-non-file --soname-synonyms=somalloc=NONE --vex-iropt-register-updates=allregs-at-mem-access --partial-loads-ok=yes --error-limit=no --trace-children=yes --child-silent-after-fork=yes --trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep,*/bin/java --tool=memcheck --stats=yes --read-inline-info=yes --fullpath-after=/mozilla/builds/nightly/mozilla"' ) 2>&1 | tee logfile

Comment 1

[Nicholas Nethercote [inactive]]

Bob, if you still have the logfile can you determine which reftest triggered the error? Even narrowing it down to a directory would be useful.

If you can re-run with Valgrind's --track-origins=yes option that would be also useful. It'll run slower but give more information in the error report.

Thank you.

Comment 2

[Bob Clary [:bc] (inactive)]

It is in the attachment

REFTEST TEST-START | file:///mozilla/builds/nightly/mozilla/layout/reftests/bugs/1081185-1.html
REFTEST TEST-LOAD | file:///mozilla/builds/nightly/mozilla/layout/reftests/bugs/1081185-1.html 

I'll see about re-running a more limited set.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What revision of mozilla-central were you testing against?  nsDisplayList.cpp changes pretty frequently, and it would be useful to know what's on line 5562.

Comment 4

[Bob Clary [:bc] (inactive)]

http://hg.mozilla.org/mozilla-central/rev/2387ada86428

  if (mFrame->Combines3DTransformWithAncestors()) {
==>    if (!mFrame->Extend3DContext() &&
        !aBuilder->GetAccumulatedRectLevels()) {
      // For preserve-3d, only leaf frames and frames start
      // preserve-3d chain have non-empty bounds.
      return rect;
    }

Comment 5

[Bob Clary [:bc] (inactive)]

(In reply to Bob Clary [:bc:] from comment #4)

I pulled the revision from the wrong log. It should have been http://hg.mozilla.org/mozilla-central/rev/23b7f289df92 pulled at 2015-10-09 06:08:01 -0700

I'm rebuilding now and will give complete answers shortly.

Comment 6

[Bob Clary [:bc] (inactive)]

Attachment: logfile.2015-10-15-2.txt

http://hg.mozilla.org/mozilla-central/rev/e193b4da0a8c1025aa76a403c64663ff1cd41709

TEST_PATH=layout/reftests/bugs/1081185-1.html
(DISPLAY=:1.0 make -C firefox-debug reftest EXTRA_TEST_ARGS=' --suite=reftest --timeout=86400   --debugger=/usr/local/bin/valgrind --debugger-args=" --track-origins=yes --fair-sched=yes --smc-check=all-non-file --soname-synonyms=somalloc=NONE --vex-iropt-register-updates=allregs-at-mem-access --partial-loads-ok=yes --error-limit=no --trace-children=yes --child-silent-after-fork=yes --trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep,*/bin/java --tool=memcheck --stats=yes --read-inline-info=yes --fullpath-after=/mozilla/builds/nightly/mozilla"' ) 2>&1 | tee logfile.2015-10-15-2.txt

==25350== Conditional jump or move depends on uninitialised value(s)
==25350==    at 0x9B83E00: nsDisplayTransform::GetBounds(nsDisplayListBuilder*, bool*) (/layout/base/nsDisplayList.cpp:5573)

  if (mFrame->Combines3DTransformWithAncestors()) {
==>    if (!mFrame->Extend3DContext() &&
        !aBuilder->GetAccumulatedRectLevels()) {
      // For preserve-3d, only leaf frames and frames start
      // preserve-3d chain have non-empty bounds.
      return rect;
    }

==25350== Conditional jump or move depends on uninitialised value(s)
==25350==    at 0xAE598A6: js::Fprinter::~Fprinter() (/js/src/vm/Printer.cpp:399)

Fprinter::~Fprinter()
{
==>    MOZ_ASSERT_IF(init_, !file_);
}

Comment 7

[Nicholas Nethercote [inactive]]

>   if (mFrame->Combines3DTransformWithAncestors()) {
> ==>    if (!mFrame->Extend3DContext() &&
>         !aBuilder->GetAccumulatedRectLevels()) {

I did some digging with the VALGRIND_CHECK_VALUE_IS_DEFINED macro from <valgrind/memcheck.h> and determined that it's the aBuilder->GetAccumulatedRectLevels() call that's the problem.

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: Initialize Preserves3DContext::mAccumulatedRectLevels

This patch is enough to satisfy Valgrind.

Comment 9

[Thinker Li [:sinker]]

Comment on attachment 8674705 [details] [diff] [review]
Initialize Preserves3DContext::mAccumulatedRectLevels

Thanks!

Comment 10

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/732305d424f42a06d3a31746bbf379bcfef08e95
Bug 1213711 - Initialize Preserves3DContext::mAccumulatedRectLevels. r=thinker.

Comment 11

[Nicholas Nethercote [inactive]]

I think this doesn't need to be security sensitive. The code that introduced the problem (bug 1097464) hasn't made it to any other channels and I think this will only cause visual problems.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/732305d424f4

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/732305d424f4

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Backed out from Aurora44 so that bug 1097464 could be backed out cleanly. It remains fixed on Fx45+ where bug 1097464 is still landed.

https://hg.mozilla.org/releases/mozilla-aurora/rev/59717b623c8f

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/59717b623c8f

Comment 16

[Ryan VanderMeulen [:RyanVM]]

This along with the change that caused this regression were backed out from Firefox 45.

https://hg.mozilla.org/releases/mozilla-aurora/rev/64ec448f156d