r202346 - [ASTUnit] Fix use-after-free bug in ASTUnit::getMainBufferWithPrecompiledPreamble().

Argyrios Kyrtzidis akyrtzi at gmail.com
Wed Feb 26 20:12:00 PST 2014


Author: akirtzidis
Date: Wed Feb 26 22:11:59 2014
New Revision: 202346

URL: http://llvm.org/viewvc/llvm-project?rev=202346&view=rev
Log:
[ASTUnit] Fix use-after-free bug in ASTUnit::getMainBufferWithPrecompiledPreamble().

With r197755 we started reading the contents of buffer file entries, but the
buffers may point to ASTReader blobs that have been disposed.

Fix this by having the CompilerInstance object keep a reference to the ASTReader
as well as having the ASTContext keep reference to the ExternalASTSource.

This was very difficult to construct a test case for.
rdar://16149782

Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/include/clang/AST/ExternalASTSource.h
    cfe/trunk/include/clang/Frontend/ASTUnit.h
    cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h
    cfe/trunk/include/clang/Frontend/CompilerInstance.h
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/Frontend/ASTUnit.cpp
    cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/lib/Frontend/FrontendAction.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Feb 26 22:11:59 2014
@@ -416,7 +416,7 @@ public:
   SelectorTable &Selectors;
   Builtin::Context &BuiltinInfo;
   mutable DeclarationNameTable DeclarationNames;
-  OwningPtr<ExternalASTSource> ExternalSource;
+  IntrusiveRefCntPtr<ExternalASTSource> ExternalSource;
   ASTMutationListener *Listener;
 
   /// \brief Contains parents of a node.
@@ -811,11 +811,13 @@ public:
   /// The external AST source provides the ability to load parts of
   /// the abstract syntax tree as needed from some external storage,
   /// e.g., a precompiled header.
-  void setExternalSource(OwningPtr<ExternalASTSource> &Source);
+  void setExternalSource(IntrusiveRefCntPtr<ExternalASTSource> Source);
 
   /// \brief Retrieve a pointer to the external AST source associated
   /// with this AST context, if any.
-  ExternalASTSource *getExternalSource() const { return ExternalSource.get(); }
+  ExternalASTSource *getExternalSource() const {
+    return ExternalSource.getPtr();
+  }
 
   /// \brief Attach an AST mutation listener to the AST context.
   ///

Modified: cfe/trunk/include/clang/AST/ExternalASTSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTSource.h?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ExternalASTSource.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTSource.h Wed Feb 26 22:11:59 2014
@@ -53,7 +53,7 @@ enum ExternalLoadResult {
 /// sources can resolve types and declarations from abstract IDs into
 /// actual type and declaration nodes, and read parts of declaration
 /// contexts.
-class ExternalASTSource {
+class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
   /// \brief Whether this AST source also provides information for
   /// semantic analysis.
   bool SemaSource;

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Wed Feb 26 22:11:59 2014
@@ -75,7 +75,7 @@ private:
   IntrusiveRefCntPtr<ASTContext>          Ctx;
   IntrusiveRefCntPtr<TargetOptions>       TargetOpts;
   IntrusiveRefCntPtr<HeaderSearchOptions> HSOpts;
-  ASTReader *Reader;
+  IntrusiveRefCntPtr<ASTReader> Reader;
   bool HadModuleLoaderFatalFailure;
 
   struct ASTWriterData;

Modified: cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h (original)
+++ cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h Wed Feb 26 22:11:59 2014
@@ -24,13 +24,13 @@ class ChainedIncludesSource : public Ext
 public:
   virtual ~ChainedIncludesSource();
 
-  static ChainedIncludesSource *create(CompilerInstance &CI);
+  static IntrusiveRefCntPtr<ChainedIncludesSource> create(CompilerInstance &CI);
 
   ExternalSemaSource &getFinalReader() const { return *FinalReader; }
 
 private:
   std::vector<CompilerInstance *> CIs;
-  OwningPtr<ExternalSemaSource> FinalReader;
+  IntrusiveRefCntPtr<ExternalSemaSource> FinalReader;
 
   
 protected:

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Wed Feb 26 22:11:59 2014
@@ -102,8 +102,8 @@ class CompilerInstance : public ModuleLo
   /// \brief The frontend timer
   OwningPtr<llvm::Timer> FrontendTimer;
 
-  /// \brief Non-owning reference to the ASTReader, if one exists.
-  ASTReader *ModuleManager;
+  /// \brief The ASTReader, if one exists.
+  IntrusiveRefCntPtr<ASTReader> ModuleManager;
 
   /// \brief The set of top-level modules that has already been loaded,
   /// along with the module map
@@ -454,8 +454,8 @@ public:
   /// @name Module Management
   /// {
 
-  ASTReader *getModuleManager() const { return ModuleManager; }
-  void setModuleManager(ASTReader *Reader) { ModuleManager = Reader; }
+  IntrusiveRefCntPtr<ASTReader> getModuleManager() const;
+  void setModuleManager(IntrusiveRefCntPtr<ASTReader> Reader);
 
   /// }
   /// @name Code Completion

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Feb 26 22:11:59 2014
@@ -791,8 +791,8 @@ void ASTContext::AddDeallocation(void (*
 }
 
 void
-ASTContext::setExternalSource(OwningPtr<ExternalASTSource> &Source) {
-  ExternalSource.reset(Source.take());
+ASTContext::setExternalSource(IntrusiveRefCntPtr<ExternalASTSource> Source) {
+  ExternalSource = Source;
 }
 
 void ASTContext::PrintStats() const {
@@ -846,7 +846,7 @@ void ASTContext::PrintStats() const {
                << NumImplicitDestructors
                << " implicit destructors created\n";
 
-  if (ExternalSource.get()) {
+  if (ExternalSource) {
     llvm::errs() << "\n";
     ExternalSource->PrintStats();
   }

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Wed Feb 26 22:11:59 2014
@@ -718,8 +718,6 @@ ASTUnit *ASTUnit::LoadFromASTFile(const
   HeaderSearch &HeaderInfo = *AST->HeaderInfo.get();
   unsigned Counter;
 
-  OwningPtr<ASTReader> Reader;
-
   AST->PP = new Preprocessor(PPOpts,
                              AST->getDiagnostics(), AST->ASTFileLangOpts,
                              /*Target=*/0, AST->getSourceManager(), HeaderInfo, 
@@ -742,21 +740,17 @@ ASTUnit *ASTUnit::LoadFromASTFile(const
   bool disableValid = false;
   if (::getenv("LIBCLANG_DISABLE_PCH_VALIDATION"))
     disableValid = true;
-  Reader.reset(new ASTReader(PP, Context,
+  AST->Reader = new ASTReader(PP, Context,
                              /*isysroot=*/"",
                              /*DisableValidation=*/disableValid,
-                             AllowPCHWithCompilerErrors));
-  
-  // Recover resources if we crash before exiting this method.
-  llvm::CrashRecoveryContextCleanupRegistrar<ASTReader>
-    ReaderCleanup(Reader.get());
+                             AllowPCHWithCompilerErrors);
 
-  Reader->setListener(new ASTInfoCollector(*AST->PP, Context,
+  AST->Reader->setListener(new ASTInfoCollector(*AST->PP, Context,
                                            AST->ASTFileLangOpts,
                                            AST->TargetOpts, AST->Target, 
                                            Counter));
 
-  switch (Reader->ReadAST(Filename, serialization::MK_MainFile,
+  switch (AST->Reader->ReadAST(Filename, serialization::MK_MainFile,
                           SourceLocation(), ASTReader::ARR_None)) {
   case ASTReader::Success:
     break;
@@ -771,21 +765,14 @@ ASTUnit *ASTUnit::LoadFromASTFile(const
     return NULL;
   }
 
-  AST->OriginalSourceFile = Reader->getOriginalSourceFile();
+  AST->OriginalSourceFile = AST->Reader->getOriginalSourceFile();
 
   PP.setCounterValue(Counter);
 
   // Attach the AST reader to the AST context as an external AST
   // source, so that declarations will be deserialized from the
   // AST file as needed.
-  ASTReader *ReaderPtr = Reader.get();
-  OwningPtr<ExternalASTSource> Source(Reader.take());
-
-  // Unregister the cleanup for ASTReader.  It will get cleaned up
-  // by the ASTUnit cleanup.
-  ReaderCleanup.unregister();
-
-  Context.setExternalSource(Source);
+  Context.setExternalSource(AST->Reader);
 
   // Create an AST consumer, even though it isn't used.
   AST->Consumer.reset(new ASTConsumer);
@@ -793,8 +780,7 @@ ASTUnit *ASTUnit::LoadFromASTFile(const
   // Create a semantic analysis object and tell the AST reader about it.
   AST->TheSema.reset(new Sema(PP, Context, *AST->Consumer));
   AST->TheSema->Initialize();
-  ReaderPtr->InitializeSema(*AST->TheSema);
-  AST->Reader = ReaderPtr;
+  AST->Reader->InitializeSema(*AST->TheSema);
 
   // Tell the diagnostic client that we have started a source file.
   AST->getDiagnostics().getClient()->BeginSourceFile(Context.getLangOpts(),&PP);
@@ -1173,7 +1159,7 @@ bool ASTUnit::Parse(llvm::MemoryBuffer *
 
   if (OverrideMainBuffer) {
     std::string ModName = getPreambleFile(this);
-    TranslateStoredDiagnostics(Clang->getModuleManager(), ModName,
+    TranslateStoredDiagnostics(Clang->getModuleManager().getPtr(), ModName,
                                getSourceManager(), PreambleDiagnostics,
                                StoredDiagnostics);
   }

Modified: cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp (original)
+++ cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp Wed Feb 26 22:11:59 2014
@@ -62,12 +62,13 @@ ChainedIncludesSource::~ChainedIncludesS
     delete CIs[i];
 }
 
-ChainedIncludesSource *ChainedIncludesSource::create(CompilerInstance &CI) {
+IntrusiveRefCntPtr<ChainedIncludesSource>
+ChainedIncludesSource::create(CompilerInstance &CI) {
 
   std::vector<std::string> &includes = CI.getPreprocessorOpts().ChainedIncludes;
   assert(!includes.empty() && "No '-chain-include' in options!");
 
-  OwningPtr<ChainedIncludesSource> source(new ChainedIncludesSource());
+  IntrusiveRefCntPtr<ChainedIncludesSource> source(new ChainedIncludesSource());
   InputKind IK = CI.getFrontendOpts().Inputs[0].getKind();
 
   SmallVector<llvm::MemoryBuffer *, 4> serialBufs;
@@ -137,13 +138,12 @@ ChainedIncludesSource *ChainedIncludesSo
       
       serialBufNames.push_back(pchName);
 
-      OwningPtr<ExternalASTSource> Reader;
-
-      Reader.reset(createASTReader(*Clang, pchName, bufs, serialBufNames, 
-        Clang->getASTConsumer().GetASTDeserializationListener()));
+      IntrusiveRefCntPtr<ASTReader> Reader;
+      Reader = createASTReader(*Clang, pchName, bufs, serialBufNames, 
+        Clang->getASTConsumer().GetASTDeserializationListener());
       if (!Reader)
         return 0;
-      Clang->setModuleManager(static_cast<ASTReader*>(Reader.get()));
+      Clang->setModuleManager(Reader);
       Clang->getASTContext().setExternalSource(Reader);
     }
     
@@ -162,13 +162,13 @@ ChainedIncludesSource *ChainedIncludesSo
   assert(!serialBufs.empty());
   std::string pchName = includes.back() + ".pch-final";
   serialBufNames.push_back(pchName);
-  OwningPtr<ASTReader> Reader;
-  Reader.reset(createASTReader(CI, pchName, serialBufs, serialBufNames));
+  IntrusiveRefCntPtr<ASTReader> Reader;
+  Reader = createASTReader(CI, pchName, serialBufs, serialBufNames);
   if (!Reader)
     return 0;
 
-  source->FinalReader.reset(Reader.take());
-  return source.take();
+  source->FinalReader = Reader;
+  return source;
 }
 
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Feb 26 22:11:59 2014
@@ -104,6 +104,13 @@ void CompilerInstance::setASTConsumer(AS
 void CompilerInstance::setCodeCompletionConsumer(CodeCompleteConsumer *Value) {
   CompletionConsumer.reset(Value);
 }
+ 
+IntrusiveRefCntPtr<ASTReader> CompilerInstance::getModuleManager() const {
+  return ModuleManager;
+}
+void CompilerInstance::setModuleManager(IntrusiveRefCntPtr<ASTReader> Reader) {
+  ModuleManager = Reader;
+}
 
 // Diagnostics
 static void SetUpDiagnosticLog(DiagnosticOptions *DiagOpts,
@@ -301,16 +308,16 @@ void CompilerInstance::createPCHExternal
                                                   bool DisablePCHValidation,
                                                 bool AllowPCHWithCompilerErrors,
                                                  void *DeserializationListener){
-  OwningPtr<ExternalASTSource> Source;
+  IntrusiveRefCntPtr<ExternalASTSource> Source;
   bool Preamble = getPreprocessorOpts().PrecompiledPreambleBytes.first != 0;
-  Source.reset(createPCHExternalASTSource(Path, getHeaderSearchOpts().Sysroot,
+  Source = createPCHExternalASTSource(Path, getHeaderSearchOpts().Sysroot,
                                           DisablePCHValidation,
                                           AllowPCHWithCompilerErrors,
                                           getPreprocessor(), getASTContext(),
                                           DeserializationListener,
                                           Preamble,
-                                       getFrontendOpts().UseGlobalModuleIndex));
-  ModuleManager = static_cast<ASTReader*>(Source.get());
+                                       getFrontendOpts().UseGlobalModuleIndex);
+  ModuleManager = static_cast<ASTReader*>(Source.getPtr());
   getASTContext().setExternalSource(Source);
 }
 
@@ -1176,9 +1183,7 @@ CompilerInstance::loadModule(SourceLocat
         getASTContext().setASTMutationListener(
           getASTConsumer().GetASTMutationListener());
       }
-      OwningPtr<ExternalASTSource> Source;
-      Source.reset(ModuleManager);
-      getASTContext().setExternalSource(Source);
+      getASTContext().setExternalSource(ModuleManager);
       if (hasSema())
         ModuleManager->InitializeSema(getSema());
       if (hasASTConsumer())

Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=202346&r1=202345&r2=202346&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Wed Feb 26 22:11:59 2014
@@ -316,12 +316,11 @@ bool FrontendAction::BeginSourceFile(Com
     
     if (!CI.getPreprocessorOpts().ChainedIncludes.empty()) {
       // Convert headers to PCH and chain them.
-      OwningPtr<ExternalASTSource> source;
-      source.reset(ChainedIncludesSource::create(CI));
+      IntrusiveRefCntPtr<ChainedIncludesSource> source;
+      source = ChainedIncludesSource::create(CI);
       if (!source)
         goto failure;
-      CI.setModuleManager(static_cast<ASTReader*>(
-         &static_cast<ChainedIncludesSource*>(source.get())->getFinalReader()));
+      CI.setModuleManager(static_cast<ASTReader*>(&source->getFinalReader()));
       CI.getASTContext().setExternalSource(source);
 
     } else if (!CI.getPreprocessorOpts().ImplicitPCHInclude.empty()) {
@@ -361,7 +360,7 @@ bool FrontendAction::BeginSourceFile(Com
   // provides the layouts from that file.
   if (!CI.getFrontendOpts().OverrideRecordLayoutsFile.empty() && 
       CI.hasASTContext() && !CI.getASTContext().getExternalSource()) {
-    OwningPtr<ExternalASTSource> 
+    IntrusiveRefCntPtr<ExternalASTSource> 
       Override(new LayoutOverrideSource(
                      CI.getFrontendOpts().OverrideRecordLayoutsFile));
     CI.getASTContext().setExternalSource(Override);





More information about the cfe-commits mailing list