Closed Bug 1763877 Opened 2 years ago Closed 2 years ago

Tuple.prototype.with() doesn't root elements properly

Categories

(Core :: JavaScript Engine, defect, P1)

Firefox 101
defect

Tracking

()

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: tjc, Assigned: tjc)

Details

Attachments

(2 files)

Attached file with.js

+++ This bug was initially created as a clone of Bug #1763874 +++

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/96.0.4664.93 Safari/537.36

Steps to reproduce:

See the attached test file, with.js .

With records and tuples enabled, this test runs a loop 500 times in which it calls Tuple.prototype.with().

Actual results:

mach jit-test record-tuple/with -g

(gdb) run
[...]

Thread 1 "js" received signal SIGSEGV, Segmentation fault.
0x000055555707428d in JS::Value::unboxGCPointer<JSObject, (JSValueTag)131068> (this=0x19785b13ff90) at /home/tjc/gecko-fork/obj-x64-debug/dist/include/js/Value.h:629
629	    MOZ_ASSERT((asBits_ & js::gc::CellAlignMask) == 0,
(gdb) bt
#0  0x000055555707428d in JS::Value::unboxGCPointer<JSObject, (JSValueTag)131068> (this=0x19785b13ff90) at /home/tjc/gecko-fork/obj-x64-debug/dist/include/js/Value.h:629
#1  0x000055555705bf31 in JS::Value::toObject (this=0x19785b13ff90) at /home/tjc/gecko-fork/obj-x64-debug/dist/include/js/Value.h:844
#2  0x0000555557063093 in js::IsObjectValueInCompartment (v=..., comp=0x7ffff694e2a0) at /home/tjc/gecko-fork/js/src/vm/JSObject.h:1065
#3  0x0000555557062c55 in js::NativeObject::checkStoredValue (this=0x19785b100018, v=...) at /home/tjc/gecko-fork/js/src/vm/NativeObject.h:1067
#4  0x00005555572b4074 in js::NativeObject::copyDenseElements (this=0x19785b100018, dstStart=0, src=0x19785b13ff90, count=1) at /home/tjc/gecko-fork/js/src/vm/NativeObject-inl.h:98
#5  0x00005555577dbb66 in js::tuple_with (cx=0x7ffff692a200, argc=2, vp=0x7ffff6928140) at /home/tjc/gecko-fork/js/src/vm/TupleType.cpp:132
#6  0x0000555557221d52 in CallJSNative (cx=0x7ffff692a200, native=0x5555577db780 <js::tuple_with(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...)
    at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:420
#7  0x000055555720f743 in js::InternalCallOrConstruct (cx=0x7ffff692a200, args=..., construct=js::NO_CONSTRUCT, reason=js::CallReason::Call) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:507
#8  0x000055555720ff10 in InternalCall (cx=0x7ffff692a200, args=..., reason=js::CallReason::Call) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:567
#9  0x000055555720fcff in js::CallFromStack (cx=0x7ffff692a200, args=...) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:571
#10 0x000055555720305d in Interpret (cx=0x7ffff692a200, state=...) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:3293
#11 0x00005555571f7a31 in js::RunScript (cx=0x7ffff692a200, state=...) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:389
#12 0x0000555557211221 in js::ExecuteKernel (cx=0x7ffff692a200, script=..., envChainArg=..., evalInFrame=..., result=...) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:760
#13 0x0000555557211594 in js::Execute (cx=0x7ffff692a200, script=..., envChain=..., rval=...) at /home/tjc/gecko-fork/js/src/vm/Interpreter.cpp:792
#14 0x000055555742025e in ExecuteScript (cx=0x7ffff692a200, envChain=..., script=..., rval=...) at /home/tjc/gecko-fork/js/src/vm/CompilationAndEvaluation.cpp:515
#15 0x000055555742037b in JS_ExecuteScript (cx=0x7ffff692a200, scriptArg=...) at /home/tjc/gecko-fork/js/src/vm/CompilationAndEvaluation.cpp:539
#16 0x0000555557055d0d in RunFile (cx=0x7ffff692a200, filename=0x7fffffffe169 "/home/tjc/gecko-fork/js/src/jit-test/tests/record-tuple/with.js", file=0x7ffff7860020, 
    compileMethod=CompileUtf8::DontInflate, compileOnly=false) at /home/tjc/gecko-fork/js/src/shell/js.cpp:1065
#17 0x0000555557055662 in Process (cx=0x7ffff692a200, filename=0x7fffffffe169 "/home/tjc/gecko-fork/js/src/jit-test/tests/record-tuple/with.js", forceTTY=false, kind=FileScript)
    at /home/tjc/gecko-fork/js/src/shell/js.cpp:1654
#18 0x000055555702cdd3 in ProcessArgs (cx=0x7ffff692a200, op=0x7fffffffd978) at /home/tjc/gecko-fork/js/src/shell/js.cpp:10896
#19 0x000055555701fdc6 in Shell (cx=0x7ffff692a200, op=0x7fffffffd978) at /home/tjc/gecko-fork/js/src/shell/js.cpp:11693
#20 0x000055555701ad6b in main (argc=13, argv=0x7fffffffdc48) at /home/tjc/gecko-fork/js/src/shell/js.cpp:12736

Expected results:

The test should pass. I have a patch coming shortly.

Previously, tuple_with() declared the following pointer to its tuple
argument's elements:
HeapSlotArray t = tuple->getDenseElements();
Because this wasn't rooted, incremental GC could cause t to be used
after freeing. Since t was only used twice, I inlined it to solve this
problem.

Attachment #9271547 - Attachment description: WIP: Bug 1763877 - Root elements in TupleType::tuple_with → WIP: Bug 1763877 - Root elements in TupleType::tuple_with r?jandem
Assignee: nobody → tjc
Status: NEW → ASSIGNED
Attachment #9271547 - Attachment description: WIP: Bug 1763877 - Root elements in TupleType::tuple_with r?jandem → Bug 1763877 - Root elements in TupleType::tuple_with r=jandem
Flags: needinfo?(jdemooij)
Flags: needinfo?(jdemooij)
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fbfe7de10894
Root elements in TupleType::tuple_with r=jandem
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: