[PATCH] D156565: Diagnose use of VLAs in C++ by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Is it expected that this introduces a warning for C code, as the commit message 
and tests appear to only affect C++? A trivial example from the Linux kernel:

https://elixir.bootlin.com/linux/v6.5.8/source/tools/lib/bpf/btf_dump.c#L1678

  #include 
  #include 
  #include 
  
  void foo(char *orig_name, char **cached_name, size_t dup_cnt)
  {
  const size_t max_len = 256;
  char new_name[max_len];
  
  snprintf(new_name, max_len, "%s___%zu", orig_name, dup_cnt);
  *cached_name = strdup(new_name);
  }



  $ clang -std=gnu89 -Wall -fsyntax-only test.c
  test.c:8:16: warning: variable length arrays are a C99 feature 
[-Wvla-extension]
  8 | char new_name[max_len];
|   ^~~
  1 warning generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156565

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

[PATCH] D157435: [Sema] Do not emit -Wmissing-variable-declarations for register variables

[Nathan Chancellor 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 rGa22d385f9656: [Sema] Do not emit 
-Wmissing-variable-declarations for register variables (authored by 
nathanchance).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157435

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-missing-variable-declarations-register.c


Index: clang/test/Sema/warn-missing-variable-declarations-register.c
===
--- /dev/null
+++ clang/test/Sema/warn-missing-variable-declarations-register.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -Wmissing-variable-declarations 
-fsyntax-only -verify %s
+// expected-no-diagnostics
+
+register unsigned long current_stack_pointer asm("rsp");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14144,6 +14144,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  var->getStorageClass() != SC_Register &&
   !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -123,6 +123,8 @@
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a 
constructor
   of a base class is not called in the constructor of its derived class.
+- Clang no longer emits ``-Wmissing-variable-declarations`` for variables 
declared
+  with the ``register`` storage class.
 
 Bug Fixes in This Version
 -


Index: clang/test/Sema/warn-missing-variable-declarations-register.c
===
--- /dev/null
+++ clang/test/Sema/warn-missing-variable-declarations-register.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -Wmissing-variable-declarations -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+register unsigned long current_stack_pointer asm("rsp");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14144,6 +14144,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  var->getStorageClass() != SC_Register &&
   !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -123,6 +123,8 @@
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a constructor
   of a base class is not called in the constructor of its derived class.
+- Clang no longer emits ``-Wmissing-variable-declarations`` for variables declared
+  with the ``register`` storage class.
 
 Bug Fixes in This Version
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157435: [Sema] Do not emit -Wmissing-variable-declarations for register variables

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 548337.
nathanchance added a comment.

- Update formatting in release notes
- Add GCC bug link to commit message
- Remove unnecessary REQUIRES: in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157435

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-missing-variable-declarations-register.c


Index: clang/test/Sema/warn-missing-variable-declarations-register.c
===
--- /dev/null
+++ clang/test/Sema/warn-missing-variable-declarations-register.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -Wmissing-variable-declarations 
-fsyntax-only -verify %s
+// expected-no-diagnostics
+
+register unsigned long current_stack_pointer asm("rsp");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14144,6 +14144,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  var->getStorageClass() != SC_Register &&
   !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -123,6 +123,8 @@
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a 
constructor
   of a base class is not called in the constructor of its derived class.
+- Clang no longer emits ``-Wmissing-variable-declarations`` for variables 
declared
+  with the ``register`` storage class.
 
 Bug Fixes in This Version
 -


Index: clang/test/Sema/warn-missing-variable-declarations-register.c
===
--- /dev/null
+++ clang/test/Sema/warn-missing-variable-declarations-register.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -Wmissing-variable-declarations -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+register unsigned long current_stack_pointer asm("rsp");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14144,6 +14144,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  var->getStorageClass() != SC_Register &&
   !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -123,6 +123,8 @@
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a constructor
   of a base class is not called in the constructor of its derived class.
+- Clang no longer emits ``-Wmissing-variable-declarations`` for variables declared
+  with the ``register`` storage class.
 
 Bug Fixes in This Version
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157435: [Sema] Do not emit -Wmissing-variable-declarations for register variables

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: aaron.ballman, nickdesaulniers.
Herald added a subscriber: pengfei.
Herald added a project: All.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When building the Linux kernel with -Wmissing-variable-declarations,
there are several instances of warnings around variables declared with
register storage:

  arch/x86/include/asm/asm.h:208:24: warning: no previous extern declaration 
for non-static variable 'current_stack_pointer' 
[-Wmissing-variable-declarations]
  register unsigned long current_stack_pointer asm(_ASM_SP);
 ^
  arch/x86/include/asm/asm.h:208:10: note: declare 'static' if the variable is 
not intended to be used outside of this translation unit
  register unsigned long current_stack_pointer asm(_ASM_SP);
   ^
  1 warning generated.

The suggestion is invalid, as the variable cannot have both static and
register storage. Do not emit -Wmissing-variable-declarations for
register variables.

Closes: https://github.com/llvm/llvm-project/issues/64509
Link: https://lore.kernel.org/202308081050.szew4cq5-...@intel.com/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157435

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-missing-variable-declarations-register.c


Index: clang/test/Sema/warn-missing-variable-declarations-register.c
===
--- /dev/null
+++ clang/test/Sema/warn-missing-variable-declarations-register.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -Wmissing-variable-declarations 
-fsyntax-only -verify %s
+// REQUIRES: x86-registered-target
+// expected-no-diagnostics
+
+register unsigned long current_stack_pointer asm("rsp");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14144,6 +14144,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  var->getStorageClass() != SC_Register &&
   !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -123,6 +123,8 @@
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a 
constructor
   of a base class is not called in the constructor of its derived class.
+- Clang no longer emits `-Wmissing-variable-declarations` for variables 
declared
+  with the `register` storage class.
 
 Bug Fixes in This Version
 -


Index: clang/test/Sema/warn-missing-variable-declarations-register.c
===
--- /dev/null
+++ clang/test/Sema/warn-missing-variable-declarations-register.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -Wmissing-variable-declarations -fsyntax-only -verify %s
+// REQUIRES: x86-registered-target
+// expected-no-diagnostics
+
+register unsigned long current_stack_pointer asm("rsp");
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14144,6 +14144,7 @@
   var->getDeclContext()->getRedeclContext()->isFileContext() &&
   var->isExternallyVisible() && var->hasLinkage() &&
   !var->isInline() && !var->getDescribedVarTemplate() &&
+  var->getStorageClass() != SC_Register &&
   !isa(var) &&
   !isTemplateInstantiation(var->getTemplateSpecializationKind()) &&
   !getDiagnostics().isIgnored(diag::warn_missing_variable_declarations,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -123,6 +123,8 @@
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a constructor
   of a base class is not called in the constructor of its derived class.
+- Clang no longer emits `-Wmissing-variable-declarations` for variables declared
+  with the `register` storage class.
 
 Bug Fixes in This Version
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D76096#4564855 , @DavidSpickett 
wrote:

> One of the kernel tests now fails to build for AArch64 and Arm:
>
>   00:00:48 ++ make 
> CC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/bin/aarch64-cc 
> LD=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/ld.lld 
> SUBLEVEL=0 EXTRAVERSION=-bisect ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- 
> HOSTCC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/clang 
> -j32 -s -k
>   00:08:26 lib/test_scanf.c:661:2: error: implicit conversion from 'int' to 
> 'unsigned char' changes value from -168 to 88 [-Werror,-Wconstant-conversion]
>   00:08:26   661 | test_number_prefix(unsigned char,   "0xA7", 
> "%2hhx%hhx", 0, 0xa7, 2, check_uchar);
>   00:08:26   | 
> ^
>   00:08:26 lib/test_scanf.c:609:29: note: expanded from macro 
> 'test_number_prefix'
>   00:08:26   609 | T result[2] = {~expect[0], ~expect[1]};
>  \
>   00:08:26   |   ~^~
>   00:08:27 1 error generated.
>   00:08:27 make[3]: *** [scripts/Makefile.build:243: lib/test_scanf.o] Error 1
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/test_scanf.c#n661
>
> Seems like the test should be fixed, though I wonder why this same error 
> isn't effecting GCC builds.

Thanks for the report! I sent a patch for this already, just waiting for it to 
be picked up: 
https://lore.kernel.org/20230807-test_scanf-wconstant-conversion-v2-1-839ca3908...@kernel.org/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096

___
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

[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

[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

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

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Thanks, I can confirm that the `__attribute__((__cleanup__(...)))` errors that 
we observed with Peter's work-in-progress guards series are resolved and that 
all the errors I observed when building the kernel after 
https://reviews.llvm.org/D154696 was merged are no longer present when that 
change is re-applied on top of this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155342

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

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

For the record, I have sent 
https://lore.kernel.org/20230718-nsecs_to_jiffies_timeout-constant-logical-operand-v1-0-36ed8fc8f...@kernel.org/
 to address those warnings for the kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142609

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

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D142609#4511579 , @nickdesaulniers 
wrote:

> In D142609#4507696 , @nathanchance 
> wrote:
>
>> but I see a new one along a similar line as those:
>>
>> https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/v3d/v3d_drv.h#L343
>
> I only see 4 instances of `NSEC_PER_SEC % HZ` in mainline. 2 already use the 
> C preprocessor (inconsistently), and 2 don't. I think the 2 that don't could 
> be converted to use the preprocessor, then that issue goes away. So I think 
> we can clean that up on the kernel side.

Is this diff what you had in mind?

  diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c 
b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
  index 4a33ad2d122b..d7dd93da2511 100644
  --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
  +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
  @@ -186,9 +186,10 @@ i915_gem_object_wait(struct drm_i915_gem_object *obj,
   static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
   {
  /* nsecs_to_jiffies64() does not guard against overflow */
  -   if (NSEC_PER_SEC % HZ &&
  -   div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
  +#if (NSEC_PER_SEC % HZ) != 0
  +   if (div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
  return MAX_JIFFY_OFFSET;
  +#endif
  
  return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
   }
  diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
  index b74b1351bfc8..af1470ee477d 100644
  --- a/drivers/gpu/drm/v3d/v3d_drv.h
  +++ b/drivers/gpu/drm/v3d/v3d_drv.h
  @@ -340,9 +340,10 @@ struct v3d_submit_ext {
   static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
   {
  /* nsecs_to_jiffies64() does not guard against overflow */
  -   if (NSEC_PER_SEC % HZ &&
  -   div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
  +#if (NSEC_PER_SEC % HZ) != 0
  +   if (div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
  return MAX_JIFFY_OFFSET;
  +#endif
  
  return min_t(u64, MAX_JIFFY_OFFSET, nsecs_to_jiffies64(n) + 1);
   }

Not sure how the kernel maintainers will like that, they generally do not like 
to use preprocessing for hiding warnings. Alternatively, we could just turn the 
checks into booleans, which is what I tested earlier.

  diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c 
b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
  index 4a33ad2d122b..d4b918fb11ce 100644
  --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
  +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
  @@ -186,7 +186,7 @@ i915_gem_object_wait(struct drm_i915_gem_object *obj,
   static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
   {
  /* nsecs_to_jiffies64() does not guard against overflow */
  -   if (NSEC_PER_SEC % HZ &&
  +   if ((NSEC_PER_SEC % HZ) != 0 &&
  div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
  return MAX_JIFFY_OFFSET;
  diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
  index b74b1351bfc8..7f664a4b2a75 100644
  --- a/drivers/gpu/drm/v3d/v3d_drv.h
  +++ b/drivers/gpu/drm/v3d/v3d_drv.h
  @@ -340,7 +340,7 @@ struct v3d_submit_ext {
   static inline unsigned long nsecs_to_jiffies_timeout(const u64 n)
   {
  /* nsecs_to_jiffies64() does not guard against overflow */
  -   if (NSEC_PER_SEC % HZ &&
  +   if ((NSEC_PER_SEC % HZ) != 0 &&
  div_u64(n, NSEC_PER_SEC) >= MAX_JIFFY_OFFSET / HZ)
  return MAX_JIFFY_OFFSET;

I don't really have a strong preference either way but warnings that show up 
with certain configuration values and not others are annoying for things like 
randconfig testing, although I suppose that is always the nature of certain 
warnings...

> @nathanchance were there many other instances of warnings triggered by this 
> patch beyond that?

No, those were the only two instances that I saw across my whole build matrix 
with the current version of the patch.


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

https://reviews.llvm.org/D142609

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

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I took the most recent version for a spin against the Linux kernel. The couple 
of issues that a previous revision of the warning flagged 
(https://github.com/ClangBuiltLinux/linux/issues/1806, 
https://github.com/ClangBuiltLinux/linux/issues/1807) are no longer visible 
(that seems intentional because both of those came from macro expressions) but 
I see a new one along a similar line as those:

https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/v3d/v3d_drv.h#L343

  In file included from drivers/gpu/drm/v3d/v3d_bo.c:25:
  drivers/gpu/drm/v3d/v3d_drv.h:343:24: warning: use of logical '&&' with 
constant operand [-Wconstant-logical-operand]
343 | if (NSEC_PER_SEC % HZ &&
| ~ ^
  drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: use '&' for a bitwise operation
343 | if (NSEC_PER_SEC % HZ &&
|   ^~
|   &
  drivers/gpu/drm/v3d/v3d_drv.h:343:24: note: remove constant to silence this 
warning
  1 warning generated.

Another minimized example showing how the warning can trigger with different 
configurations:

  $ cat x.c
  #define A 1000
  #define B 300
  #define C 250
  #define D 100
  
  int bar(void);
  int baz(void);
  
  int foo(void)
  {
  if (A % B && bar()) // 1000 % 300 = 100, warning
  return -3;
  
  if (A % C && bar()) // 1000 % 250 = 0, no warning
  return -2;
  
  if (A % D && bar()) // 1000 % 100 = 0, no warning
  return -1;
  
  return baz();
  }
  
  $ clang -Wall -fsyntax-only x.c
  x.c:11:12: warning: use of logical '&&' with constant operand 
[-Wconstant-logical-operand]
 11 | if (A % B && bar())
| ~ ^
  x.c:11:12: note: use '&' for a bitwise operation
 11 | if (A % B && bar())
|   ^~
|   &
  x.c:11:12: note: remove constant to silence this warning
  1 warning generated.

I am sure we can send a patch making that a boolean expression (although it is 
duplicated in a few places it seems so multiple patches will be needed) but I 
figured I would report it just in case there was something that should be 
changed with the warning, since I see there was already some discussion around 
not warning for macros and this seems along a similar line.


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

https://reviews.llvm.org/D142609

___
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

[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

[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] D153989: [compiler-rt] Move crt into builtins

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This change appears to break `-DCOMPILER_RT_BUILD_CRT=OFF`:

  $ cmake -B build -S llvm -DCMAKE_BUILD_TYPE=Release 
-DCOMPILER_RT_BUILD_CRT=OFF -DLLVM_ENABLE_PROJECTS='clang;compiler-rt;lld'
  ...
  CMake Error at cmake/modules/AddLLVM.cmake:1935 (add_dependencies):
The dependency target "crt" of target "check-all" does not exist.
  Call Stack (most recent call first):
cmake/modules/AddLLVM.cmake:1975 (add_lit_target)
CMakeLists.txt:1211 (umbrella_lit_testsuite_end)
  
  
  CMake Error at cmake/modules/AddLLVM.cmake:1935 (add_dependencies):
The dependency target "crt" of target "check-compiler-rt" does not exist.
  Call Stack (most recent call first):
cmake/modules/AddLLVM.cmake:1975 (add_lit_target)
/home/nathan/cbl/src/llvm-project/compiler-rt/test/CMakeLists.txt:114 
(umbrella_lit_testsuite_end)
  
  
  CMake Error at cmake/modules/AddLLVM.cmake:1935 (add_dependencies):
The dependency target "crt" of target "check-builtins" does not exist.
  Call Stack (most recent call first):
cmake/modules/AddLLVM.cmake:2001 (add_lit_target)

/home/nathan/cbl/src/llvm-project/compiler-rt/test/builtins/CMakeLists.txt:112 
(add_lit_testsuite)
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153989

___
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

[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((&kn->dir.children)
  ->rb_node) ==
   sizeof(char) ||
   sizeof((&kn->dir.children)
  ->rb_node) ==
   sizeof(short) ||
   sizeof((&kn->dir.children)
  ->rb_node) ==
 

[PATCH] D152186: [Clang] Add release note for 877210faa447

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Thanks, I landed it with those nits addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152186

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

[PATCH] D152186: [Clang] Add release note for 877210faa447

[Nathan Chancellor 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 rGdeecf89a1361: [Clang] Add release note for 877210faa447 
(authored by nathanchance).

Changed prior to commit:
  https://reviews.llvm.org/D152186?vs=528534&id=528539#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152186

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -330,6 +330,8 @@
   ``-fno-diagnostics-show-line-numbers``. At the same time, the maximum
   number of code lines it prints has been increased from 1 to 16. This
   can be controlled using ``-fcaret-diagnostics-max-lines=``.
+- Clang no longer emits ``-Wunused-variable`` warnings for variables declared
+  with ``__attribute__((cleanup(...)))`` to match GCC's behavior.
 
 Bug Fixes in This Version
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -330,6 +330,8 @@
   ``-fno-diagnostics-show-line-numbers``. At the same time, the maximum
   number of code lines it prints has been increased from 1 to 16. This
   can be controlled using ``-fcaret-diagnostics-max-lines=``.
+- Clang no longer emits ``-Wunused-variable`` warnings for variables declared
+  with ``__attribute__((cleanup(...)))`` to match GCC's behavior.
 
 Bug Fixes in This Version
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152186: [Clang] Add release note for 877210faa447

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: erichkeane, nickdesaulniers.
Herald added a project: All.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152186

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -330,6 +330,8 @@
   ``-fno-diagnostics-show-line-numbers``. At the same time, the maximum
   number of code lines it prints has been increased from 1 to 16. This
   can be controlled using ``-fcaret-diagnostics-max-lines=``.
+- Clang no longer emits ``-Wunused-variable`` for variables declared with
+  ``__attribute__((cleanup(...)))`` to match GCC.
 
 Bug Fixes in This Version
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -330,6 +330,8 @@
   ``-fno-diagnostics-show-line-numbers``. At the same time, the maximum
   number of code lines it prints has been increased from 1 to 16. This
   can be controlled using ``-fcaret-diagnostics-max-lines=``.
+- Clang no longer emits ``-Wunused-variable`` for variables declared with
+  ``__attribute__((cleanup(...)))`` to match GCC.
 
 Bug Fixes in This Version
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152180: [Sema] Do not emit -Wunused-variable for variables declared with cleanup attribute

[Nathan Chancellor 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 rG877210faa447: [Sema] Do not emit -Wunused-variable for 
variables declared with cleanup… (authored by nathanchance).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152180

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-variables.c


Index: clang/test/Sema/warn-unused-variables.c
===
--- clang/test/Sema/warn-unused-variables.c
+++ clang/test/Sema/warn-unused-variables.c
@@ -30,3 +30,8 @@
   (void)(^() { int X = 4; }); // expected-warning{{unused}}
   (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of 
undeclared identifier 'Y'}}
 }
+
+void c1(int *);
+void f4(void) {
+  int __attribute__((cleanup(c1))) X1 = 4;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1992,7 +1992,8 @@
 return false;
   }
 
-  if (D->hasAttr() || D->hasAttr())
+  if (D->hasAttr() || D->hasAttr() ||
+  D->hasAttr())
 return false;
 
   if (isa(D))


Index: clang/test/Sema/warn-unused-variables.c
===
--- clang/test/Sema/warn-unused-variables.c
+++ clang/test/Sema/warn-unused-variables.c
@@ -30,3 +30,8 @@
   (void)(^() { int X = 4; }); // expected-warning{{unused}}
   (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of undeclared identifier 'Y'}}
 }
+
+void c1(int *);
+void f4(void) {
+  int __attribute__((cleanup(c1))) X1 = 4;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1992,7 +1992,8 @@
 return false;
   }
 
-  if (D->hasAttr() || D->hasAttr())
+  if (D->hasAttr() || D->hasAttr() ||
+  D->hasAttr())
 return false;
 
   if (isa(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152180: [Sema] Do not emit -Wunused-variable for variables declared with cleanup attribute

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 528510.
nathanchance added a comment.

Change `f4()`'s return type to `void` (Nick)


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

https://reviews.llvm.org/D152180

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-variables.c


Index: clang/test/Sema/warn-unused-variables.c
===
--- clang/test/Sema/warn-unused-variables.c
+++ clang/test/Sema/warn-unused-variables.c
@@ -30,3 +30,8 @@
   (void)(^() { int X = 4; }); // expected-warning{{unused}}
   (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of 
undeclared identifier 'Y'}}
 }
+
+void c1(int *);
+void f4(void) {
+  int __attribute__((cleanup(c1))) X1 = 4;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1992,7 +1992,8 @@
 return false;
   }
 
-  if (D->hasAttr() || D->hasAttr())
+  if (D->hasAttr() || D->hasAttr() ||
+  D->hasAttr())
 return false;
 
   if (isa(D))


Index: clang/test/Sema/warn-unused-variables.c
===
--- clang/test/Sema/warn-unused-variables.c
+++ clang/test/Sema/warn-unused-variables.c
@@ -30,3 +30,8 @@
   (void)(^() { int X = 4; }); // expected-warning{{unused}}
   (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of undeclared identifier 'Y'}}
 }
+
+void c1(int *);
+void f4(void) {
+  int __attribute__((cleanup(c1))) X1 = 4;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1992,7 +1992,8 @@
 return false;
   }
 
-  if (D->hasAttr() || D->hasAttr())
+  if (D->hasAttr() || D->hasAttr() ||
+  D->hasAttr())
 return false;
 
   if (isa(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152180: [Sema] Do not emit -Wunused-variable for variables declared with cleanup attribute

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: aaron.ballman, nickdesaulniers, erichkeane.
Herald added a project: All.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A variable declared with `__attribute__((cleanup))` cannot be unused, as
its address is passed to the clean up function. Do not emit
`-Wunused-variable` for variables declared with the cleanup attribute,
which matches GCC's behavior: https://godbolt.org/z/dz5YfTsan


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152180

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-variables.c


Index: clang/test/Sema/warn-unused-variables.c
===
--- clang/test/Sema/warn-unused-variables.c
+++ clang/test/Sema/warn-unused-variables.c
@@ -30,3 +30,8 @@
   (void)(^() { int X = 4; }); // expected-warning{{unused}}
   (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of 
undeclared identifier 'Y'}}
 }
+
+void c1(int *);
+int f4(void) {
+  int __attribute__((cleanup(c1))) X1 = 4;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1992,7 +1992,8 @@
 return false;
   }
 
-  if (D->hasAttr() || D->hasAttr())
+  if (D->hasAttr() || D->hasAttr() ||
+  D->hasAttr())
 return false;
 
   if (isa(D))


Index: clang/test/Sema/warn-unused-variables.c
===
--- clang/test/Sema/warn-unused-variables.c
+++ clang/test/Sema/warn-unused-variables.c
@@ -30,3 +30,8 @@
   (void)(^() { int X = 4; }); // expected-warning{{unused}}
   (void)(^() { int X = 4; return Y + X; }); // expected-error {{use of undeclared identifier 'Y'}}
 }
+
+void c1(int *);
+int f4(void) {
+  int __attribute__((cleanup(c1))) X1 = 4;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1992,7 +1992,8 @@
 return false;
   }
 
-  if (D->hasAttr() || D->hasAttr())
+  if (D->hasAttr() || D->hasAttr() ||
+  D->hasAttr())
 return false;
 
   if (isa(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151741: [Lex] Only warn on defining or undefining language-defined builtins

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This patch cleared up all the warnings I reported, thank you for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151741

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

For what it's worth, this change is quite noisy for the Linux kernel, as there 
are certain macros undefined in header files that are included in many other 
headers. Additionally, at least one new instance is in a UAPI header, so it 
cannot be changed to my knowledge (honestly, all of these appear to be 
intentional, so I am not sure any of them should be changed). Is there any way 
for us to opt out of this? It would be a shame if we had to outright disable 
`-Wbuiltin-macro-redefined`, it has caught issues before (the kernel is 
generally against opting out on a per-location basis but does have the 
infrastructure to do so, not sure if it will be available for all these 
locations though...).

`arch/arm/include/uapi/asm/types.h`: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/include/uapi/asm/types.h?h=v6.4-rc3

  In file included from scripts/mod/devicetable-offsets.c:3:
  In file included from include/linux/mod_devicetable.h:12:
  In file included from include/uapi/linux/mei.h:10:
  In file included from include/uapi/linux/mei_uuid.h:12:
  In file included from include/linux/types.h:6:
  In file included from include/uapi/linux/types.h:5:
  arch/arm/include/uapi/asm/types.h:27:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef __INT32_TYPE__
 ^
  arch/arm/include/uapi/asm/types.h:32:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef __UINT32_TYPE__
 ^
  arch/arm/include/uapi/asm/types.h:37:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef __UINTPTR_TYPE__
 ^
  3 errors generated.

`arch/arm64/include/asm/neon-intrinsics.h`: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/include/asm/neon-intrinsics.h?h=v6.4-rc3

  In file included from arch/arm64/lib/xor-neon.c:11:
  arch/arm64/include/asm/neon-intrinsics.h:18:8: error: undefining builtin 
macro [-Werror,-Wbuiltin-macro-redefined]
  #undef __INT64_TYPE__
 ^
  arch/arm64/include/asm/neon-intrinsics.h:23:8: error: undefining builtin 
macro [-Werror,-Wbuiltin-macro-redefined]
  #undef __UINT64_TYPE__
 ^
  2 errors generated.

`include/linux/drbd_genl_api.h` (this may be unnecessary now?): 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/drbd_genl_api.h?h=v6.4-rc3

  In file included from drivers/block/drbd/drbd_debugfs.c:11:
  In file included from drivers/block/drbd/drbd_int.h:35:
  include/linux/drbd_genl_api.h:47:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef linux
 ^
  1 error generated.

`arch/mips/kernel/vmlinux.lds.S` (there is little context available as to why 
this exists): 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/kernel/vmlinux.lds.S?h=v6.4-rc3

  In file included from :357:
  :6:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef mips
 ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

@amyk thank you so much!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144967

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

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Could this be merged into `main` and backported to `release/16.x`? If this 
makes 16.0.0 final, I think the kernel can avoid working around this issue 
altogether, as `-mtune` was only wired up to do something on PowerPC in during 
the 16 development cycle; in prior versions, it was ignored so any value was 
accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144967

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

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Thanks for the fix, it appears to work fine for me against the kernel's 
`powernv_defconfig` target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144967

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

[PATCH] D140270: MIPS: fix build from IR files, nan2008 and FpAbi

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I tested this against the kernel and left my feedback above (no issues) so this 
can be merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140270

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

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> I’ll have to filter the warnings to see if there are any other instances with 
> other operators that appear problematic.

I count a single warning that triggers for an arithmetic operator (which might 
be a bug):

  ../drivers/net/wireless/ralink/rt2x00/rt2800lib.c:10067:14: warning: implicit 
conversion from 'int' to 's8' (aka 'signed char') changes value from 128 to 
-128 [-Wconstant-conversion]
  cal_val -= 128;
 ^~~

and a few that trigger for `|=` (but I think they are all false positives?):

  ../drivers/usb/gadget/udc/bdc/bdc_core.c:62:17: warning: implicit conversion 
from 'unsigned long' to 'u32' (aka 'unsigned int') changes value from 
18446744071830503424 to 2415919104 [-Wconstant-conversion]
  temp |= BDC_COS|BDC_COP_STP;
  ~~~^~~~
  
  ../drivers/net/wireless/realtek/rtlwifi/rtl8192ee/led.c:62:13: warning: 
implicit conversion from 'unsigned long' to 'u32' (aka 'unsigned int') changes 
value from 18446744073707454463 to 4292870143 [-Wconstant-conversion]
  ledcfg |= ~BIT(21);
^~~~
  
  ../drivers/net/ethernet/intel/igb/igb_main.c:1834:42: warning: implicit 
conversion from 'unsigned long' to 'u32' (aka 'unsigned int') changes value 
from 18446744073709486592 to 4294902272 [-Wconstant-conversion]
  tqavctrl |= E1000_TQAVCTRL_DATATRANTIM |
  ~~~^
  
  ../drivers/iio/adc/imx7d_adc.c:243:10: warning: implicit conversion from 
'unsigned long' to 'u32' (aka 'unsigned int') changes value from 
18446744073172680704 to 3758096384 [-Wconstant-conversion]
  cfg1 |= (IMX7D_REG_ADC_CH_CFG1_CHANNEL_EN |
  ^~~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> In D139114#4053166 , @nathanchance 
> wrote:
>
>> For what it’s worth, this triggers a LOT in the Linux kernel for the pattern 
>> that @MaskRay pointed out:
>
> Out of curiosity, has it found any true positives that you can tell?

I have not been able to look too closely since I am on mobile until Wednesday 
but of the instances I examined, they all appear to be false positives (or 
maybe true positives that we really just don’t care about) .

The Linux kernel has a macro `BIT(x)`, which is just `1UL << x`, so any 
negation is going to be a really large number but expected to truncate when 
assigning to a narrower width variable because the bit being cleared might only 
be in the first few positions. That is at least the case with the really noisy 
instance of the warning that I mentioned above

  #define FWNODE_FLAG_LINKS_ADDED   BIT(0)
  #define FWNODE_FLAG_NOT_DEVICEBIT(1)
  #define FWNODE_FLAG_INITIALIZED   BIT(2)
  #define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD  BIT(3)
  #define FWNODE_FLAG_BEST_EFFORT   BIT(4)
  
  struct fwnode_handle {
struct fwnode_handle *secondary;
const struct fwnode_operations *ops;
struct device *dev;
struct list_head suppliers;
struct list_head consumers;
u8 flags;
  };
  …
  if (initialized)
fwnode->flags |= FWNODE_FLAG_INITIALIZED;
  else
fwnode->flags &= ~FWNODE_FLAG_INITIALIZED;



>> It would be nice if this was split into a separate flag that could be 
>> disabled, as -Wconstant-conversion has not been very noisy up until this 
>> point.
>
> We can definitely split it into a separate flag. Alternatively, we could 
> investigate disabling the warning for that code pattern (or moving that bit 
> under its own flag).

Right, I would suspect that emitting this for `&=` will rarely show anything 
interesting. I’ll have to filter the warnings to see if there are any other 
instances with other operators that appear problematic. I’ll see how easy that 
is or if there is a simple way to omit those instances that can be added to the 
patch itself for testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

For what it’s worth, this triggers a LOT in the Linux kernel for the pattern 
that @MaskRay pointed out:

https://gist.github.com/nathanchance/a76c2ba0a22dda3251c4ec5ee9e3f285

  $ rg -c -- -Wconstant-conversion 
$TMP_FOLDER/4c37671a7c946ac246b52fa44a6a649b89d6310b-build.log
  28364

The breakdown shows that one site in a common header is responsible for 19,352 
of those instances, but there are a lot of one off instances, which will take a 
long time to fix:

https://gist.github.com/nathanchance/85338a1e0693590cc93dba1d3e0bb050

It would be nice if this was split into a separate flag that could be disabled, 
as -Wconstant-conversion has not been very noisy up until this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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

[PATCH] D140270: MIPS: fix build from IR files, nan2008 and FpAbi

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This does not show any issues with any of the MIPS configurations that I build 
in the Linux kernel, thanks for fixing the problems I reported!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140270

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

[PATCH] D138179: MIPS: fix build from IR files, nan2008 and FpAbi

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

For what it's worth, this breaks building the Linux kernel's `ARCH=mips 
32r1_defconfig` with clang and GNU as, with thousands of messages along the 
line of:

  /tmp/mmc_ops-d329c3.s: Assembler messages:
  /tmp/mmc_ops-d329c3.s:310: Error: .module is not permitted after generating 
code
  /tmp/mmc_ops-d329c3.s:473: Error: .module is not permitted after generating 
code
  /tmp/mmc_ops-d329c3.s:594: Error: .module is not permitted after generating 
code
  /tmp/mmc_ops-d329c3.s:687: Error: .module is not permitted after generating 
code
  /tmp/mmc_ops-d329c3.s:781: Error: .module is not permitted after generating 
code
  /tmp/mmc_ops-d329c3.s:946: Error: .module is not permitted after generating 
code
  /tmp/mmc_ops-d329c3.s:1023: Error: .module is not permitted after generating 
code
  ...

A simplified reproducer:

  void rcu_normal_store() {}
  void __attribute__ksysfs_init() {}



  $ clang --version | head -1
  ClangBuiltLinux clang version 16.0.0 (https://github.com/llvm/llvm-project 
9739bb81aed490bfcbcbbac6970da8fb7232fd34)
  
  $ clang --target=mips-linux-gnu -fno-integrated-as -c -o /dev/null ksysfs.i
  /tmp/ksysfs-9b0c50.s: Assembler messages:
  /tmp/ksysfs-9b0c50.s:44: Error: .module is not permitted after generating code
  clang-16: error: assembler command failed with exit code 1 (use -v to see 
invocation)



  $ clang --version | head -1
  ClangBuiltLinux clang version 16.0.0 (https://github.com/llvm/llvm-project 
92619956eb27ef08dd24045307593fc3d7f78db0)
  
  $ clang --target=mips-linux-gnu -fno-integrated-as -c -o /dev/null ksysfs.i


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138179

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

[PATCH] D136790: [Clang][Sema] Add -Wincompatible-function-pointer-types-strict

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8217
+  err_typecheck_convert_incompatible_function_pointer.Text>,
+  InGroup>, 
DefaultIgnore;
 def ext_typecheck_convert_discards_qualifiers : ExtWarn<

samitolvanen wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > samitolvanen wrote:
> > > > nathanchance wrote:
> > > > > nathanchance wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > samitolvanen wrote:
> > > > > > > > > nathanchance wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > samitolvanen wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > We don't typically add new off-by-default warnings 
> > > > > > > > > > > > > because we have evidence that users don't enable them 
> > > > > > > > > > > > > enough to be worth adding them. Is there a way we can 
> > > > > > > > > > > > > enable this warning by default for CFI compilation 
> > > > > > > > > > > > > units (or when the cfi sanitizer is enabled) so that 
> > > > > > > > > > > > > it's only off by default for non-CFI users? I don't 
> > > > > > > > > > > > > think we have any examples of doing this in the code 
> > > > > > > > > > > > > base, so I believe this would be breaking new ground 
> > > > > > > > > > > > > (and thus is worth thinking about more, perhaps it's 
> > > > > > > > > > > > > a bad idea for some reason).
> > > > > > > > > > > > > Is there a way we can enable this warning by default 
> > > > > > > > > > > > > for CFI compilation units (or when the cfi sanitizer 
> > > > > > > > > > > > > is enabled) so that it's only off by default for 
> > > > > > > > > > > > > non-CFI users?
> > > > > > > > > > > > 
> > > > > > > > > > > > I can look into it, but I'm not sure if there's a 
> > > > > > > > > > > > convenient way to do that. An alternative to this could 
> > > > > > > > > > > > be to enable the warning by default, but only actually 
> > > > > > > > > > > > perform the check if CFI is enabled. This way non-CFI 
> > > > > > > > > > > > users would never see the warning because this really 
> > > > > > > > > > > > isn't a concern for them, but CFI users would still get 
> > > > > > > > > > > > the warning without explicitly enabling it. Or do you 
> > > > > > > > > > > > think this behavior would be confusing?
> > > > > > > > > > > Heh, sorry for not being clear, that's actually somewhat 
> > > > > > > > > > > along the lines of how I envisioned you'd implement it 
> > > > > > > > > > > (we don't have a way in tablegen to tie `DefaultIgnore` 
> > > > > > > > > > > to some language options or a target).
> > > > > > > > > > > 
> > > > > > > > > > > I was thinking the diagnostic would be left as 
> > > > > > > > > > > `DefaultIgnore` in the .td file, but we'd take note in 
> > > > > > > > > > > the driver that CFI was enabled and pass 
> > > > > > > > > > > `-Wincompatible-function-pointer-types-strict` to the 
> > > > > > > > > > > `-cc1` invocation. If the user wrote 
> > > > > > > > > > > `-Wno-incompatible-function-pointer-types-strict` at the 
> > > > > > > > > > > driver level, that would come *after* the automatically 
> > > > > > > > > > > generated flag for cc1 and would still disable the 
> > > > > > > > > > > diagnostic in the cc1 invocation (because the last flag 
> > > > > > > > > > > wins). WDYT?
> > > > > > > > > > I do not think that is unreasonable behavior in the generic 
> > > > > > > > > > case but specifically for the Linux kernel, kCFI is 
> > > > > > > > > > disabled by default (`CONFIG_CFI_CLANG` has to be 
> > > > > > > > > > specifically enabled) but we want to see this warning 
> > > > > > > > > > regardless of the value of that configuration so that 
> > > > > > > > > > driver authors who test with clang and automated testers 
> > > > > > > > > > are more likely to find them. As a result, if the warning 
> > > > > > > > > > is not going to be default on, we would still need to 
> > > > > > > > > > specifically enable it in our CFLAGS. I suppose your 
> > > > > > > > > > suggestion would help out folks using CFI in production 
> > > > > > > > > > though so maybe it is still worth considering doing?
> > > > > > > > > > I was thinking the diagnostic would be left as 
> > > > > > > > > > `DefaultIgnore` in the .td file, but we'd take note in the 
> > > > > > > > > > driver that CFI was enabled and pass 
> > > > > > > > > > `-Wincompatible-function-pointer-types-strict` to the 
> > > > > > > > > > `-cc1` invocation.
> > > > > > > > > 
> > > > > > > > > Thanks for clarifying, that sounds reasonable. Of course, 
> > > > > > > > > enabling it by default with CFI would still mean that we'll 
> > > > > > > > > have to either explicitly disable this in the kernel (or fix 
> > > > > > > > > all the existing warnings) before submitting the Clang change.
> > > > > > > > > As a result, if the warning is not going to be default on, we 
> > > > > > >

[PATCH] D136790: [Clang][Sema] Add -Wincompatible-function-pointer-types-strict

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8217
+  err_typecheck_convert_incompatible_function_pointer.Text>,
+  InGroup>, 
DefaultIgnore;
 def ext_typecheck_convert_discards_qualifiers : ExtWarn<

aaron.ballman wrote:
> aaron.ballman wrote:
> > samitolvanen wrote:
> > > nathanchance wrote:
> > > > aaron.ballman wrote:
> > > > > samitolvanen wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > We don't typically add new off-by-default warnings because we 
> > > > > > > have evidence that users don't enable them enough to be worth 
> > > > > > > adding them. Is there a way we can enable this warning by default 
> > > > > > > for CFI compilation units (or when the cfi sanitizer is enabled) 
> > > > > > > so that it's only off by default for non-CFI users? I don't think 
> > > > > > > we have any examples of doing this in the code base, so I believe 
> > > > > > > this would be breaking new ground (and thus is worth thinking 
> > > > > > > about more, perhaps it's a bad idea for some reason).
> > > > > > > Is there a way we can enable this warning by default for CFI 
> > > > > > > compilation units (or when the cfi sanitizer is enabled) so that 
> > > > > > > it's only off by default for non-CFI users?
> > > > > > 
> > > > > > I can look into it, but I'm not sure if there's a convenient way to 
> > > > > > do that. An alternative to this could be to enable the warning by 
> > > > > > default, but only actually perform the check if CFI is enabled. 
> > > > > > This way non-CFI users would never see the warning because this 
> > > > > > really isn't a concern for them, but CFI users would still get the 
> > > > > > warning without explicitly enabling it. Or do you think this 
> > > > > > behavior would be confusing?
> > > > > Heh, sorry for not being clear, that's actually somewhat along the 
> > > > > lines of how I envisioned you'd implement it (we don't have a way in 
> > > > > tablegen to tie `DefaultIgnore` to some language options or a target).
> > > > > 
> > > > > I was thinking the diagnostic would be left as `DefaultIgnore` in the 
> > > > > .td file, but we'd take note in the driver that CFI was enabled and 
> > > > > pass `-Wincompatible-function-pointer-types-strict` to the `-cc1` 
> > > > > invocation. If the user wrote 
> > > > > `-Wno-incompatible-function-pointer-types-strict` at the driver 
> > > > > level, that would come *after* the automatically generated flag for 
> > > > > cc1 and would still disable the diagnostic in the cc1 invocation 
> > > > > (because the last flag wins). WDYT?
> > > > I do not think that is unreasonable behavior in the generic case but 
> > > > specifically for the Linux kernel, kCFI is disabled by default 
> > > > (`CONFIG_CFI_CLANG` has to be specifically enabled) but we want to see 
> > > > this warning regardless of the value of that configuration so that 
> > > > driver authors who test with clang and automated testers are more 
> > > > likely to find them. As a result, if the warning is not going to be 
> > > > default on, we would still need to specifically enable it in our 
> > > > CFLAGS. I suppose your suggestion would help out folks using CFI in 
> > > > production though so maybe it is still worth considering doing?
> > > > I was thinking the diagnostic would be left as `DefaultIgnore` in the 
> > > > .td file, but we'd take note in the driver that CFI was enabled and 
> > > > pass `-Wincompatible-function-pointer-types-strict` to the `-cc1` 
> > > > invocation.
> > > 
> > > Thanks for clarifying, that sounds reasonable. Of course, enabling it by 
> > > default with CFI would still mean that we'll have to either explicitly 
> > > disable this in the kernel (or fix all the existing warnings) before 
> > > submitting the Clang change.
> > > As a result, if the warning is not going to be default on, we would still 
> > > need to specifically enable it in our CFLAGS. I suppose your suggestion 
> > > would help out folks using CFI in production though so maybe it is still 
> > > worth considering doing?
> > 
> > It sounds like either approach will require you to specifically enable it 
> > in your CFLAGS, right? Either it's off-by-default for everyone, or it's 
> > only on-by-default for CFI enabled builds (which the Linux kernel disables 
> > by default).
> > Thanks for clarifying, that sounds reasonable. Of course, enabling it by 
> > default with CFI would still mean that we'll have to either explicitly 
> > disable this in the kernel (or fix all the existing warnings) before 
> > submitting the Clang change.
> 
> Er, is there a chicken and egg problem here? I was imagining that you could 
> land the changes in Clang and disable the flag in the Linux kernel around 
> roughly the same time (we can time when we land things to make life easier 
> for you) so it wouldn't be particularly disruptive.
> 
> If it would be disruptive, another approach would be to leave it off b

[PATCH] D136790: [Clang][Sema] Add -Wincompatible-function-pointer-types-strict

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8217
+  err_typecheck_convert_incompatible_function_pointer.Text>,
+  InGroup>, 
DefaultIgnore;
 def ext_typecheck_convert_discards_qualifiers : ExtWarn<

aaron.ballman wrote:
> samitolvanen wrote:
> > aaron.ballman wrote:
> > > We don't typically add new off-by-default warnings because we have 
> > > evidence that users don't enable them enough to be worth adding them. Is 
> > > there a way we can enable this warning by default for CFI compilation 
> > > units (or when the cfi sanitizer is enabled) so that it's only off by 
> > > default for non-CFI users? I don't think we have any examples of doing 
> > > this in the code base, so I believe this would be breaking new ground 
> > > (and thus is worth thinking about more, perhaps it's a bad idea for some 
> > > reason).
> > > Is there a way we can enable this warning by default for CFI compilation 
> > > units (or when the cfi sanitizer is enabled) so that it's only off by 
> > > default for non-CFI users?
> > 
> > I can look into it, but I'm not sure if there's a convenient way to do 
> > that. An alternative to this could be to enable the warning by default, but 
> > only actually perform the check if CFI is enabled. This way non-CFI users 
> > would never see the warning because this really isn't a concern for them, 
> > but CFI users would still get the warning without explicitly enabling it. 
> > Or do you think this behavior would be confusing?
> Heh, sorry for not being clear, that's actually somewhat along the lines of 
> how I envisioned you'd implement it (we don't have a way in tablegen to tie 
> `DefaultIgnore` to some language options or a target).
> 
> I was thinking the diagnostic would be left as `DefaultIgnore` in the .td 
> file, but we'd take note in the driver that CFI was enabled and pass 
> `-Wincompatible-function-pointer-types-strict` to the `-cc1` invocation. If 
> the user wrote `-Wno-incompatible-function-pointer-types-strict` at the 
> driver level, that would come *after* the automatically generated flag for 
> cc1 and would still disable the diagnostic in the cc1 invocation (because the 
> last flag wins). WDYT?
I do not think that is unreasonable behavior in the generic case but 
specifically for the Linux kernel, kCFI is disabled by default 
(`CONFIG_CFI_CLANG` has to be specifically enabled) but we want to see this 
warning regardless of the value of that configuration so that driver authors 
who test with clang and automated testers are more likely to find them. As a 
result, if the warning is not going to be default on, we would still need to 
specifically enable it in our CFLAGS. I suppose your suggestion would help out 
folks using CFI in production though so maybe it is still worth considering 
doing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136790

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

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D134671#3828197 , @craig.topper 
wrote:

> In D134671#3824644 , 
> @nickdesaulniers wrote:
>
>> I don't think it's an issue for us to work around downstream, but this did 
>> regress support for `-mabi=ms` used in UEFI related build scripts.
>> https://github.com/ClangBuiltLinux/linux/issues/1725
>> Noting it in case others find their way here via bisection. Thanks to 
>> @nathanchance for the report.
>
> Is there an expectation that we honor -mabi=ms and something that matches 
> gcc? The original bug report I was fixing was that we silently ignored it.

No, I don't think so. The kernel was checking `-mabi=ms` to check for the 
existence of `__attribute__((ms_abi))`, which is not as accurate of a check as 
it could have been, since the kernel has the ability to run small programs 
during configuration time, which could have just checked for 
`__attribute__((ms_abi))` directly. The kernel does not use `-mabi=ms` as part 
of its compiler flags anywhere, it only uses the attribute, so I don't think 
there is any value to trying to support `-mabi=ms` unless there is another use 
case for it. We are just going to clean up the `-mabi=ms` check and backport 
it: https://lore.kernel.org/20220929152010.835906-1-nat...@kernel.org/

> I appears clang 7 did warn for this. clang 8 stopped warning, maybe that's 
> when the Mips code was added. If it was, I don't think it was intentional.

Right, I would agree with that assessment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134671

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

[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D134831#3827790 , @samitolvanen 
wrote:

> In D134831#3827730 , 
> @nickdesaulniers wrote:
>
>> Please get the patch disabling this warning for the kernel in flight before 
>> landing this.
>
> Agreed. @nathanchance do you want to send the patch above to LKML?

I can send it on Monday, as I am offline today (or at least not at a computer 
lol). If you want to move on it quicker, feel free to draft a commit message 
and slap my Suggested-by on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134831

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

[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8686
+def warn_cast_function_type_strict : Warning,
+  InGroup, DefaultIgnore;
 def err_cast_pointer_to_non_pointer_int : Error<

kees wrote:
> aaron.ballman wrote:
> > samitolvanen wrote:
> > > nickdesaulniers wrote:
> > > > I don't think we need a new group for a warning that only contains one 
> > > > diagnostic kind?
> > > > I don't think we need a new group for a warning that only contains one 
> > > > diagnostic kind?
> > > 
> > > I might have misunderstood something, but we seem to have plenty of 
> > > groups with only one diagnostic kind. Is there a way to add a new warning 
> > > flag without adding a diagnostic group? Most users of 
> > > `-Wcast-function-type` wouldn't want to enable the stricter version, so I 
> > > would prefer to keep this separate.
> > > 
> > > I did notice that some warnings don't define the group in 
> > > DiagnosticGroups.td, but specify it directly here. For example, 
> > > `InGroup>`. I'm not sure if there are 
> > > any benefits in doing so.
> > Typically we only define a group in DiagnosticGroups.td when the group is 
> > going to be used by more than one diagnostic, otherwise we prefer using 
> > `InGroup>` to form one-off diagnostic groups.
> > 
> > However, in this case, I am wondering if we want to add 
> > `CastFunctionTypeStrict` to be a subgroup of `CastFunctionType` so that 
> > users who enable `-Wcast-function-type` get the stricter checking by 
> > default, but still have a way to disable the stricter checking if it's too 
> > noisy for them?
> > However, in this case, I am wondering if we want to add 
> > `CastFunctionTypeStrict` to be a subgroup of `CastFunctionType` so that 
> > users who enable `-Wcast-function-type` get the stricter checking by 
> > default, but still have a way to disable the stricter checking if it's too 
> > noisy for them?
> 
> I'd be for that. It'll be very noisy for the Linux kernel, but they are all 
> instances we need to fix.
> 
> cc @nathanchance
> > However, in this case, I am wondering if we want to add 
> > `CastFunctionTypeStrict` to be a subgroup of `CastFunctionType` so that 
> > users who enable `-Wcast-function-type` get the stricter checking by 
> > default, but still have a way to disable the stricter checking if it's too 
> > noisy for them?
> 
> I'd be for that. It'll be very noisy for the Linux kernel, but they are all 
> instances we need to fix.
> 
> cc @nathanchance

Right, it would be quite noisy but we have already built an escape hatch for 
this type of scenario. We can just do what we have done for other warnings and 
disable it for "normal" builds to avoid disrupting the configurations with 
`-Werror` enabled (especially since we are not the only ones testing with tip 
of tree LLVM) then turn it on for `W=1` so that the build bots will catch new 
instances of the warnings while we clean up the other ones. I think such a diff 
would just look like:

```
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 6ae482158bc4..52bd7df84fd6 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -64,6 +64,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
 KBUILD_CFLAGS += $(call cc-disable-warning, pointer-to-enum-cast)
 KBUILD_CFLAGS += -Wno-tautological-constant-out-of-range-compare
 KBUILD_CFLAGS += $(call cc-disable-warning, unaligned-access)
+KBUILD_CFLAGS += $(call cc-disable-warning, cast-function-type-strict)
 endif

 endif
```

then that can just be reverted when we have all the instances cleaned up. It 
would be nice to get a game plan together for tackling all these because they 
appear to be quite numerous.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134831

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

[PATCH] D134702: [Clang] Don't warn if deferencing void pointers in unevaluated context

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Thanks a lot for fixing this! I took it for a spin against the Linux kernel and 
all instances of `-Wvoid-ptr-dereference` disappeared :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134702

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Diff 463063 looks good against the Linux kernel with 
https://lore.kernel.org/20220925153151.2467884-1...@inclyc.cn/ applied. I see 
no additional errors/warnings and the 
`drivers/media/platform/nvidia/tegra-vde/v4l2.c` did disappear. Once that patch 
has been fully chased and accepted, this change should be able to be merged 
without any problems. Thank you again for taking a look at the kernel ahead of 
time, it is very much appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D133574#3815951 , @aaron.ballman 
wrote:

> LGTM, thank you! Please don't land until you have some indication from the 
> kernel folks that this won't be super disruptive for them. CC 
> @nickdesaulniers and @nathanchance for awareness.

Thank you a lot for the heads up! I see an in-flight change around this from 
YingChi on the mailing list:

https://lore.kernel.org/20220925153151.2467884-1...@inclyc.cn/

It would be good to have that change accepted into a maintainer's tree and on 
its way to mainline (Linus's tree) and by extension, the stable releases before 
merging this into main, as we and several other groups are actively testing all 
Linux branches with LLVM main to catch other regressions and this error would 
require patching of our CI, which we can definitely do but prefer to avoid 
because of the maintenance overhead (we try to carry zero patches at all times).

I am still running my set of builds against the kernel with this LLVM change 
and the aforementioned Linux change but I do see another error:

  drivers/media/platform/nvidia/tegra-vde/v4l2.c:816:49: error: '' cannot be 
defined in 'offsetof'
  ctx = kzalloc(offsetof(struct tegra_ctx, 
ctrls[ARRAY_SIZE(ctrl_cfgs)]),
 ^
  include/linux/kernel.h:55:59: note: expanded from macro 'ARRAY_SIZE'
  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + 
__must_be_array(arr))
^
  include/linux/compiler.h:240:28: note: expanded from macro '__must_be_array'
  #define __must_be_array(a)  BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
  ^
  include/linux/build_bug.h:16:44: note: expanded from macro 'BUILD_BUG_ON_ZERO'
  #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
 ^
  1 error generated.

I will report back with any additional problems found.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D134461: [Clang] Warn when trying to deferencing void pointers in C

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D134461#3815458 , @aaron.ballman 
wrote:

> What do folks think of that idea?

I think that all sounds reasonable to me (although I am far from an authority 
on these matters). As far as I understand it, the kernel cannot use `-pedantic` 
due to its liberal use of GNU C extensions so moving those specific constructs 
to a `-pedantic` version of the warning will be the same as just turning them 
off altogether, which obviously works fine for us. I am happy to take the 
suggested changes for a spin against the kernel once a patch is available to 
see if there are any other interesting places where this warning triggers to 
make sure it does not need further adjustment, since we openly welcome new 
diagnostics. Right now, it is pretty much impossible to sift through them all 
because of how often it fires.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134461

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

[PATCH] D134461: [Clang] Warn when trying to deferencing void pointers in C

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

This warning is quite noisy for the Linux kernel due to a couple of places 
where a `void *` is dereferenced as part of compile time checking. My original 
commentary is available on our GitHub issue tracker 
(https://github.com/ClangBuiltLinux/linux/issues/1720) but I will summarize it 
here:

The first major instance of this warning comes from the `__is_constexpr` macro, 
which is used a lot in common macros do fancy things at compile time (change 
that added this: 
https://git.kernel.org/linus/3c8ba0d61d04ced9f8d9ff93977995a9e4e96e91):

  /*
   * This returns a constant expression while determining if an argument is
   * a constant expression, most importantly without evaluating the argument.
   * Glory to Martin Uecker 
   */
  #define __is_constexpr(x) \
(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))

Sample warnings:

  In file included from scripts/mod/devicetable-offsets.c:3:
  In file included from ./include/linux/mod_devicetable.h:13:
  In file included from ./include/linux/uuid.h:12:
  In file included from ./include/linux/string.h:253:
  ./include/linux/fortify-string.h:159:10: warning: ISO C does not allow 
indirection on operand of type 'void *' [-Wvoid-ptr-dereference]
  q_len = strlen(q);
  ^
  ./include/linux/fortify-string.h:131:24: note: expanded from macro 'strlen'
  __builtin_choose_expr(__is_constexpr(__builtin_strlen(p)),  \
^~~
  ./include/linux/const.h:12:25: note: expanded from macro '__is_constexpr'
  (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
 ^~~~
  
  In file included from arch/x86/kernel/asm-offsets.c:9:
  In file included from ./include/linux/crypto.h:20:
  In file included from ./include/linux/slab.h:15:
  In file included from ./include/linux/gfp.h:7:
  In file included from ./include/linux/mmzone.h:8:
  In file included from ./include/linux/spinlock.h:55:
  In file included from ./include/linux/preempt.h:78:
  In file included from ./arch/x86/include/asm/preempt.h:7:
  In file included from ./include/linux/thread_info.h:60:
  In file included from ./arch/x86/include/asm/thread_info.h:53:
  In file included from ./arch/x86/include/asm/cpufeature.h:5:
  In file included from ./arch/x86/include/asm/processor.h:22:
  In file included from ./arch/x86/include/asm/msr.h:11:
  In file included from ./arch/x86/include/asm/cpumask.h:5:
  In file included from ./include/linux/cpumask.h:12:
  In file included from ./include/linux/bitmap.h:9:
  ./include/linux/find.h:40:17: warning: ISO C does not allow indirection on 
operand of type 'void *' [-Wvoid-ptr-dereference]
  val = *addr & GENMASK(size - 1, offset);
^
  ./include/linux/bits.h:38:3: note: expanded from macro 'GENMASK'
  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
   ^
  ./include/linux/bits.h:25:3: note: expanded from macro 'GENMASK_INPUT_CHECK'
  __is_constexpr((l) > (h)), (l) > (h), 0)))
  ^
  ./include/linux/const.h:12:25: note: expanded from macro '__is_constexpr'
  (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
 ^
  ./include/linux/build_bug.h:16:62: note: expanded from macro 
'BUILD_BUG_ON_ZERO'
  #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
   ^

The second major instance is the type checking in `container_of` when ptr is a 
`void *` (change that added this: 
https://git.kernel.org/linus/c7acec713d14c6ce8a20154f9dfda258d6bcad3b):

  /**
   * container_of - cast a member of a structure out to the containing structure
   * @ptr:  the pointer to the member.
   * @type: the type of the container struct this is embedded in.
   * @member:   the name of the member within the struct.
   *
   */
  #define container_of(ptr, type, member) ({\
void *__mptr = (void *)(ptr);   \
static_assert(__same_type(*(ptr), ((type *)0)->member) ||   \
  __same_type(*(ptr), void),\
  "pointer type mismatch in container_of()");   \
((type *)(__mptr - offsetof(type, member))); })

  #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))

Sample warning:

  In file included from sound/soc/sof/core.c:14:
  In file included from ./include/sound/sof.h:14:
  ./include/linux/pci.h:600:9: warning: ISO C does not allow indirection on 
operand of type 'void *' [-Wvoid-ptr-dereference]
  return container

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I can no longer do a two stage build on Fedora after this change.

  $ cmake \
  -B build/stage1 \
  -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_C_COMPILER=$(command -v clang) \
  -DCMAKE_CXX_COMPILER=$(command -v clang++) \
  -DLLVM_ENABLE_PROJECTS="clang;lld" \
  -DLLVM_ENABLE_TERMINFO=OFF \
  -DLLVM_TARGETS_TO_BUILD=host \
  -DLLVM_USE_LINKER=$(command -v ld.lld) \
  -G Ninja \
  -S llvm
  ...
  
  $ ninja -C build/stage1
  ...
  
  $ cmake \
  -B build/stage2 \
  -DCLANG_TABLEGEN=$PWD/build/stage1/bin/clang-tblgen \
  -DCMAKE_AR=$PWD/build/stage1/bin/llvm-ar \
  -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_C_COMPILER=$PWD/build/stage1/bin/clang \
  -DCMAKE_CXX_COMPILER=$PWD/build/stage1/bin/clang++ \
  -DCMAKE_RANLIB=$PWD/build/stage1/bin/llvm-ranlib \
  -DLLVM_ENABLE_PROJECTS="clang;lld" \
  -DLLVM_ENABLE_TERMINFO=OFF \
  -DLLVM_TABLEGEN=$PWD/build/stage1/bin/llvm-tblgen \
  -DLLVM_TARGETS_TO_BUILD=host \
  -DLLVM_USE_LINKER=$PWD/build/stage1/bin/ld.lld \
  -G Ninja \
  -S llvm
  ...
  -- Performing Test LLVM_LIBSTDCXX_MIN
  -- Performing Test LLVM_LIBSTDCXX_MIN - Failed
  CMake Error at cmake/modules/CheckCompilerVersion.cmake:88 (message):
libstdc++ version must be at least 7.1.
  Call Stack (most recent call first):
cmake/config-ix.cmake:15 (include)
CMakeLists.txt:810 (include)
  ...
  
  $ cat build/stage2/CMakeFiles/CMakeError.log
  Checking whether the ASM compiler is GNU using "--version" did not match 
"(GNU assembler)|(GCC)|(Free Software Foundation)":
  clang version 16.0.0 (https://github.com/llvm/llvm-project 
f7a33090a91015836497c75f173775392ab0304d)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: /home/nathan/cbl/src/llvm-project/build/stage1/bin
  Performing C++ SOURCE FILE Test LLVM_LIBSTDCXX_MIN failed with the following 
output:
  Change Dir: /home/nathan/cbl/src/llvm-project/build/stage2/CMakeFiles/CMakeTmp
  
  Run Build Command(s):/usr/bin/ninja-build cmTC_caa1d && [1/2] Building CXX 
object CMakeFiles/cmTC_caa1d.dir/src.cxx.o
  FAILED: CMakeFiles/cmTC_caa1d.dir/src.cxx.o
  /home/nathan/cbl/src/llvm-project/build/stage1/bin/clang++ 
-DLLVM_LIBSTDCXX_MIN  -std=c++0x -std=c++17 -MD -MT 
CMakeFiles/cmTC_caa1d.dir/src.cxx.o -MF CMakeFiles/cmTC_caa1d.dir/src.cxx.o.d 
-o CMakeFiles/cmTC_caa1d.dir/src.cxx.o -c 
/home/nathan/cbl/src/llvm-project/build/stage2/CMakeFiles/CMakeTmp/src.cxx
  In file included from 
/home/nathan/cbl/src/llvm-project/build/stage2/CMakeFiles/CMakeTmp/src.cxx:2:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/iosfwd:40:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/postypes.h:40:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/cwchar:44:
  /usr/include/wchar.h:35:10: fatal error: 'stddef.h' file not found
  #include 
   ^~
  1 error generated.
  ninja: build stopped: subcommand failed.
  
  
  Source file was:
  
  #include 
  #if defined(__GLIBCXX__)
  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < 7
  #error Unsupported libstdc++ version
  #endif
  #endif
  int main() { return 0; }
  
  $ fd stddef.h build/stage1
  build/stage1/lib/clang/16.0.0/include/stddef.h

The parent commit is fine. If there is any further information I can provide or 
patches I can test, please let me know!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130586

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

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This cleans up the objtool warnings I initially reported and does not introduce 
any other regressions in my Linux kernel build and QEMU boot tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129709

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

[PATCH] D124159: [SimplifyCFG] Thread branches on same condition in more cases (PR54980)

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I ended up reducing something down anyways. At the parent commit of 
dc969061c68e62328607d68215ed8b9ef4a1e4b1 
, there is 
no crash.

  _Bool fpsimd_context_busy;
  enum { false, true } arch_static_branch_jump() {
asm goto("" : : : : l_yes);
return false;
  l_yes:
return true;
  }
  _Bool __preempt_count_dec_and_test();
  void preempt_schedule_notrace();
  long __percpu_read_8();
  _Bool may_use_simd() {
return ({ ({ arch_static_branch_jump(); }); }) && ({
 typeof(fpsimd_context_busy) pscr_ret__ = ({
   typeof(fpsimd_context_busy) __retval = __percpu_read_8();
   if (__builtin_expect(__preempt_count_dec_and_test(), 0))
 preempt_schedule_notrace();
   __retval;
 });
 pscr_ret__;
   });
  }
  void aes_cipher_encrypt() {
if (may_use_simd())
  preempt_schedule_notrace();
  }



  $ clang --target=aarch64-linux-gnu -O2 -c -o /dev/null aes-ce-glue.i
  clang: 
/home/nathan/cbl/src/llvm-project/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:1136:
 llvm::BasicBlock *SplitBlockPredecessorsImpl(llvm::BasicBlock *, 
ArrayRef, const char *, llvm::DomTreeUpdater *, 
llvm::DominatorTree *, llvm::LoopInfo *, llvm::MemorySSAUpdater *, bool): 
Assertion `!isa(Preds[i]->getTerminator()) && "Cannot split an edge 
from a CallBrInst"' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: clang --target=aarch64-linux-gnu -O2 -c -o 
/dev/null aes-ce-glue.i
  1.   parser at end of file
  2.  Optimizer
   #0 0xd1ad36b8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/nathan/tmp/install/llvm/dc969061c68e62328607d68215ed8b9ef4a1e4b1/bin/clang-15+0x35336b8)
   #1 0xd1ad18d8 llvm::sys::RunSignalHandlers() 
(/home/nathan/tmp/install/llvm/dc969061c68e62328607d68215ed8b9ef4a1e4b1/bin/clang-15+0x35318d8)
   #2 0xd1a5e6e4 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
CrashRecoveryContext.cpp:0:0
   #3 0xd1a5e894 CrashRecoverySignalHandler(int) 
CrashRecoveryContext.cpp:0:0
   #4 0xa099e854 (linux-vdso.so.1+0x854)
   #5 0xa04b28a8 __pthread_kill_implementation 
(/lib64/libc.so.6+0x828a8)
   #6 0xa046ae40 gsignal (/lib64/libc.so.6+0x3ae40)
   #7 0xa04572f8 abort (/lib64/libc.so.6+0x272f8)
   #8 0xa0464538 __assert_fail_base (/lib64/libc.so.6+0x34538)
   #9 0xa04645a0 __assert_perror_fail (/lib64/libc.so.6+0x345a0)
  #10 0xd1ae30c0 SplitBlockPredecessorsImpl(llvm::BasicBlock*, 
llvm::ArrayRef, char const*, llvm::DomTreeUpdater*, 
llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, bool) 
BasicBlockUtils.cpp:0:0
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124159

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

[PATCH] D124159: [SimplifyCFG] Thread branches on same condition in more cases (PR54980)

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Should dc969061c68e62328607d68215ed8b9ef4a1e4b1 
 be 
reverted as well? I just bisected an assertion failure while building the Linux 
kernel for arm64 to that change:

  clang: 
/home/nathan/cbl/src/llvm-project/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:1136:
 llvm::BasicBlock *SplitBlockPredecessorsImpl(llvm::BasicBlock *, 
ArrayRef, const char *, llvm::DomTreeUpdater *, 
llvm::DominatorTree *, llvm::LoopInfo *, llvm::MemorySSAUpdater *, bool): 
Assertion `!isa(Preds[i]->getTerminator()) && "Cannot split an edge 
from a CallBrInst"' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: 
/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang -mlittle-endian 
-Qunused-arguments -fmacro-prefix-map=/home/nathan/cbl/src/linux/= -Wall 
-Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing 
-fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration 
-Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu11 
--target=aarch64-linux-gnu -fintegrated-as -Werror=unknown-warning-option 
-Werror=ignored-optimization-argument -mgeneral-regs-only -Wno-psabi 
-fno-asynchronous-unwind-tables -fno-unwind-tables 
-mbranch-protection=pac-ret+leaf+bti -fno-delete-null-pointer-checks 
-Wno-frame-address -Wno-address-of-packed-member -O2 -Wframe-larger-than=2048 
-fstack-protector-strong -Wimplicit-fallthrough -Wno-gnu 
-Wno-unused-but-set-variable -Wno-unused-const-variable -fno-omit-frame-pointer 
-fno-optimize-sibling-calls -ftrivial-auto-var-init=zero 
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang 
-fno-stack-clash-protection -Wdeclaration-after-statement -Wvla 
-Wno-pointer-sign -Wcast-function-type -fno-strict-overflow -fno-stack-check 
-Werror=date-time -Werror=incompatible-pointer-types -Wno-initializer-overrides 
-Wno-format -Wno-sign-compare -Wno-format-zero-length -Wno-pointer-to-enum-cast 
-Wno-tautological-constant-out-of-range-compare -Wno-unaligned-access 
-mstack-protector-guard=sysreg -mstack-protector-guard-reg=sp_el0 
-mstack-protector-guard-offset=1184 -Wa,-march=armv8.5-a -nostdinc 
-I/home/nathan/cbl/src/linux/arch/arm64/include 
-I./arch/arm64/include/generated -I/home/nathan/cbl/src/linux/include 
-I./include -I/home/nathan/cbl/src/linux/arch/arm64/include/uapi 
-I./arch/arm64/include/generated/uapi -I/home/nathan/cbl/src/linux/include/uapi 
-I./include/generated/uapi -include 
/home/nathan/cbl/src/linux/include/linux/compiler-version.h -include 
/home/nathan/cbl/src/linux/include/linux/kconfig.h -include 
/home/nathan/cbl/src/linux/include/linux/compiler_types.h -D__KERNEL__ 
-DKASAN_SHADOW_SCALE_SHIFT= -DCONFIG_CC_HAS_K_CONSTRAINT=1 
-DARM64_ASM_ARCH=\"armv8.5-a\" -DKASAN_SHADOW_SCALE_SHIFT= -I 
/home/nathan/cbl/src/linux/arch/arm64/crypto -I ./arch/arm64/crypto 
-DKBUILD_MODFILE=\"arch/arm64/crypto/aes-ce-cipher\" 
-DKBUILD_BASENAME=\"aes_ce_glue\" -DKBUILD_MODNAME=\"aes_ce_cipher\" 
-D__KBUILD_MODNAME=kmod_aes_ce_cipher -c 
-Wp,-MMD,arch/arm64/crypto/.aes-ce-glue.o.d -fcolor-diagnostics -o 
arch/arm64/crypto/aes-ce-glue.o 
/home/nathan/cbl/src/linux/arch/arm64/crypto/aes-ce-glue.c
  1.   parser at end of file
  2.  Optimizer
   #0 0x55aae2db8b83 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang+0x3e2fb83)
   #1 0x55aae2db6b1e llvm::sys::RunSignalHandlers() 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang+0x3e2db1e)
   #2 0x55aae2d40c13 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
CrashRecoveryContext.cpp:0:0
   #3 0x55aae2d40d8e CrashRecoverySignalHandler(int) 
CrashRecoveryContext.cpp:0:0
   #4 0x7fdd27ea18e0 (/usr/lib/libc.so.6+0x3e8e0)
   #5 0x7fdd27ef136c (/usr/lib/libc.so.6+0x8e36c)
   #6 0x7fdd27ea1838 gsignal (/usr/lib/libc.so.6+0x3e838)
   #7 0x7fdd27e8b535 abort (/usr/lib/libc.so.6+0x28535)
   #8 0x7fdd27e8b45c (/usr/lib/libc.so.6+0x2845c)
   #9 0x7fdd27e9a366 (/usr/lib/libc.so.6+0x37366)
  #10 0x55aae2dcb9b6 SplitBlockPredecessorsImpl(llvm::BasicBlock*, 
llvm::ArrayRef, char const*, llvm::DomTreeUpdater*, 
llvm::DominatorTree*, llvm::LoopInfo*, llvm::MemorySSAUpdater*, bool) 
BasicBlockUtils.cpp:0:0
  #11 0x55aae2dcba4c llvm::SplitBlockPredecessors(llvm::BasicBlock*, 
llvm::ArrayRef, char const*, llvm::DomTreeUpdater*, 
llvm::LoopInfo*, llvm::MemorySSAUpdater*, bool) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang+0x3e42a4c)
  #12 0x55aae2ef5b01 (anonymous 
namespace)::SimplifyCFGOpt::simplifyCondBranch(llvm::BranchInst*, 
llvm::IRBuilder&) 
SimplifyCFG.cpp:0:0
  #13 0x55aae2ee2e29 (anonymous 
namespace)::SimplifyCFGOpt::run(llvm::BasicBlock*) SimplifyCFG.cpp:0:0
  #14 0x55aae2edfe84 llvm::simplify

[PATCH] D125506: [PowerPC] Implement XL compat __fnabs and __fnabss builtins.

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

@amyk Thank you a lot for the fix! I'll pull it down and verify everything is 
fixed tonight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125506

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

[PATCH] D125506: [PowerPC] Implement XL compat __fnabs and __fnabss builtins.

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

I bisected a crash when compiling the Linux kernel to this change.

A reduced C reproducer:

  enum {
OP_CB_GETATTR = 3,
OP_CB_RECALL,
OP_CB_RECALL_ANY = 8,
OP_CB_RECALL_SLOT = 10,
OP_CB_NOTIFY_LOCK = 13,
OP_CB_NOTIFY_DEVICEID,
OP_CB_OFFLOAD
  } static callback_ops_0;
  int preprocess_nfs42_op_op_nr, preprocess_nfs42_op_op;
  void preprocess_nfs42_op() {
switch (preprocess_nfs42_op_op_nr)
case OP_CB_GETATTR:
case OP_CB_RECALL:
case OP_CB_RECALL_ANY:
case OP_CB_RECALL_SLOT:
case OP_CB_NOTIFY_DEVICEID:
case OP_CB_NOTIFY_LOCK:
  preprocess_nfs42_op_op = preprocess_nfs42_op_op_nr;
if (preprocess_nfs42_op_op_nr == OP_CB_OFFLOAD)
  preprocess_nfs42_op_op = callback_ops_0;
  }



  $ clang --version | head -1
  ClangBuiltLinux clang version 15.0.0 (https://github.com/llvm/llvm-project 
559b8fc17ef6f5a65ccf9a11fce5f91c0a011b00)
  
  $ clang --target=powerpc64le-linux-gnu -O2 -Wall -Wextra -c -o /dev/null 
callback_xdr.i



  $ clang --version | head -1
  ClangBuiltLinux clang version 15.0.0 (https://github.com/llvm/llvm-project 
c35ca3a1c78f693b749ad11742350b7fc6c5cd89)
  
  $ clang --target=powerpc64le-linux-gnu -O2 -Wall -Wextra -c -o /dev/null 
callback_xdr.i
  Impossible reg-to-reg copy
  UNREACHABLE executed at 
/home/nathan/cbl/src/llvm-project/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:1861!
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: clang --target=powerpc64le-linux-gnu -O2 -Wall 
-Wextra -c -o /dev/null callback_xdr.i
  1.   parser at end of file
  2.  Code generation
  3.  Running pass 'Function Pass Manager' on module 'callback_xdr.i'.
  4.  Running pass 'Post-RA pseudo instruction expansion pass' on function 
'@preprocess_nfs42_op'
   #0 0x55a3bcb4e583 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x3351583)
   #1 0x55a3bcb4c50e llvm::sys::RunSignalHandlers() 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x334f50e)
   #2 0x55a3bcad2923 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
CrashRecoveryContext.cpp:0:0
   #3 0x55a3bcad2a9e CrashRecoverySignalHandler(int) 
CrashRecoveryContext.cpp:0:0
   #4 0x7f4af3152b00 __restore_rt (/lib64/libc.so.6+0x3eb00)
   #5 0x7f4af31a2d0c __pthread_kill_implementation 
(/lib64/libc.so.6+0x8ed0c)
   #6 0x7f4af3152a56 gsignal (/lib64/libc.so.6+0x3ea56)
   #7 0x7f4af313c7f4 abort (/lib64/libc.so.6+0x287f4)
   #8 0x55a3bcad787f 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x32da87f)
   #9 0x55a3bb83b308 
llvm::PPCInstrInfo::copyPhysReg(llvm::MachineBasicBlock&, 
llvm::MachineInstrBundleIterator, llvm::DebugLoc 
const&, llvm::MCRegister, llvm::MCRegister, bool) const PPCInstrInfo.cpp:0:0
  #10 0x55a3bc1242bb (anonymous 
namespace)::ExpandPostRA::runOnMachineFunction(llvm::MachineFunction&) 
ExpandPostRAPseudos.cpp:0:0
  #11 0x55a3bbf62d4d 
llvm::MachineFunctionPass::runOnFunction(llvm::Function&) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x2765d4d)
  #12 0x55a3bc3fc107 llvm::FPPassManager::runOnFunction(llvm::Function&) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x2bff107)
  #13 0x55a3bc403b81 llvm::FPPassManager::runOnModule(llvm::Module&) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x2c06b81)
  #14 0x55a3bc3fcafc llvm::legacy::PassManagerImpl::run(llvm::Module&) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x2bffafc)
  #15 0x55a3bd2fd16d clang::EmitBackendOutput(clang::DiagnosticsEngine&, 
clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, 
clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, 
llvm::Module*, clang::BackendAction, std::unique_ptr>) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x3b0016d)
  #16 0x55a3bd6a27ee 
clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) 
CodeGenAction.cpp:0:0
  #17 0x55a3bdf46cb4 clang::ParseAST(clang::Sema&, bool, bool) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x4749cb4)
  #18 0x55a3bd5f0f50 clang::FrontendAction::Execute() 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x3df3f50)
  #19 0x55a3bd565d1f 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x3d68d1f)
  #20 0x55a3bd69be92 
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) 
(/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x3e9ee92)
  #21 0x55a3bb785028 cc1_main(llvm::ArrayRef, char const*, 
void*) (/home/nathan/tmp/build/llvm-bisect/stage1/bin/clang-15+0x1f88028)
  #22 0x55a3bb782f2f ExecuteCC1Tool(llvm::SmallVec

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

I see an instance of this warning in the Linux kernel due to the "Now, for 
unknown directives inside a skipped conditional block, we diagnose the unknown 
directive as a warning if it is sufficiently similar to a directive specific to 
preprocessor conditional blocks" part of this change:

  arch/x86/crypto/aesni-intel_asm.S:532:8: warning: invalid preprocessing 
directive, did you mean '#if'? [-Wunknown-directives]
  # in in order to perform
^
  arch/x86/crypto/aesni-intel_asm.S:547:8: warning: invalid preprocessing 
directive, did you mean '#if'? [-Wunknown-directives]
  # in in order to perform
^
  2 warnings generated.

This is a comment within an assembler file that will be preprocessed (this is a 
32-bit x86 build and the block is guarded by `#ifdef __x86_64__` on line 500):

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/crypto/aesni-intel_asm.S?h=v5.18-rc7#n532

Is there anything that can be done to improve this heuristic for this case? I 
can try to push a patch to change the comment style for this one instance but I 
always worry that a warning of this nature will appear in the future and result 
in the kernel disabling this warning entirely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D122189: [Clang][NeonEmitter] emit ret decl first for -Wdeclaration-after-statement

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This passes through all my `ARCH=arm` and `ARCH=arm64` build and boot tests on 
`next-20220321` with the instances of `-Wdeclaration-after-statement` resolved 
and no new instances of any other warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122189

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D110869#3302625 , @nickdesaulniers 
wrote:

> All built+boot. I know @nathanchance pointed out an issue with some already 
> complex code, but I'm of the opinion that fn should be attributed in kernel 
> sources.

Aside from that one diff I posted above adding 
`__attribute__((zero_call_used_regs("skip")))` to `_paravirt_ident_64()`, I 
have not noticed any issues with diff406596 on my two test machines; 
they boot right up and I am currently running them through my usual build 
tests. I will comment back if anything bad turns up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

It looks like `_paravirt_ident_64()` is the problematic function. This diff on 
top of v5.17-rc2 allows me to boot:

  diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
  index 4420499f7bb4..c1b68504136c 100644
  --- a/arch/x86/kernel/paravirt.c
  +++ b/arch/x86/kernel/paravirt.c
  @@ -96,7 +96,7 @@ static unsigned paravirt_patch_call(void *insn_buff, const 
void *target,
  
   #ifdef CONFIG_PARAVIRT_XXL
   /* identity function, which can be inlined */
  -u64 notrace _paravirt_ident_64(u64 x)
  +u64 notrace __attribute__((zero_call_used_regs("skip"))) 
_paravirt_ident_64(u64 x)
   {
  return x;
   }

Rather interesting function to have problems with as a result of this patch but 
it seems like this function is being used in a very specific way further down 
the file with the `__PV_IS_CALLEE_SAVE` macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D110869#3295012 , @void wrote:

> In D110869#3294696 , @nathanchance 
> wrote:
>
>> This diff allows me to boot on bare metal as of v5.17-rc2:
>>
>>   diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>>   index 6aef9ee28a39..8ee176dac669 100644
>>   --- a/arch/x86/kernel/Makefile
>>   +++ b/arch/x86/kernel/Makefile
>>   @@ -125,6 +125,7 @@ obj-$(CONFIG_DEBUG_NMI_SELFTEST) += nmi_selftest.o
>>   
>>obj-$(CONFIG_KVM_GUEST)+= kvm.o kvmclock.o
>>obj-$(CONFIG_PARAVIRT) += paravirt.o
>>   +CFLAGS_paravirt.o += -fzero-call-used-regs=skip
>>obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= paravirt-spinlocks.o
>>obj-$(CONFIG_PARAVIRT_CLOCK)   += pvclock.o
>>obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o
>>
>> I have uploaded the config used, the preprocessed file, and the "good" 
>> object (with the following diff) and the "bad" object (without the above 
>> diff) here: 
>> https://github.com/nathanchance/bug-files/tree/052a31e6d94c1b349cf6f312808794dace24/D110869
>>
>> If there is any more information I can give, please let me know!
>
> Thanks for the information!
>
> Could you test this patch for me? (Applied over the patch in this review.)
>
>   diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp 
> b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
>   index 968a14548813..46ae48bd6a3c 100644
>   --- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
>   +++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
>   @@ -1217,7 +1217,8 @@ void PEI::insertZeroCallUsedRegs(MachineFunction &MF) 
> {
>continue;
>   
>  MCRegister Reg = MO.getReg();
>   -  if ((MO.isDef() || MO.isUse()) && AllocatableSet[Reg])
>   +  if (AllocatableSet[Reg] && !MO.isImplicit() &&
>   +  (MO.isDef() || MO.isUse()))
>UsedRegs.set(Reg);
>}

I tested this patch but it did not change the hang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This diff allows me to boot on bare metal as of v5.17-rc2:

  diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
  index 6aef9ee28a39..8ee176dac669 100644
  --- a/arch/x86/kernel/Makefile
  +++ b/arch/x86/kernel/Makefile
  @@ -125,6 +125,7 @@ obj-$(CONFIG_DEBUG_NMI_SELFTEST) += nmi_selftest.o
  
   obj-$(CONFIG_KVM_GUEST)+= kvm.o kvmclock.o
   obj-$(CONFIG_PARAVIRT) += paravirt.o
  +CFLAGS_paravirt.o += -fzero-call-used-regs=skip
   obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= paravirt-spinlocks.o
   obj-$(CONFIG_PARAVIRT_CLOCK)   += pvclock.o
   obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o

I have uploaded the config used, the preprocessed file, and the "good" object 
(with the following diff) and the "bad" object (without the above diff) here: 
https://github.com/nathanchance/bug-files/tree/052a31e6d94c1b349cf6f312808794dace24/D110869

If there is any more information I can give, please let me know!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

The latest revision allows me to boot a kernel in QEMU now but that same kernel 
does not boot on bare metal. I'll see if I can narrow down the problematic 
translation unit with Nick's `subdir-ccflags-y` trick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D117717: [clang] Ignore -fconserve-stack

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance abandoned this revision.
nathanchance added a comment.

As discussed, this is not the right fix for the issue. I have just gone ahead 
and told the original reporter to use `scan-build` with the `--use-cc=clang` 
flag to avoid these issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117717

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

[PATCH] D117717: [clang] Ignore -fconserve-stack

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D117717#3256293 , @MaskRay wrote:

> Assuming scan-build is clang/tools/scan-build, which is in tree and can be 
> fixed instead.

Yes, that is the one.

> I am not familiar with it, but from
>
>> However, this caused issues with scan-build, as GCC is used as the compiler 
>> but clang is invoked after it with all the -f options that GCC was.
>
> if scan-build+GCC takes all GCC recognized options and forwards them to 
> Clang, I think it is a generally infeasible model.
> GCC has dozens of options not recognized by Clang...

I can barely read Perl but it seems like that is what the script does:

https://github.com/llvm/llvm-project/blob/058d2123792da54ae7460fea0946d2c90a032e1c/clang/tools/scan-build/libexec/ccc-analyzer#L661-L664

I don't really know what the fix is for that on the `scan-build` side of things 
(how do you decide what flags should be passed through or not?). If there is 
someone who should be looped in, please do so.

I agree that this is an infeasible model to support so I have commented on the 
kernel patch that brought this up: 
https://lore.kernel.org/r/YeiaAgQ+gbZYTMwD@archlinux-ax161/

I'll abandon this depending on what others have to say.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117717

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

[PATCH] D117717: [clang] Ignore -fconserve-stack

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D117717#3256180 , @MaskRay wrote:

> Clang has a long list of ignored options but most were added during the 
> catch-up phase when Clang strove to be a competitor compiler.
> I think for new ignored options there needs to be some higher standard.
> From Debian Code Search 
> https://codesearch.debian.net/search?q=fconserve-stack&literal=0&page=2 and 
> internal code search, I believe this is a fairly obscure option.
> Shall we fix the projects instead?

I'll give a little background:

The Linux kernel has a `make` macro called `cc-option` that invokes the 
compiler with a flag and if the flag is supported, it adds it to the kernel's 
CFLAGS variable. Nick recently went through and cleaned up the use of this 
macro, as it can slow down build times if it is used excessively, especially 
with incremental compiles because this macro runs during each invocation of 
`make`: https://git.kernel.org/linus/7d73c3e9c51400d3e0e755488050804e4d44737a. 
This flag was one of the ones that was cleaned up; it was moved into a block 
that is only built when GCC is used as the compiler because we know this flag 
is not supported with `clang` so there is no point in testing for it. However, 
this caused issues with `scan-build`, as GCC is used as the compiler but 
`clang` is invoked after it with all the `-f` options that GCC was.

The kernel can add back the `cc-option` call (and likely will to workaround 
this problem) but all that is going to do is drop this flag during the 
`scan-build` process, which may result in higher stack usage.

Should we just tell people to run `scan-build` only with `clang`? How 
`ccc-analyzer` works seems weird to me but I assume some of the flag machinery 
that is present here is for that reason?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117717

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

[PATCH] D117717: [clang] Ignore -fconserve-stack

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: aaron.ballman, MaskRay, srhines.
Herald added a subscriber: dang.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This flag is used when building the Linux kernel with GCC. When the
kernel is built with scan-build (which defaults to GCC for the compiler
step), ccc-analyzer passes along all '-f' flags it sees from the
compiler step, which causes the clang invocation to fail because this
flag is completely unrecognized.

Add this flag to the clang_ignored_gcc_optimization_f_Group so that it
is recognized and does not cause the analyzer step of ccc-analyzer to
fail.

Link: 
https://lore.kernel.org/r/20220119135147.1859982-1-amadeuszx.slawin...@linux.intel.com


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117717

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -307,6 +307,7 @@
 // RUN: -malign-functions=100 \
 // RUN: -malign-loops=100 \
 // RUN: -malign-jumps=100 \
+// RUN: -fconserve-stack -fno-conserve-stack  \
 // RUN: %s 2>&1 | FileCheck --check-prefix=IGNORE %s
 // IGNORE-NOT: error: unknown argument
 
@@ -372,6 +373,8 @@
 // RUN: -fno-devirtualize-speculatively   \
 // RUN: -fslp-vectorize-aggressive\
 // RUN: -fno-slp-vectorize-aggressive \
+// RUN: -fconserve-stack  \
+// RUN: -fno-conserve-stack   \
 // RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-WARNING %s
 // CHECK-WARNING-DAG: optimization flag '-finline-limit=1000' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-limit' is not supported
@@ -432,6 +435,8 @@
 // CHECK-WARNING-DAG: optimization flag '-fno-devirtualize-speculatively' is 
not supported
 // CHECK-WARNING-DAG: the flag '-fslp-vectorize-aggressive' has been 
deprecated and will be ignored
 // CHECK-WARNING-DAG: the flag '-fno-slp-vectorize-aggressive' has been 
deprecated and will be ignored
+// CHECK-WARNING-DAG: optimization flag '-fconserve-stack' is not supported
+// CHECK-WARNING-DAG: optimization flag '-fno-conserve-stack' is not supported
 
 // Test that we mute the warning on these
 // RUN: %clang -### -finline-limit=1000 -Wno-invalid-command-line-argument 
 \
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4473,6 +4473,7 @@
 defm caller_saves : BooleanFFlag<"caller-saves">, 
Group;
 defm reorder_blocks : BooleanFFlag<"reorder-blocks">, 
Group;
 defm branch_count_reg : BooleanFFlag<"branch-count-reg">, 
Group;
+defm conserve_stack : BooleanFFlag<"conserve-stack">, 
Group;
 defm default_inline : BooleanFFlag<"default-inline">, 
Group;
 defm fat_lto_objects : BooleanFFlag<"fat-lto-objects">, 
Group;
 defm float_store : BooleanFFlag<"float-store">, 
Group;


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -307,6 +307,7 @@
 // RUN: -malign-functions=100 \
 // RUN: -malign-loops=100 \
 // RUN: -malign-jumps=100 \
+// RUN: -fconserve-stack -fno-conserve-stack  \
 // RUN: %s 2>&1 | FileCheck --check-prefix=IGNORE %s
 // IGNORE-NOT: error: unknown argument
 
@@ -372,6 +373,8 @@
 // RUN: -fno-devirtualize-speculatively   \
 // RUN: -fslp-vectorize-aggressive\
 // RUN: -fno-slp-vectorize-aggressive \
+// RUN: -fconserve-stack  \
+// RUN: -fno-conserve-stack   \
 // RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-WARNING %s
 // CHECK-WARNING-DAG: optimization flag '-finline-limit=1000' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-limit' is not supported
@@ -432,6 +435,8 @@
 // CHECK-WARNING-DAG: optimization flag '-fno-devirtualize-speculatively' is not supported
 // CHECK-WARNING-DAG: the flag '-fslp-vectorize-aggressive' has been deprecated and will be ignored
 // CHECK-WARNING-

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D105169#3246378 , @nlopes wrote:

> In D105169#3244624 , @nathanchance 
> wrote:
>
>> I can reduce all of these down for you and/or I can start an email thread 
>> with the `objtool` maintainers to see if there is a way to fix or avoid 
>> these warnings on the `objtool` side and include you in that discussion, if 
>> LLVM is not really doing anything wrong here. I am by no means an expert in 
>> this area and I don't want to delay this anymore but I want to avoid 
>> regressing our builds, as `objtool` regularly helps us spot compiler bugs. 
>> Perhaps @nickdesaulniers has some other thoughts?
>
> LLVM is not doing anything wrong here. The issue is that once we have UB, 
> LLVM is not required to lay out the functions nicely. For example, issue 
> #53118 is just a "nice to fix", it's not a bug.
> On the other hand, I understand that fixing `objtool` is likely impossible if 
> we consider this UB thing as assembly doesn't have enough information to 
> distinguish between a compiler bug and a UB case. I just don't know how 
> frequent and/or useful these warnings are.
>
> I don't think we should hold this patch anymore as it's not wrong and the 
> side-effects are understood. We can and should try to reduce those kernel 
> warnings to zero, but we cannot put all that burden on this patch's author.

Fair enough. We will just try to deal with this change on the kernel side in 
one way or another.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a subscriber: nickdesaulniers.
nathanchance added a comment.

In D105169#3242949 , @hyeongyukim 
wrote:

> @nathanchance Hi, I analyzed all four warnings.

Thanks a lot for doing this!

> Warning #1: can be handled by x86-backend. filled issue #53118 
> 

This is the warning in `drivers/net/ethernet/wiznet/w5100.lto.o` if I am 
reading the issue, right? I would have thought that would be warnings #2 and #3.

> Warning #2: bug in the kernel, fixed in the next version.
> Warning #3: same reason with #2

I think this refers to `drivers/net/ethernet/renesas/ravb.lto.o`, which is 
indeed fixed by 
https://git.kernel.org/linus/d9f31aeaa1e5aefa68130878af3c3513d41c1e2d.

> Warning #4: It was not reproduced in the latest clang.

I can still see this with clang @1441ffe6a6da90e9c4800914eb0005b1087aa5d2  and 
Linux @ 112450df61b7373529b0fe4c122ad13b89d80a8a built with `KCFLAGS="-Xclang 
-enable-noundef-analysis"`. Your small reproducer has it as well:

  $ clang --version | head -1
  ClangBuiltLinux clang version 14.0.0 (https://github.com/llvm/llvm-project 
1441ffe6a6da90e9c4800914eb0005b1087aa5d2)
  
  $ clang -O2 -flto=thin -fsanitize=integer-divide-by-zero 
-fsanitize-coverage=trace-pc -Xclang -enable-noundef-analysis -c -o 
intelfb.{o,i}
  
  $ ld.lld -m elf_x86_64 -r -o intelfb.lto.o --whole-archive intelfb.o
  
  $ ./objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount 
intelfb.lto.o
  intelfb.lto.o: warning: objtool: .text.intelfbhw_validate_mode: unexpected 
end of section



> As you see, one of this patch's advantages is that it exposed some bugs in 
> the kernel!
> All issues are either identified or will be fixed soon. Is it okay to 
> recommit this patch?

I just went to go hammer on this option to how many warnings I see on the next 
version of Linux ("linux-next") [1] with `allmodconfig` (which in turn enables 
`CONFIG_UBSAN`) + `CONFIG_LTO_CLANG_THIN=y` . I think I see a lot more warnings 
there versus Linus' tree as the kernel no longer uses `-fsanitize=object-size` 
there, which potentially hides some of these?

  $ echo "CONFIG_GCOV_KERNEL=n
  CONFIG_KASAN=n
  CONFIG_LTO_CLANG_THIN=y
  CONFIG_WERROR=n" >allmod.config
  
  $ make -skj"$(nproc)" KCFLAGS="-Xclang -enable-noundef-analysis" 
KCONFIG_ALLCONFIG=1 LLVM=1 allmodconfig all
  ...
  drivers/video/fbdev/omap2/omapfb/dss/omapdss.lto.o: warning: objtool: 
.text.dsi_cio_timings: unexpected end of section
  drivers/soc/qcom/qcom_rpmh.lto.o: warning: objtool: 
.text.rpmh_rsc_write_ctrl_data: unexpected end of section
  net/tipc/tipc.lto.o: warning: objtool: .text.tipc_nl_compat_link_stat_dump: 
unexpected end of section
  drivers/scsi/mpi3mr/mpi3mr.lto.o: warning: objtool: 
.text.mpi3mr_op_request_post: unexpected end of section
  drivers/md/bcache/bcache.lto.o: warning: objtool: .text.bch_cache_show: 
unexpected end of section
  drivers/scsi/dc395x.lto.o: warning: objtool: .text.msgin_phase0: unexpected 
end of section
  ...
  fs/ocfs2/cluster/ocfs2_nodemanager.lto.o: warning: objtool: 
.text.o2hb_setup_one_bio: unexpected end of section
  drivers/video/fbdev/intelfb/intelfb.lto.o: warning: objtool: 
.text.intelfbhw_validate_mode: unexpected end of section
  drivers/net/ethernet/wiznet/w5100.lto.o: warning: objtool: 
.text.w5100_tx_skb: unexpected end of section
  drivers/net/ethernet/wiznet/w5100.lto.o: warning: objtool: 
.text.w5100_rx_skb: unexpected end of section
  drivers/gpu/drm/amd/amdgpu/amdgpu.lto.o: warning: objtool: 
.text.dqm_debugfs_hqds: unexpected end of section
  ...

Without `KCFLAGS="-Xclang -enable-noundef-analysis"`, I only see the 
`mpi3mr_op_request_post` warning, so that is probably unrelated to this 
(probably some recent change regressed this, I'll see if I can hunt that down).

Without ThinLTO, I see:

  $ echo CONFIG_WERROR=n >allmod.config
  
  $ make -skj"$(nproc)" KCFLAGS="-Xclang -enable-noundef-analysis" 
KCONFIG_ALLCONFIG=1 LLVM=1 allmodconfig all
  ...
  drivers/soc/qcom/rpmh-rsc.o: warning: objtool: rpmh_rsc_write_ctrl_data() 
falls through to next function trace_raw_output_rpmh_tx_done()
  drivers/gpu/drm/radeon/sumo_dpm.o: warning: objtool: 
sumo_dpm_set_power_state() falls through to next function 
sumo_dpm_post_set_power_state()
  ...
  drivers/scsi/mpi3mr/mpi3mr_fw.o: warning: objtool: mpi3mr_op_request_post() 
falls through to next function mpi3mr_check_rh_fault_ioc()
  drivers/net/ethernet/wiznet/w5100.o: warning: objtool: w5100_tx_skb() falls 
through to next function w5100_get_drvinfo()
  drivers/net/ethernet/wiznet/w5100.o: warning: objtool: w5100_rx_skb() falls 
through to next function w5100_mmio_probe()
  ...

Some of the same warnings, which likely have the same root cause as above but 
no LTO might make this easier to look into. Without `KCFLAGS="-Xclang 
-enable-noundef-analysis"`, same as above, I only see the 
`mpi3mr_op_request_post` warning.

I can reduce all of

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D105169#3236484 , @hyeongyukim 
wrote:

> @nathanchance
>
> I tried to reproduce the last warning (intelfbhw_validate_mode), but I failed 
> to produce it.
> I think my reproducer is correct, but it does not make any warning.
> Can you tell me which part was wrong?
>
>   clang -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o intelfb.o 
> intelfb.i
>   ld.lld -m elf_x86_64 -r -o intelfb.lto.o --whole-archive intelfb.o
>   objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount 
> intelfb.lto.o
>
> I use these commands, and I attached the `intelfb.i` file.F21595840: 
> intelfb.i 

It looks like this particular case also needs `-fsanitize-coverage=trace-pc` 
(which comes from `CONFIG_KCOV`). Once I add that with your reduced reproducer, 
I see the initial warning.

  $ clang -O2 -flto=thin -fsanitize=integer-divide-by-zero 
-fsanitize-coverage=trace-pc -c -o intelfb.{o,i}
  
  $ ld.lld -m elf_x86_64 -r -o intelfb.lto.o --whole-archive intelfb.o
  
  $ ./objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount 
intelfb.lto.o
  intelfb.lto.o: warning: objtool: .text.intelfbhw_validate_mode: unexpected 
end of section


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

The two false positive warnings in the kernel are fixed with this patch and 
there were no new warnings introduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116059

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

@hyeongyukim I hand reduced a couple of the translation units that I see issues 
in so apologies if they are a little verbose.

The full set of warnings:

  drivers/net/ethernet/renesas/ravb.lto.o: warning: objtool: 
.text.ravb_set_gti: unexpected end of section
  drivers/net/ethernet/wiznet/w5100.lto.o: warning: objtool: 
.text.w5100_tx_skb: unexpected end of section
  drivers/net/ethernet/wiznet/w5100.lto.o: warning: objtool: 
.text.w5100_readbuf: unexpected end of section
  drivers/video/fbdev/intelfb/intelfb.lto.o: warning: objtool: 
.text.intelfbhw_validate_mode: unexpected end of section

which individually can be viewed at the following links:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_main.c?h=v5.16-rc7#n2480
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/wiznet/w5100.c?h=v5.16-rc7#n798
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/wiznet/w5100.c?h=v5.16-rc7#n513
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/intelfb/intelfbhw.c?h=v5.16-rc7#n313

I have a reproducer for the first two, as that is all I had time for; if you 
would like them for the other two, I can get those for you tomorrow.

https://github.com/nathanchance/creduce-files/tree/3699d2b725a305c0f02a7672447b30578115ba51/D105169-ravb_main

  $ clang -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o ravb_main.o 
ravb_main.i
  ravb_main.i:68:5: warning: expression result unused [-Wunused-value]
  __rem;
  ^
  1 warning generated.
  
  $ ld.lld -m elf_x86_64 -r -o ravb.lto.o --whole-archive ravb_main.o
  
  $ ./objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount 
ravb.lto.o
  ravb.lto.o: warning: objtool: .text.ravb_set_gti: unexpected end of section

This one can also be reproduced without LTO:

  $ clang -O2 -fsanitize=integer-divide-by-zero -c -o ravb_main.o ravb_main.i
  
  $ ./objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount 
ravb_main.o
  ravb_main.o: warning: objtool: .text: unexpected end of section

As far as I can tell, that function already has a check to avoid dividing by 
zero.

https://github.com/nathanchance/creduce-files/tree/3699d2b725a305c0f02a7672447b30578115ba51/D105169-w5100

  $ clang -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o w5100.o w5100.i
  
  $ ld.lld -m elf_x86_64 -r -o w5100.lto.o --whole-archive w5100.o
  
  $ w5100.lto.o: warning: objtool: .text.w5100_tx_skb: unexpected end of section

Without LTO, another warning is emitted, with a similar root cause I believe:

  $ clang -O2 -fsanitize=integer-divide-by-zero -c -o w5100.o w5100.i
  
  $ ./objtool orc generate --module --no-fp --no-unreachable --uaccess --mcount 
w5100.o
  w5100.o: warning: objtool: w5100_tx_skb() falls through to next function 
w5100_probe()

In theory, `w5100_writebuf()` should probably have a check for `s0_tx_buf_size` 
being zero but the input is completely controlled by the kernel, as can be seen 
in `w5100_probe()` (`s0_tx_buf_size` cannot be zero). Most if not all kernel 
maintainers are not going to accept patches adding checks that are superfluous.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

@hyeongyukim I am currently offline for the evening but it seems like my 
reduction might have been too aggressive. It looks like this code comes from 
`ravb_set_gti()` 
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/renesas/ravb_main.c?h=v5.16-rc7#n2480),
 which checks that `rate` is not zero before using it as a divisor. I will see 
if I can get a reproducer without any undefined behavior such as this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor 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 rGbe8180af5854: [clang][driver] Warn when 
'-mno-outline-atomics' is used with a non-AArch64… (authored by 
nathanchance).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-outline-atomics.c


Index: clang/test/Driver/unsupported-outline-atomics.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-outline-atomics.c
@@ -0,0 +1,15 @@
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS-X86
+// CHECK-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS-X86
+// CHECK-NO-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "-outline-atomics"
+
+// RUN: %clang -target riscv64 -moutline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-OUTLINE-ATOMICS-RISCV
+// CHECK-OUTLINE-ATOMICS-RISCV: warning: 'riscv64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-RISCV-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target riscv64 -mno-outline-atomics -S %s -### 2>&1 | 
FileCheck %s -check-prefix=CHECK-NO-OUTLINE-ATOMICS-RISCV
+// CHECK-NO-OUTLINE-ATOMICS-RISCV: warning: 'riscv64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-RISCV-NOT: "-target-feature" "-outline-atomics"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7007,18 +7007,18 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_moutline_atomics,
options::OPT_mno_outline_atomics)) {
-if (A->getOption().matches(options::OPT_moutline_atomics)) {
-  // Option -moutline-atomics supported for AArch64 target only.
-  if (!Triple.isAArch64()) {
-D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
-<< Triple.getArchName();
-  } else {
+// Option -moutline-atomics supported for AArch64 target only.
+if (!Triple.isAArch64()) {
+  D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
+  << Triple.getArchName() << A->getOption().getName();
+} else {
+  if (A->getOption().matches(options::OPT_moutline_atomics)) {
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back("+outline-atomics");
+  } else {
+CmdArgs.push_back("-target-feature");
+CmdArgs.push_back("-outline-atomics");
   }
-} else {
-  CmdArgs.push_back("-target-feature");
-  CmdArgs.push_back("-outline-atomics");
 }
   } else if (Triple.isAArch64() &&
  getToolChain().IsAArch64OutlineAtomicsDefault(Args)) {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -568,7 +568,7 @@
   InGroup;
 
 def warn_drv_moutline_atomics_unsupported_opt : Warning<
-  "'%0' does not support '-moutline-atomics'; flag ignored">,
+  "'%0' does not support '-%1'; flag ignored">,
   InGroup;
 
 def warn_drv_darwin_sdk_invalid_settings : Warning<


Index: clang/test/Driver/unsupported-outline-atomics.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-outline-atomics.c
@@ -0,0 +1,15 @@
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OUTLINE-ATOMICS-X86
+// CHECK-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support '-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-NO-OUTLINE-ATOMICS-X86
+// CHECK-NO-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support '-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "-outline-atomics"
+
+// RUN: %clang -target riscv64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OUTLINE-ATOMICS-RISCV
+// CHECK-OUTLINE-ATOMICS-RISCV: warning: 'riscv64' does not s

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 395879.
nathanchance added a comment.
Herald added subscribers: luke957, s.egerton, simoncook.

- Move test from x86-outline-atomics.c to unsupported-outline-atomics.c (Marco).

- Add a test for riscv64 to ensure it works with multiple architectures (Marco).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-outline-atomics.c


Index: clang/test/Driver/unsupported-outline-atomics.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-outline-atomics.c
@@ -0,0 +1,15 @@
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS-X86
+// CHECK-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS-X86
+// CHECK-NO-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "-outline-atomics"
+
+// RUN: %clang -target riscv64 -moutline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-OUTLINE-ATOMICS-RISCV
+// CHECK-OUTLINE-ATOMICS-RISCV: warning: 'riscv64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-RISCV-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target riscv64 -mno-outline-atomics -S %s -### 2>&1 | 
FileCheck %s -check-prefix=CHECK-NO-OUTLINE-ATOMICS-RISCV
+// CHECK-NO-OUTLINE-ATOMICS-RISCV: warning: 'riscv64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-RISCV-NOT: "-target-feature" "-outline-atomics"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7007,18 +7007,18 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_moutline_atomics,
options::OPT_mno_outline_atomics)) {
-if (A->getOption().matches(options::OPT_moutline_atomics)) {
-  // Option -moutline-atomics supported for AArch64 target only.
-  if (!Triple.isAArch64()) {
-D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
-<< Triple.getArchName();
-  } else {
+// Option -moutline-atomics supported for AArch64 target only.
+if (!Triple.isAArch64()) {
+  D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
+  << Triple.getArchName() << A->getOption().getName();
+} else {
+  if (A->getOption().matches(options::OPT_moutline_atomics)) {
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back("+outline-atomics");
+  } else {
+CmdArgs.push_back("-target-feature");
+CmdArgs.push_back("-outline-atomics");
   }
-} else {
-  CmdArgs.push_back("-target-feature");
-  CmdArgs.push_back("-outline-atomics");
 }
   } else if (Triple.isAArch64() &&
  getToolChain().IsAArch64OutlineAtomicsDefault(Args)) {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -559,7 +559,7 @@
   InGroup;
 
 def warn_drv_moutline_atomics_unsupported_opt : Warning<
-  "'%0' does not support '-moutline-atomics'; flag ignored">,
+  "'%0' does not support '-%1'; flag ignored">,
   InGroup;
 
 def warn_drv_darwin_sdk_invalid_settings : Warning<


Index: clang/test/Driver/unsupported-outline-atomics.c
===
--- /dev/null
+++ clang/test/Driver/unsupported-outline-atomics.c
@@ -0,0 +1,15 @@
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OUTLINE-ATOMICS-X86
+// CHECK-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support '-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-NO-OUTLINE-ATOMICS-X86
+// CHECK-NO-OUTLINE-ATOMICS-X86: warning: 'x86_64' does not support '-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-X86-NOT: "-target-feature" "-outline-atomics"
+
+// RUN: %clang -target riscv64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OUTLINE-ATOMICS-RISCV
+// CHECK-OUTLINE-ATOMICS-RIS

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/test/Driver/x86-outline-atomics.c:1-7
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS
+// CHECK-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS
+// CHECK-NO-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-NOT: "-target-feature" "-outline-atomics"

nickdesaulniers wrote:
> If `clang/test/Driver/aarch64-features.c` is only testing outline atomics, I 
> wonder if this test should actually go in there? @t.p.northover , thoughts? 
> Otherwise LGTM and thanks for the patch!
I did consider that initially but:

1. It does appear to test a few other AArch64 specific things.
2. It seemed weird to add an x86_64 test to an AArch64 file.

No strong opinions though, I'll go along with whatever works and makes the 
majority happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

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

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

If there are more qualified people to review and accept this, please add them, 
I was unsure of who else to add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116128

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

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: melver, nickdesaulniers, t.p.northover.
Herald added subscribers: pengfei, kristof.beyls.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The Linux kernel has a make macro called cc-option that invokes the
compiler with an option in isolation to see if it is supported before
adding it to CFLAGS. The exit code of the compiler is used to determine
if the flag is supported and should be added to the compiler invocation.

A call to cc-option with '-mno-outline-atomics' was added to prevent
linking errors with newer GCC versions but this call succeeds with a
non-AArch64 target because there is no warning from clang with
'-mno-outline-atomics', just '-moutline-atomics'. Because the call
succeeds and adds '-mno-outline-atomics' to the compiler invocation,
there is a warning from LLVM because the 'outline-atomics target
feature is only supported by the AArch64 backend.

$ echo | clang -target x86_64 -moutline-atomics -Werror -x c -c -o /dev/null -
clang-14: error: The 'x86_64' architecture does not support -moutline-atomics; 
flag ignored [-Werror,-Woption-ignored]

$ echo $?
1

$ echo | clang -target x86_64 -mno-outline-atomics -Werror -x c -c -o /dev/null 
-
'-outline-atomics' is not a recognized feature for this target (ignoring 
feature)

$ echo $?
0

This does not match GCC's behavior, which errors when the flag is added
to a non-AArch64 target.

$ echo | gcc -moutline-atomics -x c -c -o /dev/null -
gcc: error: unrecognized command-line option ‘-moutline-atomics’; did you mean 
‘-finline-atomics’?

$ echo | gcc -mno-outline-atomics -x c -c -o /dev/null -
gcc: error: unrecognized command-line option ‘-mno-outline-atomics’; did you 
mean ‘-fno-inline-atomics’?

$ echo | aarch64-linux-gnu-gcc -moutline-atomics -x c -c -o /dev/null -

$ echo | aarch64-linux-gnu-gcc -mno-outline-atomics -x c -c -o /dev/null -

To get closer to  GCC's behavior, issue a warning when
'-mno-outline-atomics' is used without an AArch64 triple and do not add
'{-,+}outline-atomic" to the list of target features in these cases.

Link: https://github.com/ClangBuiltLinux/linux/issues/1552


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116128

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/x86-outline-atomics.c


Index: clang/test/Driver/x86-outline-atomics.c
===
--- /dev/null
+++ clang/test/Driver/x86-outline-atomics.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s 
-check-prefix=CHECK-OUTLINE-ATOMICS
+// CHECK-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-moutline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-OUTLINE-ATOMICS-NOT: "-target-feature" "+outline-atomics"
+
+// RUN: %clang -target x86_64 -mno-outline-atomics -S %s -### 2>&1 | FileCheck 
%s -check-prefix=CHECK-NO-OUTLINE-ATOMICS
+// CHECK-NO-OUTLINE-ATOMICS: warning: 'x86_64' does not support 
'-mno-outline-atomics'; flag ignored [-Woption-ignored]
+// CHECK-NO-OUTLINE-ATOMICS-NOT: "-target-feature" "-outline-atomics"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7007,18 +7007,18 @@
 
   if (Arg *A = Args.getLastArg(options::OPT_moutline_atomics,
options::OPT_mno_outline_atomics)) {
-if (A->getOption().matches(options::OPT_moutline_atomics)) {
-  // Option -moutline-atomics supported for AArch64 target only.
-  if (!Triple.isAArch64()) {
-D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
-<< Triple.getArchName();
-  } else {
+// Option -moutline-atomics supported for AArch64 target only.
+if (!Triple.isAArch64()) {
+  D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)
+  << Triple.getArchName() << A->getOption().getName();
+} else {
+  if (A->getOption().matches(options::OPT_moutline_atomics)) {
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back("+outline-atomics");
+  } else {
+CmdArgs.push_back("-target-feature");
+CmdArgs.push_back("-outline-atomics");
   }
-} else {
-  CmdArgs.push_back("-target-feature");
-  CmdArgs.push_back("-outline-atomics");
 }
   } else if (Triple.isAArch64() &&
  getToolChain().IsAArch64OutlineAtomicsDefault(Args)) {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -559,7 +559,7 @@
   InGroup;
 
 def warn_drv_moutline_atomics_unsupported_opt : Warning<
-  "'%0' does not support '-mou

[PATCH] D114848: [Analysis] Ignore casts and unary ops for uninitialized values

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I tested the problematic Linux kernel configuration that uncovered this issue 
with this patch and the issue was resolved. I tested an x86_64 allmodconfig 
build and did not see any warnings.




Comment at: clang/lib/Analysis/UninitializedValues.cpp:594
   // If the predecessor's terminator is an "asm goto" that initializes
-  // the variable, then it won't be counted as "initialized" on the
-  // non-fallthrough paths.
+  // the variable, then it's don't count it as "initialized" on the
+  // indirect paths.

Should "it's don't count" be "don't count"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114848

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Prior to the latest revert (fd9b099906c61e46574d1ea2d99b973321fe1d21 
), the 
Linux kernel's binary verifier (`objtool`) points out an issue when building 
with ThinLTO and `-fsanitize=integer-divide-by-zero`. I have no idea if this is 
an issue with the tool or this series. A simplified reproducer:

  $ cat ravb_main.i
  int ravb_set_gti_ndev_rate;
  unsigned int ravb_set_gti_ndev_inc;
  void ravb_set_gti_ndev() {
ravb_set_gti_ndev_inc = 10;
ravb_set_gti_ndev_inc = ravb_set_gti_ndev_inc / ravb_set_gti_ndev_rate;
if (ravb_set_gti_ndev_inc)
  _dev_err(ravb_set_gti_ndev_inc);
  }
  
  $ clang -std=gnu89 -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o 
ravb_main.o ravb_main.i
  
  $ llvm-ar cDPrsT ravb.o ravb_main.o
  
  $ ld.lld -m elf_x86_64 -r -o ravb.lto.o --whole-archive ravb.o
  
  $ ./objtool orc generate --no-fp --no-unreachable --retpoline --uaccess 
--mcount --module ravb.lto.o
  ravb.lto.o: warning: objtool: .text.ravb_set_gti_ndev: unexpected end of 
section

With LLVM 13.0.0, there is no warning with those commands. The original and 
reduced `.i` file, interestingness test, and static `objtool` binary are 
available here 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I bisected a crash in the Linux kernel down to the reland commit (and it is not 
fixed as of 
https://github.com/llvm/llvm-project/commit/2249ecee8d9a6237f51485bd39f01ba031575987):

  $ git bisect log
  # bad: [bdaa181007a46d317a1665acb8234eddeee82815] [TwoAddressInstructionPass] 
Update existing physreg live intervals
  # good: [d4b1cf8f9c48a49ab808df15c4ab378276a07b82] [OpenMP] Build device 
runtimes for sm_86
  git bisect start 'bdaa181007a46d317a1665acb8234eddeee82815' 
'd4b1cf8f9c48a49ab808df15c4ab378276a07b82'
  # good: [f2703c3c3353031de8de8c465a59d31488b11fb3] [DAG] 
FoldConstantArithmetic - rename NumOps -> NumElts. NFC.
  git bisect good f2703c3c3353031de8de8c465a59d31488b11fb3
  # good: [c68183b81e5257186c9403cf91f8b958af7459bc] [gn build] Use `=` for of 
-fdebug-compilation-dir
  git bisect good c68183b81e5257186c9403cf91f8b958af7459bc
  # bad: [2d8ec3c61d3c2d47b303187bb882ca23544f6fc5] [libcxx] [test] Narrow down 
XFAILs regarding a MSVC mode specific bug to "windows-dll && msvc"
  git bisect bad 2d8ec3c61d3c2d47b303187bb882ca23544f6fc5
  # good: [97c899f3c5d9bbff2824b3252b21378bf96f3f3f] [mlir] Add callback to 
provide a pass pipeline to MlirOptMain
  git bisect good 97c899f3c5d9bbff2824b3252b21378bf96f3f3f
  # bad: [4070f305f9a0c488d7177754d77c0b367e8695bf] [mlir][DialectConversion] 
Legalize all live argument conversions
  git bisect bad 4070f305f9a0c488d7177754d77c0b367e8695bf
  # bad: [3466e00716e12e32fdb100e3fcfca5c2b3e8d784] Reland "[Attr] support 
btf_type_tag attribute"
  git bisect bad 3466e00716e12e32fdb100e3fcfca5c2b3e8d784
  # good: [f64580f8d2ce5e1161857f9c89c2eee7a74c5ab8] [AArch64][GISel] Optimize 
8 and 16 bit variants of uaddo.
  git bisect good f64580f8d2ce5e1161857f9c89c2eee7a74c5ab8
  # first bad commit: [3466e00716e12e32fdb100e3fcfca5c2b3e8d784] Reland "[Attr] 
support btf_type_tag attribute"

`cvise` spits out:

  $ cat efi.i
  typedef unsigned long efi_query_variable_info_t(int);
  typedef struct {
struct {
  efi_query_variable_info_t __attribute__((regparm(0))) * 
query_variable_info;
};
  } efi_runtime_services_t;
  efi_runtime_services_t efi_0;
  
  $ clang -m32 -O2 -c -o /dev/null efi.i
  
  $ clang -m32 -O2 -g -c -o /dev/null efi.i
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: clang -m32 -O2 -g -c -o /dev/null efi.i
  1.   parser at end of file
   #0 0x02b31d13 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2b31d13)
   #1 0x02b2fb3e llvm::sys::RunSignalHandlers() 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2b2fb3e)
   #2 0x02ab8d13 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
CrashRecoveryContext.cpp:0:0
   #3 0x02ab8e8e CrashRecoverySignalHandler(int) 
CrashRecoveryContext.cpp:0:0
   #4 0x7fa9dd533870 __restore_rt sigaction.c:0:0
   #5 0x02eaa00e 
clang::CodeGen::CGDebugInfo::CreateType(clang::FunctionType const*, 
llvm::DIFile*, clang::TypeLoc) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2eaa00e)
   #6 0x02ea4598 
clang::CodeGen::CGDebugInfo::getOrCreateType(clang::QualType, llvm::DIFile*, 
clang::TypeLoc) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2ea4598)
   #7 0x02ea7cc4 
clang::CodeGen::CGDebugInfo::CreatePointerLikeType(llvm::dwarf::Tag, 
clang::Type const*, clang::QualType, llvm::DIFile*, clang::TypeLoc) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2ea7cc4)
   #8 0x02eb6558 
clang::CodeGen::CGDebugInfo::CreateTypeNode(clang::QualType, llvm::DIFile*, 
clang::TypeLoc) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2eb6558)
   #9 0x02ea4598 
clang::CodeGen::CGDebugInfo::getOrCreateType(clang::QualType, llvm::DIFile*, 
clang::TypeLoc) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2ea4598)
  #10 0x02eaa610 
clang::CodeGen::CGDebugInfo::createFieldType(llvm::StringRef, clang::QualType, 
clang::SourceLocation, clang::AccessSpecifier, unsigned long, unsigned int, 
llvm::DIFile*, llvm::DIScope*, clang::RecordDecl const*, 
llvm::MDTupleTypedArrayWrapper, clang::TypeLoc) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2eaa610)
  #11 0x02eab3c0 
clang::CodeGen::CGDebugInfo::CollectRecordNormalField(clang::FieldDecl const*, 
unsigned long, llvm::DIFile*, llvm::SmallVectorImpl&, 
llvm::DIType*, clang::RecordDecl const*) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2eab3c0)
  #12 0x02eab90c 
clang::CodeGen::CGDebugInfo::CollectRecordFields(clang::RecordDecl const*, 
llvm::DIFile*, llvm::SmallVectorImpl&, llvm::DICompositeType*) 
(/home/nathan/cbl/src/llvm-project/build/stage1/bin/clang-14+0x2eab90c)
  #13 0x02eb111a 
clang::CodeGen::CGD

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a subscriber: peter.smith.
nathanchance added a comment.

I tested this against `next-20211027`, where it resolved the build error and 
boots in QEMU master without any visible issues. Disassembly between GCC and 
clang seems to be good. I'll leave it up to @nickdesaulniers and @peter.smith 
to approve, as I am not very familiar with TableGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112600

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

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

`check-llvm` does not pass with this revision nor does it resolve the boot 
failure that Nick mentioned, just in case that was the intention of the 
revision :) I ran the kernel through `gdb` and it still does not appear to get 
to `start_kernel`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869

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

[PATCH] D110656: [clang][Sema] Warn on uninitialized array elments

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D110656#3034181 , @nathanchance 
wrote:

> I can test this on the Linux kernel tonight and report the results tomorrow 
> morning.

My apologies, I did not have time to get to this last night or today and I am 
going to be on vacation starting today for a week and a half. I can report back 
once I am back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110656

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

[PATCH] D110656: [clang][Sema] Warn on uninitialized array elments

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I can test this on the Linux kernel tonight and report the results tomorrow 
morning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110656

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

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

As far as I am aware, this iteration of the patch has no instances in the Linux 
kernel now. The instances I found with the first iteration of the patch only 
had a right hand side with side effects, not both sides, so this warning is 
effectively a no-op now (not that there won't be instances in the future). If 
any happen to show up, I will send fixes for them at that time. In other words, 
LGTM.


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

https://reviews.llvm.org/D108003

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

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I am still running a series of builds against the Linux kernel but I already 
see one instance of this warning where the suggestion would change the meaning 
of the code in an incorrect way:

  drivers/input/touchscreen.c:81:17: warning: use of bitwise '|' with boolean 
operands [-Wbool-operation]
  data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
 ^~
  drivers/input/touchscreen.c:81:17: warning: use of bitwise '|' with boolean 
operands [-Wbool-operation]
  data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-x",
 ^~
  drivers/input/touchscreen.c:94:17: warning: use of bitwise '|' with boolean 
operands [-Wbool-operation]
  data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-y",
 ^~
  drivers/input/touchscreen.c:94:17: warning: use of bitwise '|' with boolean 
operands [-Wbool-operation]
  data_present = touchscreen_get_prop_u32(dev, "touchscreen-min-y",
 ^~
  drivers/input/touchscreen.c:108:17: warning: use of bitwise '|' with boolean 
operands [-Wbool-operation]
  data_present = touchscreen_get_prop_u32(dev,
 ^
  5 warnings generated.

Which corresponds to this file: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen.c?h=v5.14-rc6

If the calls to `touchscreen_get_prop_u32` short circuit, we could use 
`maximum` or `fuzz` uninitialized. There might be a cleaner way to rewrite that 
block to avoid the warning but based on the other instances of this warning I 
see, I am not sure `|` vs. `||` is worth warning about (but I am happy to hear 
examples of how it could be a bug). Most people realize `&&` short circuits (as 
`if (a && foo(a->...))` is relatively common) but most probably are not 
thinking about `||` short circuiting (and it would be more of an optimization 
than correctness issue as far as I understand it).

Additionally, I have not caught any instances of `&` being used instead of 
`&&`, including the ones I notated previously; those were caught because only 
the right side had side effects. As was pointed out here and on the mailing 
list 
, 
the `lib/zstd/` warning is probably a bug, as the short circuit should happen 
if `offset_1` is zero, otherwise there is unnecessary work done.


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

https://reviews.llvm.org/D108003

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ed4a94d6451: [clang] Expose unreachable fallthrough 
annotation warning (authored by nathanchance).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/P30636.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 
 
 int fallthrough(int n) {
@@ -193,6 +193,26 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-fallthrough"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+  }
+#pragma clang diagnostic pop
+
   long p = static_cast(n) * n;
   switch (sizeof(p)) {
 case 9:
Index: clang/test/SemaCXX/P30636.cpp
===
--- clang/test/SemaCXX/P30636.cpp
+++ clang/test/SemaCXX/P30636.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 // expected-no-diagnostics
 
 template
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1125,7 +1125,7 @@
 // unreachable in all instantiations of the template.
 if (!IsTemplateInstantiation)
   S.Diag(AS->getBeginLoc(),
- diag::warn_fallthrough_attr_unreachable);
+ diag::warn_unreachable_fallthrough_attr);
 markFallthroughVisited(AS);
 ++AnnotatedCnt;
 break;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -682,6 +682,9 @@
 def warn_unreachable_loop_increment : Warning<
   "loop will run at most once (loop increment never executed)">,
   InGroup, DefaultIgnore;
+def warn_unreachable_fallthrough_attr : Warning<
+  "fallthrough annotation in unreachable code">,
+  InGroup, DefaultIgnore;
 def note_unreachable_silence : Note<
   "silence by adding parentheses to mark code as explicitly dead">;
 
@@ -9578,9 +9581,6 @@
   "fallthrough annotation is outside switch statement">;
 def err_fallthrough_attr_invalid_placement : Error<
   "fallthrough annotation does not directly precede switch label">;
-def warn_fallthrough_attr_unreachable : Warning<
-  "fallthrough annotation in unreachable code">,
-  InGroup, DefaultIgnore;
 
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -821,8 +821,10 @@
 //  under separate flags.
 //
 def UnreachableCodeLoopIncrement : 
DiagGroup<"unreachable-code-loop-increment">;
+def UnreachableCodeFallthrough : DiagGroup<"unreachable-code-fallthrough">;
 def UnreachableCode : DiagGroup<"unreachable-code",
-[UnreachableCodeLoopIncrement]>;
+[UnreachableCodeLoopIncrement,
+ UnreachableCodeFallthrough]>;
 def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
 def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
 def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fall

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 366733.
nathanchance marked an inline comment as not done.
nathanchance edited the summary of this revision.
nathanchance added a comment.

- Update commit message (forgot to use `arc diff --edit --verbatim`) [David]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/P30636.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 
 
 int fallthrough(int n) {
@@ -193,6 +193,26 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-fallthrough"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+  }
+#pragma clang diagnostic pop
+
   long p = static_cast(n) * n;
   switch (sizeof(p)) {
 case 9:
Index: clang/test/SemaCXX/P30636.cpp
===
--- clang/test/SemaCXX/P30636.cpp
+++ clang/test/SemaCXX/P30636.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 // expected-no-diagnostics
 
 template
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1125,7 +1125,7 @@
 // unreachable in all instantiations of the template.
 if (!IsTemplateInstantiation)
   S.Diag(AS->getBeginLoc(),
- diag::warn_fallthrough_attr_unreachable);
+ diag::warn_unreachable_fallthrough_attr);
 markFallthroughVisited(AS);
 ++AnnotatedCnt;
 break;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -682,6 +682,9 @@
 def warn_unreachable_loop_increment : Warning<
   "loop will run at most once (loop increment never executed)">,
   InGroup, DefaultIgnore;
+def warn_unreachable_fallthrough_attr : Warning<
+  "fallthrough annotation in unreachable code">,
+  InGroup, DefaultIgnore;
 def note_unreachable_silence : Note<
   "silence by adding parentheses to mark code as explicitly dead">;
 
@@ -9578,9 +9581,6 @@
   "fallthrough annotation is outside switch statement">;
 def err_fallthrough_attr_invalid_placement : Error<
   "fallthrough annotation does not directly precede switch label">;
-def warn_fallthrough_attr_unreachable : Warning<
-  "fallthrough annotation in unreachable code">,
-  InGroup, DefaultIgnore;
 
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -821,8 +821,10 @@
 //  under separate flags.
 //
 def UnreachableCodeLoopIncrement : 
DiagGroup<"unreachable-code-loop-increment">;
+def UnreachableCodeFallthrough : DiagGroup<"unreachable-code-fallthrough">;
 def UnreachableCode : DiagGroup<"unreachable-code",
-[UnreachableCodeLoopIncrement]>;
+[UnreachableCodeLoopIncrement,
+ UnreachableCodeFallthrough]>;
 def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
 def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
 def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -1,4 +1,4 @@
-

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I have sent the following patches for the Linux kernel. These were the only 
instances of the warning that I found across the code base in all of the 
configurations that we usually test so thank you for the heads up so I could 
send fixes before this lands.

https://lore.kernel.org/r/20210814234248.1755703-1-nat...@kernel.org/
https://lore.kernel.org/r/20210814235625.1780033-1-nat...@kernel.org/
https://lore.kernel.org/r/20210815004154.1781834-1-nat...@kernel.org/

Is there a reason `-Wbool-operation` is not in `-Wall`? It is with GCC 
(although now, their `-Wbool-operation` is equivalent to just 
`Wbool-operation-negation`. I know that is tangential to this patch but if we 
want to take advantage of this new warning, we will have to explicitly enable 
`-Wbool-operation` in the kernel's Makefile.


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

https://reviews.llvm.org/D108003

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance marked an inline comment as done.
nathanchance added inline comments.



Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:211-214
+case 224:
+  n += 400;
+case 225: // expected-warning{{unannotated fall-through between switch 
labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this 
warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;

nickdesaulniers wrote:
> These 4 lines don't add anything to the test coverage.  Remove them?
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 366363.
nathanchance added a comment.

- -Wimplicit-fallthrough-unreachable -> -Wunreachable-code-fallthrough, enabled 
under -Wunreachable-code (Dávid, Aaron).

- Drop "case 225" from new test, as it does not add to test coverage (Nick).

- Add -Wunreachable-code-fallthrough to PR30636 test command, as it is 
explicitly testing that there is no unreachable warning with template function 
instantiation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaCXX/P30636.cpp
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 
 
 int fallthrough(int n) {
@@ -193,6 +193,26 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-fallthrough"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+  }
+#pragma clang diagnostic pop
+
   long p = static_cast(n) * n;
   switch (sizeof(p)) {
 case 9:
Index: clang/test/SemaCXX/P30636.cpp
===
--- clang/test/SemaCXX/P30636.cpp
+++ clang/test/SemaCXX/P30636.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough 
-Wunreachable-code-fallthrough %s
 // expected-no-diagnostics
 
 template
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1125,7 +1125,7 @@
 // unreachable in all instantiations of the template.
 if (!IsTemplateInstantiation)
   S.Diag(AS->getBeginLoc(),
- diag::warn_fallthrough_attr_unreachable);
+ diag::warn_unreachable_fallthrough_attr);
 markFallthroughVisited(AS);
 ++AnnotatedCnt;
 break;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -682,6 +682,9 @@
 def warn_unreachable_loop_increment : Warning<
   "loop will run at most once (loop increment never executed)">,
   InGroup, DefaultIgnore;
+def warn_unreachable_fallthrough_attr : Warning<
+  "fallthrough annotation in unreachable code">,
+  InGroup, DefaultIgnore;
 def note_unreachable_silence : Note<
   "silence by adding parentheses to mark code as explicitly dead">;
 
@@ -9578,9 +9581,6 @@
   "fallthrough annotation is outside switch statement">;
 def err_fallthrough_attr_invalid_placement : Error<
   "fallthrough annotation does not directly precede switch label">;
-def warn_fallthrough_attr_unreachable : Warning<
-  "fallthrough annotation in unreachable code">,
-  InGroup, DefaultIgnore;
 
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -819,8 +819,10 @@
 //  under separate flags.
 //
 def UnreachableCodeLoopIncrement : 
DiagGroup<"unreachable-code-loop-increment">;
+def UnreachableCodeFallthrough : DiagGroup<"unreachable-code-fallthrough">;
 def UnreachableCode : DiagGroup<"unreachable-code",
-[UnreachableCodeLoopIncrement]>;
+[UnreachableCodeLoopIncrement,
+ UnreachableCodeFallthrough]>;
 def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
 def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
 def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
=

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D108003#2944413 , @Quuxplusone 
wrote:

> FWIW, all three of @nathanchance's detected lines look like good true 
> positives to me: even if they're not //bugs//, they all look like places the 
> programmer //meant// to write `&&` and only wrote `&` by accident. The third 
> one might even be a bug bug, since it's doing essentially `(bounds-check 
> offset_1) & (pointer-math-on offset_1)`.

Agreed, I do plan to send patches to fix these up. I will test the warning 
against a wider range of code later to help evaluate how noisy it is and look 
for false positives.


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

https://reviews.llvm.org/D108003

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D107933#2942430 , @xbolva00 wrote:

> Yes, something like that, plus I think you want put 
> UnreachableCodeFallthrough into group UnreachableCode as well.

So you would recommend adding it to `UnreachableCode` rather than 
`UnreachableCodeAggressive`?

In D107933#2942432 , @dblaikie wrote:

> Probably still worth fixing the bug too? (though maybe not a priority once 
> it's moved out into -Wunreachable-code) - though the general fix, as 
> discussed on the bug (comment 18 and 19 about why this doesn't already 
> produce an unreachable-code warning), may require a significant amount of 
> work.

I guess not warning on fallthrough attributes that are preceded by an if 
statement with an integer constant would remove all problematic instances in 
the kernel I believe. I am just not sure how to put that into code :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

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

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

With several Linux kernel `defconfig` builds, I see the following instances of 
the warning:

  drivers/net/wireless/intel/iwlwifi/mvm/scan.c:835:3: warning: bitwise and of 
boolean expressions; did you mean logical and? [-Wbool-operation-and]
  drivers/staging/rtl8192u/r8192U_core.c:4268:20: warning: bitwise and of 
boolean expressions; did you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1043:7: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1075:30: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1294:7: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1353:30: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1932:7: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1956:22: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:1977:23: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]
  lib/zstd/compress.c:2024:29: warning: bitwise and of boolean expressions; did 
you mean logical and? [-Wbool-operation-and]

Which correspond to:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/intel/iwlwifi/mvm/scan.c#n830

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/rtl8192u/r8192U_core.c#n4268

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/zstd/compress.c#n1294

I can do `allmodconfig` builds later today.


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

https://reviews.llvm.org/D108003

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

So something more along the lines of

  diff
  diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
  index 1555a9ee2465..0aa9a82e3d11 100644
  --- a/clang/include/clang/Basic/DiagnosticGroups.td
  +++ b/clang/include/clang/Basic/DiagnosticGroups.td
  @@ -823,10 +823,12 @@ def UnreachableCode : DiagGroup<"unreachable-code",
   [UnreachableCodeLoopIncrement]>;
   def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
   def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
  +def UnreachableCodeFallthrough : DiagGroup<"unreachable-code-fallthrough">;
   def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",
   [UnreachableCode,
UnreachableCodeBreak,
  - UnreachableCodeReturn]>;
  + UnreachableCodeReturn,
  + UnreachableCodeFallthrough]>;
  
   // Aggregation warning settings.
  
  diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
  index bf857f58951b..f533076a6c77 100644
  --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
  +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
  @@ -682,6 +682,9 @@ def warn_unreachable_return : Warning<
   def warn_unreachable_loop_increment : Warning<
 "loop will run at most once (loop increment never executed)">,
 InGroup, DefaultIgnore;
  +def warn_unreachable_fallthrough_attr : Warning<
  +  "fallthrough annotation in unreachable code">,
  +  InGroup, DefaultIgnore;
   def note_unreachable_silence : Note<
 "silence by adding parentheses to mark code as explicitly dead">;
  
  @@ -9578,9 +9581,6 @@ def err_fallthrough_attr_outside_switch : Error<
 "fallthrough annotation is outside switch statement">;
   def err_fallthrough_attr_invalid_placement : Error<
 "fallthrough annotation does not directly precede switch label">;
  -def warn_fallthrough_attr_unreachable : Warning<
  -  "fallthrough annotation in unreachable code">,
  -  InGroup, DefaultIgnore;
  
   def warn_unreachable_default : Warning<
 "default label in switch which covers all enumeration values">,
  diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp 
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
  index aa2602c8d925..99ce143d3559 100644
  --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
  +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
  @@ -1125,7 +1125,7 @@ namespace {
   // unreachable in all instantiations of the template.
   if (!IsTemplateInstantiation)
 S.Diag(AS->getBeginLoc(),
  - diag::warn_fallthrough_attr_unreachable);
  + diag::warn_unreachable_fallthrough_attr);
   markFallthroughVisited(AS);
   ++AnnotatedCnt;
   break;

if I understand correctly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> In D107933#2942023 , @xbolva00 
> wrote:
>
>> 
>
> GCC does not warn (with common -Wall) for this case, right? I think Clang 
> should not as well.

Correct, GCC does not warn at all about unreachable fallthrough annotations as 
far as I am aware.

In D107933#2942102 , @dblaikie wrote:

> In D107933#2942023 , @xbolva00 
> wrote:
>
>> ImplicitFallthroughUnreachable could be enabled with -Wunreachable-code, if 
>> you think we should have it.
>
> Yeah, some of that was discussed on the bug: 
> https://bugs.llvm.org/show_bug.cgi?id=51094 & I'd still be in favor of that 
> sort of direction. I might go so far as to say: Maybe we should drop this 
> warning flag (and/or move it under -Wunreachable-code - questionable, I don't 
> think anyone's really using that, it's pretty noisy still, I think) entirely 
> even if no one's willing to reimplement it more robustly... not sure.

The kernel does not use `-Wunreachable-code` for this reason and several 
others; we have explored it in the past 
 and found that there 
were pretty much no instances where the warnings indicated a bug and kernel 
maintainers were irritated with some of the patches sent. So moving this 
warning under there (as `-Wunreachable-code-fallthrough`?) would work for us. I 
do not have a strong opinion though. I am more than happy to remove the warning 
entirely as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107933

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

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: dblaikie, rsmith, nickdesaulniers.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The Linux kernel has a macro called IS_ENABLED(), which evaluates to a
constant 1 or 0 based on Kconfig selections, allowing C code to be
unconditionally enabled or disabled at build time. For example:

int foo(struct *a, int b) {

  switch (b) {
  case 1:
  if (a->flag || !IS_ENABLED(CONFIG_64BIT))
  return 1;
  __attribute__((fallthrough));
  case 2:
  return 2;
  default:
  return 3;
  }

}

There is an unreachable warning about the fallthrough annotation in the
first case because !IS_ENABLED(CONFIG_64BIT) can be evaluated to 1,
which looks like

  return 1;
  __attribute__((fallthrough));

to clang.

This type of warning is pointless for the Linux kernel because it does
this trick all over the place due to the sheer number of configuration
options that it has.

Add -Wimplicit-fallthrough-unreachable, which is default enabled with
-Wimplicit-fallthrough to keep the status quo but allows projects to opt
out of the warning when they know these annotations may be unreachable
in certain conditions.

Fixes PR51094.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107933

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -193,6 +193,28 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wimplicit-fallthrough-unreachable"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+case 225: // expected-warning{{unannotated fall-through between switch 
labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this 
warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;
+  }
+#pragma clang diagnostic pop
+
   long p = static_cast(n) * n;
   switch (sizeof(p)) {
 case 9:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9580,7 +9580,7 @@
   "fallthrough annotation does not directly precede switch label">;
 def warn_fallthrough_attr_unreachable : Warning<
   "fallthrough annotation in unreachable code">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -671,8 +671,11 @@
 def Switch : DiagGroup<"switch">;
 def ImplicitFallthroughPerFunction :
   DiagGroup<"implicit-fallthrough-per-function">;
+def ImplicitFallthroughUnreachable :
+  DiagGroup<"implicit-fallthrough-unreachable">;
 def ImplicitFallthrough  : DiagGroup<"implicit-fallthrough",
- [ImplicitFallthroughPerFunction]>;
+ [ImplicitFallthroughPerFunction,
+  ImplicitFallthroughUnreachable]>;
 def InvalidPPToken : DiagGroup<"invalid-pp-token">;
 def Trigraphs  : DiagGroup<"trigraphs">;
 


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -193,6 +193,28 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wimplicit-fallthrough-unreachable"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+case 225: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' t

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D100581#2721430 , @nickdesaulniers 
wrote:

> Huh, maybe not: https://godbolt.org/z/PnE1fMGWo

This difference has caused some confusion already for the Linux kernel: 
https://lore.kernel.org/r/202104280827.lsczw8xg-...@intel.com

I am not sure it is appropriate to warn there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D94355#2717958 , @gulfem wrote:

> @nathanchance do you prefer me to revert the patch first as it might take me 
> a while to investigate it?
> Btw, I was able to reproduce the issue by following your steps.

I would say if it is going to take longer than the end of the week to fix the 
issue, it might be nice to have it reverted so that other people's builds 
continue to work (I know a few people who build with full LTO in a two stage 
configuration every week).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D94355#2717813 , @gulfem wrote:

> Thanks for the report @nathanchance, and I'm looking at it now.

Thanks!

> Is this failure from one of the bots?

No, this was discovered by a user of my LLVM build script:

https://github.com/ClangBuiltLinux/tc-build

https://github.com/kdrag0n/proton-clang-build/runs/2371499466?check_suite_focus=true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This patch breaks a two stage build with LTO:

  $ git bisect log
  # bad: [f0bc2782f281ca05221d2f1735bbaff6c4b81ebb] [TTI] NFC: Remove unused 
'OptSize' parameter from shouldMaximizeVectorBandwidth
  # good: [9829f5e6b1bca9b61efc629770d28bb9014dec45] [CVP] 
@llvm.[us]{min,max}() intrinsics handling
  git bisect start 'f0bc2782f281ca05221d2f1735bbaff6c4b81ebb' 
'9829f5e6b1bca9b61efc629770d28bb9014dec45'
  # bad: [1af35e77f4b8c3314dc20a10d579b52f22c75a00] [TTI] NFC: Change 
getVectorInstrCost to return InstructionCost
  git bisect bad 1af35e77f4b8c3314dc20a10d579b52f22c75a00
  # bad: [45f8946a759a780e6131256d6d206977b9c128ee] [CodeView] Fix the ARM64 
CPUType enum
  git bisect bad 45f8946a759a780e6131256d6d206977b9c128ee
  # good: [0b439e4cc9dbb5c226121383b84d4f48ab669c55] [libc++] Split 
std::allocator out of 
  git bisect good 0b439e4cc9dbb5c226121383b84d4f48ab669c55
  # good: [2eb98d89ac866e32cb56727174e4d1c1413479c8] [mlir][spirv] Allow 
bitwidth emulation on runtime arrays
  git bisect good 2eb98d89ac866e32cb56727174e4d1c1413479c8
  # bad: [80aa9b0f7b3ebe53220a398b2939610d8a49e24b] [PowerPC] stop reverse mem 
op generation for some cases.
  git bisect bad 80aa9b0f7b3ebe53220a398b2939610d8a49e24b
  # good: [a8ab1f98d22cf15f39dd1c2ce77675e628fceb31] [Evaluator] Look through 
invariant.group intrinsics
  git bisect good a8ab1f98d22cf15f39dd1c2ce77675e628fceb31
  # bad: [30f591c3869f3bbe6eca1249dcef1b8337312de6] [lldb] Disable 
TestLaunchProcessPosixSpawn.py with reproducers
  git bisect bad 30f591c3869f3bbe6eca1249dcef1b8337312de6
  # good: [6c4f2508e4278ac789230cb05f2bb56a8a7297dc] Revert "[lldb] [gdb-remote 
client] Refactor handling qSupported"
  git bisect good 6c4f2508e4278ac789230cb05f2bb56a8a7297dc
  # bad: [e96df3e531f506eea75da0f13d0f8aa9a267f975] [Passes] Add relative 
lookup table converter pass
  git bisect bad e96df3e531f506eea75da0f13d0f8aa9a267f975
  # good: [1310a19af06262122a6e9e4f6fbbe9c39ebad76e] [mlir] Use MCJIT to fix 
integration tests
  git bisect good 1310a19af06262122a6e9e4f6fbbe9c39ebad76e
  # first bad commit: [e96df3e531f506eea75da0f13d0f8aa9a267f975] [Passes] Add 
relative lookup table converter pass

My reproduction steps, apologies if they are not reduced enough:

  $ mkdir -p build/stage{1,2}
  
  $ cmake \
 -B build/stage1 \
 -G Ninja \
 -DCMAKE_C_COMPILER=$(command -v clang) \
 -DCMAKE_CXX_COMPILER=$(command -v clang++) \
 -DLLVM_USE_LINKER=$(command -v ld.lld) \
 -DLLVM_ENABLE_PROJECTS="clang;lld" \
 -DLLVM_TARGETS_TO_BUILD=host \
 -DLLVM_CCACHE_BUILD=ON \
 -DCMAKE_BUILD_TYPE=Release \
 llvm
  ...
  
  $ ninja -C build/stage1 all
  ...
  
  $ cmake \
 -B build/stage2 \
 -G Ninja \
 -DCMAKE_AR=$PWD/build/stage1/bin/llvm-ar \
 -DCMAKE_C_COMPILER=$PWD/build/stage1/bin/clang \
 -DCLANG_TABLEGEN=$PWD/build/stage1/bin/clang-tblgen \
 -DCMAKE_CXX_COMPILER=$PWD/build/stage1/bin/clang++ \
 -DLLVM_USE_LINKER=$PWD/build/stage1/bin/ld.lld \
 -DLLVM_TABLEGEN=$PWD/build/stage1/bin/llvm-tblgen \
 -DCMAKE_RANLIB=$PWD/build/stage1/bin/llvm-ranlib \
 -DLLVM_ENABLE_PROJECTS=clang \
 -DLLVM_TARGETS_TO_BUILD=host \
 -DCMAKE_BUILD_TYPE=Release \
 -DLLVM_ENABLE_LTO=Full \
 llvm
  
  $ ninja -C build/stage2 lib/libclang-cpp.so.13git
  ...
  [2296/2296] Linking CXX shared library lib/libclang-cpp.so.13git
  FAILED: lib/libclang-cpp.so.13git
  ...
  ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol 
llvm::detail::unit >::value; recompile with -fPIC
  >>> defined in lto.tmp
  >>> referenced by ld-temp.o
  >>>   
lto.tmp:(.data.rel.ro..Lreltable._ZN5clang10TargetInfo21getTypeFormatModifierENS_23TransferrableTargetInfo7IntTypeE+0x8)
  
  ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol 
llvm::detail::unit >::value; recompile with -fPIC
  >>> defined in lto.tmp
  >>> referenced by ld-temp.o
  >>>   
lto.tmp:(.data.rel.ro..Lreltable._ZN5clang10TargetInfo21getTypeFormatModifierENS_23TransferrableTargetInfo7IntTypeE+0xC)
  
  ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol 
llvm::detail::unit >::value; recompile with -fPIC
  >>> defined in lto.tmp
  >>> referenced by ld-temp.o
  >>>   
lto.tmp:(.data.rel.ro..Lreltable._ZNK5clang21analyze_format_string14LengthModifier8toStringEv+0x8)
  
  ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol 
llvm::detail::unit >::value; recompile with -fPIC
  >>> defined in lto.tmp
  >>> referenced by ld-temp.o
  >>>   
lto.tmp:(.data.rel.ro..Lreltable._ZNK5clang21analyze_format_string14LengthModifier8toStringEv+0x3C)
  
  ld.lld: error: relocation R_X86_64_PC32 cannot be used against symbol 
llvm::detail::unit >::value; recompile with -fPIC
  >>> defined in lto.tmp
  >>> referenced by ld-temp.o
  >>>   
lto.tmp:(.data.rel.ro..Lreltable._ZN4llvm15MCSymbolRefExpr18getVariantKindNameENS0_11VariantKindE+0xC0)
  

[PATCH] D98881: [RISCV] Fix mcount name

[Nathan Chancellor 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 rGef58ae86ba77: [RISCV] Fix mcount name (authored by 
nathanchance).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98881

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/mcount.c


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,13 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -59,6 +59,7 @@
 WCharType = SignedInt;
 WIntType = UnsignedInt;
 HasRISCVVTypes = true;
+MCountName = "_mcount";
   }
 
   bool setCPU(const std::string &Name) override {
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -261,6 +261,9 @@
 case llvm::Triple::arm:
   this->MCountName = "__mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };
@@ -491,6 +494,9 @@
 case llvm::Triple::sparcv9:
   this->MCountName = "_mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,13 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_c

[PATCH] D98881: [RISCV] Fix mcount name

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 333185.
nathanchance added a comment.

Drop "riscv32" and "riscv64" from list of checked triples (thanks to Fangrui 
for catching it with a good explanation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98881

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/mcount.c


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,13 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -59,6 +59,7 @@
 WCharType = SignedInt;
 WIntType = UnsignedInt;
 HasRISCVVTypes = true;
+MCountName = "_mcount";
   }
 
   bool setCPU(const std::string &Name) override {
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -261,6 +261,9 @@
 case llvm::Triple::arm:
   this->MCountName = "__mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };
@@ -491,6 +494,9 @@
 case llvm::Triple::sparcv9:
   this->MCountName = "_mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,13 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -tripl

[PATCH] D98881: [RISCV] Fix mcount name

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 332811.
nathanchance retitled this revision from "[RISCV] Fix mcount name for Linux" to 
"[RISCV] Fix mcount name".
nathanchance edited the summary of this revision.
nathanchance added a comment.
Herald added subscribers: frasercrmck, apazos, jocewei, the_o, brucehoult, 
MartinMosbeck, edward-jones, zzheng, MaskRay, niosHD, sabuasal, johnrusso, rbar.

- Hoist MCountName assignment into RISCVTargetInfo
- Prevent MCountName from getting assigned in FreeBSD and OpenBSD targets
- Add more tests

I did not update Fuchsia or any other operating systems because it appears that 
the only supported configurations from GCC are -elf, -linux, -rtems, and 
-freebsd so I figured it was wise to only influence those. If anyone disagrees, 
let me know and I can update this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98881

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/mcount.c


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,15 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32 -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64 -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -59,6 +59,7 @@
 WCharType = SignedInt;
 WIntType = UnsignedInt;
 HasRISCVVTypes = true;
+MCountName = "_mcount";
   }
 
   bool setCPU(const std::string &Name) override {
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -261,6 +261,9 @@
 case llvm::Triple::arm:
   this->MCountName = "__mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };
@@ -491,6 +494,9 @@
 case llvm::Triple::sparcv9:
   this->MCountName = "_mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,15 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32 -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64 -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileChec

[PATCH] D99220: [RISCV] Fix mcount name

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb.
nathanchance requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

GCC's name for this symbol is _mcount, which the Linux kernel expects in
a few different place:

  $ echo 'int main(void) { return 0; }' | riscv32-linux-gcc -c -pg -o tmp.o -x 
c -
  
  $ llvm-objdump -dr tmp.o | grep mcount
  000c:  R_RISCV_CALL _mcount
  
  $ echo 'int main(void) { return 0; }' | riscv64-linux-gcc -c -pg -o tmp.o -x 
c -
  
  $ llvm-objdump -dr tmp.o | grep mcount
  000c:  R_RISCV_CALL _mcount
  
  $ echo 'int main(void) { return 0; }' | clang -c -pg -o tmp.o 
--target=riscv32-linux-gnu -x c -
  
  $ llvm-objdump -dr tmp.o | grep mcount
  000a:  R_RISCV_CALL_PLT mcount
  
  $ echo 'int main(void) { return 0; }' | clang -c -pg -o tmp.o 
--target=riscv64-linux-gnu -x c -
  
  $ llvm-objdump -dr tmp.o | grep mcount
  000a:  R_RISCV_CALL_PLT mcount

Set MCountName to "_mcount" in RISCVTargetInfo then prevent it from
getting overridden in certain OSTargetInfo constructors.

Signed-off-by: Nathan Chancellor 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99220

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/mcount.c


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,15 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32 -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64 -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -59,6 +59,7 @@
 WCharType = SignedInt;
 WIntType = UnsignedInt;
 HasRISCVVTypes = true;
+MCountName = "_mcount";
   }
 
   bool setCPU(const std::string &Name) override {
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -261,6 +261,9 @@
 case llvm::Triple::arm:
   this->MCountName = "__mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };
@@ -491,6 +494,9 @@
 case llvm::Triple::sparcv9:
   this->MCountName = "_mcount";
   break;
+case llvm::Triple::riscv32:
+case llvm::Triple::riscv64:
+  break;
 }
   }
 };


Index: clang/test/CodeGen/mcount.c
===
--- clang/test/CodeGen/mcount.c
+++ clang/test/CodeGen/mcount.c
@@ -12,6 +12,15 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s 

[PATCH] D98881: [RISCV] Fix mcount name for Linux

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Such a diff does not look too bad:

  diff --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
  index 539466c4f678..c32972c1e04e 100644
  --- a/clang/lib/Basic/Targets/OSTargets.h
  +++ b/clang/lib/Basic/Targets/OSTargets.h
  @@ -256,6 +256,8 @@ public:
   case llvm::Triple::ppcle:
   case llvm::Triple::ppc64:
   case llvm::Triple::ppc64le:
  +case llvm::Triple::riscv32:
  +case llvm::Triple::riscv64:
 this->MCountName = "_mcount";
 break;
   case llvm::Triple::arm:
  @@ -488,6 +490,8 @@ public:
   case llvm::Triple::ppc:
   case llvm::Triple::ppc64:
   case llvm::Triple::ppc64le:
  +case llvm::Triple::riscv32:
  +case llvm::Triple::riscv64:
   case llvm::Triple::sparcv9:
 this->MCountName = "_mcount";
 break;
  @@ -885,7 +889,14 @@ protected:
   public:
 FuchsiaTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
 : OSTargetInfo(Triple, Opts) {
  -this->MCountName = "__mcount";
  +switch (Triple.getArch()) {
  +case llvm::Triple::riscv32:
  +case llvm::Triple::riscv64:
  +  break;
  +default:
  +  this->MCountName = "__mcount";
  +  break;
  +}
   this->TheCXXABI.set(TargetCXXABI::Fuchsia);
 }
   };
  diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
  index abae51e75a19..8df6e05ebcd5 100644
  --- a/clang/lib/Basic/Targets/RISCV.h
  +++ b/clang/lib/Basic/Targets/RISCV.h
  @@ -59,6 +59,7 @@ public:
   WCharType = SignedInt;
   WIntType = UnsignedInt;
   HasRISCVVTypes = true;
  +MCountName = "_mcount";
 }
   
 bool setCPU(const std::string &Name) override {
  diff --git a/clang/test/CodeGen/mcount.c b/clang/test/CodeGen/mcount.c
  index 649f0b56949d..2883a6370d34 100644
  --- a/clang/test/CodeGen/mcount.c
  +++ b/clang/test/CodeGen/mcount.c
  @@ -12,6 +12,14 @@
   // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
   // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
   // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s 
| FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv32 -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv64 -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv32-freebsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv64-fuschia -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
  +// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
   // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck 
-check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
   // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
   // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98881

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