1252453 - Crash [@ js::ReportAllocationOverflow] or Assertion failure: !mEntered, at mozilla/ReentrancyGuard.h:39 with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision e15383656900 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --disable-debug, run with --fuzzing-safe --no-threads --disable-oom-functions):

lfcode = new Array();
gczeal(8,2);
lfcode.push(`
const root = newGlobal();
const dbg = new Debugger;
const wrappedRoot = dbg.addDebuggee(root);
dbg.memory.trackingAllocationSites = 1;
root.eval("(" + function immediate() { '_'  << foo } + "())");
`);
file = lfcode.shift();
loadFile(file);
function loadFile(lfVarx) {
    try {
        function newFunc(x) Function(x)(); 
        newFunc(lfVarx)();
    } catch (lfVare) {
        print(lfVare)
    }
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
js::ReportAllocationOverflow (cxArg=0xe5e5e5e5) at js/src/jscntxt.cpp:371
#0  js::ReportAllocationOverflow (cxArg=0xe5e5e5e5) at js/src/jscntxt.cpp:371
#1  0x08484868 in mozilla::Vector<js::Debugger::AllocationsLogEntry, 0u, js::TempAllocPolicy>::growStorageBy (this=this@entry=0xf637eda0, aIncr=aIncr@entry=1) at js/src/opt32/dist/include/mozilla/Vector.h:881
#2  0x0846c9eb in growByUninitialized (aIncr=1, this=0xf637eda0) at js/src/opt32/dist/include/mozilla/Vector.h:1028
#3  emplaceBack<JS::Rooted<JSObject*>&, double&, char const*&, JS::Rooted<JSAtom*>&, unsigned long long&, bool&> (this=0xf637eda0) at js/src/opt32/dist/include/mozilla/Vector.h:598
#4  emplaceBack<JS::Rooted<JSObject*>&, double&, char const*&, JS::Rooted<JSAtom*>&, unsigned long long&, bool&> (this=0xf637ed88) at js/src/ds/Fifo.h:111
#5  js::Debugger::appendAllocationSite (this=0xf637ed30, cx=cx@entry=0xf7a74040, obj=obj@entry=..., frame=frame@entry=..., when=when@entry=1456843454063,405) at js/src/vm/Debugger.cpp:2016
#6  0x0846cc91 in js::Debugger::slowPathOnLogAllocationSite (cx=cx@entry=0xf7a74040, obj=obj@entry=..., frame=frame@entry=..., when=when@entry=1456843454063,405, dbgs=...) at js/src/vm/Debugger.cpp:1959
#7  0x0853ac19 in onLogAllocationSite (when=1456843454063,405, frame=..., obj=0xf656e010, cx=0xf7a74040) at js/src/vm/Debugger.h:1120
#8  js::SavedStacksMetadataCallback (cx=0xf7a74040, target=...) at js/src/vm/SavedStacks.cpp:1429
#9  0x08385be0 in JSCompartment::setNewObjectMetadata (this=0xf63c6800, cx=cx@entry=0xf7a74040, obj=obj@entry=...) at js/src/jscompartment.cpp:893
#10 0x083f3da9 in SetNewObjectMetadata (obj=0xf656e010, cxArg=0xf7a74040) at js/src/jsobjinlines.h:303
#11 create (group=..., shape=..., heap=<optimized out>, kind=<optimized out>, cx=0xf7a74040) at js/src/jsobjinlines.h:377
#12 NewObject (cx=cx@entry=0xf7a74040, group=..., group@entry=..., kind=kind@entry=js::gc::OBJECT4, newKind=newKind@entry=js::GenericObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/jsobj.cpp:668
#13 0x083f4131 in js::NewObjectWithGivenTaggedProto (cxArg=cxArg@entry=0xf7a74040, clasp=clasp@entry=0x94eb100 <js::ScriptSourceObject::class_>, proto=proto@entry=..., allocKind=js::gc::OBJECT4, newKind=newKind@entry=js::GenericObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/jsobj.cpp:729
#14 0x08420602 in NewObjectWithGivenTaggedProto (initialShapeFlags=0, newKind=js::GenericObject, proto=..., clasp=0x94eb100 <js::ScriptSourceObject::class_>, cx=0xf7a74040) at js/src/jsobjinlines.h:628
#15 NewObjectWithGivenProto (newKind=js::GenericObject, proto=..., clasp=0x94eb100 <js::ScriptSourceObject::class_>, cx=0xf7a74040) at js/src/jsobjinlines.h:663
#16 js::ScriptSourceObject::create (cx=cx@entry=0xf7a74040, source=source@entry=0xf64f9a00) at js/src/jsscript.cpp:1724
#17 0x085e3ab0 in js::frontend::CreateScriptSourceObject (cx=cx@entry=0xf7a74040, options=...) at js/src/frontend/BytecodeCompiler.cpp:692
#18 0x08425a22 in CreateEmptyScriptForClone (cx=cx@entry=0xf7a74040, enclosingScope=..., enclosingScope@entry=..., src=src@entry=...) at js/src/jsscript.cpp:3687
#19 0x08428055 in js::CloneScriptIntoFunction (cx=cx@entry=0xf7a74040, enclosingScope=enclosingScope@entry=..., fun=fun@entry=..., src=src@entry=...) at js/src/jsscript.cpp:3737
#20 0x08545a8a in JSRuntime::cloneSelfHostedFunctionScript (this=0xf7a39000, cx=cx@entry=0xf7a74040, name=name@entry=..., targetFun=targetFun@entry=...) at js/src/vm/SelfHosting.cpp:2461
#21 0x083c6681 in JSFunction::createScriptForLazilyInterpretedFunction (cx=cx@entry=0xf7a74040, fun=fun@entry=...) at js/src/jsfun.cpp:1514
#22 0x0850d8a1 in getOrCreateScript (cx=0xf7a74040, this=<optimized out>) at js/src/jsfun.h:413
#23 js::Invoke (cx=0xf7a74040, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:481
#24 0x0850dfea in js::Invoke (cx=cx@entry=0xf7a74040, thisv=..., fval=..., argc=0, argv=0xffffbdc8, rval=...) at js/src/vm/Interpreter.cpp:530
#25 0x08435352 in js::DirectProxyHandler::call (this=this@entry=0x94eb3d0 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0xf7a74040, proxy=proxy@entry=..., args=...) at js/src/proxy/DirectProxyHandler.cpp:77
#26 0x08437074 in js::CrossCompartmentWrapper::call (this=0x94eb3d0 <js::CrossCompartmentWrapper::singleton>, cx=0xf7a74040, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:289
#27 0x08439378 in js::Proxy::call (cx=cx@entry=0xf7a74040, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:391
#28 0x0843a0d9 in js::proxy_Call (cx=cx@entry=0xf7a74040, argc=0, vp=0xffffbdb8) at js/src/proxy/Proxy.cpp:683
#29 0x0850da03 in CallJSNative (args=..., native=0x843a080 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, cx=0xf7a74040) at js/src/jscntxtinlines.h:235
[...]
#53 main (argc=5, argv=0xffffcc44, envp=0xffffcc5c) at js/src/shell/js.cpp:7244
eax	0xe5e5e5e5	-437918235
ebx	0x94b9e58	155950680
ecx	0xe5e5e5e5	-437918235
edx	0xe5e5e5e5	-437918235
esi	0xe5e5e5e5	-437918235
edi	0xffffb638	-18888
ebp	0xe5e5e5e5	3857049061
esp	0xffffb410	4294947856
eip	0x806c8aa <js::ReportAllocationOverflow(js::ExclusiveContext*)+24>
=> 0x806c8aa <js::ReportAllocationOverflow(js::ExclusiveContext*)+24>:	cmpl   $0x0,0xd4(%esi)
   0x806c8b1 <js::ReportAllocationOverflow(js::ExclusiveContext*)+31>:	jne    0x806c8eb <js::ReportAllocationOverflow(js::ExclusiveContext*)+89>

Comment 1

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Debugger-related + intermittent, setting needinfo? from Jim/Nick to see if the stack makes sense here.

Comment 3

[Jim Blandy :jimb]

I can reproduce. Taking.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Thanks!

Comment 5

[Jim Blandy :jimb]

Okay, I may be being confused by GDB or just my own head, but here's what I *think* is going on. needinfo'ing terrence.

1) We enter js::Debugger::slowPathOnLogAllocationSite, which creates a Rooted<GCVector<JSObject*>> and stores one Debugger JSObject pointer in it.

2) js::Debugger::appendAllocationSite calls JSCompartment::wrap, which causes a GC (js::gc::GCRuntime::runDebugGC).

3) The GC decides the Debugger in the Rooted GCVector is garbage, and calls js::Debugger::~Debugger.

At no time between 1) and 3) does control reach js::RootLists::traceStackRoots.

Comment 6

[Jim Blandy :jimb]

This behavior occurs on the last (in my case, the eighth) call to js::Debugger::slowPathOnLogAllocationSite; and indeed, it doesn't call the traceStackRoots.

But fitzgen just pointed out what the problem almost certainly is:

slowPathOnLogAllocationSite only grew that rooting attempt recently. It's rooting Debuggers that it's just taken out of some GlobalObject's DebuggerVector, *which hold its elements weakly*.

Taking weakly held objects and sticking them in a Rooted doesn't work. The snapshot-at-the-beginning invariant assumes that every object to be retained is either 1) reachable by strong references, 2) preserved by a pre-barrier, or 3) freshly allocated.

So we need a read barrier when someone extracts Debuggers from a GlobalObject's debugger vector.

NOW THAT'S WHAT I CALL THINKING

gonna get fitzgen to analyze ALL my bugs for me from now on

Comment 7

[Jim Blandy :jimb]

So, Terrence, my needinfo from you has become:

What's the best form for that read barrier to take?

Comment 8

[Terrence Cole [:terrence]]

(In reply to Jim Blandy :jimb from comment #7)
> So, Terrence, my needinfo from you has become:
> 
> What's the best form for that read barrier to take?

Is there some reason that wrapping the pointer in ReadBarriered<T> would not work? I'll look closer at the type signature tomorrow.

Comment 9

[Jim Blandy :jimb]

Attachment: Support a 'no-threads' cookie in jit-tests.

Comment 10

[Jim Blandy :jimb]

Attachment: make Debugger::slowPathOnLogAllocationSite apply a read barrier to Debugger objects.

Comment 11

[Jim Blandy :jimb]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=58cd69803ac6

Comment 12

[Jim Blandy :jimb]

Comment on attachment 8727661 [details] [diff] [review]
Support a 'no-threads' cookie in jit-tests.

There's no need for this; I didn't realize that jit-test metalines let you pass flags directly.

Comment 13

[Jim Blandy :jimb]

I've filed bug 1254376 for the larger problem of needing a read barrier on GlobalObjects' debugger vectors.

Comment 14

[Jim Blandy :jimb]

Try push looks good.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d94c77de685

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1d94c77de685

Comment 17

[Eric Faust [:efaust]]

terrence, how big a deal is this? Do we need to uplift to 47?

Comment 18

[Terrence Cole [:terrence]]

I've no idea! Jim, should we uplift this?

Comment 19

[Jim Blandy :jimb]

I don't think this needs to be in 47 at all. I don't know why I marked mozilla47 as its target milestone.

Comment 20

[Eric Faust [:efaust]]

WONTFIX 47 based on comment 19.