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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: ochameau, Assigned: jimb)

References

Details

Attachments

(2 files, 2 obsolete files)

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 :/
Blocks: 1023386
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)
No, a Debugger.Object should never have an internal function object as its referent.
Flags: needinfo?(jimb)
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.
Flags: needinfo?(poirot.alex)
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)
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
Okay, that's very helpful. Let me debug this...
QA Contact: jimb
Assignee: nobody → jimb
QA Contact: jimb
Attachment #8604803 - Attachment is obsolete: true
Attachment #8615588 - Flags: review?(shu)
Attachment #8615587 - Flags: review?(shu) → review+
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+
Yay, devtools failures! Functioning like a good assertion should...
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.
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
Flags: in-testsuite+
OS: Unspecified → All
Hardware: Unspecified → All
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.