[PATCH] ms-inline-asm: Scope inline asm labels to functions

Ehsan Akhgari ehsan.akhgari at gmail.com
Wed Sep 17 19:21:30 PDT 2014


================
Comment at: include/clang/AST/Decl.h:312
@@ -310,1 +311,3 @@
   LabelStmt *TheStmt;
+  SmallString<16> MSAsmName;
+  bool MSAsmNameResolved;
----------------
rnk wrote:
> ehsan wrote:
> > rnk wrote:
> > > ehsan wrote:
> > > > rnk wrote:
> > > > > This will cause a memory leak for names > 16 chars. When the ASTContext is destroyed, it doesn't actually call destructors of individual AST nodes, it merely frees the memory. This should be a StringRef that points to memory from 'new (Context, 1) char[Len]', probably.
> > > > Oh, I didn't know that!  What's this 'new (Contex, 1) char[Len]' trick?  I'm not familiar with it, I don't think...
> > > It's an operator new overload declared in ASTContext.h. Basically, it's equivalent to 'new char[Len]', except it goes calls the 'void* operator new(size_t Bytes, ASTContext &Context, size_t Alignment)' overload.
> > Hmm, who is responsible for freeing this memory?  The comments in ASTContext.h seem to suggest that I should call ASTContext::Deallocate on the pointer, but I'm not sure when I should do that if the destructor is not invoked.  Looking at other parts of the code base, it seems like nobody worries about deleting memory allocated this way...
> When the ASTContext gets destroyed, it frees all memory allocated with it. It's basically a simple pool allocation scheme. The AST lives until compilation is over (i.e. forever). The clang driver even passes a -disable-free flag to prevent the -cc1 process from destroying the AST because even that is slower than just ending the process.
Thanks!  This makes perfect sense.  Should I submit another patch to improve the documentation about this in ASTContext.h?

http://reviews.llvm.org/D4589






More information about the cfe-commits mailing list