817853 - Turn on --track-origins=yes for Valgrind runs on TBPL

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

There are a bunch of recent (since end-Nov) Valgrind conditional jump and uninitialized value errors in the Valgrind TBPL runs lately. See:

https://tbpl.mozilla.org/php/getParsedLog.php?id=17552900&tree=Firefox&full=1

Let's turn on --track-origins=yes for a day or two to get more complete logs and back it out later.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: patch

Comment 2

[Rail Aliiev [:rail]]

Assigning to Gary, since he has a patch.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

https://hg.mozilla.org/build/tools/rev/bcc9d17b7d60

Comment 4

[Julian Seward [:jseward]]

I get the relevant errors on any startup of Fx, x86_64-linux, gcc
4.7.2, -g -O2.

Is there a bug filed anywhere for them?

The origin is as listed below, but looking at the source, that doesn't
make much sense, so maybe something else is going on.

Thread 21:
Conditional jump or move depends on uninitialised value(s)
   at 0x79A522B: MarkRangeConservatively(JSTracer*, unsigned long const*, unsigned long const*) (RootMarking.cpp:145)
   by 0x79A551B: MarkConservativeStackRoots(JSTracer*, bool) [clone .isra.61] (RootMarking.cpp:294)
   by 0x79A5C70: js::gc::MarkRuntime(JSTracer*, bool) (RootMarking.cpp:683)
   by 0x7832AEC: IncrementalCollectSlice(JSRuntime*, long, js::gcreason::Reason, js::JSGCInvocationKind) (jsgc.cpp:2650)
   by 0x7833FD4: GCCycle(JSRuntime*, bool, long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:4007)
   by 0x7834256: Collect(JSRuntime*, bool, long, js::JSGCInvocationKind, js::gcreason::Reason) (jsgc.cpp:4125)
   by 0x6A40F59: mozilla::dom::workers::WorkerPrivate::GarbageCollectInternal(JSContext*, bool, bool) (WorkerPrivate.cpp:4008)
   by 0x6A40FC6: (anonymous namespace)::GarbageCollectRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (WorkerPrivate.cpp:1388)
   by 0x6A3FB17: mozilla::dom::workers::WorkerRunnable::Run() (WorkerPrivate.cpp:1800)
   by 0x6A457E8: mozilla::dom::workers::WorkerPrivate::DoRunLoop(JSContext*) (WorkerPrivate.cpp:2785)
   by 0x6A37B37: (anonymous namespace)::WorkerThreadRunnable::Run() (RuntimeService.cpp:470)
   by 0x7211C10: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:627)
 Uninitialised value was created by a stack allocation
   at 0x6A40FB0: (anonymous namespace)::GarbageCollectRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (WorkerPrivate.cpp:1386)

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Julian Seward from comment #4)
> I get the relevant errors on any startup of Fx, x86_64-linux, gcc
> 4.7.2, -g -O2.
> 
> Is there a bug filed anywhere for them?

I saw those as well, but am now waiting for (possibly-related) bug 817002 to land first and will see the Valgrind results after that.

If it still occurs, then let's file the bug.

Comment 6

[Julian Seward [:jseward]]

> I saw those as well, but am now waiting for (possibly-related) bug 817002 to
> land first and will see the Valgrind results after that.

I just tried the fix for 817002 -- IIUC, it's just a one-liner.  AFAICS it
does not fix the problem, although it might (not sure) reduce the amount
of complaining, which might imply that there's more than one problem.

Seems like the GC is a bit unhappy at the moment.  I just started a new
mochitests-valgrind run, but there is so much noise now that I suspect it
is pretty pointless to let it continue.

Comment 7

[Julian Seward [:jseward]]

> Seems like the GC is a bit unhappy at the moment.

I think this is fallout from the recent GC split-up.  See 
bug 815931 #16 for a fix.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Thanks to the fallout fixes for Valgrind, the latest TBPL Valgrind runs are now green:

https://tbpl.mozilla.org/?noignore=1&jobname=valgrind&rev=87f8165c5a0b

I'm proposing to leave --track-origins=yes on for the moment. There seems to only be a 10-15min time penalty while we're only running PGO tests on Valgrind TBPL runs.

We can leave it as it is and not remove it until it makes running other tests on tbpl unbearably long, when we do get around to adding it.