From 95c225de9fa3d79f70ef5008c0279580a7d9dcad Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 28 Oct 2010 08:53:48 +0000 Subject: [PATCH] Implement an indirect-goto optimization for goto *&&lbl and respect this in the scope checker. With that done, turn an indirect goto into a protected scope into a hard error; otherwise IR generation has to start worrying about declarations not dominating their scopes, as exemplified in PR8473. If this really affects anyone, I can probably adjust this to only hard-error on possible indirect gotos into VLA scopes rather than arbitrary scopes. But we'll see how people cope with the aggressive change on the marginal feature. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@117539 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Stmt.h | 11 ++++++-- include/clang/Basic/DiagnosticSemaKinds.td | 5 ++-- lib/AST/Stmt.cpp | 8 ++++-- lib/CodeGen/CGStmt.cpp | 5 ++++ lib/Sema/JumpDiagnostics.cpp | 21 +++++++++++++- test/Sema/scope-check.c | 32 +++++++++++++++++++++- test/SemaCXX/scope-check.cpp | 4 +-- 7 files changed, 75 insertions(+), 11 deletions(-) diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h index 3862cf3fba..ca42b3ea7e 100644 --- a/include/clang/AST/Stmt.h +++ b/include/clang/AST/Stmt.h @@ -1005,10 +1005,17 @@ public: void setStarLoc(SourceLocation L) { StarLoc = L; } SourceLocation getStarLoc() const { return StarLoc; } - Expr *getTarget(); - const Expr *getTarget() const; + Expr *getTarget() { return reinterpret_cast(Target); } + const Expr *getTarget() const {return reinterpret_cast(Target);} void setTarget(Expr *E) { Target = reinterpret_cast(E); } + /// getConstantTarget - Returns the fixed target of this indirect + /// goto, if one exists. + LabelStmt *getConstantTarget(); + const LabelStmt *getConstantTarget() const { + return const_cast(this)->getConstantTarget(); + } + virtual SourceRange getSourceRange() const { return SourceRange(GotoLoc, Target->getLocEnd()); } diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7efc7d5d41..17a6fe7e4f 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1980,9 +1980,8 @@ def err_switch_into_protected_scope : Error< "switch case is in protected scope">; def err_indirect_goto_without_addrlabel : Error< "indirect goto in function with no address-of-label expressions">; -def warn_indirect_goto_in_protected_scope : Warning< - "indirect goto might cross protected scopes">, - InGroup>; +def err_indirect_goto_in_protected_scope : Error< + "indirect goto might cross protected scopes">; def note_indirect_goto_target : Note<"possible target of indirect goto">; def note_protected_by_variable_init : Note< "jump bypasses variable initialization">; diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp index 4f46b35040..09d889e923 100644 --- a/lib/AST/Stmt.cpp +++ b/lib/AST/Stmt.cpp @@ -669,8 +669,12 @@ Stmt::child_iterator GotoStmt::child_begin() { return child_iterator(); } Stmt::child_iterator GotoStmt::child_end() { return child_iterator(); } // IndirectGotoStmt -Expr* IndirectGotoStmt::getTarget() { return cast(Target); } -const Expr* IndirectGotoStmt::getTarget() const { return cast(Target); } +LabelStmt *IndirectGotoStmt::getConstantTarget() { + if (AddrLabelExpr *E = + dyn_cast(getTarget()->IgnoreParenImpCasts())) + return E->getLabel(); + return 0; +} Stmt::child_iterator IndirectGotoStmt::child_begin() { return &Target; } Stmt::child_iterator IndirectGotoStmt::child_end() { return &Target+1; } diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp index 008c07514e..4de505abdf 100644 --- a/lib/CodeGen/CGStmt.cpp +++ b/lib/CodeGen/CGStmt.cpp @@ -297,6 +297,11 @@ void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) { void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) { + if (const LabelStmt *Target = S.getConstantTarget()) { + EmitBranchThroughCleanup(getJumpDestForLabel(Target)); + return; + } + // Ensure that we have an i8* for our PHI node. llvm::Value *V = Builder.CreateBitCast(EmitScalarExpr(S.getTarget()), llvm::Type::getInt8PtrTy(VMContext), diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp index b23f615af7..bbe7bd5b28 100644 --- a/lib/Sema/JumpDiagnostics.cpp +++ b/lib/Sema/JumpDiagnostics.cpp @@ -186,6 +186,17 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) { break; case Stmt::IndirectGotoStmtClass: + // "goto *&&lbl;" is a special case which we treat as equivalent + // to a normal goto. In addition, we don't calculate scope in the + // operand (to avoid recording the address-of-label use), which + // works only because of the restricted set of expressions which + // we detect as constant targets. + if (cast(S)->getConstantTarget()) { + LabelAndGotoScopes[S] = ParentScope; + Jumps.push_back(S); + return; + } + LabelAndGotoScopes[S] = ParentScope; IndirectJumps.push_back(cast(S)); break; @@ -341,6 +352,14 @@ void JumpScopeChecker::VerifyJumps() { continue; } + // We only get indirect gotos here when they have a constant target. + if (IndirectGotoStmt *IGS = dyn_cast(Jump)) { + LabelStmt *Target = IGS->getConstantTarget(); + CheckJump(IGS, Target, IGS->getGotoLoc(), + diag::err_goto_into_protected_scope); + continue; + } + SwitchStmt *SS = cast(Jump); for (SwitchCase *SC = SS->getSwitchCaseList(); SC; SC = SC->getNextSwitchCase()) { @@ -497,7 +516,7 @@ void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump, unsigned TargetScope) { assert(JumpScope != TargetScope); - S.Diag(Jump->getGotoLoc(), diag::warn_indirect_goto_in_protected_scope); + S.Diag(Jump->getGotoLoc(), diag::err_indirect_goto_in_protected_scope); S.Diag(Target->getIdentLoc(), diag::note_indirect_goto_target); unsigned Common = GetDeepestCommonScope(JumpScope, TargetScope); diff --git a/test/Sema/scope-check.c b/test/Sema/scope-check.c index 4ccb64c9aa..1a2fc2b360 100644 --- a/test/Sema/scope-check.c +++ b/test/Sema/scope-check.c @@ -133,7 +133,7 @@ int test8(int x) { void test9(int n, void *P) { int Y; int Z = 4; - goto *P; // expected-warning {{indirect goto might cross protected scopes}} + goto *P; // expected-error {{indirect goto might cross protected scopes}} L2: ; int a[n]; // expected-note {{jump bypasses initialization of variable length array}} @@ -199,3 +199,33 @@ void test13(int n, void *p) { a0: ; static void *ps[] = { &&a0 }; } + +int test14(int n) { + static void *ps[] = { &&a0, &&a1 }; + if (n < 0) + goto *&&a0; + + if (n > 0) { + int vla[n]; + a1: + vla[n-1] = 0; + } + a0: + return 0; +} + + +// PR8473: IR gen can't deal with indirect gotos past VLA +// initialization, so that really needs to be a hard error. +void test15(int n, void *pc) { + static const void *addrs[] = { &&L1, &&L2 }; + + goto *pc; // expected-error {{indirect goto might cross protected scope}} + + L1: + { + char vla[n]; // expected-note {{jump bypasses initialization}} + L2: // expected-note {{possible target}} + vla[0] = 'a'; + } +} diff --git a/test/SemaCXX/scope-check.cpp b/test/SemaCXX/scope-check.cpp index cdc3868a7b..d462af06d7 100644 --- a/test/SemaCXX/scope-check.cpp +++ b/test/SemaCXX/scope-check.cpp @@ -66,7 +66,7 @@ namespace test4 { C c0; - goto *ip; // expected-warning {{indirect goto might cross protected scopes}} + goto *ip; // expected-error {{indirect goto might cross protected scopes}} C c1; // expected-note {{jump bypasses variable initialization}} lbl1: // expected-note {{possible target of indirect goto}} return 0; @@ -90,7 +90,7 @@ namespace test5 { if (ip[1]) { D d; // expected-note {{jump exits scope of variable with non-trivial destructor}} ip += 2; - goto *ip; // expected-warning {{indirect goto might cross protected scopes}} + goto *ip; // expected-error {{indirect goto might cross protected scopes}} } return 1; } -- 2.40.0