Closed
Bug 1163520
Opened 9 years ago
Closed 9 years ago
Assertion when using DebuggerAPI getOwnPropertyNames on "Internal function object"
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: ochameau, Assigned: jimb)
References
Details
Attachments
(2 files, 2 obsolete files)
4.58 KB,
patch
|
shu
:
review+
|
Details | Diff | Splinter Review |
4.75 KB,
patch
|
Details | Diff | Splinter Review |
While adding a new call to DebuggerAPI Object.getOwnPropertyNames, I'm hitting the following assertion: Assertion failure: !IsInternalFunctionObject(obj), at /mnt/desktop/gecko/js/src/jsfun.cpp:378 #0 0x00007ffff55bb8ed in ResolveInterpretedFunctionPrototype (obj=..., cx=0x7fffe99934e0) at /mnt/desktop/gecko/js/src/jsfun.cpp:378 #1 fun_resolve (cx=0x7fffe99934e0, obj=..., id=..., resolvedp=0x7ffffffedd50) at /mnt/desktop/gecko/js/src/jsfun.cpp:458 #2 0x00007ffff51461b5 in CallResolveOp (recursedp=<synthetic pointer>, propp=..., id=..., obj=..., cx=0x7fffe99934e0) at /mnt/desktop/gecko/js/src/vm/NativeObject-inl.h:411 #3 js::LookupOwnPropertyInline<(js::AllowGC)1> (cx=cx@entry=0x7fffe99934e0, obj=obj@entry=..., id=id@entry=..., propp=..., propp@entry=..., donep=donep@entry=0x7ffffffede10) at /mnt/desktop/gecko/js/src/vm/NativeObject-inl.h:504 #4 0x00007ffff51464b4 in js::NativeHasProperty (cx=cx@entry=0x7fffe99934e0, obj=..., obj@entry=..., id=..., foundp=foundp@entry=0x7ffffffeded0) at /mnt/desktop/gecko/js/src/vm/NativeObject.cpp:1517 #5 0x00007ffff555b4bc in HasProperty (foundp=0x7ffffffeded0, id=..., obj=..., cx=0x7fffe99934e0) at /mnt/desktop/gecko/js/src/vm/NativeObject.h:1402 #6 fun_enumerate (cx=0x7fffe99934e0, obj=...) at /mnt/desktop/gecko/js/src/jsfun.cpp:68 #7 0x00007ffff5596929 in Snapshot (cx=cx@entry=0x7fffe99934e0, pobj_=..., flags=flags@entry=24, props=props@entry=0x7ffffffee1d0) at /mnt/desktop/gecko/js/src/jsiter.cpp:313 #8 0x00007ffff5596bbd in js::GetPropertyKeys (cx=cx@entry=0x7fffe99934e0, obj=..., obj@entry=..., flags=flags@entry=24, props=props@entry=0x7ffffffee1d0) at /mnt/desktop/gecko/js/src/jsiter.cpp:421 #9 0x00007ffff50bc870 in DebuggerObject_getOwnPropertyNames (cx=0x7fffe99934e0, argc=<optimized out>, vp=<optimized out>) at /mnt/desktop/gecko/js/src/vm/Debugger.cpp:6870 http://mxr.mozilla.org/mozilla-central/source/js/src/jsobjinlines.h#562 562 /* 563 * Return true if this is a compiler-created internal function accessed by 564 * its own object. Such a function object must not be accessible to script 565 * or embedding code. 566 */ 567 inline bool 568 IsInternalFunctionObject(JSObject* funobj) If that can help, I'm hitting this assertion while running this test: http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/tests/unit/test_breakpoint-21.js#54 It asserts while running this line: 54 packet = yield waitForPause(gClient); That ends up calling the new obj.getOwnPropertyNames I've added here: https://github.com/ochameau/mozilla-central/commit/c5e54e7cad7aaacd68e117a12640806cb72d5063#diff-58d9dfd9ca670d23e714c743a045d90eR3450 The object it fails getting the property names is a function, it stringifies to: function () { return (function() { return (function() { return (function() { return (function() { var x = 10; // This line gets a breakpoint return 1; })(); })(); })(); })(); } Which is the function we are testing in this xpcshell test. Should we add a if (!IsInternalFunctionObject(obj)) somewhere in the debugger API? Otherwise I would have to badly workaround that :/
Comment 1•9 years ago
|
||
Hmm. So IsInternalFunctionObject() is the lambda template that an actual lambda will get cloned from. Should debugger even have access to these things??? It sounds like we're getting the property names for one.
Flags: needinfo?(jimb)
Assignee | ||
Comment 2•9 years ago
|
||
No, a Debugger.Object should never have an internal function object as its referent.
Flags: needinfo?(jimb)
Assignee | ||
Comment 3•9 years ago
|
||
Alex, try rebuilding with this patch applied. It should catch the point at which we try to create a Debugger.Object instance that refers to an internal function; that's where the real problem has begun.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(poirot.alex)
Reporter | ||
Comment 4•9 years ago
|
||
I get this stack now, but I imagine that doesn't help? #0 0x00007ffff50eb644 in js::Debugger::wrapDebuggeeValue (this=this@entry=0x7fffe4bb7000, cx=cx@entry=0x7fffe98984e0, vp=...) at /mnt/desktop/gecko/js/src/vm/Debugger.cpp:809 #1 0x00007ffff50ec9b7 in DebuggerEnv_getCallee (cx=0x7fffe98984e0, argc=<optimized out>, vp=<optimized out>) at /mnt/desktop/gecko/js/src/vm/Debugger.cpp:7615 #2 0x00007ffff511dd72 in js::CallJSNative (cx=0x7fffe98984e0, native=0x7ffff50ec6d0 <DebuggerEnv_getCallee(JSContext*, unsigned int, JS::Value*)>, args=...) at /mnt/desktop/gecko/js/src/jscntxtinlines.h:235
Flags: needinfo?(poirot.alex)
Reporter | ||
Comment 5•9 years ago
|
||
Here is more information, that should be helpful this time. Note that you can easily reproduce this assertion while running: ./mach xpcshell-test toolkit/devtools/server/tests/unit/test_breakpoint-21.js --debugger gdb So it throws during this test, while we are waiting for pause event, here: https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/tests/unit/test_breakpoint-21.js#54 packet = yield waitForPause(gClient); The assertion is being thrown from this line: https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/script.js#3357 form.type = this.obj.callee ? "function" : "block"; Here is the JS stack: EnvironmentActor.prototype.form@resource://gre/modules/devtools/server/actors/script.js:3371:12 EnvironmentActor.prototype.form@resource://gre/modules/devtools/server/actors/script.js:3380:22 FrameActor.prototype.form@resource://gre/modules/devtools/server/actors/script.js:3110:26 ThreadActor.prototype._paused@resource://gre/modules/devtools/server/actors/script.js:1537:22 ThreadActor.prototype._pauseAndRespond@resource://gre/modules/devtools/server/actors/script.js:690:20 BreakpointActor.prototype.hit@resource://gre/modules/devtools/server/actors/script.js:3311:12 a<@/mnt/desktop/gecko/obj-firefox/_tests/xpcshell/toolkit/devtools/server/tests/unit/test_breakpoint-21.js:77:1
Assignee | ||
Comment 6•9 years ago
|
||
Okay, that's very helpful. Let me debug this...
Assignee | ||
Updated•9 years ago
|
QA Contact: jimb
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jimb
QA Contact: jimb
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8615587 -
Flags: review?(shu)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8604803 -
Attachment is obsolete: true
Attachment #8615588 -
Flags: review?(shu)
Assignee | ||
Comment 9•9 years ago
|
||
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bbee7fdd5ebb
Updated•9 years ago
|
Attachment #8615587 -
Flags: review?(shu) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8615588 [details] [diff] [review] Don't hand out internal function objects via Debugger.Environment.prototype.callee. Review of attachment 8615588 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the fix! ::: js/src/jit-test/tests/debug/Environment-callee-04.js @@ +18,5 @@ > + return 1; > + } > + })()(); > + > + `); Aside: template strings sure are nice.
Attachment #8615588 -
Flags: review?(shu) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Yay, devtools failures! Functioning like a good assertion should...
Assignee | ||
Comment 12•9 years ago
|
||
browser/devtools/debugger/test/browser_dbg_blackboxing-02.js hits the assertion. It calls Debugger.Environment.prototype.getVariable to fetch the value of the variable 'one'; said value is an internal function object named "one", referring to the function of that name from browser/devtools/debugger/test/code_blackboxing_blackboxme.js: function blackboxme(fn) { (function one() { (function two() { (function three() { fn(); }()); }()); }()); } Naturally, CallObject::createHollowForDebug calls CallObject::createForFunction, which constructs a DeclEnvObject that refers to the callee. In the hollow case, that will be the internal function.
Assignee | ||
Comment 13•9 years ago
|
||
Here's a revised patch that addresses that browser mochitest crash. Fresh try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1c4f6b96dc4
Attachment #8615588 -
Attachment is obsolete: true
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a41a3957b1c https://hg.mozilla.org/integration/mozilla-inbound/rev/827e17b69b0f
Assignee | ||
Updated•9 years ago
|
Flags: in-testsuite+
OS: Unspecified → All
Hardware: Unspecified → All
Target Milestone: --- → mozilla42
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6a41a3957b1c https://hg.mozilla.org/mozilla-central/rev/827e17b69b0f
You need to log in
before you can comment on or make changes to this bug.
Description
•