[cfe-commits] r100464 - in /cfe/trunk: include/clang/Frontend/ASTUnit.h lib/Frontend/ASTMerge.cpp lib/Frontend/ASTUnit.cpp lib/Frontend/FrontendAction.cpp tools/CIndex/CIndex.cpp
Douglas Gregor
dgregor at apple.com
Mon Apr 5 14:10:19 PDT 2010
Author: dgregor
Date: Mon Apr 5 16:10:19 2010
New Revision: 100464
URL: http://llvm.org/viewvc/llvm-project?rev=100464&view=rev
Log:
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>.
Modified:
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/lib/Frontend/ASTMerge.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/FrontendAction.cpp
cfe/trunk/tools/CIndex/CIndex.cpp
Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=100464&r1=100463&r2=100464&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Mon Apr 5 16:10:19 2010
@@ -52,6 +52,7 @@
typedef std::map<FileID, std::vector<PreprocessedEntity *> >
PreprocessedEntitiesByFileMap;
private:
+ llvm::MaybeOwningPtr<Diagnostic> Diagnostics;
llvm::OwningPtr<FileManager> FileMgr;
llvm::OwningPtr<SourceManager> SourceMgr;
llvm::OwningPtr<HeaderSearch> HeaderInfo;
@@ -116,7 +117,7 @@
ASTUnit(const ASTUnit&); // DO NOT IMPLEMENT
ASTUnit &operator=(const ASTUnit &); // DO NOT IMPLEMENT
- ASTUnit(Diagnostic &Diag, bool MainFileIsAST);
+ explicit ASTUnit(bool MainFileIsAST);
public:
class ConcurrencyCheck {
@@ -141,6 +142,9 @@
bool isMainFileAST() const { return MainFileIsAST; }
+ const Diagnostic &getDiagnostics() const { return *Diagnostics; }
+ Diagnostic &getDiagnostics() { return *Diagnostics; }
+
const SourceManager &getSourceManager() const { return *SourceMgr; }
SourceManager &getSourceManager() { return *SourceMgr; }
@@ -210,7 +214,7 @@
///
/// \returns - The initialized ASTUnit or null if the PCH failed to load.
static ASTUnit *LoadFromPCHFile(const std::string &Filename,
- Diagnostic &Diags,
+ llvm::MaybeOwningPtr<Diagnostic> Diags,
bool OnlyLocalDecls = false,
RemappedFile *RemappedFiles = 0,
unsigned NumRemappedFiles = 0,
@@ -228,7 +232,7 @@
// FIXME: Move OnlyLocalDecls, UseBumpAllocator to setters on the ASTUnit, we
// shouldn't need to specify them at construction time.
static ASTUnit *LoadFromCompilerInvocation(CompilerInvocation *CI,
- Diagnostic &Diags,
+ llvm::MaybeOwningPtr<Diagnostic> Diags,
bool OnlyLocalDecls = false,
bool CaptureDiagnostics = false);
@@ -248,7 +252,7 @@
// shouldn't need to specify them at construction time.
static ASTUnit *LoadFromCommandLine(const char **ArgBegin,
const char **ArgEnd,
- Diagnostic &Diags,
+ llvm::MaybeOwningPtr<Diagnostic> Diags,
llvm::StringRef ResourceFilesPath,
bool OnlyLocalDecls = false,
RemappedFile *RemappedFiles = 0,
@@ -256,6 +260,24 @@
bool CaptureDiagnostics = false);
};
+/// \brief Return an potentially-owning pointer for the given diagnostic engine
+/// that owns the pointer.
+inline llvm::MaybeOwningPtr<Diagnostic> OwnedDiag(Diagnostic &Diags) {
+ return llvm::MaybeOwningPtr<Diagnostic>(&Diags, true);
+}
+
+/// \brief Return a potentially-owning pointer for the given diagnostic engine
+/// that does not own the pointer.
+inline llvm::MaybeOwningPtr<Diagnostic> UnownedDiag(Diagnostic &Diags) {
+ return llvm::MaybeOwningPtr<Diagnostic>(&Diags, false);
+}
+
+/// \brief Return an potentially-owning pointer that indicates that the
+/// default diagnostic engine should be used.
+inline llvm::MaybeOwningPtr<Diagnostic> DefaultDiag() {
+ return llvm::MaybeOwningPtr<Diagnostic>();
+}
+
} // namespace clang
#endif
Modified: cfe/trunk/lib/Frontend/ASTMerge.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTMerge.cpp?rev=100464&r1=100463&r2=100464&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTMerge.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTMerge.cpp Mon Apr 5 16:10:19 2010
@@ -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;
Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=100464&r1=100463&r2=100464&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Mon Apr 5 16:10:19 2010
@@ -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)
Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=100464&r1=100463&r2=100464&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Mon Apr 5 16:10:19 2010
@@ -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;
Modified: cfe/trunk/tools/CIndex/CIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/CIndex/CIndex.cpp?rev=100464&r1=100463&r2=100464&view=diff
==============================================================================
--- cfe/trunk/tools/CIndex/CIndex.cpp (original)
+++ cfe/trunk/tools/CIndex/CIndex.cpp Mon Apr 5 16:10:19 2010
@@ -996,11 +996,7 @@
CIndexer *CXXIdx = static_cast<CIndexer *>(CIdx);
- // Configure the diagnostics.
- DiagnosticOptions DiagOpts;
- llvm::OwningPtr<Diagnostic> Diags;
- Diags.reset(CompilerInstance::createDiagnostics(DiagOpts, 0, 0));
- return ASTUnit::LoadFromPCHFile(ast_filename, *Diags,
+ return ASTUnit::LoadFromPCHFile(ast_filename, DefaultDiag(),
CXXIdx->getOnlyLocalDecls(),
0, 0, true);
}
@@ -1019,8 +1015,8 @@
// Configure the diagnostics.
DiagnosticOptions DiagOpts;
- llvm::OwningPtr<Diagnostic> Diags;
- Diags.reset(CompilerInstance::createDiagnostics(DiagOpts, 0, 0));
+ llvm::MaybeOwningPtr<Diagnostic> Diags;
+ Diags.reset(CompilerInstance::createDiagnostics(DiagOpts, 0, 0), true);
llvm::SmallVector<ASTUnit::RemappedFile, 4> RemappedFiles;
for (unsigned I = 0; I != num_unsaved_files; ++I) {
@@ -1052,7 +1048,7 @@
llvm::OwningPtr<ASTUnit> Unit(
ASTUnit::LoadFromCommandLine(Args.data(), Args.data() + Args.size(),
- *Diags,
+ Diags,
CXXIdx->getClangResourcesPath(),
CXXIdx->getOnlyLocalDecls(),
RemappedFiles.data(),
@@ -1169,7 +1165,7 @@
Diags->Report(diag::err_fe_invoking) << AllArgs << ErrMsg;
}
- ASTUnit *ATU = ASTUnit::LoadFromPCHFile(astTmpFile, *Diags,
+ ASTUnit *ATU = ASTUnit::LoadFromPCHFile(astTmpFile, Diags,
CXXIdx->getOnlyLocalDecls(),
RemappedFiles.data(),
RemappedFiles.size(),
More information about the cfe-commits
mailing list