[cfe-commits] r95847 - in /cfe/trunk/lib/Parse: ParseDecl.cpp ParseDeclCXX.cpp ParseObjc.cpp ParseStmt.cpp Parser.cpp
Ted Kremenek
kremenek at apple.com
Wed Feb 10 18:19:14 PST 2010
Author: kremenek
Date: Wed Feb 10 20:19:13 2010
New Revision: 95847
URL: http://llvm.org/viewvc/llvm-project?rev=95847&view=rev
Log:
Clean up ownership of 'AttributeList' objects in Parser. Apparently
we would just leak them all over the place, with no clear ownership of
these objects at all. AttributeList objects would get leaked on both
error and non-error paths.
Note: I introduced the usage of llvm::OwningPtr<AttributeList> to
manage these objects, which is particularly useful for methods with
multiple return sites. In at least one method I used them even when
they weren't strictly necessary because it clarified the ownership
semantics and made the code easier to read. Should the excessive
'take()' and 'reset()' calls become a performance issue we can always
re-evaluate.
Note+1: I believe I have not introduced any double-frees, but it would
be nice for someone to review this.
This fixes <rdar://problem/7635046>.
Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Parse/Parser.cpp
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=95847&r1=95846&r2=95847&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Feb 10 20:19:13 2010
@@ -1807,15 +1807,15 @@
SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc);
- AttributeList *AttrList = 0;
+ llvm::OwningPtr<AttributeList> AttrList;
// If attributes exist after struct contents, parse them.
if (Tok.is(tok::kw___attribute))
- AttrList = ParseGNUAttributes();
+ AttrList.reset(ParseGNUAttributes());
Actions.ActOnFields(CurScope,
RecordLoc, TagDecl, FieldDecls.data(), FieldDecls.size(),
LBraceLoc, RBraceLoc,
- AttrList);
+ AttrList.get());
StructScope.Exit();
Actions.ActOnTagFinishDefinition(CurScope, TagDecl, RBraceLoc);
}
@@ -1842,10 +1842,10 @@
ConsumeToken();
}
- AttributeList *Attr = 0;
+ llvm::OwningPtr<AttributeList> Attr;
// If attributes exist after tag, parse them.
if (Tok.is(tok::kw___attribute))
- Attr = ParseGNUAttributes();
+ Attr.reset(ParseGNUAttributes());
CXXScopeSpec SS;
if (getLang().CPlusPlus && ParseOptionalCXXScopeSpecifier(SS, 0, false)) {
@@ -1895,7 +1895,8 @@
bool Owned = false;
bool IsDependent = false;
DeclPtrTy TagDecl = Actions.ActOnTag(CurScope, DeclSpec::TST_enum, TUK,
- StartLoc, SS, Name, NameLoc, Attr, AS,
+ StartLoc, SS, Name, NameLoc, Attr.get(),
+ AS,
Action::MultiTemplateParamsArg(Actions),
Owned, IsDependent);
assert(!IsDependent && "didn't expect dependent enum");
@@ -1975,14 +1976,14 @@
// Eat the }.
SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc);
- AttributeList *Attr = 0;
+ llvm::OwningPtr<AttributeList> Attr;
// If attributes exist after the identifier list, parse them.
if (Tok.is(tok::kw___attribute))
- Attr = ParseGNUAttributes(); // FIXME: where do they do?
+ Attr.reset(ParseGNUAttributes()); // FIXME: where do they do?
Actions.ActOnEnumBody(StartLoc, LBraceLoc, RBraceLoc, EnumDecl,
EnumConstantDecls.data(), EnumConstantDecls.size(),
- CurScope, Attr);
+ CurScope, Attr.get());
EnumScope.Exit();
Actions.ActOnTagFinishDefinition(CurScope, EnumDecl, RBraceLoc);
@@ -2642,10 +2643,10 @@
// In either case, we need to eat any attributes to be able to determine what
// sort of paren this is.
//
- AttributeList *AttrList = 0;
+ llvm::OwningPtr<AttributeList> AttrList;
bool RequiresArg = false;
if (Tok.is(tok::kw___attribute)) {
- AttrList = ParseGNUAttributes();
+ AttrList.reset(ParseGNUAttributes());
// We require that the argument list (if this is a non-grouping paren) be
// present even if the attribute list was empty.
@@ -2655,7 +2656,7 @@
if (Tok.is(tok::kw___cdecl) || Tok.is(tok::kw___stdcall) ||
Tok.is(tok::kw___fastcall) || Tok.is(tok::kw___w64) ||
Tok.is(tok::kw___ptr64)) {
- AttrList = ParseMicrosoftTypeAttributes(AttrList);
+ AttrList.reset(ParseMicrosoftTypeAttributes(AttrList.take()));
}
// If we haven't past the identifier yet (or where the identifier would be
@@ -2686,7 +2687,7 @@
bool hadGroupingParens = D.hasGroupingParens();
D.setGroupingParens(true);
if (AttrList)
- D.AddAttributes(AttrList, SourceLocation());
+ D.AddAttributes(AttrList.take(), SourceLocation());
ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
// Match the ')'.
@@ -2703,7 +2704,7 @@
// ParseFunctionDeclarator to handle of argument list.
D.SetIdentifier(0, Tok.getLocation());
- ParseFunctionDeclarator(StartLoc, D, AttrList, RequiresArg);
+ ParseFunctionDeclarator(StartLoc, D, AttrList.take(), RequiresArg);
}
/// ParseFunctionDeclarator - We are after the identifier and have parsed the
Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=95847&r1=95846&r2=95847&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Wed Feb 10 20:19:13 2010
@@ -64,12 +64,12 @@
}
// Read label attributes, if present.
- AttributeList *AttrList = 0;
+ llvm::OwningPtr<AttributeList> AttrList;
if (Tok.is(tok::kw___attribute)) {
attrTok = Tok;
// FIXME: save these somewhere.
- AttrList = ParseGNUAttributes();
+ AttrList.reset(ParseGNUAttributes());
}
if (Tok.is(tok::equal)) {
@@ -91,7 +91,8 @@
ParseScope NamespaceScope(this, Scope::DeclScope);
DeclPtrTy NamespcDecl =
- Actions.ActOnStartNamespaceDef(CurScope, IdentLoc, Ident, LBrace, AttrList);
+ Actions.ActOnStartNamespaceDef(CurScope, IdentLoc, Ident, LBrace,
+ AttrList.get());
PrettyStackTraceActionsDecl CrashInfo(NamespcDecl, NamespaceLoc, Actions,
PP.getSourceManager(),
@@ -327,8 +328,6 @@
// Parse nested-name-specifier.
ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false);
- AttributeList *AttrList = 0;
-
// Check nested-name specifier.
if (SS.isInvalid()) {
SkipUntil(tok::semi);
@@ -350,8 +349,9 @@
}
// Parse (optional) attributes (most likely GNU strong-using extension).
+ llvm::OwningPtr<AttributeList> AttrList;
if (Tok.is(tok::kw___attribute))
- AttrList = ParseGNUAttributes();
+ AttrList.reset(ParseGNUAttributes());
// Eat ';'.
DeclEnd = Tok.getLocation();
@@ -360,7 +360,7 @@
tok::semi);
return Actions.ActOnUsingDeclaration(CurScope, AS, true, UsingLoc, SS, Name,
- AttrList, IsTypeName, TypenameLoc);
+ AttrList.get(), IsTypeName, TypenameLoc);
}
/// ParseStaticAssertDeclaration - Parse C++0x static_assert-declaratoion.
@@ -1549,10 +1549,10 @@
SourceLocation RBraceLoc = MatchRHSPunctuation(tok::r_brace, LBraceLoc);
- AttributeList *AttrList = 0;
// If attributes exist after class contents, parse them.
+ llvm::OwningPtr<AttributeList> AttrList;
if (Tok.is(tok::kw___attribute))
- AttrList = ParseGNUAttributes(); // FIXME: where should I put them?
+ AttrList.reset(ParseGNUAttributes()); // FIXME: where should I put them?
Actions.ActOnFinishCXXMemberSpecification(CurScope, RecordLoc, TagDecl,
LBraceLoc, RBraceLoc);
Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=95847&r1=95846&r2=95847&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
+++ cfe/trunk/lib/Parse/ParseObjc.cpp Wed Feb 10 20:19:13 2010
@@ -786,15 +786,15 @@
llvm::SmallVector<Declarator, 8> CargNames;
if (Tok.isNot(tok::colon)) {
// If attributes exist after the method, parse them.
- AttributeList *MethodAttrs = 0;
+ llvm::OwningPtr<AttributeList> MethodAttrs;
if (getLang().ObjC2 && Tok.is(tok::kw___attribute))
- MethodAttrs = ParseGNUAttributes();
+ MethodAttrs.reset(ParseGNUAttributes());
Selector Sel = PP.getSelectorTable().getNullarySelector(SelIdent);
DeclPtrTy Result
= Actions.ActOnMethodDeclaration(mLoc, Tok.getLocation(),
mType, IDecl, DSRet, ReturnType, Sel,
- 0, CargNames, MethodAttrs,
+ 0, CargNames, MethodAttrs.get(),
MethodImplKind);
PD.complete(Result);
return Result;
@@ -862,9 +862,9 @@
// FIXME: Add support for optional parmameter list...
// If attributes exist after the method, parse them.
- AttributeList *MethodAttrs = 0;
+ llvm::OwningPtr<AttributeList> MethodAttrs;
if (getLang().ObjC2 && Tok.is(tok::kw___attribute))
- MethodAttrs = ParseGNUAttributes();
+ MethodAttrs.reset(ParseGNUAttributes());
if (KeyIdents.size() == 0)
return DeclPtrTy();
@@ -873,9 +873,16 @@
DeclPtrTy Result
= Actions.ActOnMethodDeclaration(mLoc, Tok.getLocation(),
mType, IDecl, DSRet, ReturnType, Sel,
- &ArgInfos[0], CargNames, MethodAttrs,
+ &ArgInfos[0], CargNames,
+ MethodAttrs.get(),
MethodImplKind, isVariadic);
PD.complete(Result);
+
+ // Delete referenced AttributeList objects.
+ for (llvm::SmallVectorImpl<Action::ObjCArgInfo>::iterator
+ I = ArgInfos.begin(), E = ArgInfos.end(); I != E; ++I)
+ delete I->ArgAttrs;
+
return Result;
}
Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=95847&r1=95846&r2=95847&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Wed Feb 10 20:19:13 2010
@@ -81,6 +81,7 @@
CXX0XAttributeList Attr;
if (getLang().CPlusPlus0x && isCXX0XAttributeSpecifier())
Attr = ParseCXX0XAttributes();
+ llvm::OwningPtr<AttributeList> AttrList(Attr.AttrList);
// Cases in this switch statement should fall through if the parser expects
// the token to end in a semicolon (in which case SemiError should be set),
@@ -102,13 +103,14 @@
case tok::identifier:
if (NextToken().is(tok::colon)) { // C99 6.8.1: labeled-statement
// identifier ':' statement
- return ParseLabeledStatement(Attr.AttrList);
+ return ParseLabeledStatement(AttrList.take());
}
// PASS THROUGH.
default: {
if ((getLang().CPlusPlus || !OnlyStatement) && isDeclarationStatement()) {
SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
+ AttrList.take(); //Passing 'Attr' to ParseDeclaration transfers ownership.
DeclGroupPtrTy Decl = ParseDeclaration(Declarator::BlockContext, DeclEnd,
Attr);
return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
@@ -135,43 +137,43 @@
}
case tok::kw_case: // C99 6.8.1: labeled-statement
- return ParseCaseStatement(Attr.AttrList);
+ return ParseCaseStatement(AttrList.take());
case tok::kw_default: // C99 6.8.1: labeled-statement
- return ParseDefaultStatement(Attr.AttrList);
+ return ParseDefaultStatement(AttrList.take());
case tok::l_brace: // C99 6.8.2: compound-statement
- return ParseCompoundStatement(Attr.AttrList);
+ return ParseCompoundStatement(AttrList.take());
case tok::semi: // C99 6.8.3p3: expression[opt] ';'
return Actions.ActOnNullStmt(ConsumeToken());
case tok::kw_if: // C99 6.8.4.1: if-statement
- return ParseIfStatement(Attr.AttrList);
+ return ParseIfStatement(AttrList.take());
case tok::kw_switch: // C99 6.8.4.2: switch-statement
- return ParseSwitchStatement(Attr.AttrList);
+ return ParseSwitchStatement(AttrList.take());
case tok::kw_while: // C99 6.8.5.1: while-statement
- return ParseWhileStatement(Attr.AttrList);
+ return ParseWhileStatement(AttrList.take());
case tok::kw_do: // C99 6.8.5.2: do-statement
- Res = ParseDoStatement(Attr.AttrList);
+ Res = ParseDoStatement(AttrList.take());
SemiError = "do/while";
break;
case tok::kw_for: // C99 6.8.5.3: for-statement
- return ParseForStatement(Attr.AttrList);
+ return ParseForStatement(AttrList.take());
case tok::kw_goto: // C99 6.8.6.1: goto-statement
- Res = ParseGotoStatement(Attr.AttrList);
+ Res = ParseGotoStatement(AttrList.take());
SemiError = "goto";
break;
case tok::kw_continue: // C99 6.8.6.2: continue-statement
- Res = ParseContinueStatement(Attr.AttrList);
+ Res = ParseContinueStatement(AttrList.take());
SemiError = "continue";
break;
case tok::kw_break: // C99 6.8.6.3: break-statement
- Res = ParseBreakStatement(Attr.AttrList);
+ Res = ParseBreakStatement(AttrList.take());
SemiError = "break";
break;
case tok::kw_return: // C99 6.8.6.4: return-statement
- Res = ParseReturnStatement(Attr.AttrList);
+ Res = ParseReturnStatement(AttrList.take());
SemiError = "return";
break;
@@ -187,7 +189,7 @@
}
case tok::kw_try: // C++ 15: try-block
- return ParseCXXTryBlock(Attr.AttrList);
+ return ParseCXXTryBlock(AttrList.take());
}
// If we reached this code, the statement must end in a semicolon.
@@ -215,6 +217,7 @@
assert(Tok.is(tok::identifier) && Tok.getIdentifierInfo() &&
"Not an identifier!");
+ llvm::OwningPtr<AttributeList> AttrList(Attr);
Token IdentTok = Tok; // Save the whole token.
ConsumeToken(); // eat the identifier.
@@ -225,7 +228,7 @@
// Read label attributes, if present.
if (Tok.is(tok::kw___attribute))
- Attr = addAttributeLists(Attr, ParseGNUAttributes());
+ AttrList.reset(addAttributeLists(AttrList.take(), ParseGNUAttributes()));
OwningStmtResult SubStmt(ParseStatement());
@@ -233,6 +236,7 @@
if (SubStmt.isInvalid())
SubStmt = Actions.ActOnNullStmt(ColonLoc);
+ // FIXME: use attributes?
return Actions.ActOnLabelStmt(IdentTok.getLocation(),
IdentTok.getIdentifierInfo(),
ColonLoc, move(SubStmt));
@@ -246,6 +250,7 @@
Parser::OwningStmtResult Parser::ParseCaseStatement(AttributeList *Attr) {
assert(Tok.is(tok::kw_case) && "Not a case stmt!");
// FIXME: Use attributes?
+ delete Attr;
// It is very very common for code to contain many case statements recursively
// nested, as in (but usually without indentation):
@@ -371,6 +376,8 @@
///
Parser::OwningStmtResult Parser::ParseDefaultStatement(AttributeList *Attr) {
//FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_default) && "Not a default stmt!");
SourceLocation DefaultLoc = ConsumeToken(); // eat the 'default'.
@@ -427,6 +434,8 @@
Parser::OwningStmtResult Parser::ParseCompoundStatement(AttributeList *Attr,
bool isStmtExpr) {
//FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::l_brace) && "Not a compount stmt!");
// Enter a scope to hold everything within the compound stmt. Compound
@@ -562,6 +571,8 @@
///
Parser::OwningStmtResult Parser::ParseIfStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_if) && "Not an if stmt!");
SourceLocation IfLoc = ConsumeToken(); // eat the 'if'.
@@ -686,6 +697,8 @@
/// [C++] 'switch' '(' condition ')' statement
Parser::OwningStmtResult Parser::ParseSwitchStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_switch) && "Not a switch stmt!");
SourceLocation SwitchLoc = ConsumeToken(); // eat the 'switch'.
@@ -763,6 +776,8 @@
/// [C++] 'while' '(' condition ')' statement
Parser::OwningStmtResult Parser::ParseWhileStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_while) && "Not a while stmt!");
SourceLocation WhileLoc = Tok.getLocation();
ConsumeToken(); // eat the 'while'.
@@ -836,6 +851,8 @@
/// Note: this lets the caller parse the end ';'.
Parser::OwningStmtResult Parser::ParseDoStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_do) && "Not a do stmt!");
SourceLocation DoLoc = ConsumeToken(); // eat the 'do'.
@@ -911,6 +928,8 @@
///
Parser::OwningStmtResult Parser::ParseForStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_for) && "Not a for stmt!");
SourceLocation ForLoc = ConsumeToken(); // eat the 'for'.
@@ -1081,6 +1100,8 @@
///
Parser::OwningStmtResult Parser::ParseGotoStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_goto) && "Not a goto stmt!");
SourceLocation GotoLoc = ConsumeToken(); // eat the 'goto'.
@@ -1115,6 +1136,8 @@
///
Parser::OwningStmtResult Parser::ParseContinueStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
SourceLocation ContinueLoc = ConsumeToken(); // eat the 'continue'.
return Actions.ActOnContinueStmt(ContinueLoc, CurScope);
}
@@ -1127,6 +1150,8 @@
///
Parser::OwningStmtResult Parser::ParseBreakStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
SourceLocation BreakLoc = ConsumeToken(); // eat the 'break'.
return Actions.ActOnBreakStmt(BreakLoc, CurScope);
}
@@ -1136,6 +1161,8 @@
/// 'return' expression[opt] ';'
Parser::OwningStmtResult Parser::ParseReturnStatement(AttributeList *Attr) {
// FIXME: Use attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_return) && "Not a return stmt!");
SourceLocation ReturnLoc = ConsumeToken(); // eat the 'return'.
@@ -1447,6 +1474,8 @@
///
Parser::OwningStmtResult Parser::ParseCXXTryBlock(AttributeList* Attr) {
// FIXME: Add attributes?
+ delete Attr;
+
assert(Tok.is(tok::kw_try) && "Expected 'try'");
SourceLocation TryLoc = ConsumeToken();
Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=95847&r1=95846&r2=95847&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Wed Feb 10 20:19:13 2010
@@ -742,11 +742,11 @@
// Handle the full declarator list.
while (1) {
- Action::AttrTy *AttrList;
// If attributes are present, parse them.
+ llvm::OwningPtr<AttributeList> AttrList;
if (Tok.is(tok::kw___attribute))
// FIXME: attach attributes too.
- AttrList = ParseGNUAttributes();
+ AttrList.reset(ParseGNUAttributes());
// Ask the actions module to compute the type for this declarator.
Action::DeclPtrTy Param =
More information about the cfe-commits
mailing list