Closed Bug 1506954 Opened 2 years ago Closed 2 years ago

Assertion failure: size % pageSize == 0, at js/src/gc/Memory.cpp:940 on ARM64


(Core :: JavaScript Engine, defect, P1)




Tracking Status
firefox-esr60 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed


(Reporter: decoder, Assigned: jonco)


(4 keywords, Whiteboard: [jsbugmon:ignore][fuzzblocker][arm64:m1])


(1 file)

The following testcase crashes on mozilla-central revision f6df375b8698 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-debug --disable-jemalloc, run with --fuzzing-safe --ion-offthread-compile=off test.js):

gczeal(10, 9);
var g = newGlobal();


received signal SIGSEGV, Segmentation fault.
0x0000aaaaab33fa34 in js::gc::ProtectPages (p=<optimized out>, size=<optimized out>) at js/src/gc/Memory.cpp:942
#0  0x0000aaaaab33fa34 in js::gc::ProtectPages (p=<optimized out>, size=<optimized out>) at js/src/gc/Memory.cpp:942
#1  0x0000aaaaab2c200c in js::gc::GCRuntime::protectAndHoldArenas (this=0xaaaaacc4a8e8, arenaList=0x700001bc000) at js/src/gc/GC.cpp:3151
#2  0x0000aaaaab2d9654 in js::gc::GCRuntime::compactPhase (this=0xaaaaacc4a8e8, reason=JS::gcreason::DEBUG_GC, sliceBudget=..., session=...) at js/src/gc/GC.cpp:7145
#3  0x0000aaaaab2dcaf0 in js::gc::GCRuntime::incrementalSlice (this=0xaaaaacc4a8e8, budget=..., reason=<optimized out>, session=...) at js/src/gc/GC.cpp:7641
#4  0x0000aaaaab2de53c in js::gc::GCRuntime::gcCycle (this=0xaaaaacc4a8e8, nonincrementalByAPI=false, budget=..., reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7945
#5  0x0000aaaaab2e0198 in js::gc::GCRuntime::collect (this=0xaaaaacc4a8e8, nonincrementalByAPI=<optimized out>, budget=..., reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:8126
#6  0x0000aaaaab2aecc8 in js::gc::GCRuntime::runDebugGC (this=0xaaaaacc4a8e8) at js/src/gc/GC.cpp:8732
#7  0x0000aaaaab2ae82c in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0xaaaaacc4a8e8, cx=0xaaaaacc57a30) at js/src/gc/Allocator.cpp:336
#8  0x0000aaaaab2f14d8 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0x0, cx=0xaaaaacc57a30, kind=js::gc::AllocKind::BASE_SHAPE) at js/src/gc/Allocator.cpp:295
#9  0x0000aaaaab2f3e10 in js::Allocate<js::BaseShape, (js::AllowGC)1> (cx=0xaaaaacc57a30) at js/src/gc/Allocator.cpp:239
#10 0x0000aaaaaaf28ed0 in js::BaseShape::getUnowned (cx=0xaaaaacc57a30, base=...) at js/src/vm/Shape.cpp:1552
#11 0x0000aaaaaaf2b6a4 in js::EmptyShape::getInitialShape (cx=0xaaaaacc57a30, clasp=0xaaaaac02a518 <js::DebuggerFrame::class_>, proto=..., nfixed=7, objectFlags=0) at js/src/vm/Shape.cpp:2256
#12 0x0000aaaaaae7b384 in NewObject (cx=0xaaaaacc57a30, group=..., kind=<optimized out>, newKind=<optimized out>, initialShapeFlags=0) at js/src/vm/JSObject.cpp:798
#13 0x0000aaaaaae7a428 in js::NewObjectWithGivenTaggedProto (cx=0xaaaaacc57a30, clasp=<optimized out>, proto=..., allocKind=<optimized out>, newKind=<optimized out>, initialShapeFlags=<optimized out>) at js/src/vm/JSObject.cpp:875
#14 0x0000aaaaaae83c74 in js::NewObjectWithClassProto (cx=<optimized out>, clasp=<optimized out>, newKind=js::SingletonObject, proto=..., allocKind=<optimized out>) at js/src/vm/JSObject-inl.h:546
#15 js::NewObjectWithClassProto (newKind=js::SingletonObject, cx=<optimized out>, clasp=<optimized out>, proto=...) at js/src/vm/JSObject-inl.h:554
#16 js::NewNativeObjectWithClassProto (newKind=js::SingletonObject, cx=<optimized out>, clasp=<optimized out>, proto=...) at js/src/vm/NativeObject-inl.h:798
#17 DefineConstructorAndPrototype (cx=<optimized out>, key=<optimized out>, constructor=<optimized out>, ps=<optimized out>, fs=<optimized out>, static_ps=<optimized out>, static_fs=<optimized out>, ctorp=<optimized out>, obj=..., atom=..., protoProto=..., clasp=<optimized out>, nargs=<optimized out>) at js/src/vm/JSObject.cpp:2037
#18 js::InitClass (cx=0xaaaaacc57a30, obj=..., protoProto_=..., clasp=0xaaaaac02a518 <js::DebuggerFrame::class_>, constructor=0xaaaaaad76e60 <js::DebuggerFrame::construct(JSContext*, unsigned int, JS::Value*)>, nargs=0, ps=0xaaaaac02adc0 <js::DebuggerFrame::properties_>, fs=0xaaaaac02b090 <js::DebuggerFrame::methods_>, static_ps=0x0, static_fs=0x0, ctorp=0x0) at js/src/vm/JSObject.cpp:2167
#19 0x0000aaaaaad76dd8 in js::DebuggerFrame::initClass (cx=0xaaaaacc57a30, dbgCtor=..., global=...) at js/src/vm/Debugger.cpp:8537
#20 0x0000aaaaaad96c90 in JS_DefineDebuggerObject (cx=0xaaaaacc57a30, obj=...) at js/src/vm/Debugger.cpp:12750
#21 0x0000aaaaaabc8ed0 in NewGlobalObject (cx=0xaaaaacc57a30, options=..., principals=<optimized out>) at js/src/shell/js.cpp:10101
#22 0x0000aaaaaabd9584 in NewGlobal (cx=0xaaaaacc57a30, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:6352
#23 0x0000aaaaaac56f30 in CallJSNative (cx=0xaaaaacc57a30, native=0xaaaaaabd93f0 <NewGlobal(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:468
#36 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:11297
x0	0x0	0
x1	0x0	0
x2	0x1	1
x3	0x0	0
x4	0x0	0
x5	0x0	0
x6	0x0	0
x7	0xfffffffd	4294967293
x8	0x0	0
x9	0x6a2f6c61	8246987515394747489
x10	0x1010101	72340172838076673
x11	0x38	56
x12	0x0	0
x13	0x3ac	940
x14	0xbe2b3e08	281473872248328
x15	0x2a8	680
x16	0xbe313a60	281473872640608
x17	0xac0b0420	187650007565344
x18	0x1fff	8191
x19	0x3ac	940
x20	0xacc4a8e8	187650019731688
x21	0xacc4bd30	187650019736880
x22	0x1bc000	7696583213056
x23	0x1b7000	7696583192576
x24	0x1bc000	7696583213056
x25	0xffffc780	281474976696192
x26	0xacc4d608	187650019743240
x27	0xacc4d240	187650019742272
x28	0xad00e820	187650023680032
x29	0xffffc3c0	281474976695232
x30	0xab33fa30	187649993472560
sp	0xffffc3b0	281474976695216
pc	0xaaaaab33fa34 <js::gc::ProtectPages(void*, unsigned long)+156>
cpsr	[ EL=0 C Z ]
fpcsr	void
fpcr	0x0	0
=> 0xaaaaab33fa34 <js::gc::ProtectPages(void*, unsigned long)+156>:	str	w19, [x8]
   0xaaaaab33fa38 <js::gc::ProtectPages(void*, unsigned long)+160>:	bl	0xaaaaaac03a28 <abort()>

This is on native ARM64 and blocking further fuzzing. Marking s-s due to GC involved, but I'm not sure if this is a problem in the browser too, given how frequently this happens.
The pageSize is 64K btw:

0x0000aaaaab33fa34 in js::gc::ProtectPages (p=<optimized out>, size=<optimized out>) at /js/src/gc/Memory.cpp:942
942         MOZ_RELEASE_ASSERT(p);
(gdb) p pageSize
$1 = 65536

Jon, can you look into this or forward to someone who can?
Flags: needinfo?(jcoppeard)
Also not s-s, this is a release assert and probably some platform compat problem.
Group: javascript-core-security
We knew we were going to hit this problem, so chances are it's not too hard to correct for it.
Patch to check the system page size before trying to protect arenas.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Comment on attachment 9024989 [details] [diff] [review]

Confirmed, fixes the problem, thanks!
Attachment #9024989 - Flags: feedback+
Comment on attachment 9024989 [details] [diff] [review]

Review of attachment 9024989 [details] [diff] [review]:

When we relocate arenas in a compacting GC that was triggered with GC zeal, we use mprotect to make the relocated arenas inaccessible for a while to catch any stray accesses.  This doesn't work if the system page size is larger than the size of an arena, which it is on some platforms.   This patch just adds a check and skips this if it's not supported.
Attachment #9024989 - Flags: review?(pbone)
Whiteboard: [jsbugmon:ignore][fuzzblocker] → [jsbugmon:ignore][fuzzblocker][arm64:m1]
Priority: -- → P1
Comment on attachment 9024989 [details] [diff] [review]

Review of attachment 9024989 [details] [diff] [review]:


::: js/src/gc/GC.cpp
@@ +2476,5 @@
> +CanProtectArenas()
> +{
> +    // On some systems the page size does not match the size of an
> +    // arena so we can't change the mapping permissions per arena.
> +    return SystemPageSize() == ArenaSize;

Could make this <= in case we ever change the size of arenas.  (I don't know of anything using pages less than 4K.)
Attachment #9024989 - Flags: review?(pbone) → review+
Pushed by
Don't attempt to protect relocated pages on systems that don't support this r=pbone
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Should we land this testcase still?
Flags: needinfo?(jcoppeard)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
This is covered by existing tests.
Flags: needinfo?(jcoppeard)
You need to log in before you can comment on or make changes to this bug.