Closed Bug 932530 Opened 6 years ago Closed 6 years ago

Assertion failure: thing, at gc/Marking.cpp:126 or Crash [@ tenuredZone] with OOM

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: decoder, Assigned: terrence)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, crash, testcase)

Crash Data

Attachments

(4 files)

The following testcase asserts on mozilla-central revision 518f5bff0ae4 (run with --ion-eager):


function f(x) {
    "use strict";
    return delete (b);
}
const y = 1 + Math.pow(2, -52);
var diff = function (a, b) {
    f[0] = a;
    f[1] = b;
};
diff(1, y);
Terrence, this is one of my all time favorite long standing OOM bugs that never reduced, and now it did. Can you find someone to look and fix?
Crash Signature: [@ tenuredZone]
Flags: needinfo?(terrence)
Keywords: crash
Summary: Assertion failure: thing, at gc/Marking.cpp:126 with OOM → Assertion failure: thing, at gc/Marking.cpp:126 or Crash [@ tenuredZone] with OOM
Blocks: 912928
Flags: needinfo?(terrence)
I was not able to reproduce on the given revision with:

CC="gcc -m32" CXX="g++ -m32" ./configure --target=i686-linux-gnu --enable-optimize --enable-debug --enable-debug-symbols --enable-valgrind --enable-gczeal --enable-more-deterministic --enable-methodjit --enable-type-inference --enable-profiling --with-system-nspr --disable-tests

With gcc 4.6.3 or 4.7.3.
Manged to repro with --disable-threadsafe. It only reproduces in opt+debug builds.

Program received signal SIGSEGV, Segmentation fault.
0x080dc222 in CheckMarkedThing<js::types::TypeObject> (trc=trc@entry=0x937e170, thing=thing@entry=0x0) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:126
126	    JS_ASSERT(thing);
(gdb) bt
#0  0x080dc222 in CheckMarkedThing<js::types::TypeObject> (trc=trc@entry=0x937e170, thing=thing@entry=warning: (Internal error: pc 0x0 in read in psymtab, but not in symtab.)

0x0) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:126
#1  0x080e1b22 in MarkInternal<js::types::TypeObject> (trc=0x937e170, thingp=<optimized out>) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:174
#2  0x083d761a in js::jit::ICStub::trace (this=0x9426308, trc=0x937e170) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jit/BaselineIC.cpp:244
#3  0x08417664 in trace (trc=0x937e170, this=0x93fe618) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jit/BaselineJIT.cpp:424
#4  js::jit::BaselineScript::Trace (trc=0x937e170, script=0x93fe618) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jit/BaselineJIT.cpp:441
#5  0x08238f38 in JSScript::markChildren (this=0xf782d180, trc=0x937e170) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsscript.cpp:2752
#6  0x080e05e7 in MarkChildren (trc=0x937e170, script=0xf782d180) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:1041
#7  PushMarkStack (thing=0xf782d180, gcmarker=0x937e170) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:819
#8  MarkInternal<JSScript> (trc=0x937e170, thingp=0xf7833694) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:193
#9  0x0817736e in JSFunction::trace (this=(JSFunction * const) 0xf7833680 [object Function "diff"], trc=0x937e170) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsfun.cpp:514
#10 0x080ee5d8 in js::GCMarker::processMarkStackTop (this=this@entry=0x937e170, budget=...) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:1480
#11 0x080ea434 in js::GCMarker::drainMarkStack (this=0x937e170, budget=...) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/gc/Marking.cpp:1533
#12 0x08179c21 in DrainMarkStack (rt=rt@entry=0x937df40, sliceBudget=..., phase=phase@entry=js::gcstats::PHASE_MARK) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsgc.cpp:3869
#13 0x08187233 in IncrementalCollectSlice (rt=rt@entry=0x937df40, budget=0x883c2b0, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT, gckind=gckind@entry=js::GC_NORMAL) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsgc.cpp:4385
#14 0x08187fdc in GCCycle (rt=rt@entry=0x937df40, incremental=incremental@entry=0x0, budget=0x0, gckind=gckind@entry=js::GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsgc.cpp:4555
#15 0x08188534 in Collect (rt=rt@entry=0x937df40, incremental=incremental@entry=0x0, budget=0x0, gckind=gckind@entry=js::GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsgc.cpp:4699
#16 0x081889cd in js::GC (rt=0x937df40, gckind=js::GC_NORMAL, reason=JS::gcreason::DESTROY_CONTEXT) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsgc.cpp:4724
#17 0x08148070 in js::DestroyContext (cx=0x9390ce0, mode=js::DCM_FORCE_GC) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jscntxt.cpp:267
#18 0x080f98bc in JS_DestroyContext (cx=0x9390ce0) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/jsapi.cpp:858
#19 0x0804ef4d in DestroyContext (cx=cx@entry=0x9390ce0, withGC=withGC@entry=0x1) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/shell/js.cpp:5316
#20 0x0805fe4e in main (argc=0x3, argv=0xffffcb84, envp=0xffffcb94) at /home/terrence/moz/branch/api_rooting/mozilla/js/src/shell/js.cpp:5951

So it looks like the |type| field a baseline script's stub of kind SetElem_DenseAdd is getting set to zero.
The issue was a missing handler on a failed getType when generating the baseline stub. Unfortunately, this exposes a second issue: we're not always doing a syntax parse of the method with the invalid delete.
Or rather, my breakpoint was failing to hit in an opt build. Re-doing in a debug build, we have a better picture.

This is indeed doing the SyntaxParse, but because |(b)| is wrapped in parens, we're getting into |Node SyntaxParser::setInParens(Node pn) { return NodeGeneric; }|. Thus when we test handler.isName in TOK_DELETE, we find |delete <GenericNode>|, rather than |delete <NameNode>| and fail to error. |setInParens| has the comment "String literals enclosed by parentheses are ignored during strict mode parsing", which appears to not be quite true when generating errors.
Lets check in the first fix now without the test. I'll file a new bug to track the second issue.
Assignee: general → terrence
Status: NEW → ASSIGNED
Attachment #825963 - Flags: review?(kvijayan)
See Also: → 933809
Comment on attachment 825963 [details] [diff] [review]
oom_932530-v0.diff

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

Right.. getType is fallible..
Attachment #825963 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/173d837f2b5f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Fwiw, I am still seeing exactly this signature in fuzzing (both the assertion and the crash). Could it be that we have more places where we don't properly check that fallible call?
Flags: needinfo?(terrence)
I digged through the js code and found that most return values are checked immediately. However, these three occurrences in the baseline compiler might need a fix:

http://mxr.mozilla.org/mozilla-central/source/js/src/jit/BaselineIC.cpp#4733
http://mxr.mozilla.org/mozilla-central/source/js/src/jit/BaselineIC.cpp#9330
http://mxr.mozilla.org/mozilla-central/source/js/src/jit/BaselineIC.h#4953

Let me know if that helps :)
Thanks Christian! I made a note to do exactly this today; now I can move right to patching.
Flags: needinfo?(terrence)
And the others Christian dug up.
Attachment #826888 - Flags: review?(kvijayan)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #826888 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/mozilla-central/rev/208198d2bbdd
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Keywords: verifyme
Not reproducible locally with the testcase in comment 0 - tried with the 10/30 Fx28 JS shell on Ubuntu 13.04 32bit.

I am also not seeing this signature in Socorro. Please let me know if there is anything more specific I need to do to reproduce and verify this bug.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.