From ecc3cd256aac7321498ba679ea7da0e01a1592eb Mon Sep 17 00:00:00 2001 From: Camillo Bruni Date: Thu, 3 Mar 2022 22:38:39 +0100 Subject: [PATCH] [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/+/3480095 Reviewed-by: Jakob Kummerow Reviewed-by: Maya Lekova Commit-Queue: Camillo Bruni Cr-Commit-Position: refs/heads/main@{#79357} --- src/wasm/c-api.cc | 18 ++- src/wasm/wasm-code-manager.h | 2 + tools/gcmole/Makefile | 11 +- tools/gcmole/README | 2 +- tools/gcmole/bootstrap.sh | 34 ++-- tools/gcmole/gcmole.cc | 305 ++++++++++++++++++++--------------- tools/gcmole/gcmole.py | 78 ++++++--- tools/gcmole/package.sh | 6 +- 8 files changed, 277 insertions(+), 179 deletions(-) diff --git a/src/wasm/c-api.cc b/src/wasm/c-api.cc index 63f42014f6..a217babc73 100644 --- a/src/wasm/c-api.cc +++ b/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), \ + "C/C++ incompatibility"); \ + static_assert( \ + sizeof(wasm_##name##_t ptr_or_none) == sizeof(vec::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), \ - "C/C++ incompatibility"); \ - static_assert( \ - sizeof(wasm_##name##_t ptr_or_none) == sizeof(vec::elem_type), \ - "C/C++ incompatibility"); \ + ASSERT_VEC_BASE_SIZE(name, Name, vec, ptr_or_none) \ extern "C++" inline auto hide_##name##_vec(vec& v) \ ->wasm_##name##_vec_t* { \ return reinterpret_cast(&v); \ diff --git a/src/wasm/wasm-code-manager.h b/src/wasm/wasm-code-manager.h index 4022efbdb7..137c3074d5 100644 --- a/src/wasm/wasm-code-manager.h +++ b/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); diff --git a/tools/gcmole/Makefile b/tools/gcmole/Makefile index e1bde684a6..fe295341da 100644 --- a/tools/gcmole/Makefile +++ b/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 \ diff --git a/tools/gcmole/README b/tools/gcmole/README index 1d2acd3b1a..15828fa435 100644 --- a/tools/gcmole/README +++ b/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 diff --git a/tools/gcmole/bootstrap.sh b/tools/gcmole/bootstrap.sh index e04ef5826c..f47ba6d213 100755 --- a/tools/gcmole/bootstrap.sh +++ b/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}" +MACOSX_DEPLOYMENT_TARGET=10.5 ninja -j"${NUM_JOBS}" clang -# Strip the clang binary. -STRIP_FLAGS= -if [ "${OS}" = "Darwin" ]; then - # See http://crbug.com/256342 - STRIP_FLAGS=-x +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 diff --git a/tools/gcmole/gcmole.cc b/tools/gcmole/gcmole.cc index 3dc4baa722..9881f4c5b1 100644 --- a/tools/gcmole/gcmole.cc +++ b/tools/gcmole/gcmole.cc @@ -48,6 +48,8 @@ namespace { bool g_tracing_enabled = false; bool g_dead_vars_analysis = false; +bool g_verbose = false; +bool g_print_gc_call_chain = false; #define TRACE(str) \ do { \ @@ -80,16 +82,13 @@ typedef std::map CalleesMap; static bool GetMangledName(clang::MangleContext* ctx, const clang::NamedDecl* decl, MangledName* result) { - if (!llvm::isa(decl) && - !llvm::isa(decl)) { - llvm::SmallVector output; - llvm::raw_svector_ostream out(output); - ctx->mangleName(decl, out); - *result = out.str().str(); - return true; - } - - return false; + if (llvm::isa(decl)) return false; + if (llvm::isa(decl)) return false; + llvm::SmallVector output; + llvm::raw_svector_ostream out(output); + ctx->mangleName(decl, out); + *result = out.str().str(); + return true; } @@ -217,8 +216,7 @@ struct Resolver { class CalleesPrinter : public clang::RecursiveASTVisitor { public: - explicit CalleesPrinter(clang::MangleContext* ctx) : ctx_(ctx) { - } + explicit CalleesPrinter(clang::MangleContext* ctx) : ctx_(ctx) {} virtual bool VisitCallExpr(clang::CallExpr* expr) { const clang::FunctionDecl* callee = expr->getDirectCallee(); @@ -236,17 +234,17 @@ class CalleesPrinter : public clang::RecursiveASTVisitor { } void AnalyzeFunction(const clang::FunctionDecl* f) { + if (!InV8Namespace(f)) return; MangledName name; - if (InV8Namespace(f) && GetMangledName(ctx_, f, &name)) { - const std::string& function = f->getNameAsString(); - AddCallee(name, function); + if (!GetMangledName(ctx_, f, &name)) return; + const std::string& function = f->getNameAsString(); + AddCallee(name, function); - const clang::FunctionDecl* body = NULL; - if (f->hasBody(body) && !Analyzed(name)) { - EnterScope(name); - TraverseStmt(body->getBody()); - LeaveScope(); - } + const clang::FunctionDecl* body = NULL; + if (f->hasBody(body) && !Analyzed(name)) { + EnterScope(name); + TraverseStmt(body->getBody()); + LeaveScope(); } } @@ -303,17 +301,18 @@ class FunctionDeclarationFinder : public clang::ASTConsumer, public clang::RecursiveASTVisitor { public: - explicit FunctionDeclarationFinder(clang::DiagnosticsEngine& d, - clang::SourceManager& sm, - const std::vector& args) - : d_(d), sm_(sm) {} + explicit FunctionDeclarationFinder( + clang::DiagnosticsEngine& diagnostics_engine, + clang::SourceManager& source_manager, + const std::vector& args) + : diagnostics_engine_(diagnostics_engine), + source_manager_(source_manager) {} virtual void HandleTranslationUnit(clang::ASTContext &ctx) { - mangle_context_ = clang::ItaniumMangleContext::create(ctx, d_); + mangle_context_ = + clang::ItaniumMangleContext::create(ctx, diagnostics_engine_); callees_printer_ = new CalleesPrinter(mangle_context_); - TraverseDecl(ctx.getTranslationUnitDecl()); - callees_printer_->PrintCallGraph(); } @@ -323,8 +322,8 @@ class FunctionDeclarationFinder } private: - clang::DiagnosticsEngine& d_; - clang::SourceManager& sm_; + clang::DiagnosticsEngine& diagnostics_engine_; + clang::SourceManager& source_manager_; clang::MangleContext* mangle_context_; CalleesPrinter* callees_printer_; @@ -333,8 +332,39 @@ class FunctionDeclarationFinder static bool gc_suspects_loaded = false; static CalleesSet gc_suspects; static CalleesSet gc_functions; -static bool whitelist_loaded = false; -static CalleesSet suspects_whitelist; + +static bool allowlist_loaded = false; +static CalleesSet suspects_allowlist; + +static bool gc_causes_loaded = false; +static std::map> gc_causes; + +static void LoadGCCauses() { + if (gc_causes_loaded) return; + std::ifstream fin("gccauses"); + std::string mangled, function; + while (!fin.eof()) { + std::getline(fin, mangled, ','); + std::getline(fin, function); + if (mangled.empty()) break; + std::string parent = mangled; + // start,nested + std::getline(fin, mangled, ','); + assert(mangled.compare("start") == 0); + std::getline(fin, function); + assert(function.compare("nested") == 0); + while (true) { + std::getline(fin, mangled, ','); + std::getline(fin, function); + if (mangled.compare("end") == 0) { + assert(function.compare("nested") == 0); + break; + } + gc_causes[parent].push_back(mangled); + } + } + gc_causes_loaded = true; +} static void LoadGCSuspects() { if (gc_suspects_loaded) return; @@ -352,55 +382,51 @@ static void LoadGCSuspects() { gc_suspects_loaded = true; } -static void LoadSuspectsWhitelist() { - if (whitelist_loaded) return; +static void LoadSuspectsAllowList() { + if (allowlist_loaded) return; - std::ifstream fin("tools/gcmole/suspects.whitelist"); + // TODO(cbruni): clean up once fully migrated + std::ifstream fin("tools/gcmole/suspects.allowlist"); + if (!fin.is_open()) { + fin = std::ifstream("tools/gcmole/suspects.whitelist"); + } std::string s; - while (fin >> s) suspects_whitelist.insert(s); + while (fin >> s) suspects_allowlist.insert(s); - whitelist_loaded = true; + allowlist_loaded = true; } // Looks for exact match of the mangled name. -static bool KnownToCauseGC(clang::MangleContext* ctx, - const clang::FunctionDecl* decl) { +static bool IsKnownToCauseGC(clang::MangleContext* ctx, + const clang::FunctionDecl* decl) { LoadGCSuspects(); - if (!InV8Namespace(decl)) return false; - - if (suspects_whitelist.find(decl->getNameAsString()) != - suspects_whitelist.end()) { + if (suspects_allowlist.find(decl->getNameAsString()) != + suspects_allowlist.end()) { return false; } - MangledName name; if (GetMangledName(ctx, decl, &name)) { return gc_suspects.find(name) != gc_suspects.end(); } - return false; } // Looks for partial match of only the function name. -static bool SuspectedToCauseGC(clang::MangleContext* ctx, - const clang::FunctionDecl* decl) { +static bool IsSuspectedToCauseGC(clang::MangleContext* ctx, + const clang::FunctionDecl* decl) { LoadGCSuspects(); - if (!InV8Namespace(decl)) return false; - - LoadSuspectsWhitelist(); - if (suspects_whitelist.find(decl->getNameAsString()) != - suspects_whitelist.end()) { + LoadSuspectsAllowList(); + if (suspects_allowlist.find(decl->getNameAsString()) != + suspects_allowlist.end()) { return false; } - if (gc_functions.find(decl->getNameAsString()) != gc_functions.end()) { TRACE_LLVM_DECL("Suspected by ", decl); return true; } - return false; } @@ -449,10 +475,9 @@ class ExprEffect { intptr_t effect_; }; - -const std::string BAD_EXPR_MSG("Possible problem with evaluation order."); -const std::string DEAD_VAR_MSG("Possibly dead variable."); - +const std::string BAD_EXPR_MSG( + "Possible problem with evaluation order with interleaved GCs."); +const std::string DEAD_VAR_MSG("Possibly stale variable due to GCs."); class Environment { public: @@ -612,22 +637,16 @@ class CallProps { ExprEffect ComputeCumulativeEffect(bool result_is_raw) { ExprEffect out = ExprEffect::NoneWithEnv(env_); - if (gc_.any()) { - out.setGC(); - } + if (gc_.any()) out.setGC(); if (raw_use_.any()) out.setRawUse(); if (result_is_raw) out.setRawDef(); return out; } bool IsSafe() { - if (!gc_.any()) { - return true; - } + if (!gc_.any()) return true; std::bitset raw = (raw_def_ | raw_use_); - if (!raw.any()) { - return true; - } + if (!raw.any()) return true; bool result = gc_.count() == 1 && !((raw ^ gc_).any()); return result; } @@ -950,13 +969,10 @@ class FunctionAnalyzer { ExprEffect Parallel(clang::Expr* parent, int n, clang::Expr** exprs, const Environment& env) { CallProps props; - for (int i = 0; i < n; ++i) { props.SetEffect(i, VisitExpr(exprs[i], env)); } - if (!props.IsSafe()) ReportUnsafe(parent, BAD_EXPR_MSG); - return props.ComputeCumulativeEffect( RepresentsRawPointerType(parent->getType())); } @@ -984,27 +1000,24 @@ class FunctionAnalyzer { const clang::QualType& var_type, const std::string& var_name, const Environment& env) { - if (RepresentsRawPointerType(var_type)) { - // We currently care only about our internal pointer types and not about - // raw C++ pointers, because normally special care is taken when storing - // raw pointers to the managed heap. Furthermore, checking for raw - // pointers produces too many false positives in the dead variable - // analysis. - if (IsInternalPointerType(var_type) && !env.IsAlive(var_name) && - !HasActiveGuard() && g_dead_vars_analysis) { - ReportUnsafe(parent, DEAD_VAR_MSG); - } - return ExprEffect::RawUse(); - } - return ExprEffect::None(); + if (!g_dead_vars_analysis) return ExprEffect::None(); + if (!RepresentsRawPointerType(var_type)) return ExprEffect::None(); + // We currently care only about our internal pointer types and not about + // raw C++ pointers, because normally special care is taken when storing + // raw pointers to the managed heap. Furthermore, checking for raw + // pointers produces too many false positives in the dead variable + // analysis. + if (!IsInternalPointerType(var_type)) return ExprEffect::None(); + if (env.IsAlive(var_name)) return ExprEffect::None(); + if (HasActiveGuard()) return ExprEffect::None(); + ReportUnsafe(parent, DEAD_VAR_MSG); + return ExprEffect::RawUse(); } ExprEffect Use(const clang::Expr* parent, const clang::ValueDecl* var, const Environment& env) { - if (IsExternalVMState(var)) { - return ExprEffect::GC(); - } + if (IsExternalVMState(var)) return ExprEffect::GC(); return Use(parent, var->getType(), var->getNameAsString(), env); } @@ -1062,43 +1075,40 @@ class FunctionAnalyzer { RepresentsRawPointerType(call->getType())); clang::FunctionDecl* callee = call->getDirectCallee(); - if (callee != NULL) { - if (KnownToCauseGC(ctx_, callee)) { - out.setGC(); - scopes_.back().SetGCCauseLocation( - clang::FullSourceLoc(call->getExprLoc(), sm_)); - } + if (callee == NULL) return out; - // Support for virtual methods that might be GC suspects. - clang::CXXMethodDecl* method = - llvm::dyn_cast_or_null(callee); - if (method != NULL && method->isVirtual()) { - clang::CXXMemberCallExpr* memcall = - llvm::dyn_cast_or_null(call); - if (memcall != NULL) { - clang::CXXMethodDecl* target = method->getDevirtualizedMethod( - memcall->getImplicitObjectArgument(), false); - if (target != NULL) { - if (KnownToCauseGC(ctx_, target)) { - out.setGC(); - scopes_.back().SetGCCauseLocation( - clang::FullSourceLoc(call->getExprLoc(), sm_)); - } - } else { - // According to the documentation, {getDevirtualizedMethod} might - // return NULL, in which case we still want to use the partial - // match of the {method}'s name against the GC suspects in order - // to increase coverage. - if (SuspectedToCauseGC(ctx_, method)) { - out.setGC(); - scopes_.back().SetGCCauseLocation( - clang::FullSourceLoc(call->getExprLoc(), sm_)); - } - } - } - } + if (IsKnownToCauseGC(ctx_, callee)) { + out.setGC(); + scopes_.back().SetGCCauseLocation( + clang::FullSourceLoc(call->getExprLoc(), sm_), callee); } + // Support for virtual methods that might be GC suspects. + if (memcall == NULL) return out; + clang::CXXMethodDecl* method = + llvm::dyn_cast_or_null(callee); + if (method == NULL) return out; + if (!method->isVirtual()) return out; + + clang::CXXMethodDecl* target = method->getDevirtualizedMethod( + memcall->getImplicitObjectArgument(), false); + if (target != NULL) { + if (IsKnownToCauseGC(ctx_, target)) { + out.setGC(); + scopes_.back().SetGCCauseLocation( + clang::FullSourceLoc(call->getExprLoc(), sm_), target); + } + } else { + // According to the documentation, {getDevirtualizedMethod} might + // return NULL, in which case we still want to use the partial + // match of the {method}'s name against the GC suspects in order + // to increase coverage. + if (IsSuspectedToCauseGC(ctx_, method)) { + out.setGC(); + scopes_.back().SetGCCauseLocation( + clang::FullSourceLoc(call->getExprLoc(), sm_), method); + } + } return out; } @@ -1185,11 +1195,9 @@ class FunctionAnalyzer { } bool changed() { - if (changed_) { - changed_ = false; - return true; - } - return false; + if (!changed_) return false; + changed_ = false; + return true; } const Environment& in() { @@ -1455,7 +1463,7 @@ class FunctionAnalyzer { } bool HasActiveGuard() { - for (auto s : scopes_) { + for (const auto s : scopes_) { if (s.IsBeforeGCCause()) return true; } return false; @@ -1466,6 +1474,36 @@ class FunctionAnalyzer { d_.Report(clang::FullSourceLoc(expr->getExprLoc(), sm_), d_.getCustomDiagID(clang::DiagnosticsEngine::Warning, "%0")) << msg; + if (scopes_.empty()) return; + GCScope scope = scopes_[0]; + if (!scope.gccause_location.isValid()) return; + d_.Report(scope.gccause_location, + d_.getCustomDiagID(clang::DiagnosticsEngine::Note, + "Call might cause unexpected GC.")); + clang::FunctionDecl* gccause_decl = scope.gccause_decl; + d_.Report( + clang::FullSourceLoc(gccause_decl->getBeginLoc(), sm_), + d_.getCustomDiagID(clang::DiagnosticsEngine::Note, "GC call here.")); + + if (!g_print_gc_call_chain) return; + // TODO(cbruni, v8::10009): print call-chain to gc with proper source + // positions. + LoadGCCauses(); + MangledName name; + if (!GetMangledName(ctx_, gccause_decl, &name)) return; + std::cout << "Potential GC call chain:\n"; + std::set stack; + while (true) { + if (!stack.insert(name).second) break; + std::cout << "\t" << name << "\n"; + auto next = gc_causes.find(name); + if (next == gc_causes.end()) break; + std::vector calls = next->second; + for (MangledName call : calls) { + name = call; + if (stack.find(call) != stack.end()) break; + } + } } @@ -1484,10 +1522,11 @@ class FunctionAnalyzer { struct GCScope { clang::FullSourceLoc guard_location; clang::FullSourceLoc gccause_location; + clang::FunctionDecl* gccause_decl; // We're only interested in guards that are declared before any further GC // causing calls (see TestGuardedDeadVarAnalysisMidFunction for example). - bool IsBeforeGCCause() { + bool IsBeforeGCCause() const { if (!guard_location.isValid()) return false; if (!gccause_location.isValid()) return true; return guard_location.isBeforeInTranslationUnitThan(gccause_location); @@ -1495,9 +1534,11 @@ class FunctionAnalyzer { // After we set the first GC cause in the scope, we don't need the later // ones. - void SetGCCauseLocation(clang::FullSourceLoc gccause_location_) { + void SetGCCauseLocation(clang::FullSourceLoc gccause_location_, + clang::FunctionDecl* decl) { if (gccause_location.isValid()) return; gccause_location = gccause_location_; + gccause_decl = decl; } }; std::vector scopes_; @@ -1513,9 +1554,8 @@ class ProblemsFinder : public clang::ASTConsumer, if (args[i] == "--dead-vars") { g_dead_vars_analysis = true; } - if (args[i] == "--verbose") { - g_tracing_enabled = true; - } + if (args[i] == "--verbose-trace") g_tracing_enabled = true; + if (args[i] == "--verbose") g_verbose = true; } } @@ -1571,7 +1611,7 @@ class ProblemsFinder : public clang::ASTConsumer, clang::ItaniumMangleContext::create(ctx, d_), object_decl, maybe_object_decl, smi_decl, no_gc_mole_decl, d_, sm_); TraverseDecl(ctx.getTranslationUnitDecl()); - } else { + } else if (g_verbose) { if (object_decl == NULL) { llvm::errs() << "Failed to resolve v8::internal::Object\n"; } @@ -1609,7 +1649,6 @@ class ProblemsFinder : public clang::ASTConsumer, FunctionAnalyzer* function_analyzer_; }; - template class Action : public clang::PluginASTAction { protected: diff --git a/tools/gcmole/gcmole.py b/tools/gcmole/gcmole.py index 06bc2ad0d1..a77c57355d 100755 --- a/tools/gcmole/gcmole.py +++ b/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) + write_gcmole_results(collector, options, options.v8_root_dir) + write_gcmole_results(collector, options, options.out_dir) -def write_gc_suspects(collector, dst): +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", diff --git a/tools/gcmole/package.sh b/tools/gcmole/package.sh index bbddd5b772..ceeffc2a29 100755 --- a/tools/gcmole/package.sh +++ b/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