On Feb 10, 2010, at 9:28 PM, Ted Kremenek wrote: > Author: kremenek > Date: Wed Feb 10 23:28:37 2010 > New Revision: 95853 > > URL: http://llvm.org/viewvc/llvm-project?rev=95853&view=rev > Log: > Remove use of 'std::string' from Attr objects, using instead a byte > array allocated using the allocator in ASTContext. This addresses > these strings getting leaked when using a BumpPtrAllocator (in > ASTContext). > > Fixes: <rdar://problem/7636765>
Would it work to just make these have StringExpr*'s? The memory is already allocated by the parsing/sema code, do we really need to copy it? -Chris > > Modified: > cfe/trunk/include/clang/AST/Attr.h > cfe/trunk/lib/AST/AttrImpl.cpp > cfe/trunk/lib/AST/Decl.cpp > cfe/trunk/lib/CodeGen/CodeGenModule.cpp > cfe/trunk/lib/Frontend/PCHReaderDecl.cpp > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/lib/Sema/SemaDeclAttr.cpp > > Modified: cfe/trunk/include/clang/AST/Attr.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Attr.h?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/AST/Attr.h (original) > +++ cfe/trunk/include/clang/AST/Attr.h Wed Feb 10 23:28:37 2010 > @@ -18,7 +18,6 @@ > #include "llvm/ADT/StringRef.h" > #include <cassert> > #include <cstring> > -#include <string> > #include <algorithm> > using llvm::dyn_cast; > > @@ -120,8 +119,7 @@ > assert(Next == 0 && "Destroy didn't work"); > } > public: > - > - void Destroy(ASTContext &C); > + virtual void Destroy(ASTContext &C); > > /// \brief Whether this attribute should be merged to new > /// declarations. > @@ -157,6 +155,18 @@ > // Implement isa/cast/dyncast/etc. > static bool classof(const Attr *) { return true; } > }; > + > +class AttrWithString : public Attr { > +private: > + const char *Str; > + unsigned StrLen; > +protected: > + AttrWithString(Attr::Kind AK, ASTContext &C, llvm::StringRef s); > + llvm::StringRef getString() const { return llvm::StringRef(Str, StrLen); } > + void ReplaceString(ASTContext &C, llvm::StringRef newS); > +public: > + virtual void Destroy(ASTContext &C); > +}; > > #define DEF_SIMPLE_ATTR(ATTR) \ > class ATTR##Attr : public Attr { \ > @@ -214,12 +224,12 @@ > static bool classof(const AlignedAttr *A) { return true; } > }; > > -class AnnotateAttr : public Attr { > - std::string Annotation; > +class AnnotateAttr : public AttrWithString { > public: > - AnnotateAttr(llvm::StringRef ann) : Attr(Annotate), Annotation(ann) {} > + AnnotateAttr(ASTContext &C, llvm::StringRef ann) > + : AttrWithString(Annotate, C, ann) {} > > - const std::string& getAnnotation() const { return Annotation; } > + llvm::StringRef getAnnotation() const { return getString(); } > > virtual Attr* clone(ASTContext &C) const; > > @@ -230,12 +240,12 @@ > static bool classof(const AnnotateAttr *A) { return true; } > }; > > -class AsmLabelAttr : public Attr { > - std::string Label; > +class AsmLabelAttr : public AttrWithString { > public: > - AsmLabelAttr(llvm::StringRef L) : Attr(AsmLabel), Label(L) {} > + AsmLabelAttr(ASTContext &C, llvm::StringRef L) > + : AttrWithString(AsmLabel, C, L) {} > > - const std::string& getLabel() const { return Label; } > + llvm::StringRef getLabel() const { return getString(); } > > virtual Attr* clone(ASTContext &C) const; > > @@ -248,12 +258,12 @@ > > DEF_SIMPLE_ATTR(AlwaysInline); > > -class AliasAttr : public Attr { > - std::string Aliasee; > +class AliasAttr : public AttrWithString { > public: > - AliasAttr(llvm::StringRef aliasee) : Attr(Alias), Aliasee(aliasee) {} > + AliasAttr(ASTContext &C, llvm::StringRef aliasee) > + : AttrWithString(Alias, C, aliasee) {} > > - const std::string& getAliasee() const { return Aliasee; } > + llvm::StringRef getAliasee() const { return getString(); } > > virtual Attr *clone(ASTContext &C) const; > > @@ -322,12 +332,12 @@ > DEF_SIMPLE_ATTR(Deprecated); > DEF_SIMPLE_ATTR(Final); > > -class SectionAttr : public Attr { > - std::string Name; > +class SectionAttr : public AttrWithString { > public: > - SectionAttr(llvm::StringRef N) : Attr(Section), Name(N) {} > + SectionAttr(ASTContext &C, llvm::StringRef N) > + : AttrWithString(Section, C, N) {} > > - const std::string& getName() const { return Name; } > + llvm::StringRef getName() const { return getString(); } > > virtual Attr *clone(ASTContext &C) const; > > @@ -380,15 +390,14 @@ > static bool classof(const NonNullAttr *A) { return true; } > }; > > -class FormatAttr : public Attr { > - std::string Type; > +class FormatAttr : public AttrWithString { > int formatIdx, firstArg; > public: > - FormatAttr(llvm::StringRef type, int idx, int first) : Attr(Format), > - Type(type), formatIdx(idx), firstArg(first) {} > + FormatAttr(ASTContext &C, llvm::StringRef type, int idx, int first) > + : AttrWithString(Format, C, type), formatIdx(idx), firstArg(first) {} > > - const std::string& getType() const { return Type; } > - void setType(llvm::StringRef type) { Type = type; } > + llvm::StringRef getType() const { return getString(); } > + void setType(ASTContext &C, llvm::StringRef type); > int getFormatIdx() const { return formatIdx; } > int getFirstArg() const { return firstArg; } > > > Modified: cfe/trunk/lib/AST/AttrImpl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/AttrImpl.cpp?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/AttrImpl.cpp (original) > +++ cfe/trunk/lib/AST/AttrImpl.cpp Wed Feb 10 23:28:37 2010 > @@ -11,11 +11,45 @@ > // > //===----------------------------------------------------------------------===// > > - > #include "clang/AST/Attr.h" > #include "clang/AST/ASTContext.h" > using namespace clang; > > +void Attr::Destroy(ASTContext &C) { > + if (Next) { > + Next->Destroy(C); > + Next = 0; > + } > + this->~Attr(); > + C.Deallocate((void*)this); > +} > + > +AttrWithString::AttrWithString(Attr::Kind AK, ASTContext &C, llvm::StringRef > s) > + : Attr(AK) { > + assert(!s.empty()); > + StrLen = s.size(); > + Str = new (C) char[StrLen]; > + memcpy(const_cast<char*>(Str), s.data(), StrLen); > +} > + > +void AttrWithString::Destroy(ASTContext &C) { > + C.Deallocate(const_cast<char*>(Str)); > + Attr::Destroy(C); > +} > + > +void AttrWithString::ReplaceString(ASTContext &C, llvm::StringRef newS) { > + if (newS.size() > StrLen) { > + C.Deallocate(const_cast<char*>(Str)); > + Str = new char[newS.size()]; > + } > + StrLen = newS.size(); > + memcpy(const_cast<char*>(Str), newS.data(), StrLen); > +} > + > +void FormatAttr::setType(ASTContext &C, llvm::StringRef type) { > + ReplaceString(C, type); > +} > + > #define DEF_SIMPLE_ATTR_CLONE(ATTR) \ > Attr *ATTR##Attr::clone(ASTContext &C) const { \ > return ::new (C) ATTR##Attr; \ > @@ -66,15 +100,15 @@ > } > > Attr* AnnotateAttr::clone(ASTContext &C) const { > - return ::new (C) AnnotateAttr(Annotation); > + return ::new (C) AnnotateAttr(C, getAnnotation()); > } > > Attr *AsmLabelAttr::clone(ASTContext &C) const { > - return ::new (C) AsmLabelAttr(Label); > + return ::new (C) AsmLabelAttr(C, getLabel()); > } > > Attr *AliasAttr::clone(ASTContext &C) const { > - return ::new (C) AliasAttr(Aliasee); > + return ::new (C) AliasAttr(C, getAliasee()); > } > > Attr *ConstructorAttr::clone(ASTContext &C) const { > @@ -94,7 +128,7 @@ > } > > Attr *SectionAttr::clone(ASTContext &C) const { > - return ::new (C) SectionAttr(Name); > + return ::new (C) SectionAttr(C, getName()); > } > > Attr *NonNullAttr::clone(ASTContext &C) const { > @@ -102,7 +136,7 @@ > } > > Attr *FormatAttr::clone(ASTContext &C) const { > - return ::new (C) FormatAttr(Type, formatIdx, firstArg); > + return ::new (C) FormatAttr(C, getType(), formatIdx, firstArg); > } > > Attr *FormatArgAttr::clone(ASTContext &C) const { > > Modified: cfe/trunk/lib/AST/Decl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/Decl.cpp (original) > +++ cfe/trunk/lib/AST/Decl.cpp Wed Feb 10 23:28:37 2010 > @@ -29,15 +29,6 @@ > > using namespace clang; > > -void Attr::Destroy(ASTContext &C) { > - if (Next) { > - Next->Destroy(C); > - Next = 0; > - } > - this->~Attr(); > - C.Deallocate((void*)this); > -} > - > /// \brief Return the TypeLoc wrapper for the type source info. > TypeLoc TypeSourceInfo::getTypeLoc() const { > return TypeLoc(Ty, (void*)(this + 1)); > > Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Feb 10 23:28:37 2010 > @@ -1283,8 +1283,8 @@ > const llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType()); > > // Unique the name through the identifier table. > - const char *AliaseeName = AA->getAliasee().c_str(); > - AliaseeName = getContext().Idents.get(AliaseeName).getNameStart(); > + const char *AliaseeName = > + getContext().Idents.get(AA->getAliasee()).getNameStart(); > > // Create a reference to the named value. This ensures that it is emitted > // if a deferred decl. > > Modified: cfe/trunk/lib/Frontend/PCHReaderDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PCHReaderDecl.cpp?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/lib/Frontend/PCHReaderDecl.cpp (original) > +++ cfe/trunk/lib/Frontend/PCHReaderDecl.cpp Wed Feb 10 23:28:37 2010 > @@ -446,7 +446,7 @@ > > #define STRING_ATTR(Name) \ > case Attr::Name: \ > - New = ::new (*Context) Name##Attr(ReadString(Record, Idx)); \ > + New = ::new (*Context) Name##Attr(*Context, ReadString(Record, Idx)); \ > break > > #define UNSIGNED_ATTR(Name) \ > @@ -497,7 +497,7 @@ > std::string Type = ReadString(Record, Idx); > unsigned FormatIdx = Record[Idx++]; > unsigned FirstArg = Record[Idx++]; > - New = ::new (*Context) FormatAttr(Type, FormatIdx, FirstArg); > + New = ::new (*Context) FormatAttr(*Context, Type, FormatIdx, FirstArg); > break; > } > > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Feb 10 23:28:37 2010 > @@ -2387,7 +2387,7 @@ > if (Expr *E = (Expr*) D.getAsmLabel()) { > // The parser guarantees this is a string. > StringLiteral *SE = cast<StringLiteral>(E); > - NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getString())); > + NewVD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getString())); > } > > // Don't consider existing declarations that are in a different > @@ -2910,7 +2910,7 @@ > if (Expr *E = (Expr*) D.getAsmLabel()) { > // The parser guarantees this is a string. > StringLiteral *SE = cast<StringLiteral>(E); > - NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getString())); > + NewFD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getString())); > } > > // Copy the parameter declarations from the declarator D to the function > @@ -4315,8 +4315,8 @@ > bool HasVAListArg; > if (Context.BuiltinInfo.isPrintfLike(BuiltinID, FormatIdx, HasVAListArg)) > { > if (!FD->getAttr<FormatAttr>()) > - FD->addAttr(::new (Context) FormatAttr("printf", FormatIdx + 1, > - HasVAListArg ? 0 : FormatIdx + > 2)); > + FD->addAttr(::new (Context) FormatAttr(Context, "printf", > FormatIdx+1, > + HasVAListArg ? 0 : > FormatIdx+2)); > } > > // Mark const if we don't care about errno and that is the only > @@ -4353,15 +4353,15 @@ > // FIXME: NSLog and NSLogv should be target specific > if (const FormatAttr *Format = FD->getAttr<FormatAttr>()) { > // FIXME: We known better than our headers. > - const_cast<FormatAttr *>(Format)->setType("printf"); > + const_cast<FormatAttr *>(Format)->setType(Context, "printf"); > } else > - FD->addAttr(::new (Context) FormatAttr("printf", 1, > + FD->addAttr(::new (Context) FormatAttr(Context, "printf", 1, > Name->isStr("NSLogv") ? 0 : 2)); > } else if (Name->isStr("asprintf") || Name->isStr("vasprintf")) { > // FIXME: asprintf and vasprintf aren't C99 functions. Should they be > // target-specific builtins, perhaps? > if (!FD->getAttr<FormatAttr>()) > - FD->addAttr(::new (Context) FormatAttr("printf", 2, > + FD->addAttr(::new (Context) FormatAttr(Context, "printf", 2, > Name->isStr("vasprintf") ? 0 : > 3)); > } > } > > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=95853&r1=95852&r2=95853&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Feb 10 23:28:37 2010 > @@ -329,7 +329,7 @@ > > // FIXME: check if target symbol exists in current file > > - d->addAttr(::new (S.Context) AliasAttr(Str->getString())); > + d->addAttr(::new (S.Context) AliasAttr(S.Context, Str->getString())); > } > > static void HandleAlwaysInlineAttr(Decl *d, const AttributeList &Attr, > @@ -942,7 +942,7 @@ > return; > } > > - D->addAttr(::new (S.Context) SectionAttr(SE->getString())); > + D->addAttr(::new (S.Context) SectionAttr(S.Context, SE->getString())); > } > > > @@ -1257,7 +1257,7 @@ > return; > } > > - d->addAttr(::new (S.Context) FormatAttr(Format, Idx.getZExtValue(), > + d->addAttr(::new (S.Context) FormatAttr(S.Context, Format, > Idx.getZExtValue(), > FirstArg.getZExtValue())); > } > > @@ -1343,7 +1343,7 @@ > S.Diag(ArgExpr->getLocStart(), diag::err_attribute_not_string) > <<"annotate"; > return; > } > - d->addAttr(::new (S.Context) AnnotateAttr(SE->getString())); > + d->addAttr(::new (S.Context) AnnotateAttr(S.Context, SE->getString())); > } > > static void HandleAlignedAttr(Decl *d, const AttributeList &Attr, Sema &S) { > @@ -1924,7 +1924,7 @@ > if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...)) > IdentifierInfo *NDId = ND->getIdentifier(); > NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias()); > - NewD->addAttr(::new (Context) AliasAttr(NDId->getName())); > + NewD->addAttr(::new (Context) AliasAttr(Context, NDId->getName())); > NewD->addAttr(::new (Context) WeakAttr()); > WeakTopLevelDecl.push_back(NewD); > // FIXME: "hideous" code from Sema::LazilyCreateBuiltin > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
