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
diff --git a/lib/Frontend/ASTMerge.cpp b/lib/Frontend/ASTMerge.cpp
index 2228ea4..2465861 100644
--- a/lib/Frontend/ASTMerge.cpp
+++ b/lib/Frontend/ASTMerge.cpp
@@ -37,7 +37,8 @@
CI.getDiagnostics().SetArgToStringFn(&FormatASTNodeDiagnosticArgument,
&CI.getASTContext());
for (unsigned I = 0, N = ASTFiles.size(); I != N; ++I) {
- ASTUnit *Unit = ASTUnit::LoadFromPCHFile(ASTFiles[I], CI.getDiagnostics(),
+ ASTUnit *Unit = ASTUnit::LoadFromPCHFile(ASTFiles[I],
+ UnownedDiag(CI.getDiagnostics()),
false);
if (!Unit)
continue;
diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp
index 38aeedc..2a76d6f 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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
"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 @@
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 @@
// 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 @@
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 @@
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)
diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp
index 110612d..4c2b1e9 100644
--- a/lib/Frontend/FrontendAction.cpp
+++ b/lib/Frontend/FrontendAction.cpp
@@ -46,7 +46,8 @@
assert(hasASTSupport() && "This action does not have AST support!");
std::string Error;
- ASTUnit *AST = ASTUnit::LoadFromPCHFile(Filename, CI.getDiagnostics());
+ ASTUnit *AST = ASTUnit::LoadFromPCHFile(Filename,
+ UnownedDiag(CI.getDiagnostics()));
if (!AST)
goto failure;