[cfe-commits] r116503 - in /cfe/trunk: include/clang/Driver/CC1Options.td include/clang/Frontend/PreprocessorOptions.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/FrontendAction.cpp

[Chandler Carruth]

On Sat, May 3, 2014 at 3:20 AM, Chandler Carruth <chandlerc at gmail.com>wrote:

> First off, sorry for the commit necromancy, but I tried to fix a memory
> leak here and discovered completely broken ownership semantics that really
> need to be fixed, and need to be fixed by the person who really understands
> the intended design here.
>
> On Thu, Oct 14, 2010 at 1:14 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com>wrote:
>
>> Author: akirtzidis
>> Date: Thu Oct 14 15:14:18 2010
>> New Revision: 116503
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=116503&view=rev
>> Log:
>> Introduce command line option -dump-deserialized-decls which is used to
>> print the PCH decls that got deserialized, for testing purposes.
>>
>
> <snip>
>
>
>> Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=116503&r1=116502&r2=116503&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
>> +++ cfe/trunk/lib/Frontend/FrontendAction.cpp Thu Oct 14 15:14:18 2010
>> @@ -16,12 +16,43 @@
>>  #include "clang/Frontend/CompilerInstance.h"
>>  #include "clang/Frontend/FrontendDiagnostic.h"
>>  #include "clang/Parse/ParseAST.h"
>> +#include "clang/Serialization/ASTDeserializationListener.h"
>>  #include "llvm/Support/MemoryBuffer.h"
>>  #include "llvm/Support/Timer.h"
>>  #include "llvm/Support/ErrorHandling.h"
>>  #include "llvm/Support/raw_ostream.h"
>>  using namespace clang;
>>
>> +namespace {
>> +
>> +/// \brief Dumps deserialized declarations.
>> +class DeserializedDeclsDumper : public ASTDeserializationListener {
>> +  ASTDeserializationListener *Previous;
>> +
>> +public:
>> +  DeserializedDeclsDumper(ASTDeserializationListener *Previous)
>> +    : Previous(Previous) { }
>> +
>> +  virtual void DeclRead(serialization::DeclID ID, const Decl *D) {
>> +    llvm::outs() << "PCH DECL: " << D->getDeclKindName();
>> +    if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))
>> +      llvm::outs() << " - " << ND->getNameAsString();
>> +    llvm::outs() << "\n";
>> +
>> +    if (Previous)
>> +      Previous->DeclRead(ID, D);
>> +  }
>> +
>> +  virtual void SetReader(ASTReader *Reader) {}
>> +  virtual void IdentifierRead(serialization::IdentID ID, IdentifierInfo
>> *II) {}
>> +  virtual void TypeRead(serialization::TypeIdx Idx, QualType T) {}
>> +  virtual void SelectorRead(serialization::SelectorID iD, Selector Sel)
>> {}
>> +  virtual void MacroDefinitionRead(serialization::MacroID,
>> +                                   MacroDefinition *MD) {}
>> +};
>> +
>> +} // end anonymous namespace
>> +
>>  FrontendAction::FrontendAction() : Instance(0) {}
>>
>>  FrontendAction::~FrontendAction() {}
>> @@ -118,11 +149,15 @@
>>      /// Use PCH?
>>      if (!CI.getPreprocessorOpts().ImplicitPCHInclude.empty()) {
>>        assert(hasPCHSupport() && "This action does not have PCH
>> support!");
>> +      ASTDeserializationListener *DeserialListener
>> +          = CI.getInvocation().getFrontendOpts().ChainedPCH ?
>> +                  Consumer->GetASTDeserializationListener() : 0;
>> +      if (CI.getPreprocessorOpts().DumpDeserializedPCHDecls)
>> +        DeserialListener = new DeserializedDeclsDumper(DeserialListener);
>>
>
> This listener leaks. So does the DeserializedDeclsChecker that follows the
> same pattern. LSan catches this.
>
> I went to plug the leak by storing the listeners in the FrontendAction if
> they end up being used, and deallocating them when the action is destroyed.
> To my great surprise, this caused use-after-free bugs as these listeners
> are somehow reached *after* the action is destroyed!
>
> Could you look into this? It should be pretty easy to reproduce. Forward
> declare the classes (or make them private classes nested within
> FrontendAction, or use a raw pointer and manually cast back to the derived
> type prior to deleting) and add unique_ptrs to the action, reseting them
> with the new listeners, and just wiring up DeserialListener to the raw
> pointer side. If you're having trouble reproducing, let me know, and I can
> send you a nascent patch.
>
> -Chandler
>

Just FYI, this is also filed as PR19560.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140503/ea698abd/attachment.html>