Let ASTReader optionally delete its ASTDeserializationListener.

Use this to fix the leak of DeserializedDeclsDumper and DeserializedDeclsChecker
in FrontendAction (found by LSan), PR19560.

The "delete this" bool is necessary because both PCHGenerator and ASTUnit
return the same object from both getDeserializationListener() and
getASTMutationListener(), so ASTReader can't just have a unique_ptr.

It's also not possible to just let FrontendAction (or CompilerInstance) own
these listeners due to lifetime issues (see comments on PR19560).

Finally, ASTDeserializationListener can't easily be refcounted, since several of
the current listeners are allocated on the stack.

Having this bool isn't ideal, but it's a pattern that's used in other places in
the codebase too, and it seems better than leaking.

llvm-svn: 208277
This commit is contained in:
Nico Weber 2014-05-08 04:26:47 +00:00
parent 789d3007fb
commit 824285ec53
6 changed files with 88 additions and 83 deletions

View File

@ -570,21 +570,19 @@ public:
/// Create an external AST source to read a PCH file and attach it to the AST
/// context.
void createPCHExternalASTSource(StringRef Path,
bool DisablePCHValidation,
void createPCHExternalASTSource(StringRef Path, bool DisablePCHValidation,
bool AllowPCHWithCompilerErrors,
void *DeserializationListener);
void *DeserializationListener,
bool OwnDeserializationListener);
/// Create an external AST source to read a PCH file.
///
/// \return - The new object on success, or null on failure.
static ExternalASTSource *
createPCHExternalASTSource(StringRef Path, const std::string &Sysroot,
bool DisablePCHValidation,
bool AllowPCHWithCompilerErrors,
Preprocessor &PP, ASTContext &Context,
void *DeserializationListener, bool Preamble,
bool UseGlobalModuleIndex);
static ExternalASTSource *createPCHExternalASTSource(
StringRef Path, const std::string &Sysroot, bool DisablePCHValidation,
bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
void *DeserializationListener, bool OwnDeserializationListener,
bool Preamble, bool UseGlobalModuleIndex);
/// Create a code completion consumer using the invocation; note that this
/// will cause the source manager to truncate the input source file at the

View File

@ -27,10 +27,8 @@ class MacroInfo;
class Module;
class ASTDeserializationListener {
protected:
virtual ~ASTDeserializationListener();
public:
virtual ~ASTDeserializationListener();
/// \brief The ASTReader was initialized.
virtual void ReaderInitialized(ASTReader *Reader) { }

View File

@ -340,6 +340,7 @@ private:
/// \brief The receiver of deserialization events.
ASTDeserializationListener *DeserializationListener;
bool OwnsDeserializationListener;
SourceManager &SourceMgr;
FileManager &FileMgr;
@ -1382,7 +1383,8 @@ public:
}
/// \brief Set the AST deserialization listener.
void setDeserializationListener(ASTDeserializationListener *Listener);
void setDeserializationListener(ASTDeserializationListener *Listener,
bool TakeOwnership = false);
/// \brief Determine whether this AST reader has a global index.
bool hasGlobalIndex() const { return (bool)GlobalIndex; }

View File

@ -307,33 +307,25 @@ void CompilerInstance::createASTContext() {
// ExternalASTSource
void CompilerInstance::createPCHExternalASTSource(StringRef Path,
bool DisablePCHValidation,
bool AllowPCHWithCompilerErrors,
void *DeserializationListener){
void CompilerInstance::createPCHExternalASTSource(
StringRef Path, bool DisablePCHValidation, bool AllowPCHWithCompilerErrors,
void *DeserializationListener, bool OwnDeserializationListener) {
IntrusiveRefCntPtr<ExternalASTSource> Source;
bool Preamble = getPreprocessorOpts().PrecompiledPreambleBytes.first != 0;
Source = createPCHExternalASTSource(Path, getHeaderSearchOpts().Sysroot,
DisablePCHValidation,
AllowPCHWithCompilerErrors,
getPreprocessor(), getASTContext(),
DeserializationListener,
Preamble,
getFrontendOpts().UseGlobalModuleIndex);
Source = createPCHExternalASTSource(
Path, getHeaderSearchOpts().Sysroot, DisablePCHValidation,
AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(),
DeserializationListener, OwnDeserializationListener, Preamble,
getFrontendOpts().UseGlobalModuleIndex);
ModuleManager = static_cast<ASTReader*>(Source.getPtr());
getASTContext().setExternalSource(Source);
}
ExternalASTSource *
CompilerInstance::createPCHExternalASTSource(StringRef Path,
const std::string &Sysroot,
bool DisablePCHValidation,
bool AllowPCHWithCompilerErrors,
Preprocessor &PP,
ASTContext &Context,
void *DeserializationListener,
bool Preamble,
bool UseGlobalModuleIndex) {
ExternalASTSource *CompilerInstance::createPCHExternalASTSource(
StringRef Path, const std::string &Sysroot, bool DisablePCHValidation,
bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context,
void *DeserializationListener, bool OwnDeserializationListener,
bool Preamble, bool UseGlobalModuleIndex) {
HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
std::unique_ptr<ASTReader> Reader;
@ -346,7 +338,8 @@ CompilerInstance::createPCHExternalASTSource(StringRef Path,
UseGlobalModuleIndex));
Reader->setDeserializationListener(
static_cast<ASTDeserializationListener *>(DeserializationListener));
static_cast<ASTDeserializationListener *>(DeserializationListener),
/*TakeOwnership=*/OwnDeserializationListener);
switch (Reader->ReadAST(Path,
Preamble ? serialization::MK_Preamble
: serialization::MK_PCH,

View File

@ -37,11 +37,16 @@ namespace {
class DelegatingDeserializationListener : public ASTDeserializationListener {
ASTDeserializationListener *Previous;
bool DeletePrevious;
public:
explicit DelegatingDeserializationListener(
ASTDeserializationListener *Previous)
: Previous(Previous) { }
ASTDeserializationListener *Previous, bool DeletePrevious)
: Previous(Previous), DeletePrevious(DeletePrevious) {}
virtual ~DelegatingDeserializationListener() {
if (DeletePrevious)
delete Previous;
}
void ReaderInitialized(ASTReader *Reader) override {
if (Previous)
@ -74,8 +79,9 @@ public:
/// \brief Dumps deserialized declarations.
class DeserializedDeclsDumper : public DelegatingDeserializationListener {
public:
explicit DeserializedDeclsDumper(ASTDeserializationListener *Previous)
: DelegatingDeserializationListener(Previous) { }
explicit DeserializedDeclsDumper(ASTDeserializationListener *Previous,
bool DeletePrevious)
: DelegatingDeserializationListener(Previous, DeletePrevious) {}
void DeclRead(serialization::DeclID ID, const Decl *D) override {
llvm::outs() << "PCH DECL: " << D->getDeclKindName();
@ -96,9 +102,10 @@ class DeserializedDeclsChecker : public DelegatingDeserializationListener {
public:
DeserializedDeclsChecker(ASTContext &Ctx,
const std::set<std::string> &NamesToCheck,
ASTDeserializationListener *Previous)
: DelegatingDeserializationListener(Previous),
Ctx(Ctx), NamesToCheck(NamesToCheck) { }
ASTDeserializationListener *Previous,
bool DeletePrevious)
: DelegatingDeserializationListener(Previous, DeletePrevious), Ctx(Ctx),
NamesToCheck(NamesToCheck) {}
void DeclRead(serialization::DeclID ID, const Decl *D) override {
if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))
@ -320,17 +327,24 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
assert(hasPCHSupport() && "This action does not have PCH support!");
ASTDeserializationListener *DeserialListener =
Consumer->GetASTDeserializationListener();
if (CI.getPreprocessorOpts().DumpDeserializedPCHDecls)
DeserialListener = new DeserializedDeclsDumper(DeserialListener);
if (!CI.getPreprocessorOpts().DeserializedPCHDeclsToErrorOn.empty())
DeserialListener = new DeserializedDeclsChecker(CI.getASTContext(),
CI.getPreprocessorOpts().DeserializedPCHDeclsToErrorOn,
DeserialListener);
bool DeleteDeserialListener = false;
if (CI.getPreprocessorOpts().DumpDeserializedPCHDecls) {
DeserialListener = new DeserializedDeclsDumper(DeserialListener,
DeleteDeserialListener);
DeleteDeserialListener = true;
}
if (!CI.getPreprocessorOpts().DeserializedPCHDeclsToErrorOn.empty()) {
DeserialListener = new DeserializedDeclsChecker(
CI.getASTContext(),
CI.getPreprocessorOpts().DeserializedPCHDeclsToErrorOn,
DeserialListener, DeleteDeserialListener);
DeleteDeserialListener = true;
}
CI.createPCHExternalASTSource(
CI.getPreprocessorOpts().ImplicitPCHInclude,
CI.getPreprocessorOpts().DisablePCHValidation,
CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
DeserialListener);
CI.getPreprocessorOpts().ImplicitPCHInclude,
CI.getPreprocessorOpts().DisablePCHValidation,
CI.getPreprocessorOpts().AllowPCHWithCompilerErrors, DeserialListener,
DeleteDeserialListener);
if (!CI.getASTContext().getExternalSource())
goto failure;
}

View File

@ -578,9 +578,10 @@ void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
// AST reader implementation
//===----------------------------------------------------------------------===//
void
ASTReader::setDeserializationListener(ASTDeserializationListener *Listener) {
void ASTReader::setDeserializationListener(ASTDeserializationListener *Listener,
bool TakeOwnership) {
DeserializationListener = Listener;
OwnsDeserializationListener = TakeOwnership;
}
@ -8260,39 +8261,38 @@ void ASTReader::pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name) {
}
}
ASTReader::ASTReader(Preprocessor &PP, ASTContext &Context,
StringRef isysroot, bool DisableValidation,
bool AllowASTWithCompilerErrors,
bool AllowConfigurationMismatch,
bool ValidateSystemInputs,
ASTReader::ASTReader(Preprocessor &PP, ASTContext &Context, StringRef isysroot,
bool DisableValidation, bool AllowASTWithCompilerErrors,
bool AllowConfigurationMismatch, bool ValidateSystemInputs,
bool UseGlobalIndex)
: Listener(new PCHValidator(PP, *this)), DeserializationListener(0),
SourceMgr(PP.getSourceManager()), FileMgr(PP.getFileManager()),
Diags(PP.getDiagnostics()), SemaObj(0), PP(PP), Context(Context),
Consumer(0), ModuleMgr(PP.getFileManager()),
isysroot(isysroot), DisableValidation(DisableValidation),
AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
AllowConfigurationMismatch(AllowConfigurationMismatch),
ValidateSystemInputs(ValidateSystemInputs),
UseGlobalIndex(UseGlobalIndex), TriedLoadingGlobalIndex(false),
CurrentGeneration(0), CurrSwitchCaseStmts(&SwitchCaseStmts),
NumSLocEntriesRead(0), TotalNumSLocEntries(0),
NumStatementsRead(0), TotalNumStatements(0), NumMacrosRead(0),
TotalNumMacros(0), NumIdentifierLookups(0), NumIdentifierLookupHits(0),
NumSelectorsRead(0), NumMethodPoolEntriesRead(0),
NumMethodPoolLookups(0), NumMethodPoolHits(0),
NumMethodPoolTableLookups(0), NumMethodPoolTableHits(0),
TotalNumMethodPoolEntries(0),
NumLexicalDeclContextsRead(0), TotalLexicalDeclContexts(0),
NumVisibleDeclContextsRead(0), TotalVisibleDeclContexts(0),
TotalModulesSizeInBits(0), NumCurrentElementsDeserializing(0),
PassingDeclsToConsumer(false),
NumCXXBaseSpecifiersLoaded(0), ReadingKind(Read_None)
{
: Listener(new PCHValidator(PP, *this)), DeserializationListener(0),
OwnsDeserializationListener(false), SourceMgr(PP.getSourceManager()),
FileMgr(PP.getFileManager()), Diags(PP.getDiagnostics()), SemaObj(0),
PP(PP), Context(Context), Consumer(0), ModuleMgr(PP.getFileManager()),
isysroot(isysroot), DisableValidation(DisableValidation),
AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
AllowConfigurationMismatch(AllowConfigurationMismatch),
ValidateSystemInputs(ValidateSystemInputs),
UseGlobalIndex(UseGlobalIndex), TriedLoadingGlobalIndex(false),
CurrentGeneration(0), CurrSwitchCaseStmts(&SwitchCaseStmts),
NumSLocEntriesRead(0), TotalNumSLocEntries(0), NumStatementsRead(0),
TotalNumStatements(0), NumMacrosRead(0), TotalNumMacros(0),
NumIdentifierLookups(0), NumIdentifierLookupHits(0), NumSelectorsRead(0),
NumMethodPoolEntriesRead(0), NumMethodPoolLookups(0),
NumMethodPoolHits(0), NumMethodPoolTableLookups(0),
NumMethodPoolTableHits(0), TotalNumMethodPoolEntries(0),
NumLexicalDeclContextsRead(0), TotalLexicalDeclContexts(0),
NumVisibleDeclContextsRead(0), TotalVisibleDeclContexts(0),
TotalModulesSizeInBits(0), NumCurrentElementsDeserializing(0),
PassingDeclsToConsumer(false), NumCXXBaseSpecifiersLoaded(0),
ReadingKind(Read_None) {
SourceMgr.setExternalSLocEntrySource(this);
}
ASTReader::~ASTReader() {
if (OwnsDeserializationListener)
delete DeserializationListener;
for (DeclContextVisibleUpdatesPending::iterator
I = PendingVisibleUpdates.begin(),
E = PendingVisibleUpdates.end();