[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] Fix crash parsing pragma after a case or a default
From:       Olivier Goffart <ogoffart () kde ! org>
Date:       2013-09-18 8:09:34
Message-ID: 172467915.zOX8LkIpPH () gargamel
[Download RAW message or body]

On Monday 16 September 2013 11:43:00 Richard Smith wrote:
> I don't particularly like adding a NullStmt here -- there was no null
> statement in the source code, so this is not a faithful AST representation
> of the source.
> 
> This approach seems like it will also accept this:
> 
> switch (t) {
>   case 1:
> #pragma weak t
> }
> 
> We should probably reject this, because there is no statement after the
> case label. (That said, GCC accepts the above code, and fully treats these
> pragmas as being statement-like entities, so your patch would be
> bug-compatible with them.)
> 
> Does the same issue exist for goto labels?

The problem was the same for goto.

I attached a new patch that reject invalid code.

-- 
Olivier
["0001-Fix-crash-when-there-is-a-pragma-right-after-a-case-.patch" (0001-Fix-crash-when-there-is-a-pragma-right-after-a-case-.patch)]

From 978ab457e70390afce70d251c9b5b24324be9200 Mon Sep 17 00:00:00 2001
From: Olivier Goffart <ogoffart@woboq.com>
Date: Thu, 25 Jul 2013 14:43:11 +0200
Subject: [PATCH] Fix crash when there is a pragma right after a case
 statement

---
 lib/Parse/ParseStmt.cpp    | 49 ++++++++++++++++++++++++++--------------------
 test/CodeGen/pragma-weak.c | 22 +++++++++++++++++++++
 test/Parser/pragma-weak.c  | 15 ++++++++++++++
 3 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index b74ab25..9fd342c 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -483,7 +483,10 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributesWithRange &attrs) {
   // attributes (if present) after this point.
   MaybeParseGNUAttributes(attrs);
 
-  StmtResult SubStmt(ParseStatement());
+  StmtResult SubStmt;
+  do {
+      SubStmt = ParseStatement();
+  } while(!SubStmt.isInvalid() && !SubStmt.get());
 
   // Broken substmt shouldn't prevent the label from being added to the AST.
   if (SubStmt.isInvalid())
@@ -615,16 +618,18 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
   // If we found a non-case statement, start by parsing it.
   StmtResult SubStmt;
 
-  if (Tok.isNot(tok::r_brace)) {
-    SubStmt = ParseStatement();
-  } else {
-    // Nicely diagnose the common error "switch (X) { case 4: }", which is
-    // not valid.
-    SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
-    Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
-      << FixItHint::CreateInsertion(AfterColonLoc, " ;");
-    SubStmt = true;
-  }
+  do {
+    if (Tok.isNot(tok::r_brace)) {
+      SubStmt = ParseStatement();
+    } else {
+      // Nicely diagnose the common error "switch (X) { case 4: }", which is
+      // not valid.
+      SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
+      Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
+        << FixItHint::CreateInsertion(AfterColonLoc, " ;");
+      SubStmt = true;
+    }
+  } while(!SubStmt.isInvalid() && !SubStmt.get());
 
   // Broken sub-stmt shouldn't prevent forming the case statement properly.
   if (SubStmt.isInvalid())
@@ -664,16 +669,18 @@ StmtResult Parser::ParseDefaultStatement() {
 
   StmtResult SubStmt;
 
-  if (Tok.isNot(tok::r_brace)) {
-    SubStmt = ParseStatement();
-  } else {
-    // Diagnose the common error "switch (X) {... default: }", which is
-    // not valid.
-    SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
-    Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
-      << FixItHint::CreateInsertion(AfterColonLoc, " ;");
-    SubStmt = true;
-  }
+  do {
+    if (Tok.isNot(tok::r_brace)) {
+      SubStmt = ParseStatement();
+    } else {
+      // Diagnose the common error "switch (X) {... default: }", which is
+      // not valid.
+      SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
+      Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
+        << FixItHint::CreateInsertion(AfterColonLoc, " ;");
+      SubStmt = true;
+    }
+  } while(!SubStmt.isInvalid() && !SubStmt.get());
 
   // Broken sub-stmt shouldn't prevent forming the case statement properly.
   if (SubStmt.isInvalid())
diff --git a/test/CodeGen/pragma-weak.c b/test/CodeGen/pragma-weak.c
index 9bfb9ea..78486ff 100644
--- a/test/CodeGen/pragma-weak.c
+++ b/test/CodeGen/pragma-weak.c
@@ -165,6 +165,28 @@ void PR14046f() {
 }
 // CHECK: declare extern_weak i32 @PR14046e()
 
+// Parse #pragma weak after a label or case statement
+extern int PR16705a(void);
+extern int PR16705b(void);
+extern int PR16705c(void);
+int PR16705f(int a) {
+  switch(a) {
+  case 1:
+#pragma weak PR16705a
+    PR16705a();
+  default:
+#pragma weak PR16705b
+    PR16705b();
+  }
+label:
+  #pragma weak PR16705c
+  PR16705c();
+}
+
+// CHECK: declare extern_weak i32 @PR16705a()
+// CHECK: declare extern_weak i32 @PR16705b()
+// CHECK: declare extern_weak i32 @PR16705c()
+
 
 ///////////// TODO: stuff that still doesn't work
 
diff --git a/test/Parser/pragma-weak.c b/test/Parser/pragma-weak.c
index 7e5740b..f2a8749 100644
--- a/test/Parser/pragma-weak.c
+++ b/test/Parser/pragma-weak.c
@@ -15,3 +15,18 @@ extern int z;
 extern int a;
 /* expected-warning {{extra tokens at end of '#pragma weak'}}*/ #pragma weak a b
 /* expected-warning {{extra tokens at end of '#pragma weak'}}*/ #pragma weak a = x c
+
+
+void pragma_is_not_a_statement(int x) {
+  int t;
+  switch (x) {
+    case 1: // expected-error {{label at end of compound statement: expected statement}}
+#pragma weak t
+  }
+  switch(x) {
+    default: // expected-error {{label at end of compound statement: expected statement}}
+#pragma weak t
+  }
+label:
+#pragma weak t
+} // expected-error {{expected statement}}
-- 
1.7.12.1



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic