On Thu, Jul 28, 2022 at 5:35 AM Guo Ren <guoren@xxxxxxxxxx> wrote:
>
> Hi Palmer,
>
> The warning is from a clang problem.
>
> drivers/net/wireguard/queueing.c:
> static void __wg_prev_queue_enqueue(struct prev_queue *queue, struct
> sk_buff *skb)
> {
> WRITE_ONCE(NEXT(skb), NULL);
> WRITE_ONCE(NEXT(xchg_release(&queue->head, skb)), skb);
> }
>
> The queue->head is 64bit pointer size.
>
> #define __xchg_relaxed(ptr, new, size) \
> ({ \
> __typeof__(ptr) __ptr = (ptr); \
> __typeof__(new) __new = (new); \
> __typeof__(*(ptr)) __ret; \
> switch (size) { \
> case 2: { \
> ... Clang shouldn't give warning from here, because code won't enter the path.
> break; \
> } \
> case 4: \
> ...
> break; \
> case 8: \
> ... The case would enter this path.
> break; \
> default: \
> BUILD_BUG(); \
> } \
> __ret; \
> })
I assume it's this warning you are referring to?
>> drivers/net/wireguard/queueing.c:68:18: warning: cast to 'typeof (*((__ai_ptr)))' (aka 'struct sk_buff *') from smaller integer type 'unsigned int' [-Wint-to-pointer-cast]
WRITE_ONCE(NEXT(xchg_release(&queue->head, skb)), skb);
I don't consider this a bug in clang, it just performs the normal type checking
before dead code elimination and complains about code that clearly violates
the type rules.
I would suggest you split out the 16-bit xchg() into a properly typed inline
function and add type casts when calling it. In fact, I would love to
completely eliminate the 8-bit and 16-bit cases from the regular xchg()
and cmpxchg() interface and require all callers to explicitly call the
xchg16()/cmpxchg16() instead, as we require for cmpxchg64() on 32-bit
architectures already. This is something to do for another time though.
> > include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> > val = __raw_readb(PCI_IOBASE + addr);
> > ~~~~~~~~~~ ^
> > include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> > val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
Not your bug, but I see that CONFIG_MMU=n risc-v has the same bug that s390 has
with missing I/O space support. The correct workaround for this is to mark all
drivers using PCI I/O space as 'depends on HAS_IO_PORT' or similar and then
leaving out the definitions from the asm-generic header. Niklas Schnelle has
spent a lot of time working on patches for this, but they are somewhat stuck
in review. If RISC-V has the same problem, I hope we can get more people
interested in it. I think OpenRISC and C-Sky have this as well, but I'm not
sure if there is any plan to upstream clang support for those.
Arnd
