diff options
Diffstat (limited to 'chromium/v8/tools/gcmole')
| -rw-r--r-- | chromium/v8/tools/gcmole/BUILD.gn | 5 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/gcmole-test.cc | 5 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/gcmole-tools.tar.gz.sha1 | 2 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/gcmole.cc | 86 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/gcmole.lua | 27 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/ignored_files | 2 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/suspects.whitelist | 2 | ||||
| -rw-r--r-- | chromium/v8/tools/gcmole/test-expectations.txt | 14 |
8 files changed, 106 insertions, 37 deletions
diff --git a/chromium/v8/tools/gcmole/BUILD.gn b/chromium/v8/tools/gcmole/BUILD.gn index fc7d81e561d..9767acbf5a1 100644 --- a/chromium/v8/tools/gcmole/BUILD.gn +++ b/chromium/v8/tools/gcmole/BUILD.gn @@ -16,6 +16,7 @@ group("v8_run_gcmole") { "parallel.py", "run-gcmole.py", "suspects.whitelist", + "ignored_files", "test-expectations.txt", # The following contains all relevant source and build files. @@ -37,9 +38,7 @@ group("v8_run_gcmole") { "$target_gen_dir/../../torque-generated/", ] - deps = [ - "../../:run_torque", - ] + deps = [ "../../:run_torque" ] if (v8_gcmole) { # This assumes gcmole tools have been fetched by a hook diff --git a/chromium/v8/tools/gcmole/gcmole-test.cc b/chromium/v8/tools/gcmole/gcmole-test.cc index e0e2801f777..92f7a9eda88 100644 --- a/chromium/v8/tools/gcmole/gcmole-test.cc +++ b/chromium/v8/tools/gcmole/gcmole-test.cc @@ -189,7 +189,7 @@ void TestGuardedDeadVarAnalysisNotOnStack(Isolate* isolate) { void TestGuardedDeadVarAnalysisNested(JSObject raw_obj, Isolate* isolate) { CauseGCRaw(raw_obj, isolate); - // Shouldn't cause warning. + // Should cause warning. raw_obj.Print(); } @@ -198,6 +198,9 @@ void TestGuardedDeadVarAnalysisCaller(Isolate* isolate) { JSObject raw_obj = *isolate->factory()->NewJSObjectWithNullProto(); TestGuardedDeadVarAnalysisNested(raw_obj, isolate); + + // Shouldn't cause warning. + raw_obj.Print(); } JSObject GuardedAllocation(Isolate* isolate) { diff --git a/chromium/v8/tools/gcmole/gcmole-tools.tar.gz.sha1 b/chromium/v8/tools/gcmole/gcmole-tools.tar.gz.sha1 index 8f7876fa519..221e2e9d293 100644 --- a/chromium/v8/tools/gcmole/gcmole-tools.tar.gz.sha1 +++ b/chromium/v8/tools/gcmole/gcmole-tools.tar.gz.sha1 @@ -1 +1 @@ -d2f949820bf1ee7343a7b5f46987a3657aaea2e9
\ No newline at end of file +0af04ef475bc746a501fe17d3b56ccb03fc151fc diff --git a/chromium/v8/tools/gcmole/gcmole.cc b/chromium/v8/tools/gcmole/gcmole.cc index 606c90bb582..cce177caf9e 100644 --- a/chromium/v8/tools/gcmole/gcmole.cc +++ b/chromium/v8/tools/gcmole/gcmole.cc @@ -46,6 +46,7 @@ namespace { bool g_tracing_enabled = false; +bool g_dead_vars_analysis = false; #define TRACE(str) \ do { \ @@ -61,12 +62,14 @@ bool g_tracing_enabled = false; } \ } while (false) -#define TRACE_LLVM_DECL(str, decl) \ - do { \ - if (g_tracing_enabled) { \ - std::cout << str << std::endl; \ - decl->dump(); \ - } \ +// Node: The following is used when tracing --dead-vars +// to provide extra info for the GC suspect. +#define TRACE_LLVM_DECL(str, decl) \ + do { \ + if (g_tracing_enabled && g_dead_vars_analysis) { \ + std::cout << str << std::endl; \ + decl->dump(); \ + } \ } while (false) typedef std::string MangledName; @@ -365,6 +368,11 @@ static bool KnownToCauseGC(clang::MangleContext* ctx, if (!InV8Namespace(decl)) return false; + if (suspects_whitelist.find(decl->getNameAsString()) != + suspects_whitelist.end()) { + return false; + } + MangledName name; if (GetMangledName(ctx, decl, &name)) { return gc_suspects.find(name) != gc_suspects.end(); @@ -387,6 +395,7 @@ static bool SuspectedToCauseGC(clang::MangleContext* ctx, } if (gc_functions.find(decl->getNameAsString()) != gc_functions.end()) { + TRACE_LLVM_DECL("Suspected by ", decl); return true; } @@ -677,8 +686,7 @@ class FunctionAnalyzer { clang::CXXRecordDecl* smi_decl, clang::CXXRecordDecl* no_gc_decl, clang::CXXRecordDecl* no_heap_access_decl, - clang::DiagnosticsEngine& d, clang::SourceManager& sm, - bool dead_vars_analysis) + clang::DiagnosticsEngine& d, clang::SourceManager& sm) : ctx_(ctx), object_decl_(object_decl), maybe_object_decl_(maybe_object_decl), @@ -687,8 +695,7 @@ class FunctionAnalyzer { no_heap_access_decl_(no_heap_access_decl), d_(d), sm_(sm), - block_(NULL), - dead_vars_analysis_(dead_vars_analysis) {} + block_(NULL) {} // -------------------------------------------------------------------------- // Expressions @@ -984,7 +991,7 @@ class FunctionAnalyzer { // pointers produces too many false positives in the dead variable // analysis. if (IsInternalPointerType(var_type) && !env.IsAlive(var_name) && - !HasActiveGuard() && dead_vars_analysis_) { + !HasActiveGuard() && g_dead_vars_analysis) { ReportUnsafe(parent, DEAD_VAR_MSG); } return ExprEffect::RawUse(); @@ -1356,15 +1363,6 @@ class FunctionAnalyzer { } bool IsInternalPointerType(clang::QualType qtype) { - // Not yet assigned pointers can't get moved by the GC. - if (qtype.isNull()) { - return false; - } - // nullptr can't get moved by the GC. - if (qtype->isNullPtrType()) { - return false; - } - const clang::CXXRecordDecl* record = qtype->getAsCXXRecordDecl(); bool result = IsDerivedFromInternalPointer(record); TRACE_LLVM_TYPE("is internal " << result, qtype); @@ -1374,6 +1372,15 @@ class FunctionAnalyzer { // Returns weather the given type is a raw pointer or a wrapper around // such. For V8 that means Object and MaybeObject instances. bool RepresentsRawPointerType(clang::QualType qtype) { + // Not yet assigned pointers can't get moved by the GC. + if (qtype.isNull()) { + return false; + } + // nullptr can't get moved by the GC. + if (qtype->isNullPtrType()) { + return false; + } + const clang::PointerType* pointer_type = llvm::dyn_cast_or_null<clang::PointerType>(qtype.getTypePtrOrNull()); if (pointer_type != NULL) { @@ -1490,7 +1497,6 @@ class FunctionAnalyzer { clang::SourceManager& sm_; Block* block_; - bool dead_vars_analysis_; struct GCGuard { clang::CompoundStmt* stmt = NULL; @@ -1507,10 +1513,10 @@ class ProblemsFinder : public clang::ASTConsumer, public: ProblemsFinder(clang::DiagnosticsEngine& d, clang::SourceManager& sm, const std::vector<std::string>& args) - : d_(d), sm_(sm), dead_vars_analysis_(false) { + : d_(d), sm_(sm) { for (unsigned i = 0; i < args.size(); ++i) { if (args[i] == "--dead-vars") { - dead_vars_analysis_ = true; + g_dead_vars_analysis = true; } if (args[i] == "--verbose") { g_tracing_enabled = true; @@ -1518,7 +1524,29 @@ class ProblemsFinder : public clang::ASTConsumer, } } + bool TranslationUnitIgnored() { + if (!ignored_files_loaded_) { + std::ifstream fin("tools/gcmole/ignored_files"); + std::string s; + while (fin >> s) ignored_files_.insert(s); + ignored_files_loaded_ = true; + } + + clang::FileID main_file_id = sm_.getMainFileID(); + std::string filename = sm_.getFileEntryForID(main_file_id)->getName().str(); + + bool result = ignored_files_.find(filename) != ignored_files_.end(); + if (result) { + llvm::outs() << "Ignoring file " << filename << "\n"; + } + return result; + } + virtual void HandleTranslationUnit(clang::ASTContext &ctx) { + if (TranslationUnitIgnored()) { + return; + } + Resolver r(ctx); // It is a valid situation that no_gc_decl == NULL when the @@ -1558,10 +1586,10 @@ class ProblemsFinder : public clang::ASTConsumer, no_heap_access_decl = no_heap_access_decl->getDefinition(); if (object_decl != NULL && smi_decl != NULL && maybe_object_decl != NULL) { - function_analyzer_ = new FunctionAnalyzer( - clang::ItaniumMangleContext::create(ctx, d_), object_decl, - maybe_object_decl, smi_decl, no_gc_decl, no_heap_access_decl, d_, sm_, - dead_vars_analysis_); + function_analyzer_ = + new FunctionAnalyzer(clang::ItaniumMangleContext::create(ctx, d_), + object_decl, maybe_object_decl, smi_decl, + no_gc_decl, no_heap_access_decl, d_, sm_); TraverseDecl(ctx.getTranslationUnitDecl()); } else { if (object_decl == NULL) { @@ -1594,7 +1622,9 @@ class ProblemsFinder : public clang::ASTConsumer, private: clang::DiagnosticsEngine& d_; clang::SourceManager& sm_; - bool dead_vars_analysis_; + + bool ignored_files_loaded_ = false; + std::set<std::string> ignored_files_; FunctionAnalyzer* function_analyzer_; }; diff --git a/chromium/v8/tools/gcmole/gcmole.lua b/chromium/v8/tools/gcmole/gcmole.lua index a09c3b61ad5..95ba0433516 100644 --- a/chromium/v8/tools/gcmole/gcmole.lua +++ b/chromium/v8/tools/gcmole/gcmole.lua @@ -40,9 +40,8 @@ local FLAGS = { -- Print commands to console before executing them. verbose = false; - -- Perform dead variable analysis (generates many false positives). - -- TODO add some sort of whiteliste to filter out false positives. - dead_vars = false; + -- Perform dead variable analysis. + dead_vars = true; -- Enable verbose tracing from the plugin itself. verbose_trace = false; @@ -322,6 +321,7 @@ local WHITELIST = { -- CodeCreateEvent receives AbstractCode (a raw ptr) as an argument. "CodeCreateEvent", + "WriteField", }; local function AddCause(name, cause) @@ -477,6 +477,17 @@ local function SafeCheckCorrectnessForArch(arch, for_test) return errors, output end +-- Source: https://stackoverflow.com/a/41515925/1540248 +local function StringDifference(str1,str2) + for i = 1,#str1 do -- Loop over strings + -- If that character is not equal to its counterpart + if str1:sub(i,i) ~= str2:sub(i,i) then + return i --Return that index + end + end + return #str1+1 -- Return the index after where the shorter one ends as fallback. +end + local function TestRun() local errors, output = SafeCheckCorrectnessForArch('x64', true) if not errors then @@ -491,6 +502,16 @@ local function TestRun() if output ~= expectations then log("** Output mismatch from running tests. Please run them manually.") + local idx = StringDifference(output, expectations) + + log("Difference at byte "..idx) + log("Expected: "..expectations:sub(idx-10,idx+10)) + log("Actual: "..output:sub(idx-10,idx+10)) + + log("--- Full output ---") + log(output) + log("------") + return false end diff --git a/chromium/v8/tools/gcmole/ignored_files b/chromium/v8/tools/gcmole/ignored_files new file mode 100644 index 00000000000..05fcd7a129e --- /dev/null +++ b/chromium/v8/tools/gcmole/ignored_files @@ -0,0 +1,2 @@ +src/profiler/heap-snapshot-generator.cc +src/execution/isolate.cc diff --git a/chromium/v8/tools/gcmole/suspects.whitelist b/chromium/v8/tools/gcmole/suspects.whitelist index 01db7401f22..1ac855f2f76 100644 --- a/chromium/v8/tools/gcmole/suspects.whitelist +++ b/chromium/v8/tools/gcmole/suspects.whitelist @@ -2,3 +2,5 @@ IsConstructor IsEval IsAsync IsPromiseAll +IsPromiseAny +VisitRootPointers diff --git a/chromium/v8/tools/gcmole/test-expectations.txt b/chromium/v8/tools/gcmole/test-expectations.txt index 82134743f6b..780cea91811 100644 --- a/chromium/v8/tools/gcmole/test-expectations.txt +++ b/chromium/v8/tools/gcmole/test-expectations.txt @@ -1,4 +1,7 @@ +tools/gcmole/gcmole-test.cc:27:10: warning: Possibly dead variable. + return obj; + ^ tools/gcmole/gcmole-test.cc:45:3: warning: Possible problem with evaluation order. TwoArgumentsFunction(*CauseGC(obj1, isolate), *CauseGC(obj2, isolate)); ^ @@ -20,4 +23,13 @@ tools/gcmole/gcmole-test.cc:130:14: warning: Possible problem with evaluation or tools/gcmole/gcmole-test.cc:151:14: warning: Possible problem with evaluation order. so_handle->Method(*SomeClass::StaticCauseGC(obj1, isolate)); ^ -7 warnings generated. +tools/gcmole/gcmole-test.cc:161:3: warning: Possibly dead variable. + raw_obj.Print(); + ^ +tools/gcmole/gcmole-test.cc:193:3: warning: Possibly dead variable. + raw_obj.Print(); + ^ +tools/gcmole/gcmole-test.cc:216:3: warning: Possibly dead variable. + raw_obj.Print(); + ^ +11 warnings generated. |