A comment in noteMaybeNurseryPtr in Assembler-shared.h suggests that there is complexity there that is due to PJS. That complexity can be removed now, but it's not 100% clear what it is.

Previously this could be called off-main thread by PJS, although never with a nursery pointer. Now that PJS is gone we can assert that this is never called off main thread at all. This gives us a stronger assertion that nothing off main thread can touch nursery pointers.

Comment on attachment 8548941 [details] [diff] [review] bug1121435-note-nursery-ptr Review of attachment 8548941 [details] [diff] [review]: ----------------------------------------------------------------- Great!

(In reply to Terrence Cole [:terrence] from comment #2) Looks like I'm wrong because this assertion now fails on Windows.

(In reply to Jon Coppeard (:jonco) from comment #3) > (In reply to Terrence Cole [:terrence] from comment #2) > > Looks like I'm wrong because this assertion now fails on Windows. Jon, do you have any more context than that? On what test it failed, 32-bit, 64-bit? Absent any more information I'm likely to just change the comment to note that the assertion only holds if the pointer is indeed a nursery pointer, and leave it at that.

(In reply to Lars T Hansen [:lth] from comment #4) 32-bit build, in the package stage: https://treeherder.mozilla.org/logviewer.html#?job_id=4249168&repo=try There's no backtrace unfortunately.

These ImmMaybeNurseryPtr come from IonCaches.cpp. I don't know enough about how this works to say whether we should be seeing this type in off-main thread compilation. I guess the comment may just be out of date with regards to PJS being the only user.

I too think the comment is stale. noteMaybeNurseryPointer is reachable from eg cmpPtr in the x64 assembler, which is reachable from eg visitCompareAndBranch in the x64 code generator, which is reachable from the rest of the compiler and should thus be possible to reach during off-thread compilation.

Comment on attachment 8552959 [details] [diff] [review] Fix the comment to reflect reality Review of attachment 8552959 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nit below addressed. ::: js/src/jit/shared/Assembler-shared.h @@ +934,1 @@ > MOZ_ASSERT(GetJitContext()->runtime->onMainThread()); Please add another assert that fails if we're Ion-compiling on the main thread. For fuzzing with --no-threads/--ion-offthread-compile=off it'd be useful to catch that as well: MOZ_ASSERT(!GetJitContext()->runtime->mainThread()->ionCompiling); (Ion compilation shouldn't use nursery pointers on the main thread either.)

The problem is probably pushValue(const Value &val) in the x86 and ARM macro assemblers, it uses ImmMaybeNurseryPtr if val.isMarkable(). We could probably fix that somehow but I think those 2 asserts are good enough for now. https://dxr.mozilla.org/mozilla-central/search?q=ImmMaybeNurseryPtr

With additional assertion: https://hg.mozilla.org/integration/mozilla-inbound/rev/a72dcb722c0c