1461616 - wasm: anyref, Debugger and wrapping

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P3)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

While working on bug 1461337, I failed to understand the behavior of a few test cases. If I take the test case from there, and modify it like this:

var g = newGlobal();
g.parent = this;
g.eval(`
    var obj = { somekey: 'somevalue' };

    Debugger(parent).onEnterFrame = function(frame) {
        let v = frame.environment.getVariable('var0');
        assertEq(typeof v === 'object', true);
        assertEq(typeof v.somekey === 'string', true);
        assertEq(v.somekey === 'somevalue', true);
    };
`);
new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
  (module
    (func (export "func") (result anyref) (param $ref anyref)
        get_local $ref
    )
)`))).exports.func(g.obj);


That is, the child global creates an object; that object is then used as an argument in the function call that happens in the parent global. In this case, the second and third assertions both fail.

I suspected that this is the case because g.obj is wrapped on the call site. If I unwrap it in the interpreter entry, I can observe (with JSAPI C++ code) that the `somekey` key is present on the unwrappee; then in the debugger code (wasm::DebugFrame::getValue), it's still present. But the assertions still fail!

If I change the test case so that the `obj` object belongs to the parent global, the assertions both fail too.

I've tried many combinations in which there's no Debugger, and just two globals interacting with each other. In one scenario the parent global has the object, the child has the function and the function call happens in the parent or child; both assertions pass. In another scenario, the child global has the object, the parent has the function and the function call happens in either; both assertions pass.

So I think the issue is related to the Debugger itself, which might add supplementary wrapping. I see that the last thing DebuggerEnvironment::getVariable does before returning is wrapping its result value: https://searchfox.org/mozilla-central/source/js/src/vm/Debugger.cpp#11268

Any thoughts/ideas about what's going on here? Or who could I ping about this? Is the behavior intended and I am just misunderstanding something here?

Comment 1

[Luke Wagner [:luke]]

You are correct sir!  What you're seeing is a Debugger.Object wrapper.  The idea of it is to prevent the debugger from accidentally directly invoking debuggee code (which is so easy with proxies and valueOf etc).  See:
  https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Environment
  https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Object
I think you can call Debugger.Object.prototype.unwrap() to get the underlying object, or write the test to do it "the right way" using Debugger.Object methods to probe the debuggee value.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: test.patch

I took some time to get back on this. It's actually worse than I thought: the test wasn't testing anything. The onEnterFrame hook was never called, because the wasm call was happening in the new global that's just been created. So fixing as suggested in comment 0 made it possible to actually test something.

The documentation was referring to Debugger.Object.getProperty, which seems to be non-existing at this time, so I removed it from the docs too.

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 8988424 [details] [diff] [review]
test.patch

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

Thanks!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa826be7a24a
Fix wasm/gc/debugger test so it actually tests something; r=luke

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 4 changesets (bug 1461616, bug 1450261) for wasm related spidermonkey build bustages.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=aa826be7a24a31c2f2448d5e796b0f83eec278a8

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/82e8c7cfd3462f0087f316ef0dcfc769b95b3deb

https://treeherder.mozilla.org/logviewer.html#?job_id=185577665&repo=mozilla-inbound

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/971e33a5eaa2
Fix wasm/gc/debugger test so it actually tests something; r=luke

Comment 7

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/971e33a5eaa2