clang-cl crashes building Precompile-core.cc.obj |
||||||
Issue descriptionAfter https://codereview.chromium.org/2520863002 : C:\src\chrome\src>ninja -C out\gndbg obj/third_party/WebKit/Source/platform/platform/Precompile-platform.cc.obj obj/thir d_party/WebKit/Source/core/core/Precompile-core.cc.obj ninja: Entering directory `out\gndbg' [6/6] CXX obj/third_party/WebKit/Source/core/core/Precompile-core.cc.obj FAILED: obj/third_party/WebKit/Source/core/core/Precompile-core.cc.obj ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/third_party/WebKit/Source/c ore/core/Precompile-core.cc.obj.rsp /c ../../third_party/WebKit/Source/core/win/Precompile-core.cpp /Foobj/third_party/W ebKit/Source/core/core/Precompile-core.cc.obj /Fd"obj/third_party/WebKit/Source/core/core_cc.pdb" Assertion failed: DefaultArg && "sema forgot to instantiate default args", file E:\b\build\slave\win_upload_clang\build\ src\third_party\llvm\tools\clang\lib\CodeGen\MicrosoftCXXABI.cpp, line 3880 Could not write crash dump file: permission denied #0 0x0000000140b0a8c6 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x146a8c6) #1 0x00000001425f1426 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f51426) #2 0x00000001425ebbd8 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f4bbd8) #3 0x00000001425de57e (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f3e57e) #4 0x00000001425de61a (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f3e61a) #5 0x0000000140da17a1 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x17017a1) #6 0x0000000140d96d96 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x16f6d96) #7 0x00000001425a5fbf (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f05fbf) #8 0x00000001425a4f33 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f04f33) #9 0x00000001410c86ac (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x1a286ac) #10 0x000000014182ba02 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x218ba02) #11 0x000000014182bb8e (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x218bb8e) #12 0x000000014173ae49 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x209ae49) #13 0x000000014108e159 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x19ee159) #14 0x000000014107f631 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x19df631) #15 0x00000001410ffbed (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x1a5fbed) #16 0x000000013f6a5f6f (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x5f6f) #17 0x000000013f6a459b (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x459b) #18 0x00000001425d7ce5 (C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2f37ce5) #19 0x00000000773c59cd (C:\Windows\system32\kernel32.dll+0x159cd) #20 0x00000000774fa561 (C:\Windows\SYSTEM32\ntdll.dll+0x2a561) clang-cl.exe: error: clang frontend command failed due to signal (use -v to see invocation) clang version 4.0.0 (trunk 284979) Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: C:\src\chrome\src\third_party\llvm-build\Release+Asserts\bin clang-cl.exe: note: diagnostic msg: PLEASE submit a bug report to http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and associated run script. clang-cl.exe: note: diagnostic msg: ******************** PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT: Preprocessed source(s) and associated run script(s) are located at: clang-cl.exe: note: diagnostic msg: C:\Users\thakis\AppData\Local\Temp\Precompile-core-4e7984.sh clang-cl.exe: note: diagnostic msg: ******************** ninja: build stopped: subcommand failed.
,
Nov 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b7b04e7fa59d53b8b7917d4c0cf4081abdfb0e3 commit 5b7b04e7fa59d53b8b7917d4c0cf4081abdfb0e3 Author: thakis <thakis@chromium.org> Date: Tue Nov 22 23:55:14 2016 Disable Precompile-platform with clang-cl. It currently asserts while trying to build that PCH file. BUG= 667891 , 495697 NOTRY=true Review-Url: https://codereview.chromium.org/2520833005 Cr-Commit-Position: refs/heads/master@{#434028} [modify] https://crrev.com/5b7b04e7fa59d53b8b7917d4c0cf4081abdfb0e3/third_party/WebKit/Source/core/BUILD.gn
,
Nov 23 2016
Crash fixed in upstream r287774
,
Nov 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7dd55ef76fd36f1840677c3277ee0e598a50806a commit 7dd55ef76fd36f1840677c3277ee0e598a50806a Author: thakis <thakis@chromium.org> Date: Thu Nov 24 20:08:24 2016 Roll clang 287685:287780. Ran `tools/clang/scripts/upload_revision.py 287780`. Quick follow-up to https://codereview.chromium.org/2523073002, picks up fixes for a PCH crash and for asan x86 dll builds. BUG= 667891 , 668212 TBR=rnk Review-Url: https://codereview.chromium.org/2528733002 Cr-Commit-Position: refs/heads/master@{#434386} [modify] https://crrev.com/7dd55ef76fd36f1840677c3277ee0e598a50806a/tools/clang/scripts/update.py
,
Nov 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a0eb8ed36a1c3e2c5becc95ef03d42655c7cd95c commit a0eb8ed36a1c3e2c5becc95ef03d42655c7cd95c Author: thakis <thakis@chromium.org> Date: Fri Nov 25 17:32:09 2016 Revert of Disable Precompile-platform with clang-cl. (patchset #1 id:1 of https://codereview.chromium.org/2520833005/ ) Reason for revert: The crash is now hopefully fixed in clang-cl. Original issue's description: > Disable Precompile-platform with clang-cl. > > It currently asserts while trying to build that PCH file. > > BUG= 667891 , 495697 > NOTRY=true > > Committed: https://crrev.com/5b7b04e7fa59d53b8b7917d4c0cf4081abdfb0e3 > Cr-Commit-Position: refs/heads/master@{#434028} TBR=rnk@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 667891 , 495697 Review-Url: https://codereview.chromium.org/2526383002 Cr-Commit-Position: refs/heads/master@{#434534} [modify] https://crrev.com/a0eb8ed36a1c3e2c5becc95ef03d42655c7cd95c/third_party/WebKit/Source/core/BUILD.gn
,
Nov 28 2016
,
Nov 28 2016
Re-opening, because we assert later for a different reason: FAILED: obj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj.rsp /c ../../third_party/WebKit/Source/core/win/Precompile-core.cpp /Foobj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj /Fd"obj/third_party/WebKit/Source/core/workers/workers_cc.pdb" Assertion failed: AllowUnresolvedNodes && "Cannot handle unresolved nodes", file E:\b\build\slave\win_upload_clang\build\src\third_party\llvm\lib\IR\DIBuilder.cpp, line 38 This only happens with gn symbol_level=2, which enables debug info generation and is the default.
,
Nov 29 2016
I was only able to reproduce this with Chromium plugins. Here's the symbolized stack trace. Note that it's rooted in FindBadConstructs. Somehow AST deserialization triggered by FindBadConstructs is causing Clang to emit a global variable. I think the path from AST -> lazy deserialization -> codegen is working-as-designed, but we probably don't want generic RAV-based tools to hit it. C:\src\chromium\src\out\clang>ninja obj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj [1/1] CXX obj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj FAILED: obj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj.rsp /c ../../third_party/WebKit/Source/core/win/Precompile-core.cpp /Foobj/third_party/WebKit/Source/core/workers/workers/Precompile-core.cc.obj /Fd"obj/third_party/WebKit/Source/core/workers/workers_cc.pdb" Assertion failed: AllowUnresolvedNodes && "Cannot handle unresolved nodes", file C:\src\chromium\src\third_party\llvm\lib\IR\DIBuilder.cpp, line 38 Wrote crash dump file "C:\Users\rnk\AppData\Local\Temp\clang-cl.exe-af2ac0.dmp" #0 0x00007ff60208bc35 HandleAbort (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x13ebc35) #1 0x00007ff603cc7d1e raise d:\th\minkernel\crts\ucrt\src\appcrt\misc\signal.cpp:516:0 #2 0x00007ff603c9b744 abort d:\th\minkernel\crts\ucrt\src\appcrt\startup\abort.cpp:71:0 #3 0x00007ff603c9a402 common_assert_to_stderr<wchar_t> d:\th\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:149:0 #4 0x00007ff603c9a9ca _wassert d:\th\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:404:0 #5 0x00007ff601d59777 llvm::DIBuilder::trackIfUnresolved(class llvm::MDNode *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x10b9777) #6 0x00007ff601d56f93 llvm::DIBuilder::createReplaceableCompositeType(unsigned int,class llvm::StringRef,class llvm::DIScope *,class llvm::DIFile *,unsigned int,unsigned int,unsigned __int64,unsigned int,enum llvm::DINode::DIFlags,class llvm::StringRef) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x10b6f93) #7 0x00007ff602276c22 clang::CodeGen::CGDebugInfo::CreateLimitedType(class clang::RecordType const *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15d6c22) #8 0x00007ff6022882d5 clang::CodeGen::CGDebugInfo::getOrCreateLimitedType(class clang::RecordType const *,class llvm::DIFile *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15e82d5) #9 0x00007ff60227b635 clang::CodeGen::CGDebugInfo::CreateTypeDefinition(class clang::RecordType const *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15db635) #10 0x00007ff60227bca7 clang::CodeGen::CGDebugInfo::CreateTypeNode(class clang::QualType,class llvm::DIFile *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15dbca7) #11 0x00007ff602289c6d clang::CodeGen::CGDebugInfo::getOrCreateType(class clang::QualType,class llvm::DIFile *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15e9c6d) #12 0x00007ff602277283 clang::CodeGen::CGDebugInfo::CreateQualifiedType(class clang::QualType,class llvm::DIFile *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15d7283) #13 0x00007ff60227be02 clang::CodeGen::CGDebugInfo::CreateTypeNode(class clang::QualType,class llvm::DIFile *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15dbe02) #14 0x00007ff602289c6d clang::CodeGen::CGDebugInfo::getOrCreateType(class clang::QualType,class llvm::DIFile *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15e9c6d) #15 0x00007ff60227f15d clang::CodeGen::CGDebugInfo::EmitGlobalVariable(class llvm::GlobalVariable *,class clang::VarDecl const *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15df15d) #16 0x00007ff60229c4a1 clang::CodeGen::CodeGenModule::EmitGlobalVarDefinition(class clang::VarDecl const *,bool) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15fc4a1) #17 0x00007ff60229b7b4 clang::CodeGen::CodeGenModule::EmitGlobalDefinition(class clang::GlobalDecl,class llvm::GlobalValue *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15fb7b4) #18 0x00007ff60229b01b clang::CodeGen::CodeGenModule::EmitGlobal(class clang::GlobalDecl) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15fb01b) #19 0x00007ff60229de52 clang::CodeGen::CodeGenModule::EmitTopLevelDecl(class clang::Decl *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x15fde52) #20 0x00007ff603e6f8cf clang::CodeGenerator::GetModule(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x31cf8cf) #21 0x00007ff603e6d595 clang::BackendConsumer::HandleTopLevelDecl(class clang::DeclGroupRef) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x31cd595) #22 0x00007ff6039d07fa clang::ASTConsumer::HandleInterestingDecl(class clang::DeclGroupRef) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2d307fa) #23 0x00007ff60268a4e9 clang::MultiplexConsumer::HandleInterestingDecl(class clang::DeclGroupRef) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x19ea4e9) #24 0x00007ff602f5bb3d clang::ASTReader::PassInterestingDeclsToConsumer(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x22bbb3d) #25 0x00007ff602f522c1 clang::ASTReader::FinishedDeserializing(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x22b22c1) #26 0x00007ff603998643 clang::DeclContext::LoadLexicalDeclsFromExternalStorage(void)const (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2cf8643) #27 0x00007ff60399cd1b clang::DeclContext::decls_begin(void)const (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x2cfcd1b) #28 0x00007ff600d304ea clang::RecursiveASTVisitor<class chrome_checker::FindBadConstructsConsumer>::TraverseDeclContextHelper(class clang::DeclContext *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x904ea) #29 0x00007ff600d3c234 clang::RecursiveASTVisitor<class chrome_checker::FindBadConstructsConsumer>::TraverseNamespaceDecl(class clang::NamespaceDecl *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x9c234) #30 0x00007ff600d2fd3e clang::RecursiveASTVisitor<class chrome_checker::FindBadConstructsConsumer>::TraverseDecl(class clang::Decl *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x8fd3e) #31 0x00007ff600d30549 clang::RecursiveASTVisitor<class chrome_checker::FindBadConstructsConsumer>::TraverseDeclContextHelper(class clang::DeclContext *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x90549) #32 0x00007ff600d4f994 clang::RecursiveASTVisitor<class chrome_checker::FindBadConstructsConsumer>::TraverseTranslationUnitDecl(class clang::TranslationUnitDecl *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0xaf994) #33 0x00007ff600d30249 clang::RecursiveASTVisitor<class chrome_checker::FindBadConstructsConsumer>::TraverseDecl(class clang::Decl *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x90249) #34 0x00007ff600d1e19c chrome_checker::FindBadConstructsConsumer::Traverse(class clang::ASTContext &) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x7e19c) #35 0x00007ff60268a67c clang::MultiplexConsumer::HandleTranslationUnit(class clang::ASTContext &) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x19ea67c) #36 0x00007ff602ebb126 clang::ParseAST(class clang::Sema &,bool,bool) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x221b126) #37 0x00007ff60263a45b clang::ASTFrontendAction::ExecuteAction(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x199a45b) #38 0x00007ff603e6d1e8 clang::CodeGenAction::ExecuteAction(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x31cd1e8) #39 0x00007ff60263a2fb clang::FrontendAction::Execute(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x199a2fb) #40 0x00007ff60262af3c clang::CompilerInstance::ExecuteAction(class clang::FrontendAction &) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x198af3c) #41 0x00007ff6026dd5e3 clang::ExecuteCompilerInvocation(class clang::CompilerInstance *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x1a3d5e3) #42 0x00007ff600d04dd9 cc1_main(class llvm::ArrayRef<char const *>,char const *,void *) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x64dd9) #43 0x00007ff600cff890 clang::DiagnosticConsumer::EndSourceFile(void) (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x5f890) #44 0x00007ff600d02383 main (C:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe+0x62383) #45 0x00007ff603c8bdb5 __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253:0 #46 0x00007ff8ef1b8102 (C:\Windows\system32\KERNEL32.DLL+0x18102) #47 0x00007ff8f165c5b4 (C:\Windows\SYSTEM32\ntdll.dll+0x5c5b4)
,
Nov 29 2016
Reduction:
$ cat t.h
struct nullopt_t {
constexpr explicit nullopt_t(int) {}
};
constexpr nullopt_t nullopt(0);
$ cat t.cpp
//empty
# With Chromium's clang-cl on PATH
$ clang-cl -c -FIt.h -Yct.h t.cpp -Z7 -Xclang -add-plugin -Xclang find-bad-constructs
Assertion failed: AllowUnresolvedNodes && "Cannot handle unresolved nodes", file C:\src\chromium\src\third_party\llvm\lib\IR\DIBuilder.cpp, line 40
...
There's no assertion if you disable the plugin or disable debug info.
,
Nov 29 2016
hmm. maybe due to https://codereview.chromium.org/1135333007 -- this apparently made its way into chrome here https://codereview.chromium.org/1810243002/diff/100001/tools/clang/plugins/FindBadConstructsConsumer.cpp at the bottom, but without any checks, so now we don't delay-parse anything but system headers (!)
,
Nov 29 2016
It seems like RAV-based plugins might defeat Clang's PCH strategy. Clang's PCH is predicated on the idea that most of the serialized AST won't be touched, so it should be a win to lazily deserialize it on demand. By recursively traversing all decls in the TU, RAV plugins might erase those gains by touching everything.
That aside, the issue at hand is that IRgen's debug info generation assumes that it's not going to be called back after its ASTConsumer::HandleTranslationUnit callback returns. It finalizes llvm::DIBuilder, so subsequent attempts to generate type information will assert. Adding a multiplexed ASTConsumer breaks this assumption, because we pass the TU to CodeGen, then to FindBadConstructsConsumer, which triggers further deserialization, which calls back into CodeGen.
So, the quickest fix is to patch our plugin so that it always runs before codegen:
diff --git a/tools/clang/plugins/FindBadConstructsAction.h b/tools/clang/plugins/FindBadConstructsAction.h
index 383db84..99852e4 100644
--- a/tools/clang/plugins/FindBadConstructsAction.h
+++ b/tools/clang/plugins/FindBadConstructsAction.h
@@ -23,6 +23,8 @@ class FindBadConstructsAction : public clang::PluginASTAction {
virtual bool ParseArgs(const clang::CompilerInstance& instance,
const std::vector<std::string>& args);
+ ActionType getActionType() override { return AddBeforeMainAction; }
+
private:
Options options_;
};
This also means we don't need to pass '-Xclang -add-plugin -Xclang find-bad-constructs', but it means we can't disable the plugins on Windows, because they're always statically linked into the clang binary.
A different fix would be to change RAV or our usage of RAV to skip over top level decls from external AST sources. This might also improve the performance of PCH, so it seems like it might be worth it.
,
Nov 29 2016
Nice analysis! I'm surprised we accepted http://reviews.llvm.org/D17959 given that we've said for years that the reason we don't support this is because people are going to try and mutate asts and that's not supported. Skipping external decls sounds good I guess?
,
Nov 30 2016
r288221 and r288222 should make it so we ignore "interesting" decls deserialized by plugins. Richard confirmed that the simple RAVs in Chromium's style plugins are likely eroding most of the benefits of PCH. Before we do anything more here, we should roll past r288222, re-enable PCH, and then do a local build time measurement with and without Chrome's style plugins to see if this matter at all. If there is a big difference between PCH with plugins and PCH with no plugins, we can hack up the plugins to avoid traversing decls from AST files. If that shows an improvement, we should add an option to RAV like 'shouldVisitASTFiles()', and move the complexity into RAV. I think any style or diagnostic plugin using RAV will want to enable that option.
,
Dec 1 2016
,
Dec 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/566780fc9b55f2db1ba5a260589d3be0f6648e12 commit 566780fc9b55f2db1ba5a260589d3be0f6648e12 Author: thakis <thakis@chromium.org> Date: Mon Dec 05 17:58:02 2016 clang/win: Enable full symbols on one of the ToT and one of the pinned bots. This would've allowed us to find a crash with pchs on the bots. BUG= 667891 Review-Url: https://codereview.chromium.org/2551113002 Cr-Commit-Position: refs/heads/master@{#436346} [modify] https://crrev.com/566780fc9b55f2db1ba5a260589d3be0f6648e12/tools/mb/mb_config.pyl
,
Dec 5 2016
,
Dec 5 2016
We still need to measure if the plugin slows down builds, and if so address that, but this bug here is done.
,
Dec 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/171edcc0653bf8abfea519465208fc92998a54ce commit 171edcc0653bf8abfea519465208fc92998a54ce Author: thakis <thakis@chromium.org> Date: Mon Dec 05 23:45:44 2016 Revert of Disable Precompile-platform with clang-cl. (patchset #1 id:1 of https://codereview.chromium.org/2533853003/ ) Reason for revert: This should work after yesterday's clang roll. We now have a bot that builds with symbols, and I was able to build blink_tests with symbols locally with this reverted locally. Original issue's description: > Reland of Disable Precompile-platform with clang-cl. (patchset #1 id:1 of https://codereview.chromium.org/2526383002/ ) > > Reason for revert: > Clang still asserts when building Precompile-core.obj, but it's somewhere else: > Assertion failed: AllowUnresolvedNodes && "Cannot handle unresolved nodes"... > > Original issue's description: > > Revert of Disable Precompile-platform with clang-cl. (patchset #1 id:1 of https://codereview.chromium.org/2520833005/ ) > > > > Reason for revert: > > The crash is now hopefully fixed in clang-cl. > > > > Original issue's description: > > > Disable Precompile-platform with clang-cl. > > > > > > It currently asserts while trying to build that PCH file. > > > > > > BUG= 667891 , 495697 > > > NOTRY=true > > > > > > Committed: https://crrev.com/5b7b04e7fa59d53b8b7917d4c0cf4081abdfb0e3 > > > Cr-Commit-Position: refs/heads/master@{#434028} > > > > TBR=rnk@chromium.org > > # Not skipping CQ checks because original CL landed more than 1 days ago. > > BUG= 667891 , 495697 > > > > Committed: https://crrev.com/a0eb8ed36a1c3e2c5becc95ef03d42655c7cd95c > > Cr-Commit-Position: refs/heads/master@{#434534} > > TBR=thakis@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG= 667891 , 495697 > > Committed: https://crrev.com/c43462be8948e6a3a1c9a7d6bc6b10fc5fc789b8 > Cr-Commit-Position: refs/heads/master@{#434737} TBR=rnk@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 667891 , 495697 Review-Url: https://codereview.chromium.org/2551213002 Cr-Commit-Position: refs/heads/master@{#436458} [modify] https://crrev.com/171edcc0653bf8abfea519465208fc92998a54ce/third_party/WebKit/Source/core/BUILD.gn
,
Dec 7 2016
I built "blink_tests" in component debug (just is_clang=true) with and without the plugin, and build time was 69m15s with plugin and 66m11s. I built just once each time, so the difference could be completely due to cache effects. So from what I can tell, the plugin doesn't make pch time much worse (which is a pity, since the pch doesn't seem to help us much at all).
Here's my "disable plugins" patch:
diff --git a/build/config/clang/BUILD.gn b/build/config/clang/BUILD.gn
index db3e03c..2276947 100644
--- a/build/config/clang/BUILD.gn
+++ b/build/config/clang/BUILD.gn
@@ -12,39 +12,39 @@ config("find_bad_constructs") {
# On Windows, the plugin is built directly into clang, so there's
# no need to load it dynamically.
- if (is_mac || is_ios) {
- cflags += [
- "-Xclang",
- "-load",
- "-Xclang",
- rebase_path("${clang_base_path}/lib/libFindBadConstructs.dylib",
- root_build_dir),
- ]
- } else if (is_linux || is_android) {
- cflags += [
- "-Xclang",
- "-load",
- "-Xclang",
- rebase_path("${clang_base_path}/lib/libFindBadConstructs.so",
- root_build_dir),
- ]
- }
-
- cflags += [
- "-Xclang",
- "-add-plugin",
- "-Xclang",
- "find-bad-constructs",
- ]
-
- if ((is_linux || is_android) && !is_chromecast) {
- cflags += [
- "-Xclang",
- "-plugin-arg-find-bad-constructs",
- "-Xclang",
- "check-ipc",
- ]
- }
+ #if (is_mac || is_ios) {
+ # cflags += [
+ # "-Xclang",
+ # "-load",
+ # "-Xclang",
+ # rebase_path("${clang_base_path}/lib/libFindBadConstructs.dylib",
+ # root_build_dir),
+ # ]
+ #} else if (is_linux || is_android) {
+ # cflags += [
+ # "-Xclang",
+ # "-load",
+ # "-Xclang",
+ # rebase_path("${clang_base_path}/lib/libFindBadConstructs.so",
+ # root_build_dir),
+ # ]
+ #}
+
+ #cflags += [
+ # "-Xclang",
+ # "-add-plugin",
+ # "-Xclang",
+ # "find-bad-constructs",
+ #]
+
+ #if ((is_linux || is_android) && !is_chromecast) {
+ # cflags += [
+ # "-Xclang",
+ # "-plugin-arg-find-bad-constructs",
+ # "-Xclang",
+ # "check-ipc",
+ # ]
+ #}
}
}
diff --git a/third_party/WebKit/Source/BUILD.gn b/third_party/WebKit/Source/BUILD.gn
index 0218e80..f3b51b3 100644
--- a/third_party/WebKit/Source/BUILD.gn
+++ b/third_party/WebKit/Source/BUILD.gn
@@ -111,44 +111,44 @@ config("config") {
if (is_clang && blink_gc_plugin && clang_use_chrome_plugins) {
# On Windows, the plugin is built directly into clang, so there's
# no need to load it dynamically.
- if (!is_win) {
- _blink_gc_plugin_dll_extension = "so"
- if (is_mac || is_ios) {
- _blink_gc_plugin_dll_extension = "dylib"
- }
- cflags += [
- "-Xclang",
- "-load",
- rebase_path(
- "${clang_base_path}/lib/libBlinkGCPlugin.${_blink_gc_plugin_dll_extension}",
- root_build_dir),
- ]
- }
- cflags += [
- "-Xclang",
- "-add-plugin",
- "-Xclang",
- "blink-gc-plugin",
- ]
-
- # Add arguments for enabled GC plugin options:
- if (blink_gc_plugin_option_do_dump_graph) {
- cflags += [
- "-Xclang",
- "-plugin-arg-blink-gc-plugin",
- "-Xclang",
- "dump-graph",
- ]
- }
- if (blink_gc_plugin_option_warn_unneeded_finalizer) {
- cflags += [
- "-Xclang",
- "-plugin-arg-blink-gc-plugin",
- "-Xclang",
- "warn-unneeded-finalizer",
- ]
- }
+ #if (!is_win) {
+ # _blink_gc_plugin_dll_extension = "so"
+ # if (is_mac || is_ios) {
+ # _blink_gc_plugin_dll_extension = "dylib"
+ # }
+ # cflags += [
+ # "-Xclang",
+ # "-load",
+ # "-Xclang",
+ # rebase_path(
+ # "${clang_base_path}/lib/libBlinkGCPlugin.${_blink_gc_plugin_dll_extension}",
+ # root_build_dir),
+ # ]
+ #}
+ #cflags += [
+ # "-Xclang",
+ # "-add-plugin",
+ # "-Xclang",
+ # "blink-gc-plugin",
+ #]
+
+ ## Add arguments for enabled GC plugin options:
+ #if (blink_gc_plugin_option_do_dump_graph) {
+ # cflags += [
+ # "-Xclang",
+ # "-plugin-arg-blink-gc-plugin",
+ # "-Xclang",
+ # "dump-graph",
+ # ]
+ #}
+ #if (blink_gc_plugin_option_warn_unneeded_finalizer) {
+ # cflags += [
+ # "-Xclang",
+ # "-plugin-arg-blink-gc-plugin",
+ # "-Xclang",
+ # "warn-unneeded-finalizer",
+ # ]
+ #}
}
}
,
Dec 7 2016
Thanks for the data! What platform was that on? I guess one of two things is happening: 1. AST deserialization is just too slow 2. We are doing too much deserialization even without the plugins We should investigate 2 first, since it's much less work. We might find some clang-side improvements and chrome-side source tweaks to deserialize less. All that said, I'll open a new bug for it.
,
Dec 7 2016
Bug for PCH improvements: https://bugs.chromium.org/p/chromium/issues/detail?id=672115
,
Dec 7 2016
Windows. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by r...@chromium.org
, Nov 22 2016