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

[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(&exec);
        |                 ^
  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(&exec) {
        |         ^
  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(&exec);
          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(&exec);
               });) {
                  drm_exec_lock_obj(&exec);
                  do {
                          if (__builtin_expect(!!(drm_exec_is_contended(&exec)),
                                               0))
                                  goto *__drm_exec_retry_ptr;
                  } while (0);
          }
          drm_exec_fini(&exec);
  }



  $ 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