Closed
Bug 1163520
Opened 10 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)
(deleted),
patch
|
shu
:
review+
|
Details | Diff | Splinter Review |
(deleted),
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•10 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•10 years ago
|
||
No, a Debugger.Object should never have an internal function object as its referent.
Flags: needinfo?(jimb)
Assignee | ||
Comment 3•10 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
|
||
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
|
||
Assignee | ||
Updated•9 years ago
|
Flags: in-testsuite+
OS: Unspecified → All
Hardware: Unspecified → All
Target Milestone: --- → mozilla42
Comment 15•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•