<div dir="ltr">Hi Argyrios,<div><br></div><div>This change causes a use-after-free bug: <a href="http://llvm.org/bugs/show_bug.cgi?id=16295">http://llvm.org/bugs/show_bug.cgi?id=16295</a></div><div>We'd appreciate if you can fix or revert this asap (our ASAN bootstrap bot is red).</div>
<div><br></div><div>--kcc </div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 11, 2013 at 4:36 AM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" target="_blank">akyrtzi@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: akirtzidis<br>
Date: Mon Jun 10 19:36:55 2013<br>
New Revision: 183717<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=183717&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=183717&view=rev</a><br>
Log:<br>
[libclang] Allow building a precompiled preamble with compiler errors<br>
<br>
A while ago we allowed libclang to build a PCH that had compiler errors; this was to retain the performance<br>
afforded by a PCH even if the user's code is in an intermediate state.<br>
<br>
Extend this for the precompiled preamble as well.<br>
<br>
rdar://14109828<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Frontend/FrontendAction.h<br>
cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
cfe/trunk/lib/Frontend/ASTUnit.cpp<br>
cfe/trunk/lib/Frontend/FrontendAction.cpp<br>
cfe/trunk/lib/Serialization/GeneratePCH.cpp<br>
cfe/trunk/test/Index/preamble-reparse-with-BOM.m<br>
<br>
Modified: cfe/trunk/include/clang/Frontend/FrontendAction.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendAction.h?rev=183717&r1=183716&r2=183717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendAction.h?rev=183717&r1=183716&r2=183717&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Frontend/FrontendAction.h (original)<br>
+++ cfe/trunk/include/clang/Frontend/FrontendAction.h Mon Jun 10 19:36:55 2013<br>
@@ -94,6 +94,14 @@ protected:<br>
/// BeginSourceFileAction (and BeginSourceFile).<br>
virtual void EndSourceFileAction() {}<br>
<br>
+ /// \brief Callback at the end of processing a single input, to determine<br>
+ /// if the output files should be erased or not.<br>
+ ///<br>
+ /// By default it returns true if a compiler error occurred.<br>
+ /// This is guaranteed to only be called following a successful call to<br>
+ /// BeginSourceFileAction (and BeginSourceFile).<br>
+ virtual bool shouldEraseOutputFiles();<br>
+<br>
/// @}<br>
<br>
public:<br>
<br>
Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=183717&r1=183716&r2=183717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=183717&r1=183716&r2=183717&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)<br>
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Mon Jun 10 19:36:55 2013<br>
@@ -747,6 +747,8 @@ class PCHGenerator : public SemaConsumer<br>
SmallVector<char, 128> Buffer;<br>
llvm::BitstreamWriter Stream;<br>
ASTWriter Writer;<br>
+ bool AllowASTWithErrors;<br>
+ bool HasEmittedPCH;<br>
<br>
protected:<br>
ASTWriter &getWriter() { return Writer; }<br>
@@ -755,12 +757,15 @@ protected:<br>
public:<br>
PCHGenerator(const Preprocessor &PP, StringRef OutputFile,<br>
clang::Module *Module,<br>
- StringRef isysroot, raw_ostream *Out);<br>
+ StringRef isysroot, raw_ostream *Out,<br>
+ bool AllowASTWithErrors = false);<br>
~PCHGenerator();<br>
virtual void InitializeSema(Sema &S) { SemaPtr = &S; }<br>
virtual void HandleTranslationUnit(ASTContext &Ctx);<br>
virtual ASTMutationListener *GetASTMutationListener();<br>
virtual ASTDeserializationListener *GetASTDeserializationListener();<br>
+<br>
+ bool hasEmittedPCH() const { return HasEmittedPCH; }<br>
};<br>
<br>
} // end namespace clang<br>
<br>
Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=183717&r1=183716&r2=183717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=183717&r1=183716&r2=183717&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Mon Jun 10 19:36:55 2013<br>
@@ -974,8 +974,8 @@ class PrecompilePreambleConsumer : publi<br>
public:<br>
PrecompilePreambleConsumer(ASTUnit &Unit, const Preprocessor &PP,<br>
StringRef isysroot, raw_ostream *Out)<br>
- : PCHGenerator(PP, "", 0, isysroot, Out), Unit(Unit),<br>
- Hash(Unit.getCurrentTopLevelHashValue()) {<br>
+ : PCHGenerator(PP, "", 0, isysroot, Out, /*AllowASTWithErrors=*/true),<br>
+ Unit(Unit), Hash(Unit.getCurrentTopLevelHashValue()) {<br>
Hash = 0;<br>
}<br>
<br>
@@ -996,7 +996,7 @@ public:<br>
<br>
virtual void HandleTranslationUnit(ASTContext &Ctx) {<br>
PCHGenerator::HandleTranslationUnit(Ctx);<br>
- if (!Unit.getDiagnostics().hasErrorOccurred()) {<br>
+ if (hasEmittedPCH()) {<br>
// Translate the top-level declarations we captured during<br>
// parsing into declaration IDs in the precompiled<br>
// preamble. This will allow us to deserialize those top-level<br>
@@ -1010,9 +1010,11 @@ public:<br>
<br>
class PrecompilePreambleAction : public ASTFrontendAction {<br>
ASTUnit &Unit;<br>
+ PrecompilePreambleConsumer *PreambleConsumer;<br>
<br>
public:<br>
- explicit PrecompilePreambleAction(ASTUnit &Unit) : Unit(Unit) {}<br>
+ explicit PrecompilePreambleAction(ASTUnit &Unit)<br>
+ : Unit(Unit), PreambleConsumer(0) {}<br>
<br>
virtual ASTConsumer *CreateASTConsumer(CompilerInstance &CI,<br>
StringRef InFile) {<br>
@@ -1029,10 +1031,16 @@ public:<br>
<br>
CI.getPreprocessor().addPPCallbacks(<br>
new MacroDefinitionTrackerPPCallbacks(Unit.getCurrentTopLevelHashValue()));<br>
- return new PrecompilePreambleConsumer(Unit, CI.getPreprocessor(), Sysroot,<br>
- OS);<br>
+ PreambleConsumer = new PrecompilePreambleConsumer(Unit,CI.getPreprocessor(),<br>
+ Sysroot, OS);<br>
+ return PreambleConsumer;<br>
}<br>
<br>
+ bool hasEmittedPreamblePCH() const {<br>
+ return PreambleConsumer && PreambleConsumer->hasEmittedPCH();<br>
+ }<br>
+ virtual bool shouldEraseOutputFiles() { return !hasEmittedPreamblePCH(); }<br>
+<br>
virtual bool hasCodeCompletionSupport() const { return false; }<br>
virtual bool hasASTFileSupport() const { return false; }<br>
virtual TranslationUnitKind getTranslationUnitKind() { return TU_Prefix; }<br>
@@ -1622,7 +1630,7 @@ llvm::MemoryBuffer *ASTUnit::getMainBuff<br>
Act->Execute();<br>
Act->EndSourceFile();<br>
<br>
- if (Diagnostics->hasErrorOccurred()) {<br>
+ if (!Act->hasEmittedPreamblePCH()) {<br>
// There were errors parsing the preamble, so no precompiled header was<br>
// generated. Forget that we even tried.<br>
// FIXME: Should we leave a note for ourselves to try again?<br>
<br>
Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=183717&r1=183716&r2=183717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=183717&r1=183716&r2=183717&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Mon Jun 10 19:36:55 2013<br>
@@ -429,9 +429,9 @@ void FrontendAction::EndSourceFile() {<br>
llvm::errs() << "\n";<br>
}<br>
<br>
- // Cleanup the output streams, and erase the output files if we encountered<br>
- // an error.<br>
- CI.clearOutputFiles(/*EraseFiles=*/CI.getDiagnostics().hasErrorOccurred());<br>
+ // Cleanup the output streams, and erase the output files if instructed by the<br>
+ // FrontendAction.<br>
+ CI.clearOutputFiles(/*EraseFiles=*/shouldEraseOutputFiles());<br>
<br>
if (isCurrentFileAST()) {<br>
CI.takeSema();<br>
@@ -445,6 +445,10 @@ void FrontendAction::EndSourceFile() {<br>
setCurrentInput(FrontendInputFile());<br>
}<br>
<br>
+bool FrontendAction::shouldEraseOutputFiles() {<br>
+ return getCompilerInstance().getDiagnostics().hasErrorOccurred();<br>
+}<br>
+<br>
//===----------------------------------------------------------------------===//<br>
// Utility Actions<br>
//===----------------------------------------------------------------------===//<br>
<br>
Modified: cfe/trunk/lib/Serialization/GeneratePCH.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/GeneratePCH.cpp?rev=183717&r1=183716&r2=183717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/GeneratePCH.cpp?rev=183717&r1=183716&r2=183717&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Serialization/GeneratePCH.cpp (original)<br>
+++ cfe/trunk/lib/Serialization/GeneratePCH.cpp Mon Jun 10 19:36:55 2013<br>
@@ -28,22 +28,29 @@ PCHGenerator::PCHGenerator(const Preproc<br>
StringRef OutputFile,<br>
clang::Module *Module,<br>
StringRef isysroot,<br>
- raw_ostream *OS)<br>
+ raw_ostream *OS, bool AllowASTWithErrors)<br>
: PP(PP), OutputFile(OutputFile), Module(Module),<br>
isysroot(isysroot.str()), Out(OS),<br>
- SemaPtr(0), Stream(Buffer), Writer(Stream) {<br>
+ SemaPtr(0), Stream(Buffer), Writer(Stream),<br>
+ AllowASTWithErrors(AllowASTWithErrors),<br>
+ HasEmittedPCH(false) {<br>
}<br>
<br>
PCHGenerator::~PCHGenerator() {<br>
}<br>
<br>
void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {<br>
- if (PP.getDiagnostics().hasErrorOccurred())<br>
+ // Don't create a PCH if there were fatal failures during module loading.<br>
+ if (PP.getModuleLoader().HadFatalFailure)<br>
+ return;<br>
+<br>
+ bool hasErrors = PP.getDiagnostics().hasErrorOccurred();<br>
+ if (hasErrors && !AllowASTWithErrors)<br>
return;<br>
<br>
// Emit the PCH file<br>
assert(SemaPtr && "No Sema?");<br>
- Writer.WriteAST(*SemaPtr, OutputFile, Module, isysroot);<br>
+ Writer.WriteAST(*SemaPtr, OutputFile, Module, isysroot, hasErrors);<br>
<br>
// Write the generated bitstream to "Out".<br>
Out->write((char *)&Buffer.front(), Buffer.size());<br>
@@ -53,6 +60,8 @@ void PCHGenerator::HandleTranslationUnit<br>
<br>
// Free up some memory, in case the process is kept alive.<br>
Buffer.clear();<br>
+<br>
+ HasEmittedPCH = true;<br>
}<br>
<br>
ASTMutationListener *PCHGenerator::GetASTMutationListener() {<br>
<br>
Modified: cfe/trunk/test/Index/preamble-reparse-with-BOM.m<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-reparse-with-BOM.m?rev=183717&r1=183716&r2=183717&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-reparse-with-BOM.m?rev=183717&r1=183716&r2=183717&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Index/preamble-reparse-with-BOM.m (original)<br>
+++ cfe/trunk/test/Index/preamble-reparse-with-BOM.m Mon Jun 10 19:36:55 2013<br>
@@ -2,5 +2,7 @@<br>
@interface I2<br>
@end<br>
<br>
-// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_FAILONERROR=1 \<br>
-// RUN: c-index-test -test-load-source-reparse 1 local %s<br>
+// RUN: env CINDEXTEST_EDITING=1 \<br>
+// RUN: c-index-test -test-load-source-reparse 1 local %s | FileCheck %s<br>
+<br>
+// CHECK: preamble-reparse-with-BOM.m:2:12: ObjCInterfaceDecl=I2:2:12 Extent=[2:1 - 3:5]<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>