[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-22 Thread Nathan Chancellor via Phabricator via cfe-commits 
nathanchance added a comment.

In D154696#4525254 , @cor3ntin wrote:

> Can you let me know what the linux folks think? Thanks a lot!

Sure thing, you should be able to follow along on the web as well:

https://lore.kernel.org/2023070637.GA138486@dev-arch.thelio-3990X/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-22 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

In D154696#4525100 , @nathanchance 
wrote:

> The error added by this patch triggers on some recently added code to the 
> Linux kernel's -next tree, which I failed to test above due to its generally 
> unstable nature:
>
>   drivers/gpu/drm/tests/drm_exec_test.c:41:3: error: cannot jump from this 
> indirect goto statement to one of its possible targets
>  41 | drm_exec_retry_on_contention();
> | ^
>   include/drm/drm_exec.h:96:4: note: expanded from macro 
> 'drm_exec_retry_on_contention'
>  96 | goto *__drm_exec_retry_ptr; \
> | ^
>   drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: possible target of 
> indirect goto statement
>  39 | drm_exec_until_all_locked() {
> | ^
>   include/drm/drm_exec.h:79:33: note: expanded from macro 
> 'drm_exec_until_all_locked'
>  79 | __label__ __drm_exec_retry; \
> | ^
>   drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: jump enters a statement 
> expression
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/drm/drm_exec.h
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/tests/drm_exec_test.c
>
> A standalone reproducer from the kernel sources:
>
>   struct drm_exec {};
>   int drm_exec_cleanup(struct drm_exec *);
>   void drm_exec_fini(struct drm_exec *);
>   void drm_exec_init(struct drm_exec *);
>   int drm_exec_is_contended(struct drm_exec *);
>   void drm_exec_lock_obj(struct drm_exec *);
>   
>   void test_lock(void)
>   {
>   struct drm_exec exec;
>   
>   drm_exec_init();
>   for (void *__drm_exec_retry_ptr; ({
>__label__ __drm_exec_retry;
>__drm_exec_retry:
>__drm_exec_retry_ptr = &&__drm_exec_retry;
>(void)__drm_exec_retry_ptr;
>drm_exec_cleanup();
>});) {
>   drm_exec_lock_obj();
>   do {
>   if 
> (__builtin_expect(!!(drm_exec_is_contended()),
>0))
>   goto *__drm_exec_retry_ptr;
>   } while (0);
>   }
>   drm_exec_fini();
>   }
>
>
>
>   $ clang -fsyntax-only test.c
>   test.c:24:5: error: cannot jump from this indirect goto statement to one of 
> its possible targets
>  24 | goto *__drm_exec_retry_ptr;
> | ^
>   test.c:15:6: note: possible target of indirect goto statement
>  15 |  __drm_exec_retry:
> |  ^
>   test.c:13:35: note: jump enters a statement expression
>  13 | for (void *__drm_exec_retry_ptr; ({
> |  ^
>   1 error generated.
>
> I am not sure this is a problem with the patch, since if I am reading GCC's 
> documentation on statement expressions correctly 
> (https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html), the construct the 
> kernel is using with labels as values to jump into a statement expression 
> with a computed goto is undefined behavior, but I figured I would bring it up 
> just in case there is a problem.
>
>> Jumping into a statement expression with goto or using a switch statement 
>> outside the statement expression with a case or default label inside the 
>> statement expression is not permitted. Jumping into a statement expression 
>> with a computed goto (see Labels as Values) has undefined behavior.
>
> If this is a problem on the kernel side, I can bring this up to them. GCC 
> 13.1.0 at least does not error on this but if it is documented as UB, the 
> construct should still be avoided.

At least GCC documentation says this is UB. It's interesting GCC does not 
diagnose.
I'd be reluctant to revert the patch at this point as it ensure we don't jump 
into unevaluated statements like sizeof, etc.
Can you let me know what the linux folks think? Thanks a lot!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-22 Thread Nathan Chancellor via Phabricator via cfe-commits 
nathanchance added a comment.

The error added by this patch triggers on some recently added code to the Linux 
kernel's -next tree, which I failed to test above due to its generally unstable 
nature:

  drivers/gpu/drm/tests/drm_exec_test.c:41:3: error: cannot jump from this 
indirect goto statement to one of its possible targets
 41 | drm_exec_retry_on_contention();
| ^
  include/drm/drm_exec.h:96:4: note: expanded from macro 
'drm_exec_retry_on_contention'
 96 | goto *__drm_exec_retry_ptr; \
| ^
  drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: possible target of indirect 
goto statement
 39 | drm_exec_until_all_locked() {
| ^
  include/drm/drm_exec.h:79:33: note: expanded from macro 
'drm_exec_until_all_locked'
 79 | __label__ __drm_exec_retry; \
| ^
  drivers/gpu/drm/tests/drm_exec_test.c:39:2: note: jump enters a statement 
expression

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/drm/drm_exec.h
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/tests/drm_exec_test.c

A standalone reproducer from the kernel sources:

  struct drm_exec {};
  int drm_exec_cleanup(struct drm_exec *);
  void drm_exec_fini(struct drm_exec *);
  void drm_exec_init(struct drm_exec *);
  int drm_exec_is_contended(struct drm_exec *);
  void drm_exec_lock_obj(struct drm_exec *);
  
  void test_lock(void)
  {
  struct drm_exec exec;
  
  drm_exec_init();
  for (void *__drm_exec_retry_ptr; ({
   __label__ __drm_exec_retry;
   __drm_exec_retry:
   __drm_exec_retry_ptr = &&__drm_exec_retry;
   (void)__drm_exec_retry_ptr;
   drm_exec_cleanup();
   });) {
  drm_exec_lock_obj();
  do {
  if (__builtin_expect(!!(drm_exec_is_contended()),
   0))
  goto *__drm_exec_retry_ptr;
  } while (0);
  }
  drm_exec_fini();
  }



  $ clang -fsyntax-only test.c
  test.c:24:5: error: cannot jump from this indirect goto statement to one of 
its possible targets
 24 | goto *__drm_exec_retry_ptr;
| ^
  test.c:15:6: note: possible target of indirect goto statement
 15 |  __drm_exec_retry:
|  ^
  test.c:13:35: note: jump enters a statement expression
 13 | for (void *__drm_exec_retry_ptr; ({
|  ^
  1 error generated.

I am not sure this is a problem with the patch, since if I am reading GCC's 
documentation on statement expressions correctly 
(https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html), the construct the 
kernel is using with labels as values to jump into a statement expression with 
a computed goto is undefined behavior, but I figured I would bring it up just 
in case there is a problem.

> Jumping into a statement expression with goto or using a switch statement 
> outside the statement expression with a case or default label inside the 
> statement expression is not permitted. Jumping into a statement expression 
> with a computed goto (see Labels as Values) has undefined behavior.

If this is a problem on the kernel side, I can bring this up to them. GCC 
13.1.0 at least does not error on this but if it is documented as UB, the 
construct should still be avoided.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-21 Thread Corentin Jabot via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG202191060602: [Clang] Diagnose jumps into statement 
expressions (authored by cor3ntin).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -15,7 +15,7 @@
   } @finally {// expected-note {{jump bypasses initialization of @finally block}}
 L3: ;
   }
-  
+
   @try {
 goto L4; // expected-error{{cannot jump}}
 goto L5; // expected-error{{cannot jump}}
@@ -27,8 +27,8 @@
   } @finally { // expected-note {{jump bypasses initialization of @finally block}}
   L4: ;
   }
- 
-  
+
+
   @try { // expected-note 2 {{jump bypasses initialization of @try block}}
   L7: ;
   } @catch (C *c) {
@@ -36,21 +36,18 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
-  // rdar://6810106
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
@@ -90,7 +87,7 @@
 goto L0; // expected-error {{cannot jump}}
 typedef int A[n];  // expected-note {{jump bypasses initialization of VLA typedef}}
   L0:
-
+
 goto L1;  // expected-error {{cannot jump}}
 A b, c[10];// expected-note 2 {{jump bypasses initialization of variable length array}}
   L1:
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -232,3 +233,27 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; }); // expected-note {{jump enters a statement expression}}
+  }
+  {
+(void)({ goto N; 1;

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-21 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LG!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-21 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin updated this revision to Diff 542813.
cor3ntin added a comment.

Rebase + add test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -15,7 +15,7 @@
   } @finally {// expected-note {{jump bypasses initialization of @finally block}}
 L3: ;
   }
-  
+
   @try {
 goto L4; // expected-error{{cannot jump}}
 goto L5; // expected-error{{cannot jump}}
@@ -27,8 +27,8 @@
   } @finally { // expected-note {{jump bypasses initialization of @finally block}}
   L4: ;
   }
- 
-  
+
+
   @try { // expected-note 2 {{jump bypasses initialization of @try block}}
   L7: ;
   } @catch (C *c) {
@@ -36,21 +36,18 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
-  // rdar://6810106
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
@@ -90,7 +87,7 @@
 goto L0; // expected-error {{cannot jump}}
 typedef int A[n];  // expected-note {{jump bypasses initialization of VLA typedef}}
   L0:
-
+
 goto L1;  // expected-error {{cannot jump}}
 A b, c[10];// expected-note 2 {{jump bypasses initialization of variable length array}}
   L1:
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -232,3 +233,27 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; }); // expected-note {{jump enters a statement expression}}
+  }
+  {
+(void)({ goto N; 1; });  // ok
+N: ;
+  }
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;
+  }
+  {
+(void)sizeof(({goto

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-20 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

In D154696#4513510 , @cor3ntin wrote:

> FYI, this can be relanded after https://reviews.llvm.org/D155342 is merged

D155342  is all merged up


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

FYI, this can be relanded after https://reviews.llvm.org/D155342 is merged


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-13 Thread Nathan Chancellor via Phabricator via cfe-commits 
nathanchance added a comment.

I suspect this is the same issue but our CI pointed out another instance of 
this error in some x86 code 
(https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2SW8BY7moEweMJC6DJpzidlGYt8/build.log),
 which does not appear to use local labels with the same name, but rather 
unique label names. `cvise` spits out:

  void emulator_cmpxchg_emulated() {
int __ret = ({
  asm goto(" .pushsection \"__ex_table\",\"a\"\n"
   " .popsection\n"
   :
   :
   :
   : efaultu16);
  __ret;
});
  efaultu16:
({
  asm goto(" .pushsection \"__ex_table\",\"a\"\n"
   " .popsection\n"
   :
   :
   :
   : efaultu32);
efaultu32:;
});
  }



  $ clang -fsyntax-only x86.i
  x86.i:3:5: error: cannot jump from this asm goto statement to one of its 
possible targets
  3 | asm goto(" .pushsection \"__ex_table\",\"a\"\n"
| ^
  x86.i:19:3: note: possible target of asm goto statement
 19 |   efaultu32:;
|   ^
  x86.i:12:3: note: jump enters a statement expression
 12 |   ({
|   ^
  1 error generated.

which certainly seems bogus to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-12 Thread Nathan Chancellor via Phabricator via cfe-commits 
nathanchance added a comment.

In D154696#4494899 , @cor3ntin wrote:

> The issue is  that `VerifyIndirectOrAsmJumps` does not consider that label 
> may be local label at all, I'm not exactly sure how to improve that.
> The fact it works currently seems kind of brittle, what prevent incorrect 
> jumps to be ill-formed is that we looked if a label is defined in the same 
> scope as the `__local__` label introduction, but for the purpose of checking 
> jumps this is not helping at all, not sure if there is a way to break the 
> current implementation in funny ways

That is likely the same reason that we see a similar error when introducing a 
variable with `__attribute__((__cleanup__(...)))`, which involves the same 
PowerPC `asm goto` code: https://github.com/ClangBuiltLinux/linux/issues/1886


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

The issue is  that `VerifyIndirectOrAsmJumps` does not consider that label may 
be local label at all, I'm not exactly sure how to improve that.
The fact it works currently seems kind of brittle, what prevent incorrect jumps 
to be ill-formed is that we looked if a label is defined in the same scope as 
the `__local__` label introduction, but for the purpose of checking jumps this 
is not helping at all, not sure if there is a way to break the current 
implementation in funny ways


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin reopened this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.

@nathanchance Thanks for letting me know. I did revert the patch and plan to 
investigate later this week


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-12 Thread Nathan Chancellor via Phabricator via cfe-commits 
nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

This patch breaks building the Linux kernel for PowerPC, which uses `asm goto` 
with a local label in a statement expression for the WARN macro. It seems like 
something with the scoping is going wrong.

I have two reproducers. The first is manually reduced from the kernel sources:

  struct rb_node {
unsigned long __rb_parent_color;
struct rb_node *rb_right;
struct rb_node *rb_left;
  } __attribute__((aligned(sizeof(long;
  
  struct rb_root {
struct rb_node *rb_node;
  };
  struct kernfs_elem_dir {
struct rb_root children;
  };
  
  struct kernfs_node {
struct kernfs_elem_dir dir;
unsigned short  flags;
  };
  
  enum kernfs_node_flag {
KERNFS_ACTIVATED= 0x0010,
KERNFS_NS   = 0x0020,
KERNFS_HAS_SEQ_SHOW = 0x0040,
KERNFS_HAS_MMAP = 0x0080,
KERNFS_LOCKDEP  = 0x0100,
KERNFS_HIDDEN   = 0x0200,
KERNFS_SUICIDAL = 0x0400,
KERNFS_SUICIDED = 0x0800,
KERNFS_EMPTY_DIR= 0x1000,
KERNFS_HAS_RELEASE  = 0x2000,
KERNFS_REMOVING = 0x4000,
  };
  
  enum kernfs_node_type {
KERNFS_DIR  = 0x0001,
KERNFS_FILE = 0x0002,
KERNFS_LINK = 0x0004,
  };
  
  #define KERNFS_TYPE_MASK  0x000f
  
  struct bug_entry {
signed int bug_addr_disp;
signed int file_disp;
unsigned short line;
unsigned short flags;
  };
  
  static enum kernfs_node_type kernfs_type(struct kernfs_node *kn)
  {
return kn->flags & KERNFS_TYPE_MASK;
  }
  
  void kernfs_enable_ns(struct kernfs_node *kn)
  {
({
int __ret_warn_on = !!(kernfs_type(kn) != KERNFS_DIR);
if (__builtin_expect(!!(__ret_warn_on), 0))
do {
__label__ __label_warn_on;
asm goto("1:"
 "twi 31, 0, 0"
 "\n"
 ".section __ex_table,\"a\";"
 " "
 ".balign 4;"
 " "
 ".long (1b) - . ;"
 " "
 ".long (%l[__label_warn_on]) - . ;"
 " "
 ".previous"
 " "
 ".section __bug_table,\"aw\"\n"
 "2:.4byte 1b - .\n"
 "  .4byte %0 - .\n"
 "  .short %1, %2\n"
 ".org 2b+%3\n"
 ".previous\n"
 :
 : "i"("include/linux/kernfs.h"),
   "i"(378),
   "i"((1 << 0) |
   ((1 << 1) | ((9) << 8))),
   "i"(sizeof(struct bug_entry))
 :
 : __label_warn_on);
do {
} while (0);
__builtin_unreachable();
  __label_warn_on:
break;
} while (0);
__builtin_expect(!!(__ret_warn_on), 0);
});
({
int __ret_warn_on = !!(!(
({
do {
__attribute__((
__noreturn__)) extern void
__compiletime_assert_244(void)
__attribute__((__error__(
"Unsupported access 
size for {READ,WRITE}_ONCE().")));
if (!((sizeof((>dir.children)
  ->rb_node) ==
   sizeof(char) ||
   sizeof((>dir.children)
  ->rb_node) ==
   sizeof(short) ||
   sizeof((>dir.children)
  ->rb_node) ==

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

Thanks for the review :D


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
cor3ntin marked 2 inline comments as done.
Closed by commit rGb0cc947b5d0a: [Clang] Diagnose jumps into statement 
expressions (authored by cor3ntin).

Changed prior to commit:
  https://reviews.llvm.org/D154696?vs=538252=539251#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -15,7 +15,7 @@
   } @finally {// expected-note {{jump bypasses initialization of @finally block}}
 L3: ;
   }
-  
+
   @try {
 goto L4; // expected-error{{cannot jump}}
 goto L5; // expected-error{{cannot jump}}
@@ -27,8 +27,8 @@
   } @finally { // expected-note {{jump bypasses initialization of @finally block}}
   L4: ;
   }
- 
-  
+
+
   @try { // expected-note 2 {{jump bypasses initialization of @try block}}
   L7: ;
   } @catch (C *c) {
@@ -36,20 +36,19 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
+
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
@@ -88,7 +87,7 @@
 goto L0; // expected-error {{cannot jump}}
 typedef int A[n];  // expected-note {{jump bypasses initialization of VLA typedef}}
   L0:
-
+
 goto L1;  // expected-error {{cannot jump}}
 A b, c[10];// expected-note 2 {{jump bypasses initialization of variable length array}}
   L1:
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -231,3 +232,27 @@
 }
 
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; });

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-11 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM though there might be a minor typo with a test comment.




Comment at: clang/test/Sema/asm-goto.cpp:56
+  // expected-note@+1 {{jump enters a statement expression}}
+  return ({int a[n];label_true: 2;}); // expectednote
   // expected-note@+1 {{jump bypasses initialization of variable length array}}




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin marked 2 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:571
   (`#61758 `_)
+- Correcly diagnose jumps into statement expressions.
+  (`#63682 `_)

shafik wrote:
> Maybe remark this conforms with gcc behavior.
I added something. should we link to GCC documentation?



Comment at: clang/lib/Sema/JumpDiagnostics.cpp:489
+Scopes.push_back(GotoScope(ParentScope,
+   diag::note_enters_statement_expression, 0,
+   SE->getBeginLoc()));

shafik wrote:
> I guess we don't have a variable that stands or no-diagnostic.
nope, we use 0 all over the place in that file 



Comment at: clang/lib/Sema/SemaExpr.cpp:16468
   PushExpressionEvaluationContext(ExprEvalContexts.back().Context);
+  setFunctionHasBranchProtectedScope();
 }

shafik wrote:
> I see why we need this but I am not sure how someone would know that need to 
> do this in addition to the change in `BuildScopeInformation(...)`
I added a comment.
I may have spent some time trying to figure out that i needed that line :)



Comment at: clang/test/Sema/scope-check.c:251
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;

shafik wrote:
> I am sorry for this example:
> 
> ```
> int main() {
>sizeof(({goto label;}), 0);
>return 3;
>   ({label:1;});  
> }
> ```
> 
> see godbolt: https://godbolt.org/z/aeb6TbsoG
> 
> Note gcc's behavior Vs clangs.
I added it. It's gnarly :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin updated this revision to Diff 538252.
cor3ntin marked an inline comment as done.
cor3ntin added a comment.

Fix test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -36,21 +36,20 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
+
   // rdar://6810106
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -232,3 +233,27 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; }); // expected-note {{jump enters a statement expression}}
+  }
+  {
+(void)({ goto N; 1; });  // ok
+N: ;
+  }
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;
+  }
+  {
+(void)sizeof(({goto P;}), 0); // expected-error {{cannot jump from this goto statement to its label}}
+return;
+(void)({P:1;});  // expected-note {{jump enters a statement expression}}
+  }
+}
Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -50,9 +50,10 @@
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
-  // expected-note@+2 {{jump bypasses initialization of variable length array}}
-  // expected-note@+1

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin updated this revision to Diff 538250.
cor3ntin marked an inline comment as done.
cor3ntin added a comment.

Missed a comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -36,21 +36,20 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
+
   // rdar://6810106
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -232,3 +233,27 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; }); // expected-note {{jump enters a statement expression}}
+  }
+  {
+(void)({ goto N; 1; });  // ok
+N: ;
+  }
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;
+  }
+  {
+(void)sizeof(({goto P;}), 0); // expected-error {{cannot jump from this goto statement to its label}}
+return 3;
+(void)({P:1;});  // expected-note {{jump enters a statement expression}}
+  }
+}
Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -50,9 +50,10 @@
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
-  // expected-note@+2 {{jump bypasses initialization of variable length array}}
-  // expected-note@+1

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin updated this revision to Diff 538249.
cor3ntin added a comment.

Address Shafik's feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -36,21 +36,20 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
+
   // rdar://6810106
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -232,3 +233,27 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; }); // expected-note {{jump enters a statement expression}}
+  }
+  {
+(void)({ goto N; 1; });  // ok
+N: ;
+  }
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;
+  }
+  {
+(void)sizeof(({goto P;}), 0); // expected-error {{cannot jump from this goto statement to its label}}
+return 3;
+(void)({P:1;});  // expected-note {{jump enters a statement expression}}
+  }
+}
Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -50,9 +50,10 @@
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
-  // expected-note@+2 {{jump bypasses initialization of variable length array}}
-  // expected-note@+1 {{possible target of asm goto

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:571
   (`#61758 `_)
+- Correcly diagnose jumps into statement expressions.
+  (`#63682 `_)

Maybe remark this conforms with gcc behavior.



Comment at: clang/lib/Sema/JumpDiagnostics.cpp:489
+Scopes.push_back(GotoScope(ParentScope,
+   diag::note_enters_statement_expression, 0,
+   SE->getBeginLoc()));

I guess we don't have a variable that stands or no-diagnostic.



Comment at: clang/lib/Sema/SemaExpr.cpp:16468
   PushExpressionEvaluationContext(ExprEvalContexts.back().Context);
+  setFunctionHasBranchProtectedScope();
 }

I see why we need this but I am not sure how someone would know that need to do 
this in addition to the change in `BuildScopeInformation(...)`



Comment at: clang/test/Sema/scope-check.c:251
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;

I am sorry for this example:

```
int main() {
   sizeof(({goto label;}), 0);
   return 3;
  ({label:1;});  
}
```

see godbolt: https://godbolt.org/z/aeb6TbsoG

Note gcc's behavior Vs clangs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154696/new/

https://reviews.llvm.org/D154696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin created this revision.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Such jumps are not allowed by GCC and allowing them
can lead to situations where we jumps into unevaluated
statements.

Fixes #63682


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154696

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/scope-check.c
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaObjC/scope-check.m

Index: clang/test/SemaObjC/scope-check.m
===
--- clang/test/SemaObjC/scope-check.m
+++ clang/test/SemaObjC/scope-check.m
@@ -36,21 +36,20 @@
   } @finally {
 goto L7; // expected-error{{cannot jump}}
   }
-  
+
   goto L8;  // expected-error{{cannot jump}}
-  @try { 
+  @try {
   } @catch (A *c) {
   } @catch (B *c) {
   } @catch (C *c) { // expected-note {{jump bypasses initialization of @catch block}}
   L8: ;
   }
-  
+
   // rdar://6810106
   id X;
   goto L9;// expected-error{{cannot jump}}
-  goto L10;   // ok
-  @synchronized// expected-note {{jump bypasses initialization of @synchronized block}}
-  ( ({ L10: ; X; })) {
+  @synchronized (X)  // expected-note {{jump bypasses initialization of @synchronized block}}
+  {
   L9:
 ;
   }
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -831,8 +831,9 @@
 case 0:
   return 0;
 
-  ({
-case 1: // expected-note {{not supported}}
+  ({  // expected-note {{jump enters a statement expression}}
+case 1:// expected-error {{cannot jump from switch statement to this case label}} \
+   // expected-note  {{not supported}}
   return 1;
   });
 }
Index: clang/test/Sema/scope-check.c
===
--- clang/test/Sema/scope-check.c
+++ clang/test/Sema/scope-check.c
@@ -65,7 +65,8 @@
 
   // Statement expressions.
   goto L3;   // expected-error {{cannot jump from this goto statement to its label}}
-  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  int Y = ({  int a[x];   // expected-note {{jump bypasses initialization of variable length array}} \
+  // expected-note {{jump enters a statement expression}}
L3: 4; });
   
   goto L4; // expected-error {{cannot jump from this goto statement to its label}}
@@ -107,25 +108,25 @@
4; })];
   L10:; // bad
   }
-  
+
   {
 // FIXME: Crashes goto checker.
 //goto L11;// ok
 //int A[({   L11: 4; })];
   }
-  
+
   {
 goto L12;
-
+
 int y = 4;   // fixme-warn: skips initializer.
   L12:
 ;
   }
-  
+
   // Statement expressions 2.
   goto L1; // expected-error {{cannot jump from this goto statement to its label}}
-  return x == ({
- int a[x];   // expected-note {{jump bypasses initialization of variable length array}}  
+  return x == ({ // expected-note {{jump enters a statement expression}}
+ int a[x];   // expected-note {{jump bypasses initialization of variable length array}}
L1:
  42; });
 }
@@ -232,3 +233,22 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+void GH63682() {
+  {
+goto L; // expected-error {{cannot jump from this goto statement to its label}}
+(void)sizeof (int){({ L:; 1; })}; // expected-note {{jump enters a statement expression}}
+  }
+  {
+goto M; // expected-error {{cannot jump from this goto statement to its label}}
+(void)({ M:; 1; }); // expected-note {{jump enters a statement expression}}
+  }
+  {
+(void)({ goto N; 1; });  // ok
+N: ;
+  }
+  {
+(void)sizeof (int){({ goto O; 1; })}; // ok (not evaluated)
+O: ;
+  }
+}
Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -50,9 +50,10 @@
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
-  // expected-note@+2 {{jump bypasses initialization of variable length array}}
-  // expected-note@+1 {{possible target of asm goto statement}}
-  return ({int a[n];label_true: 2;});
+  //