1668489 - Intermittent hazards | unrooted 'se' of type 'js::frontend::SwitchEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2217 + other unrooted failures

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: csabou [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=317255015&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/c8AJzyP8RryGMH5HXLI1tg/runs/0/artifacts/public/logs/live_backing.log

---

[task 2020-10-01T09:45:29.436Z] LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/builds/worker/workspace/obj-haz-shell/dist/bin" PATH="/builds/worker/fetches/sixgill/usr/bin:${PATH}" XDB='/builds/worker/fetches/sixgill/usr/bin/xdb.so' SOURCE='/builds/worker/checkouts/gecko' ANALYZED_OBJDIR='/builds/worker/workspace/obj-analyzed' /builds/worker/workspace/obj-haz-shell/dist/bin/js /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/analyzeRoots.js gcFunctions.lst gcEdges.txt limitedFunctions.lst gcTypes.txt typeInfo.txt 15 15 tmp.15 > rootingHazards.15
[task 2020-10-01T09:45:29.436Z] cat rootingHazards.1 rootingHazards.2 rootingHazards.3 rootingHazards.4 rootingHazards.5 rootingHazards.6 rootingHazards.7 rootingHazards.8 rootingHazards.9 rootingHazards.10 rootingHazards.11 rootingHazards.12 rootingHazards.13 rootingHazards.14 rootingHazards.15 > rootingHazards.txt
[task 2020-10-01T09:45:29.436Z] Running explain to generate ('hazards.txt', 'unnecessary.txt', 'refs.txt')
[task 2020-10-01T09:45:29.436Z] LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/builds/worker/workspace/obj-haz-shell/dist/bin" PATH="/builds/worker/fetches/sixgill/usr/bin:${PATH}" XDB='/builds/worker/fetches/sixgill/usr/bin/xdb.so' SOURCE='/builds/worker/checkouts/gecko' ANALYZED_OBJDIR='/builds/worker/workspace/obj-analyzed' python2.7 /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/explain.py rootingHazards.txt gcFunctions.txt hazards.txt unnecessary.txt refs.txt
[task 2020-10-01T09:45:29.571Z] Wrote explained_hazards.tmp
[task 2020-10-01T09:45:29.571Z] Wrote unnecessary.tmp
[task 2020-10-01T09:45:29.571Z] Wrote refs.tmp
[task 2020-10-01T09:45:29.571Z] Found 214 hazards 137 unsafe references 0 missing
[task 2020-10-01T09:45:29.572Z] Running heapwrites to generate heapWriteHazards.txt
[task 2020-10-01T09:45:29.572Z] LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/builds/worker/workspace/obj-haz-shell/dist/bin" PATH="/builds/worker/fetches/sixgill/usr/bin:${PATH}" XDB='/builds/worker/fetches/sixgill/usr/bin/xdb.so' SOURCE='/builds/worker/checkouts/gecko' ANALYZED_OBJDIR='/builds/worker/workspace/obj-analyzed' /builds/worker/workspace/obj-haz-shell/dist/bin/js /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/analyzeHeapWrites.js > heapWriteHazards.txt
[task 2020-10-01T09:45:33.115Z] + check_hazards /builds/worker/workspace/analysis
[task 2020-10-01T09:45:33.115Z] + set +e
[task 2020-10-01T09:45:33.115Z] ++ grep -c 'Function.*has unrooted.*live across GC call' /builds/worker/workspace/analysis/rootingHazards.txt
[task 2020-10-01T09:45:33.117Z] + NUM_HAZARDS=214
[task 2020-10-01T09:45:33.117Z] ++ grep -c '^Function.*takes unsafe address of unrooted' /builds/worker/workspace/analysis/refs.txt
[task 2020-10-01T09:45:33.119Z] + NUM_UNSAFE=137
[task 2020-10-01T09:45:33.119Z] ++ grep -c '^Function.* has unnecessary root' /builds/worker/workspace/analysis/unnecessary.txt
[task 2020-10-01T09:45:33.121Z] + NUM_UNNECESSARY=1050
[task 2020-10-01T09:45:33.121Z] ++ grep -c '^Dropped CFG' /builds/worker/workspace/analysis/build_xgill.log
[task 2020-10-01T09:45:33.123Z] + NUM_DROPPED=0
[task 2020-10-01T09:45:33.123Z] ++ perl -lne 'print $1 if m!found (\d+)/\d+ allowed errors!' /builds/worker/workspace/analysis/heapWriteHazards.txt
[task 2020-10-01T09:45:33.125Z] + NUM_WRITE_HAZARDS=0
[task 2020-10-01T09:45:33.125Z] ++ grep -c '^Function.*expected hazard.*but none were found' /builds/worker/workspace/analysis/rootingHazards.txt
[task 2020-10-01T09:45:33.126Z] + NUM_MISSING=0
[task 2020-10-01T09:45:33.126Z] + set +x
[task 2020-10-01T09:45:33.126Z] TinderboxPrint: rooting hazards<br/>214
[task 2020-10-01T09:45:33.126Z] TinderboxPrint: (unsafe references to unrooted GC pointers)<br/>137
[task 2020-10-01T09:45:33.126Z] TinderboxPrint: (unnecessary roots)<br/>1050
[task 2020-10-01T09:45:33.126Z] TinderboxPrint: missing expected hazards<br/>0
[task 2020-10-01T09:45:33.126Z] TinderboxPrint: heap write hazards<br/>0
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'se' of type 'js::frontend::SwitchEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2217
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'noe' of type 'js::frontend::NameOpEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2349
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'traceLog' of type 'js::frontend::AutoFrontendTraceLog' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2401
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'emitterScope' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2401
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'lexicalEmitterScope' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2428
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'traceLog' of type 'js::frontend::AutoFrontendTraceLog' live across GC call at js/src/frontend/BytecodeEmitter.cpp:2537
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifAlreadyDone' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:3519
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifDone' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:3519
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifDone:1' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:3589
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'noe' of type 'js::frontend::NameOpEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:4042
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'noe' of type 'mozilla::Maybe<js::frontend::NameOpEmitter>' live across GC call at js/src/frontend/BytecodeEmitter.cpp:4224
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'noe' of type 'mozilla::Maybe<js::frontend::NameOpEmitter>' live across GC call at js/src/frontend/BytecodeEmitter.cpp:4418
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'tryCatch' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:4729
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'lse' of type 'js::frontend::LexicalScopeEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:4929
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'emitterScope' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:4999
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifAsyncIterIsUndefined' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:5160
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'noe' of type 'js::frontend::NameOpEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:5352
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'forOf' of type 'js::frontend::ForOfEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:5414
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'noe' of type 'js::frontend::NameOpEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:5493
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'bce2' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:5776
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'loc' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:6067
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifCanSkip' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:6223
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifKind' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:6344
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifThrowMethodIsNotDefined' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:6430
[task 2020-10-01T09:45:33.128Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifReturnMethodIsDefined' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:6540
[task 2020-10-01T09:45:33.129Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'ifReturnDone' of type 'mozilla::detail::MaybeStorage::NonConstT' live across GC call at js/src/frontend/BytecodeEmitter.cpp:6573
[task 2020-10-01T09:45:33.129Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'cone' of type 'js::frontend::CallOrNewEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:7699
[task 2020-10-01T09:45:33.129Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'cone' of type 'js::frontend::CallOrNewEmitter' live across GC call at js/src/frontend/BytecodeEmitter.cpp:7826```

Comment 1

[Cosmin Sabou [:CosminS]]

Tooru, could you have a look over these hazard failures? Thank you.

Comment 2

[Cosmin Sabou [:CosminS]]

Failing on js/src/frontend/Parser.cpp: and other files: https://treeherder.mozilla.org/logviewer.html#?job_id=317257686&repo=mozilla-central

Comment 3

[Tooru Fujisawa [:arai]]

This sounds like my patch screwed up rooting analysis (it says struct TokenPos { uint32_t begin; uint32_t end; ...} needs rooting...), and I think I cannot solve the issue immediately.
can you backout bug 1667740 patches for now?

Comment 4

[Cosmin Sabou [:CosminS]]

Hopefully fixed by backout: https://hg.mozilla.org/integration/autoland/rev/2224900e3db5d978e2b2207336a6cfa412595ded

Comment 5

[Cosmin Sabou [:CosminS]]

The backout didn't fix this: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=Jd7hyjiQQH2l0nVED1UaFg.0&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=2224900e3db5d978e2b2207336a6cfa412595ded&searchStr=linux%2Cx64%2Cdebug%2Cspidermonkey%2Cbuilds%2Chazard-linux64-shell-haz%2Fdebug%2Ch

Waiting for retriggers to see if it's from Bug 1667336: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=hazard&tochange=00e795e5aeb16945e822e0e8dc33974326d3c014&fromchange=9a5e08c779234c36a7e2e71df5626e637180a1be&selectedTaskRun=OCB2qN7gRn6aghJEePUybw.0

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

Fixed by backing out bug 1667336

Comment 7

[Jon Coppeard (:jonco)]

On matrix arai said:

the issue about the rooting hazard seems to be whether mozilla::detail::MaybeStorage::NonConstT is marked as GC pointer or not
it's listed under GCInvalidated in typeInfo.txt, only in failing case

Comment 8

[Jon Coppeard (:jonco)]

The immediate issue of the shell hazard build failing has been fixed by backing out bug 1667336, but the underlying problem is likely to still be present so I'm reopening this.

It occurs to me that the patch in bug 1667336 removed an early use of Maybe<> in threading/Mutex.h. Maybe this is something something to do with the order dependence mentioned here: https://searchfox.org/mozilla-central/source/js/src/devtools/rootAnalysis/computeGCTypes.js#191-194 ?

Comment 9

[Steve Fink [:sfink] [:s:]]

The problem: mozilla::detail::MaybeStorage::NonConstT is claimed to be a GC pointer because (from gcTypes.txt, which is hidden in the hazardIntermediates.tar.xz artifact):

GCPointer: mozilla::detail::MaybeStorage::NonConstT
  which is annotated as a GCPointer

That's... not good. NonConstT is not an actual type. It's declared with using NonConstT = std::remove_const_t<T>; for the template parameter T. Whether or not it is a GC pointer or not depends on the actual type. That would explain the nondetermism: all Maybe<T> are getting treated as if they contain the same type of storage, and either the first or last one will win (depending on how the types are processed; I haven't checked). And the order of all of these supposedly duplicate types will depend on compile order.

What's interesting is that this is a change in behavior. I looked at an old gcTypes.txt file, and it did not merge everything into a bogus NonConstT type. That "type" never appeared. Although come to think of it, that may have been due to a Maybe implementation change. It formerly used an opaque AlignedStorage2 or something; now it has a field that the hazard analysis can see into.

Comment 10

[Steve Fink [:sfink] [:s:]]

One difference when looking at the old gcTypes.txt:

GCPointer: mozilla::Maybe<JSString*>
  contains field 'template-param-JSString*' of type (inherited annotations from JSString*)

This is what I expect and want: Maybe<JSString*> is a GC pointer because Maybe<T> is annotated with MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS, and JSString* is a GC pointer.

In the erroneous gcTypes.txt, it actually does even better:

GCPointer: mozilla::Maybe<JSString*>
  inherits from mozilla::detail::MaybeStorage<JSString*, true>
    contains field 'mStorage' of type mozilla::detail::MaybeStorage<JSString*, true>::Union
      contains field 'val' pointing to type JSString
        inherits from js::gc::CellWithLengthAndFlags
          inherits from js::gc::Cell
            which is annotated as a GCThing
        contains field 'd' of type JSString::Data
          ...redundant reason elided...
  contains field 'template-param-JSString*' of type (inherited annotations from JSString*)

The old version implemented its storage with an opaque buffer, which necessitated the MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS annotation. As can be seen above, that is no longer needed; the hazard analysis can now see though its mStorage to figure out for itself whether it holds a GC pointer.

Comment 11

[Steve Fink [:sfink] [:s:]]

Ok, here's a weird discrepancy: looking at AutoSweepBase in the old shows

GCPointer: js::AutoSweepBase
  contains field 'nogc' of type JS::AutoCheckCannotGC
    which is annotated as a GCPointer

whereas the new erroneous gcTypes.txt says:

GCPointer: js::AutoSweepBase
  contains field 'nogc' of type mozilla::detail::MaybeStorage::NonConstT
    which is annotated as a GCPointer

Uh... that field is still just a JS::AutoCheckCannotGC. There's no Maybe involved anywhere.

I need to look at the analysis database files.

Comment 12

[Intermittent Failures Robot]

8 failures in 4195 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 6
• mozilla-central: 2

Platform breakdown:

• linux64: 8

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1668489&startday=2020-09-28&endday=2020-10-04&tree=all

Comment 13

[Intermittent Failures Robot]

1 failures in 4130 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform breakdown:

• linux64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1668489&startday=2020-10-05&endday=2020-10-11&tree=all

Comment 14

[Steve Fink [:sfink] [:s:]]

Just an update: I have something that I believe should fix this, basically switching to using gcc's builtin type->string conversion code instead of handrolling it. It takes a flag to "chase" typedefs.

Unfortunately, it also causes gcc to crash when stringifying some of our hairy GC template code when some of the components' type info is not yet available. It's a bug in gcc, though I haven't isolated it enough to file a test case (and I don't know if I could trigger it without a plugin; gcc doesn't stringify types much. It's only for error messages normally, I think?)

Right now, I'm checking to see whether upgrading from gcc 9.1.0 -> 9.3.0 will fix it.

If not, I'll have to go back to the handrolled code, and add in some typedef chasing. I would really rather use gcc's code for this, though!

Comment 15

[Steve Fink [:sfink] [:s:]]

gcc 9.3.0 did not work. Same crash. I think it's dying on either IncrementalIter<Iter>::Elem IncrementalIter<Iter>::get() const [with Iter = ContainerIter<mozilla::Vector<mozilla::UniquePtr<js::gc::SweepAction, JS::DeletePolicy<js::gc::SweepAction> >, 0, js::SystemAllocPolicy> >; IncrementalIter<Iter>::Elem = const mozilla::UniquePtr<js::gc::SweepAction, JS::DeletePolicy<js::gc::SweepAction> >&] or a related type.

Worse, it's dying within a strip_typedefs call, which suggests that fixing up the handrolled code might not be straightforward either, since that's exactly what I was intending to call.

I may need to sniff at the type and blacklist whatever it is that's causing the crash. It's tricky, because this isn't reproducing on my local machine -- I'm trying to debug via an interactive loaner (they're working again!) But the relevant code is in sixgill, which gets compiled in a separate toolchain job. I'd need to set up its dev environment to make changes.

I guess my next step is to try to figure out why it's not crashing locally. Perhaps there's something about the environment that I can replicate for the CI job?

Comment 16

[Steve Fink [:sfink] [:s:]]

Oh yeah. I should mention that the specific line it's crashing on is:

js/src/gc/GC.cpp:5976:52: internal compiler error: Segmentation fault
  5976 |   Elem get() const { return maybeIter.ref().get(); }

Comment 17

[Steve Fink [:sfink] [:s:]]

Never mind, with the latest setup it is reproducing locally. \o/

Comment 18

[Steve Fink [:sfink] [:s:]]

Filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97419 which will probably be ignored for either requiring a plugin to reproduce, or not being on the latest gcc.

Comment 19

[Intermittent Failures Robot]

5 failures in 3694 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 4
• try: 1

Platform breakdown:

• linux64: 5

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1668489&startday=2020-10-12&endday=2020-10-18&tree=all

Comment 20

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1668489 - Switch to sixgill that qualifies `using`-aliased template members to avoid collisions between different types

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/027e6c6901ec
Switch to sixgill that qualifies `using`-aliased template members to avoid collisions between different types r=jonco

Comment 22

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/027e6c6901ec

Comment 23

[Intermittent Failures Robot]

2 failures in 4230 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 1
• autoland: 1

Platform breakdown:

• linux64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1668489&startday=2020-10-19&endday=2020-10-25&tree=all

Comment 24

[Steve Fink [:sfink] [:s:]]

Hopefully that last comment 23 will be the final occurrence of this. (It claims to cover the last 7 days, and this landed 2 days ago, so hopefully those failures are all from the first 5.)