Assertion failure: !obj->runtimeFromMainThread()->gc.nursery().isInside(src.dataPointer()), at vm/ArrayBufferObject.cpp:2070
Categories
(Core :: JavaScript: GC, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr115 | --- | unaffected |
firefox120 | --- | unaffected |
firefox121 | --- | wontfix |
firefox122 | --- | wontfix |
People
(Reporter: decoder, Assigned: sfink, NeedInfo)
References
(Regression)
Details
(4 keywords, Whiteboard: [bugmon:update,bisect])
Attachments
(3 files)
The following testcase crashes on mozilla-central revision 20231124-dbd1a6aef0a7 (debug build, run with --fuzzing-safe --ion-offthread-compile=off):
evalInWorker(`
gczeal(14);
a = [];
for (b = 0;;)
a.push(new ArrayBuffer);
`);
Backtrace:
received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xf2dffb40 (LWP 18735)]
0x585e9282 in js::ArrayBufferObject::objectMoved(JSObject*, JSObject*) ()
#0 0x585e9282 in js::ArrayBufferObject::objectMoved(JSObject*, JSObject*) ()
#1 0x588d7a5d in js::gc::ArenaList::relocateArenas(js::gc::Arena*, js::gc::Arena*, js::SliceBudget&, js::gcstats::Statistics&) ()
#2 0x588d9160 in js::gc::ArenaLists::relocateArenas(js::gc::Arena*&, JS::GCReason, js::SliceBudget&, js::gcstats::Statistics&) ()
#3 0x588d5297 in js::gc::GCRuntime::relocateArenas(JS::Zone*, JS::GCReason, js::gc::Arena*&, js::SliceBudget&) ()
#4 0x588d4ec7 in js::gc::GCRuntime::compactPhase(JS::GCReason, js::SliceBudget&, js::gc::AutoGCSession&) ()
#5 0x58900b0e in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, JS::GCReason, bool) ()
#6 0x58903d1b in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget const&, JS::GCReason) ()
#7 0x589053da in js::gc::GCRuntime::collect(bool, js::SliceBudget const&, JS::GCReason) ()
#8 0x588ce430 in js::gc::GCRuntime::gc(JS::GCOptions, JS::GCReason) ()
#9 0x588ceb57 in js::gc::GCRuntime::runDebugGC() ()
#10 0x588ccecc in bool js::gc::CellAllocator::PreAllocChecks<(js::AllowGC)1>(JSContext*, js::gc::AllocKind) ()
#11 0x57f205d6 in void* js::gc::CellAllocator::AllocNurseryOrTenuredCell<(JS::TraceKind)0, (js::AllowGC)1>(JSContext*, js::gc::AllocKind, unsigned int, js::gc::Heap, js::gc::AllocSite*) ()
#12 0x57f204b7 in js::NativeObject* js::gc::CellAllocator::NewObject<js::NativeObject, (js::AllowGC)1>(JSContext*, js::gc::AllocKind, js::gc::Heap, JSClass const*, js::gc::AllocSite*) ()
#13 0x57f1f5cf in js::NativeObject::create(JSContext*, js::gc::AllocKind, js::gc::Heap, JS::Handle<js::SharedShape*>, js::gc::AllocSite*) ()
#14 0x585e5371 in NewArrayBufferObject(JSContext*, JS::Handle<JSObject*>, js::gc::AllocKind) ()
#15 0x585e5d44 in std::tuple<js::ArrayBufferObject*, unsigned char*> js::ArrayBufferObject::createBufferAndData<(js::ArrayBufferObject::FillContents)0>(JSContext*, unsigned int, js::AutoSetNewObjectMetadata&, JS::Handle<JSObject*>) ()
#16 0x585de023 in js::ArrayBufferObject::createZeroed(JSContext*, unsigned int, JS::Handle<JSObject*>) ()
#17 0x585dde2d in js::ArrayBufferObject::class_constructor(JSContext*, unsigned int, JS::Value*) ()
#18 0x27bd83c7 in ?? ()
#19 0x27bcc5a2 in ?? ()
#20 0x27ba87ed in ?? ()
#21 0x58ae071f in js::jit::EnterBaselineInterpreterAtBranch(JSContext*, js::InterpreterFrame*, unsigned char*) ()
#22 0x57f8b149 in js::Interpret(JSContext*, js::RunState&) ()
#23 0x57f829af in MaybeEnterInterpreterTrampoline(JSContext*, js::RunState&) ()
#24 0x57f8250f in js::RunScript(JSContext*, js::RunState&) ()
#25 0x57f86006 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) ()
#26 0x57f865aa in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) ()
#27 0x580f29a8 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) ()
#28 0x580f2783 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) ()
#29 0x57e088e6 in WorkerMain(mozilla::UniquePtr<WorkerInput, JS::DeletePolicy<WorkerInput> >) ()
#30 0x57e08efb in js::detail::ThreadTrampoline<void (&)(mozilla::UniquePtr<WorkerInput, JS::DeletePolicy<WorkerInput> >), mozilla::UniquePtr<WorkerInput, JS::DeletePolicy<WorkerInput> > >::Start(void*) ()
#31 0x57e51942 in set_alt_signal_stack_and_start(PthreadCreateParams*) ()
#32 0xf7fb226a in start_thread (arg=0xf2dffb40) at pthread_create.c:333
#33 0xf7c8e41e in clone () from /lib32/libc.so.6
eax 0x567a78bb 1450866875
ebx 0x598cd1b8 1502400952
ecx 0x598ceddc 1502408156
edx 0x0 0
esi 0xf23fffd0 -230686768
edi 0x0 0
ebp 0xf2dfdf58 4074757976
esp 0xf2dfdf20 4074757920
eip 0x585e9282 <js::ArrayBufferObject::objectMoved(JSObject*, JSObject*)+834>
=> 0x585e9282 <_ZN2js17ArrayBufferObject11objectMovedEP8JSObjectS2_+834>: movl $0x816,0x0
0x585e928c <_ZN2js17ArrayBufferObject11objectMovedEP8JSObjectS2_+844>: call 0x57e51230 <abort>
Seems to be 32-bit only, likely something involving OOM.
Reporter | ||
Comment 1•9 months ago
|
||
Reporter | ||
Comment 2•9 months ago
|
||
Comment 3•9 months ago
|
||
Unable to reproduce bug 1866518 using build mozilla-central 20231124092418-dbd1a6aef0a7. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.
Updated•9 months ago
|
Comment 4•9 months ago
|
||
Steve, could you take look at this when you have a chance.
Comment 5•9 months ago
|
||
Changing Severity/priority since this now marked Sec-High
Updated•9 months ago
|
Comment 6•9 months ago
|
||
Not sure about the regressor, setting it as Bug 1862530 but :sdetar/:sfink if you could confirm?
Assignee | ||
Comment 7•9 months ago
|
||
Recently-added assertion that is checking more than it wants to.
What's going on is that we have an empty ArrayBuffer
that gets compacted to a memory location that happens to land right before the nursery. That means that its dataPointer()
(which is its inlineDataPointer()
since it's zero-length) points to the very beginning of the nursery. There's no assertion for this, so it continues on. A later compaction then moves it to another location, but this time there's an assertion that the source ABO's data pointer is not inside the nursery, and that assertion fails. But it's spurious, because even though the pointer points to the nursery, it's pointing to zero bytes of data, so it's fine.
Fuzzers gotta fuzz.
Assignee | ||
Updated•9 months ago
|
Updated•9 months ago
|
Comment 8•9 months ago
|
||
:jonco, since you are the author of the regressor, bug 1859335, could you take a look?
For more information, please visit BugBot documentation.
Assignee | ||
Comment 9•9 months ago
|
||
BugBot, leave jonco alone. ;-) I have a patch, but I'm looking for more places that might need it first.
Comment 10•9 months ago
|
||
It turned out that this is just an overly strict assertion, not a security issue, so I don't think it needs to be tracked.
Assignee | ||
Comment 11•9 months ago
|
||
Ok, I'll pester Jon instead. Jon, this is trickier than I expected. I made a Nursery::isInside(void*, size_t)
to return false
for a zero-length region, passing in the length where it made sense, but that revealed that we have both situations: in the above case, we have a zero-length pointer to the beginning of the nursery that logically belongs with a Cell that immediately precedes the nursery. But in another case, we have a zero-length nursery buffer that is being used for a different ArrayBuffer (I'm not sure it started out as zero-length, it might have gotten detached). So for that, a pointer to a zero-length region at the beginning of the nursery should return true
.
Currently this can definitely happen to ArrayBuffers. I don't think it can happen to JSStrings, since dependent strings will never point to inline base strings' chars. I'm not sure if it can happen with NativeObject or not. BigInt would be another one to worry about.
Options are:
- disallow using the final Cell, Arena, or Chunk before a nursery Chunk
- disallow inline pointers past the end of a Cell's memory, or at pointers to zero-length regions
- skip the first
CellAlignBytes
of every nursery Chunk when allocating a buffer - make every
isInline
caller specify what a pointer to the beginning of the nursery means
I just thought of that 3rd one while writing this, so I think I'll try that first.
Comment 12•9 months ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #11)
skip the first
CellAlignBytes
of every nursery Chunk when allocating a buffer
This should happen already since there's a header at the start of the chunk.
disallow inline pointers past the end of a Cell's memory, or at pointers to zero-length regions
There have been problems with this before so I'm sympathetic to this idea but I guess it's simpler to have a pointer to a zero length region than have a possibly-null pointer and have to check it every time.
Assignee | ||
Comment 13•9 months ago
|
||
Oops, forgot to post this last night:
Uh, I'm an idiot. We already disallow using the beginning of a NurseryChunk, or any Chunk for that matter. There's a header. I'll have to go back and see why I was getting a pointer to there.
I'm not sure why I was seeing what seemed to be a nursery pointer to the beginning of the nursery, but it was as part of a incomplete patch so I was probably just messing something up. The actual fix is trivial and seems to resolve the problem. I still need to figure out how to land a test, since the fuzz test takes over 2 minutes to eventually OOM.
disallow inline pointers past the end of a Cell's memory, or at pointers to zero-length regions
There have been problems with this before so I'm sympathetic to this idea but I guess it's simpler to have a pointer to a zero length region than have a possibly-null pointer and have to check it every time.
I would probably have it point to a shared empty space rather than a nullptr. But it still seems like a nonobvious restriction.
Assignee | ||
Comment 14•9 months ago
|
||
Updated•9 months ago
|
Comment 15•9 months ago
|
||
Pushed by sfink@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8038f0b90bbd Pointers to the beginning of a NurseryChunk should not be automatically considered to be in the nursery r=jonco
Comment 16•9 months ago
•
|
||
Backed out for causing SM bustages on bug1866518-nursery-AB.js
Jit test failure log. Assertion failure log
Comment 17•9 months ago
|
||
Pushed by sfink@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8a70af1e5277 Pointers to the beginning of a NurseryChunk should not be automatically considered to be in the nursery r=jonco
Comment 18•9 months ago
|
||
Backed out for causing assertions on Nursery-inl.h
- backout: https://hg.mozilla.org/integration/autoland/rev/9eb376416795d16b034a0f88d5dd3a9a5c8763cb
- push: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=eccvPwxbQqeuGv7TKOU4sg.0&revision=8a70af1e52778c46ce7cb3c560763afd1937eaf1
- failure log: https://treeherder.mozilla.org/logviewer?job_id=438832940&repo=autoland&lineNumber=36615
[task 2023-12-05T00:54:35.530Z] TEST-PASS | js/src/jit-test/tests/gc/pretenure-array-long-then-short-lived.js | Success (code 0, args "") [8.4 s]
[task 2023-12-05T00:54:36.031Z] Assertion failure: isInside(oldData), at /builds/worker/checkouts/gecko/js/src/gc/Nursery-inl.h:82
[task 2023-12-05T00:54:36.031Z] #01: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1db953c]
[task 2023-12-05T00:54:36.031Z] #02: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x2356217]
[task 2023-12-05T00:54:36.031Z] #03: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x2351c74]
[task 2023-12-05T00:54:36.031Z] #04: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x235155e]
[task 2023-12-05T00:54:36.031Z] #05: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x2352f5f]
[task 2023-12-05T00:54:36.031Z] #06: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x2354a6c]
[task 2023-12-05T00:54:36.031Z] #07: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x2353963]
[task 2023-12-05T00:54:36.031Z] #08: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x235370d]
[task 2023-12-05T00:54:36.031Z] #09: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x2313152]
[task 2023-12-05T00:54:36.031Z] #10: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x230fdb8]
[task 2023-12-05T00:54:36.031Z] #11: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x230f1ed]
[task 2023-12-05T00:54:36.031Z] #12: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x22950b9]
[task 2023-12-05T00:54:36.031Z] #13: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x22618c7]
[task 2023-12-05T00:54:36.031Z] #14: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x22610ba]
[task 2023-12-05T00:54:36.031Z] #15: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x193eea4]
[task 2023-12-05T00:54:36.031Z] #16: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1f31277]
[task 2023-12-05T00:54:36.031Z] #17: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1f30a57]
[task 2023-12-05T00:54:36.031Z] #18: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1f30961]
[task 2023-12-05T00:54:36.031Z] #19: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x25d57d9]
[task 2023-12-05T00:54:36.031Z] #20: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x25d1de7]
[task 2023-12-05T00:54:36.031Z] #21: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x25dc51b]
[task 2023-12-05T00:54:36.031Z] #22: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x25dcc40]
[task 2023-12-05T00:54:36.031Z] #23: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x245ed40]
[task 2023-12-05T00:54:36.031Z] #24: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1910abd]
[task 2023-12-05T00:54:36.031Z] #25: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1909cff]
[task 2023-12-05T00:54:36.031Z] #26: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x190985f]
[task 2023-12-05T00:54:36.031Z] #27: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x190d376]
[task 2023-12-05T00:54:36.031Z] #28: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x190d8ba]
[task 2023-12-05T00:54:36.031Z] #29: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1a7b428]
[task 2023-12-05T00:54:36.031Z] #30: JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>)[/builds/worker/workspace/obj-spider/dist/bin/js +0x1a7b5fe]
[task 2023-12-05T00:54:36.031Z] #31: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x187311f]
[task 2023-12-05T00:54:36.031Z] #32: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1872353]
[task 2023-12-05T00:54:36.031Z] #33: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x180cad5]
[task 2023-12-05T00:54:36.031Z] #34: ???[/builds/worker/workspace/obj-spider/dist/bin/js +0x1806d86]
[task 2023-12-05T00:54:36.031Z] Exit code: -11
[task 2023-12-05T00:54:36.031Z] FAIL - gc/pretenuring.js
[task 2023-12-05T00:54:36.031Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/pretenuring.js | Assertion failure: isInside(oldData), at /builds/worker/checkouts/gecko/js/src/gc/Nursery-inl.h:82 (code -11, args "") [0.7 s]
Updated•9 months ago
|
Assignee | ||
Comment 19•8 months ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #12)
(In reply to Steve Fink [:sfink] [:s:] from comment #11)
skip the first
CellAlignBytes
of every nursery Chunk when allocating a bufferThis should happen already since there's a header at the start of the chunk.
Unfortunately, this isn't enough to fix the problem. A pointer just past the end of a nursery chunk is ambiguous as to whether it is in the nursery or not. The above change of taking the header into account removes the problem of incorrectly treating a pointer to the beginning of a nursery chunk as being in the nursery, since there is no valid data there until you get past the header. But you could answer the above question as either "no, not inside the nursery, it's pointing to the beginning of a malloced page" or "yes, inside the nursery, it's a zero-length pointer just past the end of a nursery cell". And both are used in practice. In order for the header to help, we would need a header on all non-nursery chunks as well.
I guess the straightforward fix is to add a nursery chunk footer as well. :-( I'll give that a try.
Comment 20•8 months ago
|
||
There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:sfink, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.
Comment 21•8 months ago
|
||
The patch is not r+ed.
Updated•7 months ago
|
Description
•