r183717 - [libclang] Allow building a precompiled preamble with compiler errors

Kostya Serebryany kcc at google.com
Tue Jun 11 08:05:22 PDT 2013


confirmed, our bot is green again, thanks!


On Tue, Jun 11, 2013 at 5:08 PM, Benjamin Kramer <benny.kra at gmail.com>wrote:

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


More information about the cfe-commits mailing list