build: add flag to build V8 with OBJECT_PRINT by mmarchini · Pull Request #32834 · nodejs/node · GitHub

Add a configure flag to build V8 with -DOBJECT_PRINT, which will
expose auxiliar functions to inspect heap objects using native
debuggers.

Fixes: #32402
Signed-off-by: Mary Marchini mmarchini@netflix.com

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

@mmarchini Just landed #32787 so you can avoid conflicts with it :)

So it works on gdb, but not on lldb (all commands give empty results on lldb). One caveat though: any command that requires v8:: will not work, which includes jlh, one of the most useful commands. That's not a concern of this PR, but something we should be aware of (and it means this PR doesn't fix #32402).

@mmarchini jlh works when you replace v8::internal::Object with void in its definition 🤷‍♀️

I'd be okay with turning it on unconditionally. We already do that for the builtin disassembler and that pulls in a lot more code than does object printing.

diff --git a/common.gypi b/common.gypi
index d39668b100..7026f41565 100644
--- a/common.gypi
+++ b/common.gypi
@@ -53,6 +53,9 @@
     # Enable disassembler for `--print-code` v8 options
     'v8_enable_disassembler': 1,
 
+    # Sets -dOBJECT_PRINT.
+    'v8_enable_object_print%': 1,
+
     # https://github.com/nodejs/node/pull/22920/files#r222779926
     'v8_enable_handle_zapping': 0,

I'd be okay with turning it on unconditionally. We already do that for the builtin disassembler and that pulls in a lot more code than does object printing.

The binary maybe much bigger. If we make it enable by default, maybe need to check binary size first. If the size turns out okay, I think we can close #32402.

The binary maybe much bigger. If we make it enable by default, maybe need to check binary size first. If the size turns out okay, I think we can close #32402.

Shouldn't be that much, but I'll double check.

cc @nodejs/v8 any performance or stability concerns if we enable OBJECT_PRINT by default (considering we already have ENABLE_DISASSEMBLER enabled by default)?

@gengjiawen 72724 Kb (current build) vs 72972 Kb (with OBJECT_PRINT), so 248 Kb increase, which is not concerning IMO.

This shouldn't affect anything beyond binary size and more verbose printing. It makes sense for debugging.

The idea is to set it by default on release builds as well, so native module developers can use those debugging features as well.

If you can live with the binary size increase that sounds fine. This code isn't battle tested, but it seems somewhat unlikely to cause unexpected issues; especially if you don't accept random untrusted input as JS.

Thanks

I'll follow up with a PR to set this as default (as suggested by @bnoordhuis).

@mmarchini This needs rebase.

Add a configure flag to build V8 with `-DOBJECT_PRINT`, which will expose auxiliar functions to inspect heap objects using native debuggers. Fixes: nodejs#32402 Signed-off-by: Mary Marchini <mmarchini@netflix.com>

Landed in dd0c522

Add a configure flag to build V8 with `-DOBJECT_PRINT`, which will expose auxiliar functions to inspect heap objects using native debuggers. Fixes: #32402 Signed-off-by: Mary Marchini <mmarchini@netflix.com> PR-URL: #32834 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org> Reviewed-By: Jiawen Geng <technicalcute@gmail.com>

The flag improves the experience of debugging V8 with native debuggers. It doens't incur performance penality, the only downside is an increase in binary size by approximately 248 Kb. Ref: nodejs#32834

The flag improves the experience of debugging V8 with native debuggers. It doens't incur performance penality, the only downside is an increase in binary size by approximately 248 Kb. Ref: #32834 PR-URL: #34705 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>