[PATCH] D56658: [MSP430] Add msp430 toochain

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: cfe/trunk/lib/CodeGen/CodeGenModule.cpp:141
+  (!CodeGenOpts.RelaxedAliasing && CodeGenOpts.OptimizationLevel > 0)) {
+fprintf(stderr, "TBAA enabled\n");
 TBAA.reset(new CodeGenTBAA(Context, TheModule, CodeGenOpts, getLangOpts(),

I'm just curious, was committing this change intentional? Seems like something 
for debugging purposes, rather than a useful diagnostic, since it doesn't 
appear to be related to the rest of the patch (as far as I can tell).

I see it quite a bit on a Linux kernel build: 
https://gist.github.com/nathanchance/0b2d3b16d5b30396038d86f5cba3


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56658



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

[PATCH] D56658: [MSP430] Add msp430 toochain

[Nathan Chancellor via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/CodeGenModule.cpp:141
+  (!CodeGenOpts.RelaxedAliasing && CodeGenOpts.OptimizationLevel > 0)) {
+fprintf(stderr, "TBAA enabled\n");
 TBAA.reset(new CodeGenTBAA(Context, TheModule, CodeGenOpts, getLangOpts(),

asl wrote:
> nathanchance wrote:
> > I'm just curious, was committing this change intentional? Seems like 
> > something for debugging purposes, rather than a useful diagnostic, since it 
> > doesn't appear to be related to the rest of the patch (as far as I can 
> > tell).
> > 
> > I see it quite a bit on a Linux kernel build: 
> > https://gist.github.com/nathanchance/0b2d3b16d5b30396038d86f5cba3
> Oh... Sorry, maybe unclean tree or something like this. Will fix!
Thanks for the quick reply, looks like Peter Collingbourne beat you to it :) 
https://reviews.llvm.org/rC351241


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56658



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

[PATCH] D62445: [test] Fix plugin tests

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: cfe/trunk/lib/Analysis/plugins/CMakeLists.txt:1
+if(LLVM_ENABLE_PLUGINS)
+  add_subdirectory(SampleAnalyzer)

I think this should have a dependency on `CLANG_ENABLE_STATIC_ANALYZER` like in 
`clang/test/CMakeLists.txt`, otherwise my build (which disables 
`CLANG_ENABLE_STATIC_ANALYZER`) fails because these plugins are being added to 
the build.

```
[2058/2605] Linking CXX shared module lib/CheckerOptionHandlingAnalyzerPlugin.so
FAILED: lib/CheckerOptionHandlingAnalyzerPlugin.so 
: && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
-fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w -fdiagnostics-color 
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
-fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete -fuse-ld=gold   -Wl,-O3 
-Wl,--gc-sections  
-Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandlingAnalyzerPlugin.exports"
 -shared  -o lib/CheckerOptionHandlingAnalyzerPlugin.so 
tools/clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeFiles/CheckerOptionHandlingAnalyzerPlugin.dir/CheckerOptionHandling.cpp.o
  -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a lib/libLLVMRemarks.a 
lib/libLLVMMC.a lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm 
lib/libLLVMDemangle.a && :
/usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
/usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
collect2: error: ld returned 1 exit status
[2059/2605] Building CXX object 
tools/clang/tools/clang-refactor/CMakeFiles/clang-refactor.dir/TestSupport.cpp.o
[2060/2605] Linking CXX shared module 
lib/CheckerDependencyHandlingAnalyzerPlugin.so
FAILED: lib/CheckerDependencyHandlingAnalyzerPlugin.so 
: && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
-fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w -fdiagnostics-color 
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
-fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete -fuse-ld=gold   -Wl,-O3 
-Wl,--gc-sections  
-Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandlingAnalyzerPlugin.exports"
 -shared  -o lib/CheckerDependencyHandlingAnalyzerPlugin.so 
tools/clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeFiles/CheckerDependencyHandlingAnalyzerPlugin.dir/CheckerDependencyHandling.cpp.o
  -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a lib/libLLVMRemarks.a 
lib/libLLVMMC.a lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm 
lib/libLLVMDemangle.a && :
/usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
/usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
collect2: error: ld returned 1 exit status
[2061/2605] Building CXX object 
tools/clang/lib/Tooling/CMakeFiles/obj.clangTooling.dir/CompilationDatabase.cpp.o
[2062/2605] Linking CXX shared module lib/SampleAnalyzerPlugin.so
FAILED: lib/SampleAnalyzerPlugin.so 
: && /usr/bin/g++ -fPIC -O2 -march=native -mtune=native -fPIC 
-fvisibility-inlines-hidden -Werror=date-time -std=c++11 -w -fdiagnostics-color 
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
-fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,nodelete -fuse-ld=gold   -Wl,-O3 
-Wl,--gc-sections  
-Wl,--version-script,"/tc-build2/build/llvm/stage1/tools/clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.exports"
 -shared  -o lib/SampleAnalyzerPlugin.so 
tools/clang/lib/Analysis/plugins/SampleAnalyzer/CMakeFiles/SampleAnalyzerPlugin.dir/MainCallChecker.cpp.o
  -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libclangAnalysis.a 
lib/libclangAST.a -lclangStaticAnalyzerCore -lclangStaticAnalyzerFrontend 
lib/libLLVMSupport.a lib/libclangASTMatchers.a lib/libclangAST.a 
lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a lib/libLLVMRemarks.a 
lib/libLLVMMC.a lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lz -lrt -ldl -lpthread -lm 
lib/libLLVMDemangle.a && :
/usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerCore
/usr/bin/ld.gold: error: cannot find -lclangStaticAnalyzerFrontend
collect2: error: ld returned 1 exit status
```


Repository:
  rL LLVM

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

https://revi

[PATCH] D62873: Avoid building analyzer plugins if CLANG_ENABLE_STATIC_ANALYZER is OFF

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Thanks for the quick fix, looks good to me!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62873



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

[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I tested this patch on top of r366728 and saw no regressions in my set of 
kernel builds.


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

https://reviews.llvm.org/D65108



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

[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This change breaks building the Linux kernel for arm32 (at least):

  ...
YACCscripts/dtc/dtc-parser.tab.c
HOSTCC  scripts/dtc/yamltree.o
HOSTCC  scripts/dtc/dtc-parser.tab.o
HOSTCC  scripts/dtc/dtc-lexer.lex.o
HOSTLD  scripts/dtc/dtc
UPD include/config/kernel.release
UPD include/generated/utsrelease.h
HOSTCC  scripts/asn1_compiler
HOSTCC  scripts/extract-cert
HOSTCC  scripts/sortextable
HOSTCC  scripts/kallsyms
HOSTCC  scripts/conmakehash
SYSNR   arch/arm/include/generated/asm/unistd-nr.h
SYSTBL  arch/arm/include/generated/calls-oabi.S
GEN arch/arm/include/generated/asm/mach-types.h
SYSTBL  arch/arm/include/generated/calls-eabi.S
HOSTCC  scripts/mod/mk_elfconfig
CC  scripts/mod/devicetable-offsets.s
CC  scripts/mod/empty.o
  clang-9: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,'
  make[2]: *** [scripts/Makefile.build:112: scripts/mod/devicetable-offsets.s] 
Error 1
  make[2]: *** Waiting for unfinished jobs
  clang-9: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,'
  make[2]: *** [scripts/Makefile.build:279: scripts/mod/empty.o] Error 1
  make[1]: *** [Makefile:1118: prepare0] Error 2
  make: *** [Makefile:325: __build_one_by_one] Error 2

The full command line that causes the issue is:

  /home/nathan/cbl/tc-build/build/llvm/stage1/bin/clang 
-Wp,-MD,scripts/mod/.empty.o.d  -nostdinc -isystem 
/home/nathan/cbl/tc-build/build/llvm/stage1/lib/clang/9.0.0/include 
-I./arch/arm/include -I./arch/arm/include/generated  -I./include 
-I./arch/arm/include/uapi -I./arch/arm/include/generated/uapi -I./include/uapi 
-I./include/generated/uapi -include ./include/linux/kconfig.h -include 
./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian 
-Qunused-arguments -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs 
-fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE 
-Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security 
-std=gnu89 --target=arm-linux-gnueabi --prefix=/home/nathan/cbl/usr/bin/ 
--gcc-toolchain=/home/nathan/cbl/usr -no-integrated-as 
-Werror=unknown-warning-option -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mfpu=vfp 
-funwind-tables -marm -Wa,-mno-warn-deprecated -D__LINUX_ARM_ARCH__=7 
-march=armv7-a -msoft-float -Uarm -O2 -fstack-protector-strong 
-Wno-format-invalid-specifier -Wno-gnu -Wno-tautological-compare 
-mno-global-merge -fomit-frame-pointer -Wdeclaration-after-statement -Wvla 
-Wno-pointer-sign -Wno-initializer-overrides -Wno-unused-value -Wno-format 
-Wno-sign-compare -Wno-format-zero-length-DKBUILD_BASENAME='"empty"' 
-DKBUILD_MODNAME='"empty"' -c -o scripts/mod/empty.o scripts/mod/empty.c

I'm not in the right state of mind (exhausted) to debug this but I wanted to 
let you know in case you have any immediate ideas.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64527



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This does not take into account `CLANG_DEFAULT_LINKER`, resulting in a 
`check-clang` failure:

  $ cmake -GNinja \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DLLVM_ENABLE_PROJECTS=clang \
-DPYTHON_EXECUTABLE=$(command -v python3) \
../llvm && ninja check-clang
  ...
  Testing Time: 163.80s
Expected Passes: 17055
Expected Failures  : 23
Unsupported Tests  : 55
  
  $ cmake -GNinja \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DLLVM_ENABLE_PROJECTS=clang \
-DPYTHON_EXECUTABLE=$(command -v python3) \
-DCLANG_DEFAULT_LINKER=lld \
../llvm && ninja check-clang
  ...
  Testing Time: 176.55s
  
  Failing Tests (1):
  Clang :: Driver/hexagon-toolchain-elf.c
  
Expected Passes: 17050
Expected Failures  : 23
Unsupported Tests  : 59
Unexpected Failures: 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> Weird -- the baseline didn't take CLANG_DEFAULT_LINKER into account either, 
> though, right?

I assume by baseline you mean the test before this change? It does not seem 
like it.

> Is it a regression?

I guess not but the test is still broken when `CLANG_DEFAULT_LINKER` is set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D77498: [Hexagon] Select lld as the default linker for linux-musl target

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> These are new tests how do you get the generic lld driver to work?

It doesn't, Clang invokes the appropriate driver in 
`ToolChain::GetLinkerPath()`: 
https://github.com/llvm/llvm-project/blob/6011627f5118dd64a0c33694b604c70e766d8c40/clang/lib/Driver/ToolChain.cpp#L537-L541


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77498



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

[PATCH] D81390: [KernelAddressSanitizer] Make globals constructors compatible with kernel

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I do not know enough about KASAN enough to review this patch but I can say that 
against mainline at 
https://git.kernel.org/linus/af7b4801030c07637840191c69eb666917e4135d, there 
appear to be no visible regressions with this version of the patch on top of 
caa2fddce72f2e8ca3d6346cc2c8fe85252b91d8 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81390



___
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] 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] 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.

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] 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.

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.

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#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] 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] 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] 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] 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] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This test case has never passed for me with `check-clang`, is there something 
environment related that is needed with this or are others experiencing this 
too?

  [147/148] Running the Clang regression tests
  llvm-lit: 
/home/nathan/cbl/git/tc-build/llvm-project/llvm/utils/lit/lit/llvm/config.py:340:
 note: using clang: /home/nathan/cbl/git/tc-build/build/llvm/stage1/bin/clang
  FAIL: Clang :: Driver/check-time-trace-sections.cpp (4955 of 15278)
   TEST 'Clang :: Driver/check-time-trace-sections.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 2';   
/home/nathan/cbl/git/tc-build/build/llvm/stage1/bin/clang --driver-mode=g++ -S 
-ftime-trace -ftime-trace-granularity=0 -o 
/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections
 
/home/nathan/cbl/git/tc-build/llvm-project/clang/test/Driver/check-time-trace-sections.cpp
  : 'RUN: at line 3';   cat 
/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.json
 | "/usr/bin/python" 
/home/nathan/cbl/git/tc-build/llvm-project/clang/test/Driver/check-time-trace-sections.py
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  Time trace json-file dumped to 
/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.json
  Use chrome://tracing or Speedscope App (https://www.speedscope.app) for 
flamegraph visualization
  Not all CodeGen sections are inside any Frontend section!
  
  --
  
  
  Testing Time: 38.46s
  
  Failing Tests (1):
  Clang :: Driver/check-time-trace-sections.cpp
  
Expected Passes: 14008
Expected Failures  : 15
Unsupported Tests  : 1254
Unexpected Failures: 1
  FAILED: tools/clang/test/CMakeFiles/check-clang 

This is on bdceb9fb14595d10f7d94e1dd950cf2d94d2f2d3 
 using 
this script 's command: 
`./build-llvm.py --build-stage1-only --check-targets clang --projects clang 
--targets X86`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63325



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

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Done, thanks for looking into this!

F9847579: check-time-trace-sections.json 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63325



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

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> Could it be an issue with python? What is the version you are using?

I would assume so...

  $ /usr/bin/python --version
  Python 3.7.4


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63325



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

[PATCH] D66873: [Test][Time profiler] Fix test for python3

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

The test case passes for me after this, thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66873



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

[PATCH] D66873: [Test][Time profiler] Fix test for python3

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance accepted this revision.
nathanchance added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66873



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

[PATCH] D66404: [CFG] Make destructor calls more accurate

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This commit causes some warnings in the Linux kernel that appear to be false 
positives. For example:

CC  drivers/base/cpu.o
  drivers/base/cpu.c:404:1: error: control may reach end of non-void function 
[-Werror,-Wreturn-type]
  }
  ^
  1 error generated.

Corresponds to this function 
:

  struct device *get_cpu_device(unsigned cpu)
  {
if (cpu < nr_cpu_ids && cpu_possible(cpu))
return per_cpu(cpu_sys_devices, cpu);
else
return NULL;
  }

creduce spits out:

  a() {
if (b())
  return ({
while (0)
  ;
0;
  });
else
  return 0;
  }


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66404



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

[PATCH] D66404: [CFG] Make destructor calls more accurate

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

@NoQ thank you for the fix, I can confirm it works!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66404



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

[PATCH] D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I am seeing the same failure that @leonardchan reported above. This is related 
to `-DCLANG_DEFAULT_LINKER=lld`:

  $ cmake -G Ninja \
-Wno-dev \
-DCMAKE_BUILD_TYPE=Release \
-DCLANG_DEFAULT_LINKER=lld \
-DPYTHON_EXECUTABLE=$(command -v python3) \
-DLLVM_ENABLE_PROJECTS="clang;lld" \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
../llvm && \
  ninja lld check-clang
  ...
  Failing Tests (1):
  Clang :: Driver/hexagon-toolchain-elf.c
  
Expected Passes: 16530
Expected Failures  : 21
Unsupported Tests  : 98
Unexpected Failures: 1
  ...
  $ cmake -G Ninja \
-Wno-dev \
-DCMAKE_BUILD_TYPE=Release \
-DPYTHON_EXECUTABLE=$(command -v python3) \
-DLLVM_ENABLE_PROJECTS="clang;lld" \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
../llvm && \
  ninja lld check-clang
  ...
Expected Passes: 16531
Expected Failures  : 21
Unsupported Tests  : 98
  ...

The test assumes that if `-fuse-ld` is not passed, we are not using `ld.lld` 
but that is clearly false when `-DCLANG_DEFAULT_LINKER=lld` is set. It seems 
like an explicit `-fuse-ld=ld` should be used so we are guaranteed to hit 
https://elixir.bootlin.com/llvm/llvmorg-10-init/source/clang/lib/Driver/ToolChain.cpp#L501?

  diff --git a/clang/test/Driver/hexagon-toolchain-elf.c 
b/clang/test/Driver/hexagon-toolchain-elf.c
  index cf7d8bdb393..c9b5e20c41a 100644
  --- a/clang/test/Driver/hexagon-toolchain-elf.c
  +++ b/clang/test/Driver/hexagon-toolchain-elf.c
  @@ -538,11 +538,12 @@
   // CHECK080:  "-Wreturn-type"
  
   // 
-
  -// Default, not passing -fuse-ld
  +// Default, using system linker
   // 
-
   // RUN: %clang -### -target hexagon-unknown-elf \
   // RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
   // RUN:   -mcpu=hexagonv60 \
  +// RUN:   -fuse-ld=ld \
   // RUN:   %s 2>&1 \
   // RUN:   | FileCheck -check-prefix=CHECK081 %s
   // REQUIRES: hexagon-registered-target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70919



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

[PATCH] D72668: [Driver][test] Fix Driver/hexagon-toolchain-elf.c for -DCLANG_DEFAULT_LINKER=lld builds

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance accepted this revision.
nathanchance added a comment.
This revision is now accepted and ready to land.

This fixes the error for me and looks better from a documentation perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72668



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

[PATCH] D89158: [NewPM] Provide method to run all pipeline callbacks, used for -O0

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

  $ cmake \
  -G Ninja \
  -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_C_COMPILER=$(command -v clang) \
  -DCMAKE_CXX_COMPILER=$(command -v clang++) \
  -DLLVM_CCACHE_BUILD=ON \
  -DLLVM_ENABLE_PROJECTS="clang;polly" \
  -DLLVM_TARGETS_TO_BUILD=X86 \
  -DLLVM_USE_LINKER=$(command -v ld.lld) \
  ../llvm
  
  $ ninja check-clang
  ...
  [0/1] Running the Clang regression tests
  -- Testing: 26861 tests, 64 workers --
  Testing: llvm-lit: /tmp/llvm-project/llvm/utils/lit/lit/llvm/config.py:347: 
note: using clang: /tmp/llvm-project/build/bin/clang
   0.. 10
  FAIL: Clang :: CodeGen/lto-newpm-pipeline.c (3731 of 26861)
   TEST 'Clang :: CodeGen/lto-newpm-pipeline.c' FAILED 

  Script:
  --
  : 'RUN: at line 3';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O0 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-FULL-O0
  : 'RUN: at line 5';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O0 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-THIN-O0
  : 'RUN: at line 7';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O1 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-FULL-OPTIMIZED
  : 'RUN: at line 9';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O1 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-THIN-OPTIMIZED
  : 'RUN: at line 11';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O2 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-FULL-OPTIMIZED
  : 'RUN: at line 13';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O2 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-THIN-OPTIMIZED
  : 'RUN: at line 15';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -O3 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-FULL-OPTIMIZED
  : 'RUN: at line 17';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=thin -O3 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c
-check-prefix=CHECK-THIN-OPTIMIZED
  : 'RUN: at line 19';   /tmp/llvm-project/build/bin/clang -cc1 
-internal-isystem /tmp/llvm-project/build/lib/clang/12.0.0/include 
-nostdsysteminc -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-fexperimental-new-pass-manager -fdebug-pass-manager -flto=full -Os 
/tmp/llvm-project/clang/test/CodeGen/lto-newpm-pipeline.c 2>&1 | 
/tmp/llvm-project/build/bin/FileCheck 
/tmp/llvm-project/clang/

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I think that not warning upon falling through to `goto` or `return` is a 
mistake. We have a confirmed case of a bug where `clang` would catch this bug 
whereas `gcc` would not: 
https://lore.kernel.org/lkml/20201121124019.21116-1-ogab...@kernel.org/. I 
suspect that warnings from falling through to `break` and `;` make up the vast 
majority of the noise in the Linux kernel and I highly doubt that those are 
bugs. The other two generally have code flow implications that should be 
annotated. I will modify this patch tonight and see how many warnings we get 
from `goto` and `return` statements against mainline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91895

___
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] 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] 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] 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] 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 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] 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.

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.

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 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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<

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] 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.

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] 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.

> 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] 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] 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] 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.

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] 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] 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] 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] 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] 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] 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]

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] 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] 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 a comment.

In D154696#4494899 , @cor3ntin wrote:

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154696

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

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

[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] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D72231#1881855 , @rjmccall wrote:

> In D72231#1881797 , @nickdesaulniers 
> wrote:
>
> > In D72231#1881784 , @rjmccall 
> > wrote:
> >
> > > In D72231#1881760 , 
> > > @nickdesaulniers wrote:
> > >
> > > > In D72231#1879347 , @rjmccall 
> > > > wrote:
> > > >
> > > > > In D72231#1878528 , 
> > > > > @nathanchance wrote:
> > > > >
> > > > > > There appear to a be semantic difference between GCC and clang with 
> > > > > > the current version of this patch which results in a lot of 
> > > > > > additional warnings in the Linux kernel: 
> > > > > > https://godbolt.org/z/eHFJd8
> > > > >
> > > > >
> > > > > Warning about casting to an enum seems clearly correct and in scope 
> > > > > for this warning.  Warning about casting to `_Bool` seems clearly 
> > > > > incorrect and should not be warned about at all.
> > > >
> > > >
> > > > Maybe we should only warn if the size of the `void*` is smaller than 
> > > > the size of the `enum`? (32b `void*`, 64b `enum`)? 
> > > > https://godbolt.org/z/oAts-u
> > > >
> > > > Otherwise this warning creates a massive mess for us to clean up, and I 
> > > > suspect Linux kernel developers will just end up disabling the warning.
> > >
> > >
> > > If deployment is easier if we split out a subgroup that we can turn off, 
> > > that seems fine.  But I don't see any good abstract justification for 
> > > warning about a cast to `int` and not a cast to an `int`-sized `enum`.  
> > > What would the reasoning be, just that the latter "couldn't possibly" be 
> > > intended to preserve the original pointer value, so it must be an opaque 
> > > value being represented as a `void*`?  That seems pretty weak to me.
> >
> >
> > Less about enums, more about casts to/from void*, since you might use that 
> > in place of a union that would be too large to describe.  Specifically, 
> > this `struct` is used throughout the kernel for most drivers: 
> > https://elixir.bootlin.com/linux/v5.5.4/source/include/linux/mod_devicetable.h#L260
> >   It is exceedingly common to stuff whatever data in there: 
> > https://elixir.bootlin.com/linux/v5.5.4/source/drivers/ata/ahci_brcm.c#L428 
> > so long as the driver is careful not to reinterpret the data as the 
> > incorrect type.  Describing such a union for ever possible enum packed in 
> > there would not be fun.
>
>
> No, I understand the pattern, but they must have already done some sort of 
> pass over the code to make it warning-clean when they're working with a 
> smaller integer type.  Or do they just in practice never store smaller 
> integers in there, whereas it's hard to control size with an enum?


Yes, if the data is a regular `int`, rather than an `enum`, all of the 
callsites either cast to `long` or `uintptr_t` (which is typedef'd in the 
kernel to `unsigned long`). There are a lot fewer of those spots in the kernel 
(at least from my super quick `rg` search), most of the spots use an `enum`, 
maybe to purposefully avoid this warning? Most, if not all the sites, only 
store a number that is less than 5 because they use that number to determine 
exactly which device is present from the match data so the driver can handle 
different quirks with things like case statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72231



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

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/test/Parser/asm-qualifiers.c:20
+
+void combinations(void) {
+  asm volatile inline("");

I'm probably being dense but what is intended to be tested differently between 
`combinations` and `permutations`? I assume the order of the qualifiers? 
Wouldn't it just be better to merge `combinations` into `permutations` or was 
there some deeper reasoning for the compartmentalization?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563



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

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added inline comments.



Comment at: clang/test/Parser/asm-qualifiers.c:20
+
+void combinations(void) {
+  asm volatile inline("");

nickdesaulniers wrote:
> nathanchance wrote:
> > I'm probably being dense but what is intended to be tested differently 
> > between `combinations` and `permutations`? I assume the order of the 
> > qualifiers? Wouldn't it just be better to merge `combinations` into 
> > `permutations` or was there some deeper reasoning for the 
> > compartmentalization?
> `combinations` tests a combination of different `asm-qualifiers` together. 
> `permutations` are just permutations of the combinations that have not been 
> tested above. I may not even have my nomenclature correct.  Shall I combine 
> them?
I assume that you want permutations since you want to make sure that the 
ordering does not matter, right? If you just care about combinations then

```
  asm inline goto volatile("" foo);
  asm inline volatile goto("" foo);

  asm goto inline volatile("" foo);
  asm goto volatile inline("" foo);

  asm volatile goto inline("" foo); // note, this one should probably be 
added in permutations
  asm volatile inline goto("" foo);
``` 

could just be distilled down to one of those since they are the same 
combination of qualifiers (combinations do not care about order). I would say 
that moving `combinations` into `permutations` would be wise since 
`permutations` tests the same thing that `combinations` does and more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563



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

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

As an FYI, this appears to cause several false positive warnings with the Linux 
kernel:

  ../drivers/video/fbdev/core/fbmem.c:665:3: warning: misleading indentation; 
statement is not part of the previous 'else' [-Wmisleading-indentation]
  if (fb_logo.depth > 4 && depth > 4) {
  ^
  ../drivers/video/fbdev/core/fbmem.c:661:2: note: previous statement is here
  else
  ^
  ../drivers/video/fbdev/core/fbmem.c:1075:3: warning: misleading indentation; 
statement is not part of the previous 'if' [-Wmisleading-indentation]
  return ret;
  ^
  ../drivers/video/fbdev/core/fbmem.c:1072:2: note: previous statement is here
  if (!ret)
  ^
  2 warnings generated.

Corresponding to:

https://elixir.bootlin.com/linux/v5.4.1/source/drivers/video/fbdev/core/fbmem.c#L665

and

https://elixir.bootlin.com/linux/v5.4.1/source/drivers/video/fbdev/core/fbmem.c#L1072


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> IMO that is misleading indentation. A single space followed by a tab? WTF

I guess I will take a look over all of the warnings that cropped up and see if 
this is the case for all of them. If it is, I will just send patches to fix 
those (with the justification of the warning and the fact that it would be a 
checkpatch warning), rather than adding `-ftabstop` to `KBUILD_CFLAGS`.

  $ ./scripts/checkpatch.pl --terse --types LEADING_SPACE -f 
drivers/video/fbdev/core/fbmem.c
  drivers/video/fbdev/core/fbmem.c:665: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:666: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:667: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:668: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:669: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:670: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:671: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:672: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:673: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:674: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:675: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:676: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:677: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:678: WARNING: please, no spaces at the start 
of a line
  drivers/video/fbdev/core/fbmem.c:1063: WARNING: please, no spaces at the 
start of a line
  drivers/video/fbdev/core/fbmem.c:1064: WARNING: please, no spaces at the 
start of a line
  drivers/video/fbdev/core/fbmem.c:1070: WARNING: please, no spaces at the 
start of a line
  drivers/video/fbdev/core/fbmem.c:1075: WARNING: please, no spaces at the 
start of a line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

As a follow up to my previous post, I have sent patches to clean up all of the 
warnings that I see in the Linux kernel. However, I found one that I do believe 
is a false positive:

  ../drivers/staging/uwb/allocator.c:353:3: warning: misleading indentation; 
statement is not part of the previous 'else' [-Wmisleading-indentation]
alloc_found:
^
  ../drivers/staging/uwb/allocator.c:350:2: note: previous statement is here
  else
  ^
  1 warning generated.

Corresponding to 
https://github.com/torvalds/linux/blob/2187f215ebaac73ddbd814696d7c7fa34f0c3de0/drivers/staging/uwb/allocator.c#L346-L353.

Simplified:

  $ cat test.c
  int a(int b, int c) {
if (b)
goto label;
  
if (c)
return 0;
  
label:
return 1;
  }
  
  $ clang -Wmisleading-indentation -o /dev/null -c test.c
  test.c:8:3: warning: misleading indentation; statement is not part of the 
previous 'if' [-Wmisleading-indentation]
label:
^
  test.c:5:2: note: previous statement is here
  if (c)
  ^
  1 warning generated.

goto labels are unaffected by indentation so there should be no warning here. 
While I think that the labels should be unindented for style, the driver is 
marked as obsolete and is scheduled to be deleted so I am not sure such a patch 
would be welcomed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: Mordante, rjmccall.
Herald added a project: clang.

GCC does not warn on casts from pointers to enumerators, while clang
currently does: https://godbolt.org/z/3DFDVG

This causes a bunch of extra warnings in the Linux kernel, where
certain structs contain a void pointer to avoid using a gigantic
union for all of the various types of driver data, such as
versions.

Add a diagnostic that allows certain projects like the kernel to
disable the warning just for enums, which allowst those projects to
keep full compatibility with GCC but keeps the intention of treating
casts to integers and enumerators the same by default so that other
projects have the opportunity to catch issues not noticed before (or
follow suite and disable the warning).

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75758

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/cast.c


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -186,3 +186,23 @@
 void *intToPointerCast3() {
   return (void*)(1 + 3);
 }
+
+void voidPointerToEnumCast(VoidPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'VoidPtr' (aka 'void *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // no-warning
+#pragma clang diagnostic pop
+}
+
+void pointerToEnumCast(CharPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+#pragma clang diagnostic pop
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2777,11 +2777,18 @@
   // If the result cannot be represented in the integer type, the behavior
   // is undefined. The result need not be in the range of values of any
   // integer type.
-  unsigned Diag = Self.getLangOpts().MicrosoftExt
-  ? diag::ext_ms_pointer_to_int_cast
-  : SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
+  unsigned Diag;
+  if (Self.getLangOpts().MicrosoftExt)
+Diag = diag::ext_ms_pointer_to_int_cast;
+  else if (SrcType->isVoidPointerType())
+if (DestType->isEnumeralType())
+  Diag = diag::warn_void_pointer_to_enum_cast;
+else
+  Diag = diag::warn_void_pointer_to_int_cast;
+  else if (DestType->isEnumeralType())
+Diag = diag::warn_pointer_to_enum_cast;
+  else
+Diag = diag::warn_pointer_to_int_cast;
   Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
 }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3669,9 +3669,15 @@
 def warn_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_pointer_to_enum_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;
 def warn_void_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_void_pointer_to_enum_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;
 def ext_ms_pointer_to_int_cast : ExtWarn<
   "cast to smaller integer type %1 from %0 is a Microsoft extension">,
 InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -838,9 +838,13 @@
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
-def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast">;
+def VoidPointerToEnumCast : DiagGroup<"void-pointer-to-enum-cast">;
+def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast",
+ [VoidPointerToEnumCast]>;
+def PointerToEnumCast : Di

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance updated this revision to Diff 248824.
nathanchance edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75758

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/cast.c


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -186,3 +186,23 @@
 void *intToPointerCast3() {
   return (void*)(1 + 3);
 }
+
+void voidPointerToEnumCast(VoidPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'VoidPtr' (aka 'void *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // no-warning
+#pragma clang diagnostic pop
+}
+
+void pointerToEnumCast(CharPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+#pragma clang diagnostic pop
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2777,11 +2777,18 @@
   // If the result cannot be represented in the integer type, the behavior
   // is undefined. The result need not be in the range of values of any
   // integer type.
-  unsigned Diag = Self.getLangOpts().MicrosoftExt
-  ? diag::ext_ms_pointer_to_int_cast
-  : SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
+  unsigned Diag;
+  if (Self.getLangOpts().MicrosoftExt)
+Diag = diag::ext_ms_pointer_to_int_cast;
+  else if (SrcType->isVoidPointerType())
+if (DestType->isEnumeralType())
+  Diag = diag::warn_void_pointer_to_enum_cast;
+else
+  Diag = diag::warn_void_pointer_to_int_cast;
+  else if (DestType->isEnumeralType())
+Diag = diag::warn_pointer_to_enum_cast;
+  else
+Diag = diag::warn_pointer_to_int_cast;
   Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
 }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3669,9 +3669,15 @@
 def warn_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_pointer_to_enum_cast : Warning<
+  warn_pointer_to_int_cast.Text>,
+  InGroup;
 def warn_void_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_void_pointer_to_enum_cast : Warning<
+  warn_void_pointer_to_int_cast.Text>,
+  InGroup;
 def ext_ms_pointer_to_int_cast : ExtWarn<
   "cast to smaller integer type %1 from %0 is a Microsoft extension">,
 InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -838,9 +838,13 @@
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
-def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast">;
+def VoidPointerToEnumCast : DiagGroup<"void-pointer-to-enum-cast">;
+def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast",
+ [VoidPointerToEnumCast]>;
+def PointerToEnumCast : DiagGroup<"pointer-to-enum-cast",
+  [VoidPointerToEnumCast]>;
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
- [VoidPointerToIntCast]>;
+ [PointerToEnumCast, VoidPointerToIntCast]>;
 
 def Move : DiagGroup<"move", [
 PessimizingMove,


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -186,3 +186,23 @@
 void *intToPointerCast3() {
   return (void*)(1 + 3);
 }
+
+void voidPointerToEnumCast(VoidPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 'VoidPtr' (aka 'void *')}}
+ 

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance marked 2 inline comments as done.
nathanchance added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3673
+def warn_pointer_to_enum_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;

xbolva00 wrote:
> You can reuse warning text:
> 
> warn_pointer_to_int_cast.Text
> 
> I think..
Thanks, that is indeed cleaner!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75758



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

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

[Nathan Chancellor via Phabricator via cfe-commits]

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

Thanks for the review and accepting. I do not have commit rights, would you 
mind doing that on my behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75758



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

[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

There appear to a be semantic difference between GCC and clang with the current 
version of this patch which results in a lot of additional warnings in the 
Linux kernel: https://godbolt.org/z/eHFJd8


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72231



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

[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D72231#1879347 , @rjmccall wrote:

> Warning about casting to an enum seems clearly correct and in scope for this 
> warning.  Warning about casting to `_Bool` seems clearly incorrect and should 
> not be warned about at all.


Fair enough. There are 97 unique instances of the enum variant of the warning 
across the various configs that I test so I will start sending patches to add 
`uintptr_t` casts to silence it.

https://gist.github.com/nathanchance/2c2892e9e4b411fa78770ed3624812b4


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72231



___
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] 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.

@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] 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