<div dir="ltr">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.<div class="gmail_extra">
<br><div class="gmail_quote">On Thu, Oct 14, 2010 at 1:14 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" target="_blank" class="cremed">akyrtzi@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: akirtzidis<br>
Date: Thu Oct 14 15:14:18 2010<br>
New Revision: 116503<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=116503&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=116503&view=rev</a><br>
Log:<br>
Introduce command line option -dump-deserialized-decls which is used to print the PCH decls that got deserialized, for testing purposes.<br></blockquote><div><br></div><div><snip></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=116503&r1=116502&r2=116503&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=116503&r1=116502&r2=116503&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Thu Oct 14 15:14:18 2010<br>
@@ -16,12 +16,43 @@<br>
 #include "clang/Frontend/CompilerInstance.h"<br>
 #include "clang/Frontend/FrontendDiagnostic.h"<br>
 #include "clang/Parse/ParseAST.h"<br>
+#include "clang/Serialization/ASTDeserializationListener.h"<br>
 #include "llvm/Support/MemoryBuffer.h"<br>
 #include "llvm/Support/Timer.h"<br>
 #include "llvm/Support/ErrorHandling.h"<br>
 #include "llvm/Support/raw_ostream.h"<br>
 using namespace clang;<br>
<br>
+namespace {<br>
+<br>
+/// \brief Dumps deserialized declarations.<br>
+class DeserializedDeclsDumper : public ASTDeserializationListener {<br>
+  ASTDeserializationListener *Previous;<br>
+<br>
+public:<br>
+  DeserializedDeclsDumper(ASTDeserializationListener *Previous)<br>
+    : Previous(Previous) { }<br>
+<br>
+  virtual void DeclRead(serialization::DeclID ID, const Decl *D) {<br>
+    llvm::outs() << "PCH DECL: " << D->getDeclKindName();<br>
+    if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))<br>
+      llvm::outs() << " - " << ND->getNameAsString();<br>
+    llvm::outs() << "\n";<br>
+<br>
+    if (Previous)<br>
+      Previous->DeclRead(ID, D);<br>
+  }<br>
+<br>
+  virtual void SetReader(ASTReader *Reader) {}<br>
+  virtual void IdentifierRead(serialization::IdentID ID, IdentifierInfo *II) {}<br>
+  virtual void TypeRead(serialization::TypeIdx Idx, QualType T) {}<br>
+  virtual void SelectorRead(serialization::SelectorID iD, Selector Sel) {}<br>
+  virtual void MacroDefinitionRead(serialization::MacroID,<br>
+                                   MacroDefinition *MD) {}<br>
+};<br>
+<br>
+} // end anonymous namespace<br>
+<br>
 FrontendAction::FrontendAction() : Instance(0) {}<br>
<br>
 FrontendAction::~FrontendAction() {}<br>
@@ -118,11 +149,15 @@<br>
     /// Use PCH?<br>
     if (!CI.getPreprocessorOpts().ImplicitPCHInclude.empty()) {<br>
       assert(hasPCHSupport() && "This action does not have PCH support!");<br>
+      ASTDeserializationListener *DeserialListener<br>
+          = CI.getInvocation().getFrontendOpts().ChainedPCH ?<br>
+                  Consumer->GetASTDeserializationListener() : 0;<br>
+      if (CI.getPreprocessorOpts().DumpDeserializedPCHDecls)<br>
+        DeserialListener = new DeserializedDeclsDumper(DeserialListener);<br></blockquote><div><br></div><div>This listener leaks. So does the DeserializedDeclsChecker that follows the same pattern. LSan catches this.</div>
<div><br></div><div>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!</div>
<div><br></div><div>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.</div>
<div><br></div><div>-Chandler</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
       CI.createPCHExternalASTSource(<br>
                                 CI.getPreprocessorOpts().ImplicitPCHInclude,<br>
                                 CI.getPreprocessorOpts().DisablePCHValidation,<br>
-                                CI.getInvocation().getFrontendOpts().ChainedPCH?<br>
-                                 Consumer->GetASTDeserializationListener() : 0);<br>
+                                DeserialListener);<br>
       if (!CI.getASTContext().getExternalSource())<br>
         goto failure;<br>
     }<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>