[tools] Improve gcmole part II

Prepare gcmole.cc for the next update:
- Print possible GC locations when discovering stale/dead variables
- Make error messages less confusing for the modern V8 engineer
- Prepare gcmole to read suspects.allowlist instead of .whitelist
- Use more readable variable names
- Only log non-found types with --verbose
- Change the currently unusued gccauses format in gcmole.py and
  support loading it back in gcmole.cc
- Implemented first basic gc call-chain printing (disabled by default)

GCmole packaging:
- Add debug mode to bootstrap.sh build script
- Update gcmole.py run instructions in bootstrap.sh and package.sh

Bug: v8:10009
Change-Id: I369d48baa2980455d2e8f57e7a803d0384fe83f1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3480095Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79357}

src/wasm/c-api.cc

@@ -2293,12 +2293,20 @@ struct borrowed_vec {
 
 // Vectors
 
+#ifdef V8_GC_MOLE
+#define ASSERT_VEC_BASE_SIZE(name, Name, vec, ptr_or_none)
+
+#else
+#define ASSERT_VEC_BASE_SIZE(name, Name, vec, ptr_or_none)                 \
+  static_assert(sizeof(wasm_##name##_vec_t) == sizeof(vec<Name>),          \
+                "C/C++ incompatibility");                                  \
+  static_assert(                                                           \
+      sizeof(wasm_##name##_t ptr_or_none) == sizeof(vec<Name>::elem_type), \
+      "C/C++ incompatibility");
+#endif
+
 #define WASM_DEFINE_VEC_BASE(name, Name, vec, ptr_or_none)                     \
-  static_assert(sizeof(wasm_##name##_vec_t) == sizeof(vec<Name>),              \
-                "C/C++ incompatibility");                                      \
-  static_assert(                                                               \
-      sizeof(wasm_##name##_t ptr_or_none) == sizeof(vec<Name>::elem_type),     \
-      "C/C++ incompatibility");                                                \
+  ASSERT_VEC_BASE_SIZE(name, Name, vec, ptr_or_none)                           \
   extern "C++" inline auto hide_##name##_vec(vec<Name>& v)                     \
       ->wasm_##name##_vec_t* {                                                 \
     return reinterpret_cast<wasm_##name##_vec_t*>(&v);                         \

src/wasm/wasm-code-manager.h

@@ -495,7 +495,9 @@ class V8_EXPORT_PRIVATE WasmCode final {
 // often for rather small functions.
 // Increase the limit if needed, but first check if the size increase is
 // justified.
+#ifndef V8_GC_MOLE
 STATIC_ASSERT(sizeof(WasmCode) <= 88);
+#endif
 
 WasmCode::Kind GetCodeKind(const WasmCompilationResult& result);
 

tools/gcmole/Makefile

@@ -32,11 +32,18 @@ LLVM_BUILD_INCLUDE:=$(BUILD_ROOT)/include
 CLANG_SRC_INCLUDE:=$(CLANG_SRC_ROOT)/include
 CLANG_BUILD_INCLUDE:=$(BUILD_ROOT)/tools/clang/include
 
+CXXFLAGS = -O3 -g3
+all: libgcmole.so
+Release: libgcmole.so
+
+Debug: CXXFLAGS = -O1 -DDEBUG -g
+Debug: libgcmole.so
+
 libgcmole.so: gcmole.cc
 	$(CXX) -I$(LLVM_BUILD_INCLUDE) -I$(LLVM_SRC_INCLUDE)                  \
-	-I$(CLANG_BUILD_INCLUDE) -I$(CLANG_SRC_INCLUDE) -I. -D_DEBUG          \
+	-I$(CLANG_BUILD_INCLUDE) -I$(CLANG_SRC_INCLUDE) -I. ${CXXFLAGS}          \
 	-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS         \
-	-D__STDC_LIMIT_MACROS -O3 -fomit-frame-pointer -fno-exceptions        \
+	-D__STDC_LIMIT_MACROS -fomit-frame-pointer -fno-exceptions        \
 	-fno-rtti -fPIC -Woverloaded-virtual -Wcast-qual -fno-strict-aliasing \
 	-pedantic -Wno-long-long -Wall -W -Wno-unused-parameter               \
 	-Wwrite-strings -static-libstdc++ -std=c++0x -shared -o libgcmole.so  \

tools/gcmole/README

@@ -109,7 +109,7 @@ script "bootstrap.sh" mentioned above).
 
 TROUBLESHOOTING ---------------------------------------------------------------
 
-gcmole is tighly coupled with the AST structure that Clang produces. Therefore
+gcmole is tightly coupled with the AST structure that Clang produces. Therefore
 when upgrading to a newer Clang version, it might start producing bogus output
 or completely stop outputting warnings. In such occasion, one might start the
 debugging process by checking weather a new AST node type is introduced which

tools/gcmole/bootstrap.sh

@@ -35,6 +35,8 @@
 
 LLVM_RELEASE=9.0.1
 
+BUILD_TYPE="Release"
+# BUILD_TYPE="Debug"
 THIS_DIR="$(readlink -f "$(dirname "${0}")")"
 LLVM_PROJECT_DIR="${THIS_DIR}/bootstrap/llvm"
 BUILD_DIR="${THIS_DIR}/bootstrap/build"
@@ -99,29 +101,35 @@ if [ ! -e "${BUILD_DIR}" ]; then
 fi
 cd "${BUILD_DIR}"
 cmake -GNinja -DCMAKE_CXX_FLAGS="-static-libstdc++" -DLLVM_ENABLE_TERMINFO=OFF \
-    -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang \
+    -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DLLVM_ENABLE_PROJECTS=clang \
     -DLLVM_ENABLE_Z3_SOLVER=OFF "${LLVM_PROJECT_DIR}/llvm"
-MACOSX_DEPLOYMENT_TARGET=10.5 ninja -j"${NUM_JOBS}"
-
-# Strip the clang binary.
-STRIP_FLAGS=
-if [ "${OS}" = "Darwin" ]; then
-  # See http://crbug.com/256342
-  STRIP_FLAGS=-x
+MACOSX_DEPLOYMENT_TARGET=10.5 ninja -j"${NUM_JOBS}" clang
+
+if [[ "${BUILD_TYPE}" = "Release" ]]; then
+  # Strip the clang binary.
+  STRIP_FLAGS=
+  if [ "${OS}" = "Darwin" ]; then
+    # See http://crbug.com/256342
+    STRIP_FLAGS=-x
+  fi
+  strip ${STRIP_FLAGS} bin/clang
 fi
-strip ${STRIP_FLAGS} bin/clang
 cd -
 
 # Build libgcmole.so
 make -C "${THIS_DIR}" clean
 make -C "${THIS_DIR}" LLVM_SRC_ROOT="${LLVM_PROJECT_DIR}/llvm" \
     CLANG_SRC_ROOT="${LLVM_PROJECT_DIR}/clang" \
-    BUILD_ROOT="${BUILD_DIR}" libgcmole.so
+    BUILD_ROOT="${BUILD_DIR}" $BUILD_TYPE
 
 set +x
 
-echo
-echo You can now run gcmole using this command:
-echo
-echo CLANG_BIN=\"tools/gcmole/gcmole-tools/bin\" python tools/gcmole/gcmole.py
+echo '#########################################################################'
+echo 'Congratulations you compiled clang and libgcmole.so'
+echo 
+echo '# You can now run gcmole:'
+echo 'tools/gcmole/gcmole.py \'
+echo '   --clang-bin-dir="tools/gcmole/bootstrap/build/bin" \'
+echo '   --clang-plugins-dir="tools/gcmole" \'
+echo '   --v8-target-cpu=$CPU'
 echo

tools/gcmole/gcmole.py

@@ -6,7 +6,6 @@
 # This is main driver for gcmole tool. See README for more details.
 # Usage: CLANG_BIN=clang-bin-dir python tools/gcmole/gcmole.py [arm|arm64|ia32|x64]
 
-
 # for py2/py3 compatibility
 from __future__ import print_function
 
@@ -14,13 +13,13 @@ from multiprocessing import cpu_count
 
 import collections
 import difflib
+import json
 import optparse
 import os
 import re
 import subprocess
 import sys
 import threading
-import json
 
 if sys.version_info.major > 2:
   from pathlib import Path
@@ -82,6 +81,15 @@ else:
 ArchCfg = collections.namedtuple(
     "ArchCfg", ["name", "cpu", "triple", "arch_define", "arch_options"])
 
+# TODO(cbruni): use gn desc by default for platform-specific settings
+OPTIONS_64BIT = [
+    "-DV8_COMPRESS_POINTERS",
+    "-DV8_COMPRESS_POINTERS_IN_SHARED_CAGE",
+    "-DV8_EXTERNAL_CODE_SPACE",
+    "-DV8_SHORT_BUILTIN_CALLS",
+    "-DV8_SHARED_RO_HEAP",
+]
+
 ARCHITECTURES = {
     "ia32":
         ArchCfg(
@@ -99,14 +107,15 @@ ARCHITECTURES = {
             arch_define="V8_TARGET_ARCH_ARM",
             arch_options=["-m32"],
         ),
+    # TODO(cbruni): Use detailed settings:
+    #   arch_options = OPTIONS_64BIT + [ "-DV8_WIN64_UNWINDING_INFO" ]
     "x64":
         ArchCfg(
             name="x64",
             cpu="x64",
             triple="x86_64-unknown-linux",
             arch_define="V8_TARGET_ARCH_X64",
-            arch_options=[],
-        ),
+            arch_options=[]),
     "arm64":
         ArchCfg(
             name="arm64",
@@ -148,7 +157,7 @@ def make_clang_command_line(plugin, plugin_args, options):
   icu_src_dir = options.v8_root_dir / 'third_party/icu/source'
   return ([
       options.clang_bin_dir / "clang++",
-      "-std=c++14",
+      "-std=c++17",
       "-c",
       "-Xclang",
       "-load",
@@ -164,11 +173,13 @@ def make_clang_command_line(plugin, plugin_args, options):
       "-Xclang",
       arch_cfg.triple,
       "-fno-exceptions",
+      "-Wno-everything",
       "-D",
       arch_cfg.arch_define,
       "-DENABLE_DEBUGGER_SUPPORT",
-      "-DV8_INTL_SUPPORT",
       "-DV8_ENABLE_WEBASSEMBLY",
+      "-DV8_GC_MOLE",
+      "-DV8_INTL_SUPPORT",
       "-I{}".format(options.v8_root_dir),
       "-I{}".format(options.v8_root_dir / 'include'),
       "-I{}".format(options.v8_build_dir / 'gen'),
@@ -253,7 +264,7 @@ def invoke_clang_plugin_for_each_file(filenames, plugin, plugin_args, options):
           else:
             break
         filename, returncode, stdout, stderr = output
-        log(filename, level=1)
+        log(filename, level=2)
         if returncode != 0:
           sys.stderr.write(stderr)
           sys.exit(returncode)
@@ -439,25 +450,44 @@ def generate_gc_suspects(files, options):
     collector.parse(stdout.splitlines())
   collector.propagate()
   # TODO(cbruni): remove once gcmole.cc is migrated
-  write_gc_suspects(collector, options.v8_root_dir)
-  write_gc_suspects(collector, options.out_dir)
-  log("GCSuspects generated for {}", options.v8_target_cpu)
-
-
-def write_gc_suspects(collector, dst):
+  write_gcmole_results(collector, options, options.v8_root_dir)
+  write_gcmole_results(collector, options, options.out_dir)
+
+
+def write_gcmole_results(collector, options, dst):
+  # gcsuspects contains a list("mangled_full_name,name") of all functions that
+  # could cause a gc (directly or indirectly).
+  #
+  # EXAMPLE
+  # _ZN2v88internal4Heap16CreateApiObjectsEv,CreateApiObjects
+  # _ZN2v88internal4Heap17CreateInitialMapsEv,CreateInitialMaps
+  # ...
   with open(dst / "gcsuspects", "w") as out:
-    for name, value in collector.gc.items():
+    for name, value in list(collector.gc.items()):
       if value:
         out.write(name + "\n")
-
+  # gccauses contains a map["mangled_full_name,name"] => list(inner gcsuspects)
+  # Where the inner gcsuspects are functions directly called in the outer
+  # function that can cause a gc. The format is encoded for simplified
+  # deserialization in gcmole.cc.
+  #
+  # EXAMPLE:
+  # _ZN2v88internal4Heap17CreateHeapObjectsEv,CreateHeapObjects
+  # start,nested
+  # _ZN2v88internal4Heap16CreateApiObjectsEv,CreateApiObjects
+  # _ZN2v88internal4Heap17CreateInitialMapsEv,CreateInitialMaps
+  # ...
+  # end,nested
+  # ...
   with open(dst / "gccauses", "w") as out:
-    out.write("GC = {\n")
-    for name, causes in collector.gc_caused.items():
-      out.write("  '{}': [\n".format(name))
+    for name, causes in list(collector.gc_caused.items()):
+      out.write("{}\n".format(name))
+      out.write("start,nested\n")
       for cause in causes:
-        out.write("    '{}',\n".format(cause))
-      out.write("  ],\n")
-    out.write("}\n")
+        out.write("{}\n".format(cause))
+      out.write("end,nested\n")
+  log("GCSuspects and gccauses generated for {} in '{}'", options.v8_target_cpu,
+      dst)
 
 
 # ------------------------------------------------------------------------------
@@ -498,7 +528,7 @@ def check_correctness_for_arch(options, for_test):
       sys.stdout.write(stderr)
 
   log("Done processing {} files.", processed_files)
-  log("Errors found" if errors_found else "## No errors found")
+  log("Errors found" if errors_found else "No errors found")
 
   return errors_found, output
 
@@ -598,7 +628,7 @@ def main(args):
   parser.add_option(
       "--out-dir",
       metavar="DIR",
-      help="Output location for gcsuspect and gcauses file."
+      help="Output location for the gcsuspect and gcauses file."
       "Default: BUILD_DIR/gen/tools/gcmole")
   parser.add_option(
       "--is-bot",
@@ -639,9 +669,9 @@ def main(args):
       action="store_true",
       default=True,
       dest="allowlist",
-      help="""When building gcsuspects allowlist certain functions as if they can be
-  causing GC. Currently used to reduce number of false positives in dead
-  variables analysis. See TODO for ALLOWLIST in gcmole.py""")
+      help="When building gcsuspects allowlist certain functions as if they can be "
+      "causing GC. Currently used to reduce number of false positives in dead "
+      "variables analysis. See TODO for ALLOWLIST in gcmole.py")
   group.add_option(
       "--test-run",
       action="store_true",

tools/gcmole/package.sh

@@ -14,6 +14,7 @@ PACKAGE_DIR="${THIS_DIR}/gcmole-tools"
 PACKAGE_FILE="${THIS_DIR}/gcmole-tools.tar.gz"
 PACKAGE_SUM="${THIS_DIR}/gcmole-tools.tar.gz.sha1"
 BUILD_DIR="${THIS_DIR}/bootstrap/build"
+V8_ROOT_DIR= `realpath "${THIS_DIR}/../.."`
 
 # Echo all commands
 set -x
@@ -72,5 +73,8 @@ echo "sudo chroot \$CHROOT_DIR bash -c 'PATH=/docs/depot_tools:\$PATH; /docs/v8/
 echo
 echo You can now run gcmole using this command:
 echo
-echo CLANG_BIN=\"tools/gcmole/gcmole-tools/bin\" python tools/gcmole/gcmole.py
+echo 'tools/gcmole/gcmole.py \'
+echo '   --clang-bin-dir="tools/gcmole/gcmole-tools/bin" \'
+echo '   --clang-plugins-dir="tools/gcmole/gcmole-tools" \'
+echo '   --v8-target-cpu=$CPU'
 echo