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}
This adds ExecutionContextDescription.uniqueId for a system-unique
way to identify an execution context and supports it in Runtime.evaluate.
This allows a client to avoid accidentally executing an expression
in a context different from that originally intended if a navigation
occurs while Runtime.evaluate is in flight.
Design doc: https://docs.google.com/document/d/1vGVWvKP9FTTX6kimcUJR_PAfVgDeIzXXITFpl0SyghQ
Bug: v8:11268, chromium:1101897
Change-Id: I4c6bec562ffc85312559316f639d641780144039
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2594538
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71869}
Test creates out-of-memory condition. Running that test in the
stress_concurrent_allocation variant might lead to "ineffective GCs"
failure before going OOM. Simply do not run this test for that variant.
Bug: v8:11272
Change-Id: I114686ec345f7a38f871347b62983d7591dc6ba3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2594769
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71788}
So far we reported the script ID, but DevTools ignores that and uses the
source url instead. That url was just set to "wasm ", which the frontend
couldn't make any sense of.
This CL fixes this by passing the source URL to the code create event,
and also setting the position of the code inside the script (i.e.
wasm module).
R=thibaudm@chromium.org, petermarshall@chromium.org
Bug: chromium:1125986
Change-Id: Ic41dcd2768c60fd6748468d3a89fc4ffccb35932
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2581543
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71695}
Function prototypes can be lazily allocated. This means they go into the
temporary objects set that debug-eval uses to figure out if a write
will be side-effect free.
We were incorrectly classifying writes to function prototypes as
side-effect free because the prototype happened to be lazily allocated
when we first accessed it during debug-eval, but was actually reachable
from the function (not allocated temporarily).
To do this we introduced a way to temporarily turn off the temporary
object tracking, and we use it when lazily allocating function
prototypes.
This could mean that we incorrectly report side-effects when writing to
function prototypes for functions which were themselves created during
debug-eval side-effect free mode. However, it's unclear if this is a
problem, because function declarations set global variables which would
already throw due to side-effects.
Bug: chromium:1154193
Change-Id: I444a673662095f6deabaafdce3cdf3d86b71446d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2581968
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71692}
Looks like this was accidentally added in https://crrev.com/c/979952.
The file is not loaded by any other test, hence we don't need the
dependency.
R=machenbach@chromium.org
Cq-Include-Trybots: luci.v8.try:v8_android_arm64_n5x_rel_ng
Change-Id: I02f25924980c02e6091bd5d275763adb66bd0b27
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2578977
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71682}
We currently report "wasm " as the source URL on all wasm code, with no
position information. This will change in a follow-up CL. To make that
difference visible, extend a test to show the URL and position reported
for wasm code.
R=thibaudm@chromium.org
Bug: chromium:1125986
Change-Id: I09f1820d591f27c1ff3c2acb41f8e279ac08a9e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2575071
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71680}
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}
This is a reland of ab4d9717f2.
The original CL did a std::move before the final use of the NativeModule.
PS2 removes that.
TBR=petermarshall@chromium.org, thibaudm@chromium.org
Original change's description:
> [wasm] Pass the script ID to code logging
>
> We didn't pass a script ID with the code creation events for profiling.
> This made DevTools lose the connection to the wasm script, hence
> jumping from the profiler entry to the source did not work.
>
> This CL changes the timing of code logging a bit such that the script is
> always allocated before logging. In the queue of code to be logged we
> then also store the script ID, and finally set it on the {CodeEntry}
> object.
>
> R=thibaudm@chromium.org
>
> Bug: chromium:1125986
> Change-Id: I2248c1d520bc819436bbe732373f7a3446b64f48
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2575057
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71654}
Bug: chromium:1125986
Cq-Include-Trybots: luci.v8.try:v8_linux64_ubsan_rel_ng
Change-Id: I2a7c5fe04fff726836b1279e3d05b1702a4efb76
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2578980
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71663}
This reverts commit ab4d9717f2.
Reason for revert: UBSan issues: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20UBSan/14184/overview
Original change's description:
> [wasm] Pass the script ID to code logging
>
> We didn't pass a script ID with the code creation events for profiling.
> This made DevTools lose the connection to the wasm script, hence
> jumping from the profiler entry to the source did not work.
>
> This CL changes the timing of code logging a bit such that the script is
> always allocated before logging. In the queue of code to be logged we
> then also store the script ID, and finally set it on the {CodeEntry}
> object.
>
> R=thibaudm@chromium.org
>
> Bug: chromium:1125986
> Change-Id: I2248c1d520bc819436bbe732373f7a3446b64f48
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2575057
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71654}
TBR=petermarshall@chromium.org,clemensb@chromium.org,thibaudm@chromium.org
Change-Id: I03c90c77b55e770797a6d66b1d778992a047e07a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1125986
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2575070
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71660}
We didn't pass a script ID with the code creation events for profiling.
This made DevTools lose the connection to the wasm script, hence
jumping from the profiler entry to the source did not work.
This CL changes the timing of code logging a bit such that the script is
always allocated before logging. In the queue of code to be logged we
then also store the script ID, and finally set it on the {CodeEntry}
object.
R=thibaudm@chromium.org
Bug: chromium:1125986
Change-Id: I2248c1d520bc819436bbe732373f7a3446b64f48
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2575057
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71654}
Import wrappers were only logged if logging was enabled during
compilation. If the profiler is enabled later, and regular wasm code is
logged via {NativeModule::LogWasmCodes}, the import wrappers were
missing.
This CL fixes the long-standing TODO, and adds tests which triggered
that code path. Those tests were hanging before because the expected
functions did never appear in the profile.
Drive-by: If {WasmEngine::LogOutstandingCodesForIsolate} detects that
code logging is disabled by now, it should still clear the {code_to_log}
vector.
R=thibaudm@chromium.org
Bug: chromium:1125986, chromium:1141787
Change-Id: I2566ef369bb61a09488f2d932b6c10d92e4cb12f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2574696
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71645}
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}
We currently do not report a script ID for wasm code, i.e. the script id
is 0. We cannot just print the script ID itself, as it is considered
unstable. Thus this CL only makes us print whether it is set or not.
In a follow-up CL where we fix setting script IDs for wasm code events
the output will change.
R=thibaudm@chromium.org
Bug: chromium:1125986
Change-Id: Ibc52829ea8a5a5c9506e36390eb4c608bcab4624
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2571120
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71616}
This change completes the necessary API changes for import assertions
discussed in
https://docs.google.com/document/d/1yuXgNHSbTAPubT1Mg0JXp5uTrfirkvO1g5cHHCe-LmY.
The old ResolveCallback is deprecated and replaced with a
ResolveModuleCallback that includes import assertions. Until
ResolveCallback is removed, InstantiateModule and associated functions
are modified to accept both types of callback, using the new one if it
was supplied and the old one otherwise. An alternative that I chose not
to go with would be to just duplicate InstantiateModule and associated
functions for both callback types.
SyntheticModule::PrepareInstantiate's callback parameter was unused so I
removed it.
Bug: v8:10958
Change-Id: I8e9fbaf9c2853b076b13da02473fbbe039b9db57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2551919
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Dan Clark <daniec@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#71506}
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}
Since one of the latest Clang rolls, ASan builds on MacOS appear
to be using bigger stack frames, so reduce the maximum recursion
depth a bit in that configuration.
Fixed: v8:11176
Change-Id: I00942194a6c4d8046ec6abd24219912ebd153e57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2563465
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71501}
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}
Function tables have been removed from the scope object in
https://crrev.com/c/2507696, hence the code for printing them is dead
now.
R=bmeurer@chromium.org
Change-Id: Ib36fb314ae54468239737f100a6594d8d2031218
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2557982
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71412}
- Use C++ primitives (int, bool) for the ScriptOrigin constructor.
- Deprecate the old accessors and constructor
Bug: v8:11195
Change-Id: I739edd6b4c58e19a8a16ddce863eea14ec933697
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555005
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71384}
We had a test which first enabled the profiler, and then compiled wasm
code. In this case, all code objects were registered correctly and the
profile looked as expected.
This CL extends the test for also test another order: First compile the
wasm code, then enable the profiler. In that case, we were reporting a
wrong debug name of the exported wasm function. The name of that
function is spec'ed to be the string representation of the function
index. But for debugging, we want to see a more meaningful name,
identical to the name we show when reporting the code during
compilation.
This fix requires handlifying the {SharedFunctionInfo::DebugName}
method, because for exported wasm functions, it needs to allocate a new
name on the JS heap.
In order to avoid this allocation where possible, a second variant is
added which returns a unique_ptr directly. This can be used in all
places where the name is just being printed, which turned out to be the
majority of cases ({DebugName().ToCString()}).
R=petermarshall@chromium.org
Bug: chromium:1141787
Change-Id: I0343c2f06f0b852007535ff07459b712801ead01
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2543931
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71308}
Add a requirement to String::GetChars that we either have a string
access lock, or a string access lock is not needed. This prevents us
from reading strings during internalization that may be in the middle
of being made external.
To avoid taking the lock too often when known to be unnecessary (e.g.
for strings that were only just created), there's now a static
SharedStringAccessGuardIfNeeded::NotNeeded(). This is hopefully ugly
enough that it's used sparingly.
One fix required for this is to enter the Isolate when tearing down
IsolateData in inspector tests -- this is so that the V8Inspector
instance being torn down will see the current Isolate and be able to
verify its thread id against the current thread.
Bug: chromium:1011762, chromium:1148680
Change-Id: Ic5d29c1b066ebae5a351c7b4bb116b9b1bf61889
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2536465
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71197}
Currently, we assume that stack trace creation always succeeds while
filling in the `exceptionDetails` structure. Stack trace creation can
fail under some circumstances so this CL introduces a null check.
R=clemensb@chromium.org
Bug: chromium:1147552
Change-Id: I4055d5276bbb7bf178b648bfc7bd84a288626c09
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2532310
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71169}
The fuzzer is expected to generate a lot of syntax and runtime errors,
and the respective messages just flood the fuzzer output. By always
putting a {TryCatch} scope around the execution, we prevent those
messages from being printed.
At the same time, inspector tests need to properly propagate uncaught
exceptions in the backend to the inspector, and fail on uncaught
exceptions in the frontend.
This CL allows for all these behaviours by extending the
{CatchExceptions} enum and the {TryCatch} logic in the task runner.
Drive-by: Use {base::OS::ExitProcess} instead of the explicit
{fflush} and {_exit}.
R=szuend@chromium.org
Bug: chromium:1142437
Change-Id: Ic2cb3b0de2399d25bd8c53090575308cb0e09ab0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2529135
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71152}
For the fuzzer it's unwise to exit on uncaught exceptions, as this
terminates the whole fuzzing process. Just ignore those exceptions
instead.
Drive-by: Fix a typo.
R=szuend@chromium.org
Bug: chromium:1142437
Change-Id: Ided1c0f35840c158f157acd8c0bb1c12ecf8a37f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2526386
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71059}
Instead of passing two bools to the {TaskRunner} constructor, pass to
enums. This makes the semantics more clear in the caller.
In the fuzzer, we actually *do not* want to catch exceptions. This
semantic fix will be done in a follow-up CL, such that this CL is a pure
refactoring.
R=szuend@chromium.org
Bug: v8:11074
Change-Id: I7f6df3a3f344524deb08db10b9317a6734b7ea42
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2526385
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71056}
Joining the thread from the watchdog is problematic, since e.g.
{pthread_join} (the implementation of {Thread::Join} on POSIX systems)
has undefined behaviour if multiple threads try to join at the same
time. In practice, this leads to deadlocks.
Thus implement termination by just calling {TaskRunner::Terminate}, but
not {TaskRunner::Join}. This fixes the deadlocks in the inspector
fuzzer.
The inspector test binary is fixed simarly, even though there it seems
to not cause problems so far.
In both files, the {Terminate} function is inlined into callers because
it's only a single line now, with one to two users.
Also, replace the single fuzzer test (which is invalid javascript) by
two tests: One called "invalid" explicitly, still with invalid
javascript, and one empty file, which is valid input. That one
reproduced the deadlock.
R=szuend@chromium.org
Bug: chromium:1142437
Change-Id: I8fb98b0cdbf3ceff6af6849397e5da5a4e9acd3c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2526384
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71054}
Asan complains about the alloc-dealloc-mismatch because the startup data
is allocated via "new[]" in snapshot.cc and deallocated via "delete" in
inspector-test.cc.
A more failure-proof fix would be to have {StartupData} manage the
lifetime of the contained char*, but since this is in an API object, the
refactoring might be more involved. Since other users also just dealloc
explicitly via "delete[]", this CL just fixes the issue in
inspector-test.cc.
R=szuend@chromium.org
Bug: chromium:1142437, v8:11107
Change-Id: I84438b2f12ce8eb6b653d4861e899a2f003e1227
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2523200
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71045}
The {ToV8Vector} method returns a {i::Vector} pointing to heap-allocated
memory, but that memory was never free'd. Since we already have a
{ToVector} method returning a {std::vector}, this CL switches to that
one instead.
R=szuend@chromium.org
Bug: chromium:1142437, v8:11107
Change-Id: I8ee0177f7dcfe2ecb435e684674b0cda6f613658
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2523198
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71040}
Rename the field "deffered_queue_" to "deferred_queue_".
R=szuend@chromium.orgCC=yangguo@chromium.org
Bug: chromium:1142437
Change-Id: I004082b7a798c8b7df92c7adea32e71cb11d7bef
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2520899
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71009}
While writing a new test I realized that the test did not fail if
running into a CHECK or UNREACHABLE *after* printing the last expected
line.
That is because both stderr and the exit status are ignored. This CL
fixes that.
This will uncover a lot of memory leaks, which I plan to address in
follow-up CLs.
R=machenbach@chromium.org
CC=szuend@chromium.org
Bug: chromium:1142437, v8:11107
Cq-Include-Trybots: luci.v8.try:v8_linux64_asan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_mac64_asan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_win64_asan_rel_ng
Change-Id: I65f325abf102e063bb4f449353c47e94d84de652
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2519567
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71006}
Keep tasks in unique_ptrs, such that they are freed independent of
whether they have been executed or not.
R=szuend@chromium.org
Bug: chromium:1142437, v8:11107, v8:11074
Change-Id: Ia265df3187c724b63e0f576d33235c1bfa522c4f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2517694
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71005}
The inspector fuzzer is running into timeouts most of the time
currently, because the test explicitly needs to quit execution.
Make fuzzing more efficient by adding a watchdog thread which stop
execution after 2 seconds. This will still result in valid test cases,
i.e. everything that was executed within those two seconds will count as
covered code.
Drive-by: Slightly simplify the storage of task runners. No need to
clear the vector after termination.
R=szuend@chromium.org
Bug: chromium:1142437, chromium:1145285
Change-Id: I7b5fe7ddcbce731fbc3d74ee8c43f7249f34b918
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2520906
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71002}
