<div dir="ltr">confirmed, our bot is green again, thanks! </div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 11, 2013 at 5:08 PM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
On 11.06.2013, at 11:36, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
<br>
> Hi Argyrios,<br>
><br>
> This change causes a use-after-free bug: <a href="http://llvm.org/bugs/show_bug.cgi?id=16295" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=16295</a><br>
> We'd appreciate if you can fix or revert this asap (our ASAN bootstrap bot is red).<br>
<br>
</div>Should be fixed in r183741.<br>
<br>
- Ben<br>
<div class="HOEnZb"><div class="h5">><br>
> --kcc<br>
><br>
><br>
> On Tue, Jun 11, 2013 at 4:36 AM, Argyrios Kyrtzidis <<a href="mailto:akyrtzi@gmail.com">akyrtzi@gmail.com</a>> wrote:<br>
> 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>
><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>
<br>
</div></div></blockquote></div><br></div>