Closed
Bug 1387115
Opened 7 years ago
Closed 7 years ago
[wasm] Expose WebAssembly instance memory and globals via debugger scope
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: yury, Assigned: yury)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
Currently, we provide access to the wasm function locals via scope we are creating for the debugger. The wasm code often uses its (internal) memory and globals too, so it will be nice to inspects these values too during debugging as well. A WebAssembly memory is just an object that contains ArrayBuffer. Currently, no devtools can display content of an ArrayBuffer. So some type of watch expression needs to be provided to inspect memory content, e.g. `new DataView(memory0.buffer).getUint32(0x1000, true)`. For that we need to implement an eval functionality for the wasm debug frames.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8893485 [details] Bug 1387115 - Expose WebAssembly instance memory and globals via debugger scope. https://reviewboard.mozilla.org/r/164556/#review169972 Great! ::: js/src/vm/Scope.cpp:1264 (Diff revision 1) > +/* static */ WasmInstanceScope* > +WasmInstanceScope::create(JSContext* cx, WasmInstanceObject* instance) > +{ > + Rooted<WasmInstanceScope*> wasmInstanceScope(cx); > + > + { Is this { } block wrapping most of the function body necessary? ::: js/src/wasm/WasmDebug.cpp:572 (Diff revision 1) > + break; > + case ValType::F32: > + vp.set(NumberValue(value.f32())); > + break; > + case ValType::F64: > + vp.set(NumberValue(value.f64())); I think you have to CanonicalizeNaN() these since the bits come from the .wasm. Actually, could you also add CanonicalizeNaN() the f32/f64 constant cases in GetGlobalExport()? It's missing there for the same reason. ::: js/src/wasm/WasmJS.cpp:1065 (Diff revision 1) > > + Rooted<WasmInstanceScope*> instanceScope(cx, WasmInstanceScope::create(cx, obj)); > + if (!instanceScope) > + return nullptr; > + > + obj->initReservedSlot(INSTANCE_SCOPE_SLOT, PrivateGCThingValue(instanceScope)); Could you create the WasmInstanceScope lazily, changing the infallible WasmInstanceObject::scope() to a fallible getScope()?
Attachment #8893485 -
Flags: review?(luke) → review+
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8893486 [details] Bug 1387115 - Allow debugger to eval using WebAssembly frames. https://reviewboard.mozilla.org/r/164558/#review170028 Nice! Could you add tests for all these new paths?
Attachment #8893486 -
Flags: review?(luke) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8893485 [details] Bug 1387115 - Expose WebAssembly instance memory and globals via debugger scope. https://reviewboard.mozilla.org/r/164556/#review169972 > Is this { } block wrapping most of the function body necessary? I moved comment from WasmFunctionScope::create about CGPtr for Data structure that explain block. However it is not longer needed in WasmFunctionScope::create, shall I remove { } block wrapping there?
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
(In reply to Yury Delendik (:yury) from comment #7) Ah, I see now this is an idiomatic pattern introduced so it's clear that the Rooted<> has to be the outermost stack lifetime. n/m this comment, but it does make sense to take out the {} for WasmFunctionScope::create() now that it has no such hazard.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
Pushed by ydelendik@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a301b2aa1267 Expose WebAssembly instance memory and globals via debugger scope. r=luke https://hg.mozilla.org/integration/autoland/rev/4f6eab9d705f Allow debugger to eval using WebAssembly frames. r=luke
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a301b2aa1267 https://hg.mozilla.org/mozilla-central/rev/4f6eab9d705f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Updated•7 years ago
|
Blocks: wasm-tools
You need to log in
before you can comment on or make changes to this bug.
Description
•