--cpu-prof crashes in debug builds when code cache is enabled #27307
Labels
confirmed-bug
Issues with confirmed bugs.
inspector
Issues and PRs related to the V8 inspector protocol
v8 engine
Issues and PRs related to the V8 dependency.
Comments
2 tasks
joyeecheung
added
inspector
|
Another stack trace without optimized debug |
joyeecheung
added a commit
that referenced
this issue
Apr 19, 2019
The CPU profiler crashes in debug builds when code cache is enabled. Skip the test temporarily until it's fixed. PR-URL: #27308 Refs: #27307 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
|
https://gist.github.com/addaleax/139800e79fd687776f409edbaf35b981 fixes this for me, locally – @joyeecheung does that change look right to you? I can open a CL if you think so, but I haven’t done this kind of change in V8 before… like, whether this is enough or the handle itself should be passed as the argument instead of dereferencing it for the function calls, etc. |
|
@addaleax I am not sure if this is sufficient either..please open a CL in the upstream, thanks! |
2 tasks
|
Is this fixed on master now ? $ out/Debug/node -p process.features
{
inspector: true,
debug: true,
uv: true,
ipv6: true,
tls_alpn: true,
tls_sni: true,
tls_ocsp: true,
tls: true,
cached_builtins: true
}
$ out/Debug/node --cpu-prof test/fixtures/workload/fibonacci.js
9227465 |
|
@gengjiawen Not until https://chromium-review.googlesource.com/c/v8/v8/+/1575698 is landed but it depends on GC to reproduce so it could disappear at times |
|
@nodejs/v8-update I think it would take a while for https://chromium-review.googlesource.com/c/v8/v8/+/1575698 to roll back into master. Should we cherry pick this? It does reproduce on master in tests, we only disabled a particular build config that is more likely to trigger the crash. |
|
Sure. Anything can be cherry-picked if there is a good reason for it. |
joyeecheung
added a commit
to joyeecheung/node
that referenced
this issue
Apr 26, 2019
Original commit message:
[snapshot] Use Handle to track name in `CodeSerializer::Deserialize`
The `Script::InitLineEnds(Handle<Script>(script, isolate));` line
may lead to objects being moved around on the heap, so it’s necessary
to use a `Handle` to track that.
This was causing crashes in Node.js in Debug mode when using the
code cache in combination with the CPU profiler.
Refs: nodejs#27307
Change-Id: I392b4c00c6ebad44753f87fcbf2e3278ea7799a6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1575698
Reviewed-by: Jakob Gruber <[email protected]>
Reviewed-by: Peter Marshall <[email protected]>
Commit-Queue: Peter Marshall <[email protected]>
Cr-Commit-Position: refs/heads/master@{#61036}
Refs: v8/v8@5d0cf6b
oleavr
pushed a commit
to frida/v8
that referenced
this issue
Apr 26, 2019
The `Script::InitLineEnds(Handle<Script>(script, isolate));` line may lead to objects being moved around on the heap, so it’s necessary to use a `Handle` to track that. This was causing crashes in Node.js in Debug mode when using the code cache in combination with the CPU profiler. Refs: nodejs/node#27307 Change-Id: I392b4c00c6ebad44753f87fcbf2e3278ea7799a6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1575698 Reviewed-by: Jakob Gruber <[email protected]> Reviewed-by: Peter Marshall <[email protected]> Commit-Queue: Peter Marshall <[email protected]> Cr-Commit-Position: refs/heads/master@{#61036}
targos
pushed a commit
that referenced
this issue
Apr 29, 2019
Original commit message:
[snapshot] Use Handle to track name in `CodeSerializer::Deserialize`
The `Script::InitLineEnds(Handle<Script>(script, isolate));` line
may lead to objects being moved around on the heap, so it’s necessary
to use a `Handle` to track that.
This was causing crashes in Node.js in Debug mode when using the
code cache in combination with the CPU profiler.
Refs: #27307
Change-Id: I392b4c00c6ebad44753f87fcbf2e3278ea7799a6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1575698
Reviewed-by: Jakob Gruber <[email protected]>
Reviewed-by: Peter Marshall <[email protected]>
Commit-Queue: Peter Marshall <[email protected]>
Cr-Commit-Position: refs/heads/master@{#61036}
Refs: v8/v8@5d0cf6b
PR-URL: #27423
Fixes: #27307
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos
pushed a commit
to targos/node
that referenced
this issue
May 1, 2019
Original commit message:
[snapshot] Use Handle to track name in `CodeSerializer::Deserialize`
The `Script::InitLineEnds(Handle<Script>(script, isolate));` line
may lead to objects being moved around on the heap, so it’s necessary
to use a `Handle` to track that.
This was causing crashes in Node.js in Debug mode when using the
code cache in combination with the CPU profiler.
Refs: nodejs#27307
Change-Id: I392b4c00c6ebad44753f87fcbf2e3278ea7799a6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1575698
Reviewed-by: Jakob Gruber <[email protected]>
Reviewed-by: Peter Marshall <[email protected]>
Commit-Queue: Peter Marshall <[email protected]>
Cr-Commit-Position: refs/heads/master@{#61036}
Refs: v8/v8@5d0cf6b
PR-URL: nodejs#27423
Fixes: nodejs#27307
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This currently fails the debug build on master, the CPU profiler crashes when code cache is enabled (for some reason,
thisinv8::internal::ProfilerListener::InferScriptNameturns into a nullptr in the middle of the profiling).See stack trace
It does not crash if I build it with
code_cache_stub.cc, or build it in release mode.cc @nodejs/v8 @nodejs/v8-inspector @psmarshall
The text was updated successfully, but these errors were encountered: