[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
llvm-svn: 202346
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 2152390..6ed7c08 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -416,7 +416,7 @@
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 @@
/// 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.
///
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index b077426..54e96d9 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -53,7 +53,7 @@
/// 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;
diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h
index 4a64b24..adc920a 100644
--- a/clang/include/clang/Frontend/ASTUnit.h
+++ b/clang/include/clang/Frontend/ASTUnit.h
@@ -75,7 +75,7 @@
IntrusiveRefCntPtr<ASTContext> Ctx;
IntrusiveRefCntPtr<TargetOptions> TargetOpts;
IntrusiveRefCntPtr<HeaderSearchOptions> HSOpts;
- ASTReader *Reader;
+ IntrusiveRefCntPtr<ASTReader> Reader;
bool HadModuleLoaderFatalFailure;
struct ASTWriterData;
diff --git a/clang/include/clang/Frontend/ChainedIncludesSource.h b/clang/include/clang/Frontend/ChainedIncludesSource.h
index aa30460..bc05c07 100644
--- a/clang/include/clang/Frontend/ChainedIncludesSource.h
+++ b/clang/include/clang/Frontend/ChainedIncludesSource.h
@@ -24,13 +24,13 @@
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:
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index be1e1ea..c8ffd32 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -102,8 +102,8 @@
/// \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 @@
/// @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
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 46d0451..65bb024 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -791,8 +791,8 @@
}
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 @@
<< NumImplicitDestructors
<< " implicit destructors created\n";
- if (ExternalSource.get()) {
+ if (ExternalSource) {
llvm::errs() << "\n";
ExternalSource->PrintStats();
}
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 8d46a8f..93429cb 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -718,8 +718,6 @@
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 @@
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 @@
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 @@
// 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 @@
if (OverrideMainBuffer) {
std::string ModName = getPreambleFile(this);
- TranslateStoredDiagnostics(Clang->getModuleManager(), ModName,
+ TranslateStoredDiagnostics(Clang->getModuleManager().getPtr(), ModName,
getSourceManager(), PreambleDiagnostics,
StoredDiagnostics);
}
diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp b/clang/lib/Frontend/ChainedIncludesSource.cpp
index 442177e..ab5c81f 100644
--- a/clang/lib/Frontend/ChainedIncludesSource.cpp
+++ b/clang/lib/Frontend/ChainedIncludesSource.cpp
@@ -62,12 +62,13 @@
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 @@
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 @@
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;
}
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 3fe8f0e..cdc4b64 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -104,6 +104,13 @@
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 @@
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 @@
getASTContext().setASTMutationListener(
getASTConsumer().GetASTMutationListener());
}
- OwningPtr<ExternalASTSource> Source;
- Source.reset(ModuleManager);
- getASTContext().setExternalSource(Source);
+ getASTContext().setExternalSource(ModuleManager);
if (hasSema())
ModuleManager->InitializeSema(getSema());
if (hasASTConsumer())
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 6cfa121..f028a56 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -316,12 +316,11 @@
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 @@
// 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);
