Alter the internal representation of the condition variable in

if/while/switch/for statements to ensure that walking the children of
these statements actually works. Previously, we stored the condition
variable as a VarDecl. However, StmtIterator isn't able to walk from a
VarDecl to a set of statements, and would (in some circumstances) walk
beyond the end of the list of statements, cause Bad Behavior.

In this change, we've gone back to representing the condition
variables as DeclStmts. While not as memory-efficient as VarDecls, it
greatly simplifies iteration over the children. 

Fixes the remainder of <rdar://problem/8104754>.

llvm-svn: 106504
This commit is contained in:
Douglas Gregor 2010-06-21 23:44:13 +00:00
parent c9fec975e6
commit 27b98eae80
5 changed files with 198 additions and 88 deletions

View File

@ -615,24 +615,16 @@ public:
/// IfStmt - This represents an if/then/else.
///
class IfStmt : public Stmt {
enum { COND, THEN, ELSE, END_EXPR };
enum { VAR, COND, THEN, ELSE, END_EXPR };
Stmt* SubExprs[END_EXPR];
/// \brief If non-NULL, the declaration in the "if" statement.
VarDecl *Var;
SourceLocation IfLoc;
SourceLocation ElseLoc;
public:
IfStmt(SourceLocation IL, VarDecl *var, Expr *cond, Stmt *then,
SourceLocation EL = SourceLocation(), Stmt *elsev = 0)
: Stmt(IfStmtClass), Var(var), IfLoc(IL), ElseLoc(EL) {
SubExprs[COND] = reinterpret_cast<Stmt*>(cond);
SubExprs[THEN] = then;
SubExprs[ELSE] = elsev;
}
IfStmt(ASTContext &C, SourceLocation IL, VarDecl *var, Expr *cond,
Stmt *then, SourceLocation EL = SourceLocation(), Stmt *elsev = 0);
/// \brief Build an empty if/then/else statement
explicit IfStmt(EmptyShell Empty) : Stmt(IfStmtClass, Empty) { }
@ -644,8 +636,8 @@ public:
/// printf("x is %d", x);
/// }
/// \endcode
VarDecl *getConditionVariable() const { return Var; }
void setConditionVariable(VarDecl *V) { Var = V; }
VarDecl *getConditionVariable() const;
void setConditionVariable(ASTContext &C, VarDecl *V);
const Expr *getCond() const { return reinterpret_cast<Expr*>(SubExprs[COND]);}
void setCond(Expr *E) { SubExprs[COND] = reinterpret_cast<Stmt *>(E); }
@ -687,9 +679,8 @@ protected:
/// SwitchStmt - This represents a 'switch' stmt.
///
class SwitchStmt : public Stmt {
enum { COND, BODY, END_EXPR };
enum { VAR, COND, BODY, END_EXPR };
Stmt* SubExprs[END_EXPR];
VarDecl *Var;
// This points to a linked list of case and default statements.
SwitchCase *FirstCase;
SourceLocation SwitchLoc;
@ -698,12 +689,7 @@ protected:
virtual void DoDestroy(ASTContext &Ctx);
public:
SwitchStmt(VarDecl *Var, Expr *cond)
: Stmt(SwitchStmtClass), Var(Var), FirstCase(0)
{
SubExprs[COND] = reinterpret_cast<Stmt*>(cond);
SubExprs[BODY] = NULL;
}
SwitchStmt(ASTContext &C, VarDecl *Var, Expr *cond);
/// \brief Build a empty switch statement.
explicit SwitchStmt(EmptyShell Empty) : Stmt(SwitchStmtClass, Empty) { }
@ -717,8 +703,8 @@ public:
/// // ...
/// }
/// \endcode
VarDecl *getConditionVariable() const { return Var; }
void setConditionVariable(VarDecl *V) { Var = V; }
VarDecl *getConditionVariable() const;
void setConditionVariable(ASTContext &C, VarDecl *V);
const Expr *getCond() const { return reinterpret_cast<Expr*>(SubExprs[COND]);}
const Stmt *getBody() const { return SubExprs[BODY]; }
@ -766,18 +752,12 @@ public:
/// WhileStmt - This represents a 'while' stmt.
///
class WhileStmt : public Stmt {
enum { COND, BODY, END_EXPR };
VarDecl *Var;
enum { VAR, COND, BODY, END_EXPR };
Stmt* SubExprs[END_EXPR];
SourceLocation WhileLoc;
public:
WhileStmt(VarDecl *Var, Expr *cond, Stmt *body, SourceLocation WL)
: Stmt(WhileStmtClass), Var(Var)
{
SubExprs[COND] = reinterpret_cast<Stmt*>(cond);
SubExprs[BODY] = body;
WhileLoc = WL;
}
WhileStmt(ASTContext &C, VarDecl *Var, Expr *cond, Stmt *body,
SourceLocation WL);
/// \brief Build an empty while statement.
explicit WhileStmt(EmptyShell Empty) : Stmt(WhileStmtClass, Empty) { }
@ -790,8 +770,8 @@ public:
/// // ...
/// }
/// \endcode
VarDecl *getConditionVariable() const { return Var; }
void setConditionVariable(VarDecl *V) { Var = V; }
VarDecl *getConditionVariable() const;
void setConditionVariable(ASTContext &C, VarDecl *V);
Expr *getCond() { return reinterpret_cast<Expr*>(SubExprs[COND]); }
const Expr *getCond() const { return reinterpret_cast<Expr*>(SubExprs[COND]);}
@ -873,23 +853,14 @@ public:
/// specified in the source.
///
class ForStmt : public Stmt {
enum { INIT, COND, INC, BODY, END_EXPR };
enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
VarDecl *CondVar;
SourceLocation ForLoc;
SourceLocation LParenLoc, RParenLoc;
public:
ForStmt(Stmt *Init, Expr *Cond, VarDecl *condVar, Expr *Inc, Stmt *Body,
SourceLocation FL, SourceLocation LP, SourceLocation RP)
: Stmt(ForStmtClass), CondVar(condVar), ForLoc(FL), LParenLoc(LP),
RParenLoc(RP)
{
SubExprs[INIT] = Init;
SubExprs[COND] = reinterpret_cast<Stmt*>(Cond);
SubExprs[INC] = reinterpret_cast<Stmt*>(Inc);
SubExprs[BODY] = Body;
}
ForStmt(ASTContext &C, Stmt *Init, Expr *Cond, VarDecl *condVar, Expr *Inc,
Stmt *Body, SourceLocation FL, SourceLocation LP, SourceLocation RP);
/// \brief Build an empty for statement.
explicit ForStmt(EmptyShell Empty) : Stmt(ForStmtClass, Empty) { }
@ -904,8 +875,8 @@ public:
/// // ...
/// }
/// \endcode
VarDecl *getConditionVariable() const { return CondVar; }
void setConditionVariable(VarDecl *V) { CondVar = V; }
VarDecl *getConditionVariable() const;
void setConditionVariable(ASTContext &C, VarDecl *V);
Expr *getCond() { return reinterpret_cast<Expr*>(SubExprs[COND]); }
Expr *getInc() { return reinterpret_cast<Expr*>(SubExprs[INC]); }

View File

@ -499,14 +499,101 @@ void DeclStmt::DoDestroy(ASTContext &C) {
DG.getDeclGroup().Destroy(C);
}
IfStmt::IfStmt(ASTContext &C, SourceLocation IL, VarDecl *var, Expr *cond,
Stmt *then, SourceLocation EL, Stmt *elsev)
: Stmt(IfStmtClass), IfLoc(IL), ElseLoc(EL)
{
setConditionVariable(C, var);
SubExprs[COND] = reinterpret_cast<Stmt*>(cond);
SubExprs[THEN] = then;
SubExprs[ELSE] = elsev;
}
VarDecl *IfStmt::getConditionVariable() const {
if (!SubExprs[VAR])
return 0;
DeclStmt *DS = cast<DeclStmt>(SubExprs[VAR]);
return cast<VarDecl>(DS->getSingleDecl());
}
void IfStmt::setConditionVariable(ASTContext &C, VarDecl *V) {
if (!V) {
SubExprs[VAR] = 0;
return;
}
SubExprs[VAR] = new (C) DeclStmt(DeclGroupRef(V),
V->getSourceRange().getBegin(),
V->getSourceRange().getEnd());
}
void IfStmt::DoDestroy(ASTContext &C) {
BranchDestroy(C, this, SubExprs, END_EXPR);
}
ForStmt::ForStmt(ASTContext &C, Stmt *Init, Expr *Cond, VarDecl *condVar,
Expr *Inc, Stmt *Body, SourceLocation FL, SourceLocation LP,
SourceLocation RP)
: Stmt(ForStmtClass), ForLoc(FL), LParenLoc(LP), RParenLoc(RP)
{
SubExprs[INIT] = Init;
setConditionVariable(C, condVar);
SubExprs[COND] = reinterpret_cast<Stmt*>(Cond);
SubExprs[INC] = reinterpret_cast<Stmt*>(Inc);
SubExprs[BODY] = Body;
}
VarDecl *ForStmt::getConditionVariable() const {
if (!SubExprs[CONDVAR])
return 0;
DeclStmt *DS = cast<DeclStmt>(SubExprs[CONDVAR]);
return cast<VarDecl>(DS->getSingleDecl());
}
void ForStmt::setConditionVariable(ASTContext &C, VarDecl *V) {
if (!V) {
SubExprs[CONDVAR] = 0;
return;
}
SubExprs[CONDVAR] = new (C) DeclStmt(DeclGroupRef(V),
V->getSourceRange().getBegin(),
V->getSourceRange().getEnd());
}
void ForStmt::DoDestroy(ASTContext &C) {
BranchDestroy(C, this, SubExprs, END_EXPR);
}
SwitchStmt::SwitchStmt(ASTContext &C, VarDecl *Var, Expr *cond)
: Stmt(SwitchStmtClass), FirstCase(0)
{
setConditionVariable(C, Var);
SubExprs[COND] = reinterpret_cast<Stmt*>(cond);
SubExprs[BODY] = NULL;
}
VarDecl *SwitchStmt::getConditionVariable() const {
if (!SubExprs[VAR])
return 0;
DeclStmt *DS = cast<DeclStmt>(SubExprs[VAR]);
return cast<VarDecl>(DS->getSingleDecl());
}
void SwitchStmt::setConditionVariable(ASTContext &C, VarDecl *V) {
if (!V) {
SubExprs[VAR] = 0;
return;
}
SubExprs[VAR] = new (C) DeclStmt(DeclGroupRef(V),
V->getSourceRange().getBegin(),
V->getSourceRange().getEnd());
}
void SwitchStmt::DoDestroy(ASTContext &C) {
// Destroy the SwitchCase statements in this switch. In the normal
// case, this loop will merely decrement the reference counts from
@ -521,6 +608,35 @@ void SwitchStmt::DoDestroy(ASTContext &C) {
BranchDestroy(C, this, SubExprs, END_EXPR);
}
WhileStmt::WhileStmt(ASTContext &C, VarDecl *Var, Expr *cond, Stmt *body,
SourceLocation WL)
: Stmt(WhileStmtClass)
{
setConditionVariable(C, Var);
SubExprs[COND] = reinterpret_cast<Stmt*>(cond);
SubExprs[BODY] = body;
WhileLoc = WL;
}
VarDecl *WhileStmt::getConditionVariable() const {
if (!SubExprs[VAR])
return 0;
DeclStmt *DS = cast<DeclStmt>(SubExprs[VAR]);
return cast<VarDecl>(DS->getSingleDecl());
}
void WhileStmt::setConditionVariable(ASTContext &C, VarDecl *V) {
if (!V) {
SubExprs[VAR] = 0;
return;
}
SubExprs[VAR] = new (C) DeclStmt(DeclGroupRef(V),
V->getSourceRange().getBegin(),
V->getSourceRange().getEnd());
}
void WhileStmt::DoDestroy(ASTContext &C) {
BranchDestroy(C, this, SubExprs, END_EXPR);
}
@ -572,26 +688,26 @@ Stmt::child_iterator LabelStmt::child_end() { return &SubStmt+1; }
// IfStmt
Stmt::child_iterator IfStmt::child_begin() {
return child_iterator(Var, &SubExprs[0]);
return &SubExprs[0];
}
Stmt::child_iterator IfStmt::child_end() {
return child_iterator(0, &SubExprs[0]+END_EXPR);
return &SubExprs[0]+END_EXPR;
}
// SwitchStmt
Stmt::child_iterator SwitchStmt::child_begin() {
return child_iterator(Var, &SubExprs[0]);
return &SubExprs[0];
}
Stmt::child_iterator SwitchStmt::child_end() {
return child_iterator(0, &SubExprs[0]+END_EXPR);
return &SubExprs[0]+END_EXPR;
}
// WhileStmt
Stmt::child_iterator WhileStmt::child_begin() {
return child_iterator(Var, &SubExprs[0]);
return &SubExprs[0];
}
Stmt::child_iterator WhileStmt::child_end() {
return child_iterator(0, &SubExprs[0]+END_EXPR);
return &SubExprs[0]+END_EXPR;
}
// DoStmt
@ -600,10 +716,10 @@ Stmt::child_iterator DoStmt::child_end() { return &SubExprs[0]+END_EXPR; }
// ForStmt
Stmt::child_iterator ForStmt::child_begin() {
return child_iterator(CondVar, &SubExprs[0]);
return &SubExprs[0];
}
Stmt::child_iterator ForStmt::child_end() {
return child_iterator(0, &SubExprs[0]+END_EXPR);
return &SubExprs[0]+END_EXPR;
}
// ObjCForCollectionStmt

View File

@ -196,7 +196,8 @@ unsigned PCHStmtReader::VisitLabelStmt(LabelStmt *S) {
unsigned PCHStmtReader::VisitIfStmt(IfStmt *S) {
VisitStmt(S);
S->setConditionVariable(cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setConditionVariable(*Reader.getContext(),
cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setCond(cast<Expr>(StmtStack[StmtStack.size() - 3]));
S->setThen(StmtStack[StmtStack.size() - 2]);
S->setElse(StmtStack[StmtStack.size() - 1]);
@ -207,7 +208,8 @@ unsigned PCHStmtReader::VisitIfStmt(IfStmt *S) {
unsigned PCHStmtReader::VisitSwitchStmt(SwitchStmt *S) {
VisitStmt(S);
S->setConditionVariable(cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setConditionVariable(*Reader.getContext(),
cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setCond(cast<Expr>(StmtStack[StmtStack.size() - 2]));
S->setBody(StmtStack.back());
S->setSwitchLoc(SourceLocation::getFromRawEncoding(Record[Idx++]));
@ -229,7 +231,8 @@ unsigned PCHStmtReader::VisitSwitchStmt(SwitchStmt *S) {
unsigned PCHStmtReader::VisitWhileStmt(WhileStmt *S) {
VisitStmt(S);
S->setConditionVariable(cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setConditionVariable(*Reader.getContext(),
cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setCond(cast_or_null<Expr>(StmtStack[StmtStack.size() - 2]));
S->setBody(StmtStack.back());
S->setWhileLoc(SourceLocation::getFromRawEncoding(Record[Idx++]));
@ -250,7 +253,8 @@ unsigned PCHStmtReader::VisitForStmt(ForStmt *S) {
VisitStmt(S);
S->setInit(StmtStack[StmtStack.size() - 4]);
S->setCond(cast_or_null<Expr>(StmtStack[StmtStack.size() - 3]));
S->setConditionVariable(cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setConditionVariable(*Reader.getContext(),
cast_or_null<VarDecl>(Reader.GetDecl(Record[Idx++])));
S->setInc(cast_or_null<Expr>(StmtStack[StmtStack.size() - 2]));
S->setBody(StmtStack.back());
S->setForLoc(SourceLocation::getFromRawEncoding(Record[Idx++]));

View File

@ -65,6 +65,7 @@ class JumpScopeChecker {
public:
JumpScopeChecker(Stmt *Body, Sema &S);
private:
void BuildScopeInformation(Decl *D, unsigned &ParentScope);
void BuildScopeInformation(Stmt *S, unsigned ParentScope);
void VerifyJumps();
void VerifyIndirectJumps();
@ -148,13 +149,33 @@ static std::pair<unsigned,unsigned>
return std::make_pair(0U, 0U);
}
/// \brief Build scope information for a declaration that is part of a DeclStmt.
void JumpScopeChecker::BuildScopeInformation(Decl *D, unsigned &ParentScope) {
bool isCPlusPlus = this->S.getLangOptions().CPlusPlus;
// If this decl causes a new scope, push and switch to it.
std::pair<unsigned,unsigned> Diags
= GetDiagForGotoScopeDecl(D, isCPlusPlus);
if (Diags.first || Diags.second) {
Scopes.push_back(GotoScope(ParentScope, Diags.first, Diags.second,
D->getLocation()));
ParentScope = Scopes.size()-1;
}
// If the decl has an initializer, walk it with the potentially new
// scope we just installed.
if (VarDecl *VD = dyn_cast<VarDecl>(D))
if (Expr *Init = VD->getInit())
BuildScopeInformation(Init, ParentScope);
}
/// BuildScopeInformation - The statements from CI to CE are known to form a
/// coherent VLA scope with a specified parent node. Walk through the
/// statements, adding any labels or gotos to LabelAndGotoScopes and recursively
/// walking the AST as needed.
void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
bool SkipFirstSubStmt = false;
// If we found a label, remember that it is in ParentScope scope.
switch (S->getStmtClass()) {
case Stmt::LabelStmtClass:
@ -172,8 +193,16 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
IndirectJumps.push_back(cast<IndirectGotoStmt>(S));
break;
case Stmt::GotoStmtClass:
case Stmt::SwitchStmtClass:
// Evaluate the condition variable before entering the scope of the switch
// statement.
if (VarDecl *Var = cast<SwitchStmt>(S)->getConditionVariable()) {
BuildScopeInformation(Var, ParentScope);
SkipFirstSubStmt = true;
}
// Fall through
case Stmt::GotoStmtClass:
// Remember both what scope a goto is in as well as the fact that we have
// it. This makes the second scan not have to walk the AST again.
LabelAndGotoScopes[S] = ParentScope;
@ -186,33 +215,22 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
for (Stmt::child_iterator CI = S->child_begin(), E = S->child_end(); CI != E;
++CI) {
if (SkipFirstSubStmt) {
SkipFirstSubStmt = false;
continue;
}
Stmt *SubStmt = *CI;
if (SubStmt == 0) continue;
bool isCPlusPlus = this->S.getLangOptions().CPlusPlus;
// If this is a declstmt with a VLA definition, it defines a scope from here
// to the end of the containing context.
if (DeclStmt *DS = dyn_cast<DeclStmt>(SubStmt)) {
// The decl statement creates a scope if any of the decls in it are VLAs
// or have the cleanup attribute.
for (DeclStmt::decl_iterator I = DS->decl_begin(), E = DS->decl_end();
I != E; ++I) {
// If this decl causes a new scope, push and switch to it.
std::pair<unsigned,unsigned> Diags
= GetDiagForGotoScopeDecl(*I, isCPlusPlus);
if (Diags.first || Diags.second) {
Scopes.push_back(GotoScope(ParentScope, Diags.first, Diags.second,
(*I)->getLocation()));
ParentScope = Scopes.size()-1;
}
// If the decl has an initializer, walk it with the potentially new
// scope we just installed.
if (VarDecl *VD = dyn_cast<VarDecl>(*I))
if (Expr *Init = VD->getInit())
BuildScopeInformation(Init, ParentScope);
}
I != E; ++I)
BuildScopeInformation(*I, ParentScope);
continue;
}

View File

@ -304,7 +304,7 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, DeclPtrTy CondVar,
DiagnoseUnusedExprResult(elseStmt);
CondResult.release();
return Owned(new (Context) IfStmt(IfLoc, ConditionVar, ConditionExpr,
return Owned(new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr,
thenStmt, ElseLoc, elseStmt));
}
@ -543,7 +543,7 @@ Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, ExprArg Cond,
return StmtError();
}
SwitchStmt *SS = new (Context) SwitchStmt(ConditionVar, CondExpr);
SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, CondExpr);
getSwitchStack().push_back(SS);
return Owned(SS);
}
@ -927,8 +927,8 @@ Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
DiagnoseUnusedExprResult(bodyStmt);
CondResult.release();
return Owned(new (Context) WhileStmt(ConditionVar, ConditionExpr, bodyStmt,
WhileLoc));
return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr,
bodyStmt, WhileLoc));
}
Action::OwningStmtResult
@ -997,9 +997,10 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
first.release();
body.release();
return Owned(new (Context) ForStmt(First, SecondResult.takeAs<Expr>(),
ConditionVar, Third, Body,
ForLoc, LParenLoc, RParenLoc));
return Owned(new (Context) ForStmt(Context, First,
SecondResult.takeAs<Expr>(), ConditionVar,
Third, Body, ForLoc, LParenLoc,
RParenLoc));
}
Action::OwningStmtResult