This moves the logic for the debug name heuristic, which derives names
for imported and exported entities from the relevant tables, into
wasm-debug.{cc,h} and stores these maps on the DebugInfoImpl rather than
on the WasmModule.
Drive-by-fix: Also use the import table based heuristic for function
names, just like we use it for everything else.
Bug: chromium:1164305
Change-Id: I8a21e0880c680079f63e6607b5b62c788049b9e1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2625870
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72061}
This adds the following internal properties to `WasmInstanceObject`
values in DevTools:
- `[[Module]]` pointing to the `WasmModuleObject`, allowing the
developer to find the module to an instance no matter where in
DevTools front-end the instance is inspected.
- `[[Functions]]`, `[[Globals]]`, `[[Memories]]`, and `[[Tables]]`
are shown (when they aren't empty), allowing developers to inspect
the entities within an instance no matter where in DevTools front-end
it's inspected.
This also updates the _Module_ scope for Wasm frames to show the entity
containers (`functions`, `globals`, `memories` and `tables`) in addition
to the `instance` and `module` to make it easier accessible (fewer
clicks to get there), but also to align it better with the _Add property
path to Watch_ and _Copy property path_ features (since exactly the same
names are exposed via Debug Evaluate on Wasm frames).
```
> Stack
> Locals
v Module
> module
> instance
> functions
> globals
> memories
> tables
```
Drive-by-fix: Move GetWasmModuleObjectInternalProperties() logic into
debug-wasm-support.cc
Screenshot: https://imgur.com/ksEHG2I.png
Doc: http://bit.ly/devtools-wasm-entities
Fixed: chromium:1165294
Bug: chromium:1071432, chromium:1164241, chromium:1165304
Change-Id: Ia88fb2705287c79988ff2b432e4a33ac34e098f5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2622912
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72042}
This change unifies the locals, stack, and globals objects exposed for
WebAssembly frames via the Scope view and via DebugEvaluate to use the
same underlying objects (implemented via interceptors). This also
means that for locals and globals we now consistently expose names
prefixed by a dollar symbol everywhere.
Drive-by-fix: Move the debug::ScopeIterator implementation for WasmFrame
into debug-wasm-support.cc, so WebAssembly scope details are all found
in one place instead of scattered around the code.
Drive-by-cleanup: Rename GetJSDebugProxy to GetWasmDebugProxy for
consistency. GetJSDebugProxy is a bit misleading, since the debug proxy
is not about JavaScript, but just exposed to JavaScript.
Doc: http://bit.ly/devtools-wasm-entities
Bug: chromium:1159307, chromium:1127914, chromium:1162229
Change-Id: If932bd06bbce72542823f63dac1bd976ab33937a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2615348
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72009}
New internal properties expose the byte length of an ArrayBuffer as well
as the pointer to the backing store, which will serve as a unique ID
to show when SharedArrayBuffers in different workers are the same buffer.
Bug: chromium:1163800
Change-Id: I49930765cb38f75ba5c6cee5a0a6827f4fec42d5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2618242
Commit-Queue: Eric Leese <leese@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72000}
Previously the Debug Proxy API that's exposed on Wasm frames by
Runtime.evaluateOnCallFrame() was implemented via actual JSProxy
instances. That means that all entities such as "memories", "tables",
"stack", "globals", etc. were JSProxy instances with "get" and "has"
traps. But that has a couple of down-sides:
1. In DevTools front-end, the proxies are shown as JSProxy, which is not
very useful to developers, since they cannot interact with them nor
can they inspect their contents. And the object preview also only
shows "Proxy {}" for them.
2. The performance doesn't scale well, which becomes a painful
bottleneck with larger Wasm modules that contain hundreds of
thousands of functions or globals.
3. We cannot use the JSProxy instances in the Scope view (for the
reasons outlined in 1.) and hence we have different logic to provide
Scope values than values in the rest of DevTools, which led to subtle
but annoying bugs and inconsistencies.
This also changes the "locals" implementation by querying the values
ahead of time, similar to the object exposed to the Scope view, instead
of on-demand, since the "locals" object might survive the current
debugger pause and peeking into the stack afterwards would read invalid
memory (and might even be a security issue). For being able to change
locals we need to look into a similar solution as what we have for
JavaScript locals already. The expression stack already works this way.
For performance reasons (especially scaling to huge, realistic Wasm
modules), we cache the per-instance proxies ("functions", "memories",
"tables" and "globals") on the WasmInstanceObject and reuse them (which
is safe since they have a `null` prototype and are non-extensible), and
we also cache the proxy maps (with the interceptors) on the
JSGlobalObject per native context.
Doc: http://bit.ly/devtools-wasm-entities
Bug: chromium:1127914, chromium:1159402, chromium:1071432, chromium:1164241
Change-Id: I6191035fdfd887835ae533fcdaabb5bbc8e661ae
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2606058
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71981}
Previously we had introduced a special `v8::internal::WasmValue` type
which we used to expose Wasm values to the Scope view in Chromium
DevTools. The problem however is that these values cannot be exposed to
JavaScript (and in particular not to Debug Evaluate), which means that
particularly for v128 and i64 we have inconsistent representations
across the various parts of DevTools.
This change removes the `wasm` type from the RemoteObject and all the
adjacent logic, and paves the way for a uniform representation of Wasm
values throughout DevTools. For i64 we will simply use BigInt
consistently everywhere, and for i32, f32 and f64 we'll just use Number.
For externref we will represent the values as-is directly. For v128
values we currently use a Uint8Array, but will introduce a dedicated
WasmSimd128 class in a follow-up CL.
Bug: chromium:1071432
Fixed: chromium:1159402
Change-Id: I0671e5736c9c27d7ca376e23ed74f16d36e03c80
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2614428
Reviewed-by: Zhi An Ng <zhin@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71962}
The wasm-scope-info-liftoff.js and wasm-set-breakpoint-liftoff.js tests
were originally testing the Liftoff path (when we still had the Wasm
interpreter), and have received some updates along the way. Nowadays the
interpreter is going and the non-liftoff versions of these tests don't
provide any additional test coverage, but are merely a slightly less
updated version of the liftoff test.
Bug: chromium:1162229
Change-Id: Ifc9933d47f33674a83b99425ef9d0e4bc5550323
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2609415
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71909}
Consistently use InspectorTest.runAsyncTestSuite() in wasm inspector
tests to make tests easier to debug (they'll fail instead of timing
out in case of errors).
Bug: chromium:1162229, chromium:1071432
Change-Id: I7aada196f9e34071aa1bb059bb45f85f75226060
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2609414
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71908}
The "imports" and "exports" that were exposed on WebAssembly frames via
Debug-Evaluate aren't useful for the DWARF C/C++ extension (and likely
not for any other language extension), since they only expose static
information that's easily available (upfront) by reading the Wasm wire
bytes.
In fact, there are already standardized functions in the WebAssembly
specification, namely `WebAssembly.Module.imports(module)` and
`WebAssembly.Module.exports(module)`, which yield static information
about the imports and exports of a Wasm module.
So instead of exposing special, non-standard "imports" and "exports", we
now instead expose both the "instance" and the "module" objects via both
the Debug Proxy and the Scope view, and also add internal [[Exports]]
and [[Imports]] properties to WasmModuleObject, which under the hood use
the standard methods mentioned above.
Fixed: chromium:1162069
Bug: chromium:1071432, chromium:1083146
Screenshot: https://imgur.com/lcaW2jL.png
Doc: https://docs.google.com/document/d/1rqbu0jKTl3q_xCxLnKzkjGXWEsHnJ9aERVhKV9RNDgE#bookmark=id.925bb2qgou38
Change-Id: Ie27e55bb08ea5f90493c57375bf2b48dfb11a4d2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2606050
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71893}
For JSArrayBuffer instances (which map to both v8::ArrayBuffer and
v8::SharedArrayBuffer), we add a couple of synthetic views to its
ValueMirror to make it easy for developers to peak into the contents of
the JSArrayBuffer. These were previously real properties, but that's
just wrong (both intuitively and semantically), and they should instead
be internal properties.
Drive-by-fix: The [[IsDetached]] internal property should only be shown
on actually detached JSArrayBuffer's to reduce visual clutter. And for
detached JSArrayBuffers creating views on them throws TypeErrors per
specification, so we shouldn't attempt to display views on them.
Bug: v8:9308, chromium:1162229
Change-Id: Ia006de7873ca4b27aae7d00d46e1b69d2e326449
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2606047
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71892}
With https://crrev.com/c/2087396 we introduced a new CDP method
`Debugger.executeWasmEvaluator()`, which we originally intended
to use as the foundation for Debug-Evaluate on Wasm frames.
However in the process of prototyping we learned that it is too
costly and too inefficient to use WebAssembly modules here, and
we switched to regular Debug-Evaluate with JavaScript instead
(with a special debug proxy exposed that allows JavaScript to
peak into the Wasm frame), since JavaScript is better suited
for short-lived / short-running snippets and we don't need
clang and wasm-ld then to generate these snippets.
The JavaScript exposed debug proxy (as described in [1]) not
only enables more powerful and flexible Debug-Evaluate for the
DWARF C/C++ extension, but also serves as the basis for various
aspects of the Basic Wasm Developer Experience.
In order to pay down technical debt and to keep the maintenance
overhead low, we should remove the initial prototype now, also
to ensure that we don't accidentally attract other users of CDP
to rely on this unsupported API (despite it being marked as
"experimental").
[1]: https://docs.google.com/document/d/1VZOJrU2VsqOZe3IUzbwQWQQSZwgGySsm5119Ust1gUA
Fixed: chromium:1162062
Bug: chromium:1020120, chromium:1068571, chromium:1127914
Change-Id: I6dba8c906a8675ce6c29a52e3c32bb6626a27247
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2605186
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71882}
If we attempt to pause, we'd check whether frames are framework code
which we pattern match with a regexp. That could cause re-entering
regexp, which is not allowed.
Fixed: chromium:1125934
Change-Id: I3b52b202a5570f7929def39176cfe5e52be3dfd5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2602948
Commit-Queue: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71876}
JavaScript scopes are reported from inner-most to outer-most, while
previously we would report WebAssembly frames from outer-most to
inner-most. This is quite confusing for developers, and also doesn't
really make sense, so this CL fixes this inconsistency.
Bug: chromium:1071432
Change-Id: I6a4742f13b9a0df33e50c6fcd40992873996aaf5
Fixed: chromium:1159309
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2602947
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71875}
Since there is no dependence defined in gn, the other file will not be
uploaded to android devices for testing.
We could add this dependence, but not selectively for the one test which
actually needs that dependence. Hence fix it by duplicating the test
body instead.
R=mslekova@chromium.orgCC=machenbach@chromium.org
Change-Id: Ic65eea05a865cf4f521f66e293c4725bc2861444
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2577475
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71679}
Previously V8 would wrap the WebAssembly.Memory backing stores into
Uint8Arrays and report that as memories, but that's confusing to the
developer, since that's not what's really being used. The way that
DevTools presents the backing stores of memories, it's still perfectly
possible to get hold of an Uint8Array if that's what the developer is
looking for.
To make it possible to easily identify the WebAssembly.Memory objects
in the DevTools front-end (in particular for the memory inspector) we
add a 'webassemblymemory' subtype to the Chrome DevTools Protocol. We
also improve the description for the memories to include the number
of active pages.
Fixed: chromium:1155566
Screenshot: https://imgur.com/8enx57u.png
Change-Id: I63dbabe0e372e9ad6dcc8e6642cdb743147a620c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2574699
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71641}
The streaming decoder computed the code section start from the passed
"offset". That offset is computed from the module offset *after* the
number of functions has been read. Hence 1 is subtracted, with the
comment:
// The offset passed to {ProcessCodeSectionHeader} is an error offset and
// not the start offset of a buffer. Therefore we need the -1 here.
That subtraction of 1 worked when the number of functions was encoded in
a 1-byte LEB, otherwise it was off.
This CL fixes the immediate issue of passing the right code offset. The
usage of the previously existing offset also seems wrong, and I will try
to clean that up in a follow-up CL.
R=ahaas@chromium.orgCC=szuend@chromium.org
Bug: chromium:1150303
Change-Id: I64bb2ececeb4749b7ba2096cd148ccb4079eca4f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2562383
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71503}
This is a reland of 4719dae1a4.
The "V8 Linux64 TSAN - stress-incremental-marking" bot adds the
--stress-incremental-marking flag for all variants, hence the SKIP in
the status file was not triggered. We just explicitly disable the
--stress-incremental-marking flag for the two new tests. This works for
the "stress_incremental_marking" variant as well as the specific bot.
Original change's description:
> [wasm][inspector][test] Add more tests for code offsets
>
> The code offsets are sometimes wrong when compiled with streaming
> compilation. Thus add more tests for synchronous, asynchronous, and
> streaming compilation. The reported code offsets should all match. This
> will be fixed in a follow-up CL.
>
> In order to make asynchronous WebAssembly compilation finish, the
> inspector-test executable needs to pump the message loop before waiting
> for new tasks to come in, just as other executables like d8.
> This is added in this CL, but because of another bug this is skipped in
> the stress-incremental-marking variant. Hence the new tests are also
> skipped there.
>
> R=szuend@chromium.org
> CC=ahaas@chromium.org
>
> Bug: chromium:1150303, v8:10748
> Change-Id: Ie1d63c8d6795e61627d838b7fa7b21e6728befc0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2562382
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71483}
Bug: chromium:1150303
Bug: v8:10748
Change-Id: I9adb9fc0250fab5c43dc85b695f4d338a9c7183c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2565128
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71492}
This reverts commit 4719dae1a4.
Reason for revert: Timeouts with --stress-incremental-marking: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20stress-incremental-marking/1093
Original change's description:
> [wasm][inspector][test] Add more tests for code offsets
>
> The code offsets are sometimes wrong when compiled with streaming
> compilation. Thus add more tests for synchronous, asynchronous, and
> streaming compilation. The reported code offsets should all match. This
> will be fixed in a follow-up CL.
>
> In order to make asynchronous WebAssembly compilation finish, the
> inspector-test executable needs to pump the message loop before waiting
> for new tasks to come in, just as other executables like d8.
> This is added in this CL, but because of another bug this is skipped in
> the stress-incremental-marking variant. Hence the new tests are also
> skipped there.
>
> R=szuend@chromium.org
> CC=ahaas@chromium.org
>
> Bug: chromium:1150303, v8:10748
> Change-Id: Ie1d63c8d6795e61627d838b7fa7b21e6728befc0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2562382
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71483}
TBR=ahaas@chromium.org,clemensb@chromium.org,szuend@chromium.org
Change-Id: Ia4361183bfafeca3cc7d71ffe12d0ec2b0722b49
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1150303
Bug: v8:10748
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2565126
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71484}
The code offsets are sometimes wrong when compiled with streaming
compilation. Thus add more tests for synchronous, asynchronous, and
streaming compilation. The reported code offsets should all match. This
will be fixed in a follow-up CL.
In order to make asynchronous WebAssembly compilation finish, the
inspector-test executable needs to pump the message loop before waiting
for new tasks to come in, just as other executables like d8.
This is added in this CL, but because of another bug this is skipped in
the stress-incremental-marking variant. Hence the new tests are also
skipped there.
R=szuend@chromium.orgCC=ahaas@chromium.org
Bug: chromium:1150303, v8:10748
Change-Id: Ie1d63c8d6795e61627d838b7fa7b21e6728befc0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2562382
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71483}
This specific case was not implemented or tested before. Implementing it
actually simplifies some of the existing logic, since StepOut can now
reuse the generic logic in debug.cc for all cases (Wasm->Wasm, Wasm->JS,
JS->Wasm).
Drive-by:
1) Fix typo ("skip" -> "step").
2) Move the check for Liftoff code from debug.cc to wasm-debug.cc, where
it fits better.
3) Remove a TODO which is done already.
R=thibaudm@chromium.org, szuend@chromium.org
Bug: chromium:1145176
Change-Id: I415ca1d8bacef5b21bf1dafd9e16417ec2d12c7c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2560719
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71428}
Building these objects takes a lot of time and memory for realistic
applications and exposing them via the Scope view in DevTools isn't
practical either. We have a replacement in the Console now, and if
this needs more exposure we can think about other, more scalable
ways with better UX.
Fixed: v8:10986
Bug: chromium:1141781
Change-Id: I6177d63a987749889a9880cf0738031191eb5705
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2507696
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70894}
This is a reland of 8f7e915839
Original change's description:
> [debugger] Try to trigger pause-on-oom flakes with an extra printf
>
> We have an issue that we can't repro locally. Enable back the
> pause-on-oom tests with an extra printf with DEBUG. We will be able to
> better assess the failures when they appear on the bot.
>
> Bug: v8:10876
> Change-Id: I066539c4b5865ecb6f2e589e9543e8c9ebd4830b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474782
> Reviewed-by: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70558}
Bug: v8:10876
Change-Id: Ice31c9455830da320ab057293c341f69e1f0c510
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2484799
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70643}
This changes remoteObjectId format from
"{injectedScriptId:123,id:456}" to "<isolateId>.<contextId>.<id>".
Prepending isolateId fixes the problem that
remote object ids clash between processes. This is especially
troubling during cross-process navigation in Chromium, see bug.
We also stop producing and parsing unnecessary json for object ids.
Drive-by: fixed some tests dumping object ids. Most tests avoid
dumping unstable values like ids, but there were few that still did.
BUG=chromium:1137143
Change-Id: Ia019757fb95704ccb718d3ea6cc54bde1a133382
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461731
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70592}
It makes inspector tests a lot more readable if the opcode of the pause
location is being printed. Since we already have a list of all opcodes
available in wasm-module-builder.js, we can just reuse that to build a
reverse lookup map.
This CL implements this for single-byte opcodes only, which is enough
for all tests that we currently have. It will have to be extended for
prefixed opcodes once that is being used.
R=thibaudm@chromium.org, kimanh@chromium.org
Change-Id: I085fea99d2f5f2dc6cc084448e5f7444cce5c78b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474789
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70578}
This reverts commit 8f7e915839.
Reason for revert: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20-%20node.js%20integration%20ng/10707?
Original change's description:
> [debugger] Try to trigger pause-on-oom flakes with an extra printf
>
> We have an issue that we can't repro locally. Enable back the
> pause-on-oom tests with an extra printf with DEBUG. We will be able to
> better assess the failures when they appear on the bot.
>
> Bug: v8:10876
> Change-Id: I066539c4b5865ecb6f2e589e9543e8c9ebd4830b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474782
> Reviewed-by: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70558}
TBR=rmcilroy@chromium.org,petermarshall@chromium.org,solanes@chromium.org
Change-Id: I1b8a146d9496e889957636456b383f8d496658dc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10876
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2479004
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70559}
We have an issue that we can't repro locally. Enable back the
pause-on-oom tests with an extra printf with DEBUG. We will be able to
better assess the failures when they appear on the bot.
Bug: v8:10876
Change-Id: I066539c4b5865ecb6f2e589e9543e8c9ebd4830b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474782
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70558}
... at function start. Otherwise we run into a position mismatch:
In a non-flooded function, we add the function-entry breakpoint (for
"hook on function call") with the position of the first opcode.
In the flooded function though, we skip that special breakpoint because
we will stop at the first instruction anyway. But then the first
instruction is non-breakable, so we don't actually emit a breakpoint for
it.
Hence during OSR we do not find a corresponding position in the new
code.
This CL fixes this by postponing the function-entry breakpoint until the
first breakable opcode is found, and only emits it if that position does
not have a breakpoint anyway.
This way, we can also move the handling for function-entry breakpoints
from {StartFunctionBody} to {EmitDebuggingInfo}, where it fits much
better.
R=thibaudm@chromium.org
Bug: chromium:1137710
Change-Id: Idfa658fa0897cca89ba5ee3066cd414f68864d06
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474774
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70529}
Stepping always happens in Liftoff now, and always by byte offset. Thus
remove the redundant "wasm-stepping-byte-offset" test, which was fully
subsumed by "wasm-stepping-liftoff". Also, rename
"wasm-stepping-liftoff" to "wasm-stepping".
R=thibaudm@chromium.org
Bug: chromium:1137710
Change-Id: Ifb68ce795ecdcbb1f85500dc4be4c2e64d15a9c6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474116
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70528}
The call to "GetSpilledRegistersForInspection" was invalidated by the
call to "GetUnusedRegister" a few lines below.
R=clemensb@chromium.org
Bug: v8:10957
Change-Id: I1e0110d9b28ca23a2a8b9ff4b4c39143bfbe5510
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2466118
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70478}
Other WebAssembly tools like wabt and wasmparser ignore empty strings
for local variable and parameter names, and just generate their own
names for it. Update V8 to comply with this convention.
Bug: chromium:1134531
Change-Id: Ic724482d93398feaf6b0797eec5a55f8ca508ca5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2448457
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70305}
A break may cause the session disconnect (and therefore agents destruction)
on a nested message loop. The runtime agent code is generally prepared to
handle this during evaluate, but the code outside of it may be not. Besides,
having a break before the console API installed is generally not what
user wants or expects, so just disable all breaks while installing the API.
Bug: chromium:1122487
Change-Id: I1d40f5007f2e1e4ec07a50ef57988513d0309b7e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2437383
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70209}
For now, V128 values are converted to String16 (since they are not
serializable). It is shown as a list of 16 uint8_t (hex). This
description can be tweaked as necessary.
Some updates to ARM64 required to push/pop the full Q register.
Bug: v8:10347
Bug: chromium:1130474
Change-Id: I1bffbb49f47c06da3cd26d830addae0416a4441a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2422082
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Zhi An Ng <zhin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70096}
Instead of forcing GC right away, the function now post a task and
performance GC from the task with an empty stack to avoid false positive
pointers in conservative stack scanning.
Bug: chromium:1098187
Change-Id: I88864845a1e395056c5d5f6e867ad774b87dbb6a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2307217
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69444}
This extends the skip list feature from step over to step into.
On a step into we can pass a skipList, which contains locations
that we do not want to stop at.
Bug: chromium:1105765
Change-Id: I70a4ded3f6a7eada14f54ae9c2f994c155c7305b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2345224
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69376}
This change adds support for skipping locations that are in a skipList
on step over. This feature is useful for when we are debugging
C++ applications that have DWARF information we only want to stop on
every breakable location in C++, not non every breakable location
on wasm level.
Bug: chromium:1105765
Change-Id: Ie835b011a00cf31e0c5b2df1ac96ebd89f53d23a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339458
Reviewed-by: Eric Leese <leese@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69329}
Add missing source position for stack check, used by OSR to find the
correct return address.
R=clemensb@chromium.org
Bug: v8:10235
Change-Id: Ie26dd3b2079168e846f84b3a4ffe18b838649be7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339625
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69309}
Spill registers before stack checks so that we can inspect them, similar
to traps.
OSR during a stack check is still unsupported and will be fixed in a
follow-up CL.
R=clemensb@chromium.org
Bug: v8:10235
Change-Id: I22c2da6b3f79b30c3838c568f9680204afc85d36
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339467
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69277}
This function was only used in a single test, and it tests a scenario
which cannot happen any more with the module cache: Having two copies of
the same NativeModule in an isolate.
Hence remove the respective runtime function and the test.
R=ahaas@chromium.org
Change-Id: Id7cdffbdf1bdf95a7eb31fdeb7d75b8e326bb90e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339100
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69246}
Currently, only a scriptURL is reported, which can be over-written by
sourceURL comments of the script. This means a script can basically
claim to come from anywhere. This means that DevTools doesn't know the
resource name the embedder provided if there is a sourceURL comment.
This CL adds a `embedderName` field to the scriptParsed and
scriptFailedToParse events that reports the name the embedder
associated with the script.
Bug: chromium:974543
Change-Id: I9863f878f57638174847890d9a3818952b1efc27
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2317310
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69078}
Only the first four elements of the array will be used. Also, the fifth
element sais 'stepInfo' instead of 'stepInto'.
R=thibaudm@chromium.org
Change-Id: I258a8b95795f0cfbcaf500b7d174786680914d36
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2316110
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69062}
This adds an internal property [[IsDetached]] to the inspector preview
of ArrayBuffer instances, which indicates whether the ArrayBuffer was
detached (i.e. transfered via `postMessage`). Previously it was rather
impossible to tell whether an ArrayBuffer was detached, you had to know
that V8 violates the ECMAScript specification and simply sets the
byteLength accessor to 0 upon detaching an ArrayBuffer (but even then it
was still impossible to tell whether that ArrayBuffer wasn't simply an
empty one from the get go).
Before: https://imgur.com/UcOF83c
After: https://imgur.com/WjmTehZ
Fixed: chromium:1109102
Change-Id: I8fb6e2be2fbfe5c62b05dc9d2a0f18378eb4de6c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2316075
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69034}
For debugging code, disable opcode merging. Otherwise, the effect of the
first merged opcode would not be observable when stepping.
R=thibaudm@chromium.org
Bug: v8:10350
Change-Id: Id656c9dee8f9676bf3d7881f3782e5ead76b5e71
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2306802
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68960}
We currently still merge opcodes (i.e. i32 comparisons plus a br_if).
This CL adds a test for this, which checks for the current behaviour.
A follow-up CL will fix this and update the expected output accordingly.
R=thibaudm@chromium.org
Bug: v8:10350
Change-Id: I846aa931a3ec1a27043f04e830503d5732ae473e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2307232
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68957}
