[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]

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


>        CI.createPCHExternalASTSource(
>
>  CI.getPreprocessorOpts().ImplicitPCHInclude,
>
>  CI.getPreprocessorOpts().DisablePCHValidation,
> -
>  CI.getInvocation().getFrontendOpts().ChainedPCH?
> -
> Consumer->GetASTDeserializationListener() : 0);
> +                                DeserialListener);
>        if (!CI.getASTContext().getExternalSource())
>          goto failure;
>      }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140503/6c2eefe8/attachment.html>