Christian Holler (:decoder) Reporter
	Description • 6 years ago

The following testcase crashes on mozilla-central revision a6a32fb286fa+ (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --cpu-count=2 --disable-oom-functions --disable-oom-functions --ion-offthread-compile=off --spectre-mitigations=on --arm-hwcap=vfp --arm-asm-nop-fill=1 --nursery-strings=on):

See attachment.


Backtrace:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x08a930e8 in js::gc::GCRuntime::tryNewNurseryObject<(js::AllowGC)1> (this=0xf660f468, cx=0xf661d800, thingSize=48, nDynamicSlots=0, clasp=0x8df27c4 <js::RegExpObject::class_>) at js/src/gc/Allocator.cpp:99
#1  0x08aa84e8 in js::Allocate<JSObject, (js::AllowGC)1> (cx=0xf661d800, kind=js::gc::AllocKind::OBJECT4_BACKGROUND, nDynamicSlots=0, heap=js::gc::DefaultHeap, clasp=0x8df27c4 <js::RegExpObject::class_>) at js/src/gc/Allocator.cpp:56
#2  0x08645217 in js::NativeObject::create (cx=0xf661d800, kind=js::gc::AllocKind::OBJECT4_BACKGROUND, heap=js::gc::DefaultHeap, shape=..., group=...) at js/src/vm/NativeObject-inl.h:538
#3  0x08777e1d in NewObject (cx=0xf661d800, group=..., kind=<optimized out>, newKind=js::GenericObject, initialShapeFlags=0) at js/src/vm/JSObject.cpp:728
#4  0x08779ade in js::NewObjectWithGroupCommon (cx=0xf661d800, group=..., allocKind=js::gc::AllocKind::OBJECT4_BACKGROUND, newKind=js::GenericObject) at js/src/vm/JSObject.cpp:898
#5  0x087dec64 in js::NewObjectWithGroup<js::RegExpObject> (newKind=js::GenericObject, allocKind=<optimized out>, group=..., cx=0xf661d800) at js/src/vm/JSObject-inl.h:753
#6  js::NewObjectWithGroup<js::RegExpObject> (newKind=js::GenericObject, group=..., cx=0xf661d800) at js/src/vm/JSObject-inl.h:763
#7  js::CloneRegExpObject (cx=0xf661d800, regex=...) at js/src/vm/RegExpObject.cpp:1345
#8  0x0857e5bd in js::jit::Simulator::softwareInterrupt (this=0xf665a000, instr=0xf6643734) at js/src/jit/arm/Simulator-arm.cpp:2674
[...]
#26 0x08082b0c in main (argc=12, argv=0xffe2efe4, envp=0xffe2f018) at js/src/shell/js.cpp:9405
eax	0x0	0
ebx	0x8df7ff4	148865012
ecx	0xf7517864	-145655708
edx	0x0	0
esi	0xf661d800	-161359872
edi	0xf661d800	-161359872
ebp	0xffe2dd48	4293057864
esp	0xffe2dd40	4293057856
eip	0x8a930e8 <js::gc::GCRuntime::tryNewNurseryObject<(js::AllowGC)1>(JSContext*, unsigned int, unsigned int, js::Class const*)+504>
=> 0x8a930e8 <js::gc::GCRuntime::tryNewNurseryObject<(js::AllowGC)1>(JSContext*, unsigned int, unsigned int, js::Class const*)+504>:	movl   $0x0,0x0
   0x8a930f2 <js::gc::GCRuntime::tryNewNurseryObject<(js::AllowGC)1>(JSContext*, unsigned int, unsigned int, js::Class const*)+514>:	ud2



This is very likely an OOM problem, it only reproduces in an ARM build. In a 32-bit build I get an unhandlable OOM and a 64-bit shell is killed by the OOM killer. I was not able to reproduce this as a standalone test. The attached test has the full execlog and it also reproduces for me from execution.log.ok inside that archive.

Steve, I gave you the driver-dcd.js a few days ago, with that you should be able to reproduce this locally using a debug armsim build. I assume it is just some unchecked OOM condition in the new nursery strings code.

	Steve Fink [:sfink] [:s:] Assignee
	Comment 2 • 6 years ago

It's not simple. The assertion it's crashing on was added long, long ago, probably with the landing of generational GC. But the nursery strings stuff very possibly invalidated a subtle property it was depending on.

What's going on is that we fail to nursery-allocate an object. That's fine, that happens whenever the nursery fills up. So we collect the nursery. It appears that we expect that if we OOM while collecting the nursery, that the nursery will end up disabled. But it isn't -- it's still enabled, yet when attempting to allocate from it, we fail.

Note that we're not allocating *strings* here, just the same objects that we always have. But in this brittle setup, I can believe that the nursery strings landing may have upset things. I'm not sure if that invariant was truly intended to begin with; it might have been one of those "aw, this will never happen" types of things. (Exceeding gcMaxBytes and thus disabling the nursery *is* expected. But other things might not have been. Or we might have added additional non-GC allocations within Nursery::allocateObject since this assert was added.)

The simple fix would be to remove the assert and allow this to fail the allocation. But I'll look at the code a little to see whether that's right.

	Steve Fink [:sfink] [:s:] Assignee
	Comment 3 • 6 years ago

We totally don't maintain that invariant today. Object allocation can fail for multiple reasons without disabling the nursery. String allocation might actually uphold this, ironically, but I don't think so -- if you fail to allocate a chunk, it doesn't look like the nursery gets disabled.

It doesn't seem that useful an invariant anyway? Why bend over backwards to make a fallible call slightly less fallible?

	Jon Coppeard (:jonco) (PTO until 14th September)
	Comment 4 • 6 years ago

Comment on attachment 8958900 [details] [diff] [review]
Nursery allocation is not infallible, even right after collecting the nursery

Review of attachment 8958900 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, sound fine.

	Pulsebot
	Comment 5 • 6 years ago

Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb463eac0f09
Nursery allocation is not infallible, even right after collecting the nursery, r=jonco

	Cristian Brindusan [:cbrindusan]
	Comment 6 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/fb463eac0f09