aaron.ballman added a comment. In D134461#3815298 <https://reviews.llvm.org/D134461#3815298>, @nathanchance wrote:
> 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. Thank you for letting us know! > Against an x86_64 allmodconfig build (which builds the majority of the code > in the kernel), I see: > > $ rg -c -- -Wvoid-ptr-dereference build.log > 1588001 > > 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 <martin.uec...@med.uni-goettingen.de> > */ > #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))) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This one is interesting to me -- I was on the fence about whether we want to diagnose this in an unevaluated context or not. The `clang/test/Analysis/misc-ps.m` test was why I was considering it, and I eventually convinced myself that the dereference there was a harmless UB when we support pointer arithmetic on void as an extension, but UB nonetheless. Now I wonder whether we want different behavior in the `sizeof` case -- it seems odd to me that we warn by default on `sizeof(*vp)` but not `sizeof(void)`: https://godbolt.org/z/PcMrW7b5W so I think we might want to silence `sizeof(*vp)` diagnostics. WDYT? > 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)) > > This is another interesting case involving an unevaluated operand. `typeof` doesn't evaluate its argument and so it seems somewhat defensible to allow `typeof(*vp)` for generic programming cases; it should just yield `void` as a type. > 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_of(priv, struct pci_host_bridge, private); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/container_of.h:20:21: note: expanded from macro > 'container_of' > __same_type(*(ptr), void), \ > ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/compiler_types.h:295:63: note: expanded from macro > '__same_type' > #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) > ^ > ./include/linux/build_bug.h:77:50: note: expanded from macro 'static_assert' > #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) > ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./include/linux/build_bug.h:78:56: note: expanded from macro > '__static_assert' > #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) > ^~~~ > > I am aware these are quite specialized cases so warning is likely > appropriate. If so, we can just disable this warning for the kernel but I > figured it was worth a comment in case the diagnostic can be improved in any > way. All this said, I think in both cases we want `-pedantic` to warn on these situations even if we would silence by default. e.g., void func(void *vp) { sizeof(void); // No warning by default, does get pedantic warning sizeof(*vp); // No warning by default, does get pedantic warning sizeof(*(vp))`; // No warning by default, does get pedantic warning void inner(typeof(*vp)); // No warning by default, does get pedantic warning (void)*vp; // Warning by default } What do folks think of that idea? 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
