Last Comment Bug 745802 - JS OOM Testing: Crash [@ js::HeapId::operator jsid]
: JS OOM Testing: Crash [@ js::HeapId::operator jsid]
Status: RESOLVED FIXED
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- critical (vote)
: mozilla15
Assigned To: Jason Orendorff [:jorendorff]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 624094
  Show dependency treegraph
 
Reported: 2012-04-16 10:13 PDT by Christian Holler (:decoder)
Modified: 2012-05-05 03:34 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (897 bytes, patch)
2012-04-26 09:08 PDT, Jason Orendorff [:jorendorff]
bhackett1024: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-04-16 10:13:58 PDT
The following command crashes on mozilla-central revision fd06332733e5 (dbg build):

js -m -n -a -A 8936 -f js/src/jit-test/tests/debug/Environment-identity-02.js
Comment 1 Christian Holler (:decoder) 2012-04-16 10:15:47 PDT
This crash looks like a null-pointer deref in Valgrind at first glance. However, the test itself (without valgrind) does not reliably reproduce which could indicate a memory corruption problem:


==10637== Invalid read of size 8
==10637==    at 0x405206: js::HeapId::operator jsid() const (Barrier.h:451)
==10637==    by 0x4059C1: js::types::Property::getKey(js::types::Property*) (jsinfer.h:644)
==10637==    by 0x44A484: js::types::Property* js::types::HashSetLookup<jsid, js::types::Property, js::types::Property>(js::types::Property**, unsigned int, jsid) (jsinferinlines.h:994)
==10637==    by 0x427548: js::types::TypeObject::maybeGetProperty(JSContext*, jsid) (jsinferinlines.h:1277)
==10637==    by 0x4E717C: ObjectStateChange(JSContext*, js::types::TypeObject*, bool, bool) (jsinfer.cpp:1641)
==10637==    by 0x4EBAE9: js::types::TypeObject::setFlags(JSContext*, unsigned int) (jsinfer.cpp:2967)
==10637==    by 0x42728D: js::types::MarkTypeObjectFlags(JSContext*, JSObject*, unsigned int) (jsinferinlines.h:403)
==10637==    by 0x6E9B44: js::mjit::Compiler::compile() (Compiler.cpp:154)
==10637==    by 0x6ECE69: js::mjit::CanMethodJIT(JSContext*, JSScript*, unsigned char*, bool, js::mjit::CompileRequest) (Compiler.cpp:992)
==10637==    by 0x78C955: UncachedInlineCall(js::VMFrame&, js::InitialFrameFlags, void**, bool*, unsigned int) (InvokeHelpers.cpp:308)
==10637==    by 0x78D151: js::mjit::stubs::UncachedCallHelper(js::VMFrame&, unsigned int, bool, js::mjit::stubs::UncachedCallResult*) (InvokeHelpers.cpp:459)
==10637==    by 0x78CEA9: js::mjit::stubs::UncachedCall(js::VMFrame&, unsigned int) (InvokeHelpers.cpp:416)
==10637==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
Comment 2 Christian Holler (:decoder) 2012-04-16 10:17:49 PDT
The test seems to be jsdbg2 only, so removing s-s and Ccing jsdbg2 devs.
Comment 3 Jason Orendorff [:jorendorff] 2012-04-25 13:22:34 PDT
Out of memory first happens here:

#0  js_ReportOutOfMemory (cx=0x100b152a0) at /Users/jorendorff/dev/mi/js/src/jscntxt.cpp:362
#1  0x00000001000df2bb in js::types::TypeCompartment::setPendingNukeTypes (this=0x1010534e0, cx=0x100b152a0) at /Users/jorendorff/dev/mi/js/src/jsinfer.cpp:2115
#2  0x00000001000e6944 in js::types::TypeObject::addProperty (this=0x100f12220, cx=0x100b152a0, id={asBits = 4}, pprop=0x100f12240) at /Users/jorendorff/dev/mi/js/src/jsinfer.cpp:2768
#3  0x000000010033fdbb in js::types::TypeObject::getProperty (this=0x100f12220, cx=0x100b152a0, id={asBits = 4}, assign=false) at jsinferinlines.h:1251
#4  0x00000001000e8099 in js::types::TypeSet::HasObjectFlags (cx=0x100b152a0, object=0x100f12220, flags=524288) at /Users/jorendorff/dev/mi/js/src/jsinfer.cpp:1626

This causes TypeObject::getProperty to return false with this->propertySet == NULL.

We error out; but later we do eventually try to run this code again, and then

#0  0x00000001000b86e4 in js::HeapId::operator jsid (this=0x0) at Barrier.h:466
#1  0x000000010014b559 in js::types::Property::getKey (p=0x0) at jsinfer.h:644
#2  0x0000000100024622 in js::types::HashSetLookup<jsid, js::types::Property, js::types::Property> (values=0x0, count=1, key={asBits = 4}) at jsinferinlines.h:994
#3  0x0000000100050a03 in js::types::TypeObject::maybeGetProperty (this=0x100f12220, cx=0x100b152a0, id={asBits = 4}) at jsinferinlines.h:1277
#4  0x00000001000df3ba in ObjectStateChange (cx=0x100b152a0, object=0x100f12220, markingUnknown=false, force=false) at /Users/jorendorff/dev/mi/js/src/jsinfer.cpp:1641
#5  0x00000001000e2b4d in js::types::TypeObject::setFlags (this=0x100f12220, cx=0x100b152a0, flags=524288) at /Users/jorendorff/dev/mi/js/src/jsinfer.cpp:2967
#6  0x000000010005048e in js::types::MarkTypeObjectFlags (cx=0x100b152a0, obj=0x100f227c0, flags=524288) at jsinferinlines.h:403
#7  0x000000010031bae8 in js::mjit::Compiler::compile (this=0x7fff5fbf8a20) at /Users/jorendorff/dev/mi/js/src/methodjit/Compiler.cpp:154

we crash because propertySet is NULL. I confirmed that the TypeObject in frame 3 is the same one from before.
Comment 4 Jason Orendorff [:jorendorff] 2012-04-25 14:22:10 PDT
Oh, we bump the property count and then we can fail without rolling that back.

1250	        setBasePropertyCount(propertyCount);
1251	        if (!addProperty(cx, id, pprop))
1252	            return NULL;
Comment 5 Jason Orendorff [:jorendorff] 2012-04-26 09:08:17 PDT
Created attachment 618679 [details] [diff] [review]
v1
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2012-05-05 03:34:15 PDT
https://hg.mozilla.org/mozilla-central/rev/5e80edf4c2dd

Note You need to log in before you can comment on or make changes to this bug.