[cfe-commits] r95853 - in /cfe/trunk: include/clang/AST/Attr.h lib/AST/AttrImpl.cpp lib/AST/Decl.cpp lib/CodeGen/CodeGenModule.cpp lib/Frontend/PCHReaderDecl.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp

Ted Kremenek kremenek at apple.com
Wed Feb 10 21:54:07 PST 2010


Not all of these attributes use strings based on StringExprs, but for those that do that's a good idea.

On Feb 10, 2010, at 9:38 PM, Chris Lattner wrote:

> 
> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 





More information about the cfe-commits mailing list