1918438 - Assertion failure: cx->nursery().isEmpty(), at js/src/gc/GCAPI.cpp:62

Status: RESOLVED FIXED

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

Description

[Nils Bars]

Attachment: bug.js

Steps to reproduce:

Checkout commit 181e5bb2645236a617d42e3740420098097f7a0f and execute the shell as shown below.

js --fuzzing-safe --no-ggc <test-case>

Actual results:

This bug initially caused the assertion below, but I could not trigger it again:

Assertion failure: prevState == JS::HeapState::Idle || (prevState == JS::HeapState::MajorCollecting && heapState == JS::HeapState::MinorCollecting), at src/gc/GC.cpp:3557

This is the assertion triggered by the provided test case

Assertion failure: cx->nursery().isEmpty(), at js/src/gc/GCAPI.cpp:62
#0 0x55f719f03c5b in JS::AutoDisableGenerationalGC::AutoDisableGenerationalGC(JSContext*) js/src/gc/GCAPI.cpp:62:3
#1 0x55f719fbba08 in js::VerifyPreTracer::VerifyPreTracer(JSRuntime*) js/src/gc/Verifier.cpp:108:9
#2 0x55f719fbba08 in js::VerifyPreTracer* js_new<js::VerifyPreTracer, JSRuntime* const&>(JSRuntime* const&) reproducebuild/dist/include/js/Utility.h:545:1
#3 0x55f719fbba08 in js::gc::GCRuntime::startVerifyPreBarriers() js/src/gc/Verifier.cpp:203:26
#4 0x55f71924c211 in js::Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3501:9
#5 0x55f719233371 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:490:13
#6 0x55f719238551 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:877:13
#7 0x55f719238d5c in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:910:10
#8 0x55f7194841b9 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) js/src/vm/CompilationAndEvaluation.cpp:496:10
#9 0x55f719484437 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) js/src/vm/CompilationAndEvaluation.cpp:520:10
#10 0x55f71919d1ce in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool, bool) js/src/shell/js.cpp:1316:10
#11 0x55f71919c535 in Process(JSContext*, char const*, bool, FileKind) js/src/shell/js.cpp
#12 0x55f71915714e in ProcessArgs(JSContext*, js::cli::OptionParser*) js/src/shell/js.cpp:11414:10
#13 0x55f71915714e in Shell(JSContext*, js::cli::OptionParser*) js/src/shell/js.cpp:11666:12
#14 0x55f71914e87d in main js/src/shell/js.cpp:12223:12
#15 0x4a590d2acd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#16 0x4a590d2ace3f in __libc_start_main csu/../csu/libc-start.c:392:3
#17 0x55f719117e28 in _start (reproducebuild/dist/bin/js+0x1c38e28) (BuildId: 1965a1e0f9319cb7b7639a327e6d7dd3)

Comment 1

[Steven DeTar [:sdetar]]

I believe this should be in the JavaScript: GC component

Comment 2

[Daniel Veditz [:dveditz]]

The testcase is the letter a (parsed as an identifier) after a couple of shell config settings... is this some weird regression? If it falls over this easy it's hard to imagine how anything runs.

gcparam("nurseryEnabled", true)
gczeal(4)
a

Comment 3

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

From an initial look, the script following the settings likely matters very little (it may need to trigger a nursery allocation?). It's running with generational GC disabled, then it enables it with gcparam, then it turns on a zeal mode (VerifierPre) that verifies pre write barriers—which are for incremental GC, not generational GC. But the only thing that matters here is that it runs the verifier, and the verifier asserts that the nursery is in an expected state. It is not, due to the disabling then enabling.

Anyway, I will look into exactly what's going on, but this is almost certainly a test-only failure. I'll want to fix it to keep it from getting in the way of fuzzing.

Comment 4

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

Yes, if I play around with it, the a is necessary. If I replace it with something else that creates a nursery object, it still crashes (eg new Object();). If I swap it for something that creates a tenured object, it does not crash (eg new ArrayBuffer(10)).

Comment 5

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

The problem is that we have a main mechanism for enabling/disabling the nursery, and then there's this secondary thing that is specific to the verifier (AutoDisableGenerationalGC). The verifier maintains its own generationalDisabled state, and uses it to decide when to call the main mechanism. The main mechanism does not know about the verifier's stuff.

Which means that the nursery can get enabled by an explicit call even when the verifier is relying on it being (and staying!) disabled. That's what is happening here. The call to gcparam("nurseryEnabled", true) is enabling the nursery while the verifiers' generationDisabled level is > 0. That allows objects to be allocated in the nursery when the verifier does not expect them to be.

The added wrinkle is that the --no-ggc command-line flag uses AutoDisableGenerationalGC, while gcparam uses the main mechanism that doesn't look at that.

GCRuntime::setParameter pauses verification before tweaking parameters, which is insufficient for this case because it doesn't undo the effects of --no-ggc. I could make the nursery enabling fail if it is being suppressed. I'd also have to make disabling fail as well, because if the disable counter goes from 1->0 in ~AutoDisableGenerationalGC, it would re-enable. I'll give that a go.

Comment 6

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

Attachment: Bug 1918438 - Do not allow gcparam() to enable/disable nursery while GGC is disabled

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51aaf4c444c4
Do not allow gcparam() to enable/disable nursery while GGC is disabled r=jonco

Comment 8

[Serban Stanca [:SerbanS]]

Backed out for causing reftests failures in regress-1918438.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST TEST-UNEXPECTED-FAIL | js/src/tests/non262/regress/regress-1918438.js | Unknown file:///opt/worker/tasks/task_172773022101716/build/tests/jsreftest/tests/js/src/tests/shell.js:127: TypeError: Assertion failed: got "false", expected "true" item 1

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0288e0f2e0e5
Do not allow gcparam() to enable/disable nursery while GGC is disabled r=jonco

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/0288e0f2e0e5