Clarify the ownership semantics of the Diagnostic object used by

ASTUnit. Previously, we would end up with use-after-free errors
because the Diagnostic object would be creating in one place (say,
CIndex) and its ownership would not be transferred into the
ASTUnit. Fixes <rdar://problem/7818608>.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@100464 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 52 insertions, 23 deletions

diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp
index 38aeedc39a..2a76d6f37b 100644
--- a/lib/Frontend/ASTUnit.cpp
+++ b/lib/Frontend/ASTUnit.cpp
@@ -35,12 +35,9 @@
 #include "llvm/System/Path.h"
 using namespace clang;
 
-ASTUnit::ASTUnit(Diagnostic &Diag, bool _MainFileIsAST)
-  : MainFileIsAST(_MainFileIsAST), 
-    ConcurrencyCheckValue(CheckUnlocked) {
-  FileMgr.reset(new FileManager);
-  SourceMgr.reset(new SourceManager(Diag));
-}
+ASTUnit::ASTUnit(bool _MainFileIsAST)
+  : MainFileIsAST(_MainFileIsAST), ConcurrencyCheckValue(CheckUnlocked) { }
+
 ASTUnit::~ASTUnit() {
   ConcurrencyCheckValue = CheckLocked;
   for (unsigned I = 0, N = TemporaryFiles.size(); I != N; ++I)
@@ -144,17 +141,30 @@ const std::string &ASTUnit::getPCHFileName() {
 }
 
 ASTUnit *ASTUnit::LoadFromPCHFile(const std::string &Filename,
-                                  Diagnostic &Diags,
+                                  llvm::MaybeOwningPtr<Diagnostic> Diags,
                                   bool OnlyLocalDecls,
                                   RemappedFile *RemappedFiles,
                                   unsigned NumRemappedFiles,
                                   bool CaptureDiagnostics) {
-  llvm::OwningPtr<ASTUnit> AST(new ASTUnit(Diags, true));
+  llvm::OwningPtr<ASTUnit> AST(new ASTUnit(true));
+  
+  if (Diags.get())
+    AST->Diagnostics = Diags;
+  else {
+    // No diagnostics engine was provided, so create our own diagnostics object
+    // with the default options.
+    DiagnosticOptions DiagOpts;
+    AST->Diagnostics.reset(CompilerInstance::createDiagnostics(DiagOpts, 0, 0),
+                           true);
+  }
+  
   AST->OnlyLocalDecls = OnlyLocalDecls;
+  AST->FileMgr.reset(new FileManager);
+  AST->SourceMgr.reset(new SourceManager(AST->getDiagnostics()));
   AST->HeaderInfo.reset(new HeaderSearch(AST->getFileManager()));
 
   // If requested, capture diagnostics in the ASTUnit.
-  CaptureDroppedDiagnostics Capture(CaptureDiagnostics, Diags, 
+  CaptureDroppedDiagnostics Capture(CaptureDiagnostics, AST->getDiagnostics(),
                                     AST->StoredDiagnostics);
 
   for (unsigned I = 0; I != NumRemappedFiles; ++I) {
@@ -164,7 +174,7 @@ ASTUnit *ASTUnit::LoadFromPCHFile(const std::string &Filename,
                                     RemappedFiles[I].second->getBufferSize(),
                                              0);
     if (!FromFile) {
-      Diags.Report(diag::err_fe_remap_missing_from_file)
+      AST->getDiagnostics().Report(diag::err_fe_remap_missing_from_file)
         << RemappedFiles[I].first;
       delete RemappedFiles[I].second;
       continue;
@@ -188,7 +198,7 @@ ASTUnit *ASTUnit::LoadFromPCHFile(const std::string &Filename,
   llvm::OwningPtr<ExternalASTSource> Source;
 
   Reader.reset(new PCHReader(AST->getSourceManager(), AST->getFileManager(),
-                             Diags));
+                             AST->getDiagnostics()));
   Reader->setListener(new PCHInfoCollector(LangInfo, HeaderInfo, TargetTriple,
                                            Predefines, Counter));
 
@@ -198,7 +208,7 @@ ASTUnit *ASTUnit::LoadFromPCHFile(const std::string &Filename,
 
   case PCHReader::Failure:
   case PCHReader::IgnorePCH:
-    Diags.Report(diag::err_fe_unable_to_load_pch);
+    AST->getDiagnostics().Report(diag::err_fe_unable_to_load_pch);
     return NULL;
   }
 
@@ -214,8 +224,10 @@ ASTUnit *ASTUnit::LoadFromPCHFile(const std::string &Filename,
   TargetOpts.CPU = "";
   TargetOpts.Features.clear();
   TargetOpts.Triple = TargetTriple;
-  AST->Target.reset(TargetInfo::CreateTargetInfo(Diags, TargetOpts));
-  AST->PP.reset(new Preprocessor(Diags, LangInfo, *AST->Target.get(),
+  AST->Target.reset(TargetInfo::CreateTargetInfo(AST->getDiagnostics(),
+                                                 TargetOpts));
+  AST->PP.reset(new Preprocessor(AST->getDiagnostics(), LangInfo, 
+                                 *AST->Target.get(),
                                  AST->getSourceManager(), HeaderInfo));
   Preprocessor &PP = *AST->PP.get();
 
@@ -278,7 +290,7 @@ public:
 }
 
 ASTUnit *ASTUnit::LoadFromCompilerInvocation(CompilerInvocation *CI,
-                                             Diagnostic &Diags,
+                                         llvm::MaybeOwningPtr<Diagnostic> Diags,
                                              bool OnlyLocalDecls,
                                              bool CaptureDiagnostics) {
   // Create the compiler instance to use for building the AST.
@@ -286,10 +298,17 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocation(CompilerInvocation *CI,
   llvm::OwningPtr<ASTUnit> AST;
   llvm::OwningPtr<TopLevelDeclTrackerAction> Act;
 
+  if (!Diags.get()) {
+    // No diagnostics engine was provided, so create our own diagnostics object
+    // with the default options.
+    DiagnosticOptions DiagOpts;
+    Diags.reset(CompilerInstance::createDiagnostics(DiagOpts, 0, 0), true);
+  }
+  
   Clang.setInvocation(CI);
 
-  Clang.setDiagnostics(&Diags);
-  Clang.setDiagnosticClient(Diags.getClient());
+  Clang.setDiagnostics(Diags.get());
+  Clang.setDiagnosticClient(Diags->getClient());
 
   // Create the target instance.
   Clang.setTarget(TargetInfo::CreateTargetInfo(Clang.getDiagnostics(),
@@ -312,7 +331,10 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocation(CompilerInvocation *CI,
          "FIXME: AST inputs not yet supported here!");
 
   // Create the AST unit.
-  AST.reset(new ASTUnit(Diags, false));
+  AST.reset(new ASTUnit(false));
+  AST->Diagnostics = Diags;
+  AST->FileMgr.reset(new FileManager);
+  AST->SourceMgr.reset(new SourceManager(AST->getDiagnostics()));
   AST->OnlyLocalDecls = OnlyLocalDecls;
   AST->OriginalSourceFile = Clang.getFrontendOpts().Inputs[0].second;
 
@@ -364,12 +386,19 @@ error:
 
 ASTUnit *ASTUnit::LoadFromCommandLine(const char **ArgBegin,
                                       const char **ArgEnd,
-                                      Diagnostic &Diags,
+                                      llvm::MaybeOwningPtr<Diagnostic> Diags,
                                       llvm::StringRef ResourceFilesPath,
                                       bool OnlyLocalDecls,
                                       RemappedFile *RemappedFiles,
                                       unsigned NumRemappedFiles,
                                       bool CaptureDiagnostics) {
+  if (!Diags.get()) {
+    // No diagnostics engine was provided, so create our own diagnostics object
+    // with the default options.
+    DiagnosticOptions DiagOpts;
+    Diags.reset(CompilerInstance::createDiagnostics(DiagOpts, 0, 0), true);
+  }
+  
   llvm::SmallVector<const char *, 16> Args;
   Args.push_back("<clang>"); // FIXME: Remove dummy argument.
   Args.insert(Args.end(), ArgBegin, ArgEnd);
@@ -380,7 +409,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(const char **ArgBegin,
 
   // FIXME: We shouldn't have to pass in the path info.
   driver::Driver TheDriver("clang", "/", llvm::sys::getHostTriple(),
-                           "a.out", false, false, Diags);
+                           "a.out", false, false, *Diags);
 
   // Don't check that inputs exist, they have been remapped.
   TheDriver.setCheckInputsExist(false);
@@ -395,13 +424,13 @@ ASTUnit *ASTUnit::LoadFromCommandLine(const char **ArgBegin,
     llvm::SmallString<256> Msg;
     llvm::raw_svector_ostream OS(Msg);
     C->PrintJob(OS, C->getJobs(), "; ", true);
-    Diags.Report(diag::err_fe_expected_compiler_job) << OS.str();
+    Diags->Report(diag::err_fe_expected_compiler_job) << OS.str();
     return 0;
   }
 
   const driver::Command *Cmd = cast<driver::Command>(*Jobs.begin());
   if (llvm::StringRef(Cmd->getCreator().getName()) != "clang") {
-    Diags.Report(diag::err_fe_expected_clang_command);
+    Diags->Report(diag::err_fe_expected_clang_command);
     return 0;
   }
 
@@ -409,7 +438,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(const char **ArgBegin,
   llvm::OwningPtr<CompilerInvocation> CI(new CompilerInvocation);
   CompilerInvocation::CreateFromArgs(*CI, (const char**) CCArgs.data(),
                                      (const char**) CCArgs.data()+CCArgs.size(),
-                                     Diags);
+                                     *Diags);
 
   // Override any files that need remapping
   for (unsigned I = 0; I != NumRemappedFiles; ++I)