| Message ID | 20240526213617.12890-2-palmer@rivosinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | RISC-V: io: Don't have a void* PCI_IOBASE | expand |
On Mai 26 2024, Palmer Dabbelt wrote: > I recently started noticing warnings along the lines of > > include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > insw(addr, buffer, count); > ^~~~~~~~~~~~~~~~~~~~~~~~~ > arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' > #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) > > which are triggered by having PCI_IOBASE be a "void __iomem *". I'm not > quite sure what the right thing to do is here: having it as u8 to make > the pointer arithmetic work seems reasonable to me, A u8 null pointer is still a null pointer. Are you sure you are quoting the right warning? AFAICS, PCI_IOBASE is not a null pointer.
On Mon, May 27, 2024 at 09:44:13AM +0200, Andreas Schwab wrote: > On Mai 26 2024, Palmer Dabbelt wrote: > > > I recently started noticing warnings along the lines of > > > > include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > insw(addr, buffer, count); > > ^~~~~~~~~~~~~~~~~~~~~~~~~ > > arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' > > #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) > > > > which are triggered by having PCI_IOBASE be a "void __iomem *". I'm not > > quite sure what the right thing to do is here: having it as u8 to make > > the pointer arithmetic work seems reasonable to me, > > A u8 null pointer is still a null pointer. Are you sure you are quoting > the right warning? AFAICS, PCI_IOBASE is not a null pointer. Null pointer has nothing to do with this. The warning is about arithmetic operation on void*, which is undefined behavior. Compilers usually do arithmetic on void* the same way as for u8*, but that is not defined by C. Best regards, Nam
On Mon, May 27, 2024 at 09:55:15AM +0200, Nam Cao wrote: > On Mon, May 27, 2024 at 09:44:13AM +0200, Andreas Schwab wrote: > > On Mai 26 2024, Palmer Dabbelt wrote: > > > > > I recently started noticing warnings along the lines of > > > > > > include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] > > > insw(addr, buffer, count); > > > ^~~~~~~~~~~~~~~~~~~~~~~~~ > > > arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' > > > #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) > > > > > > which are triggered by having PCI_IOBASE be a "void __iomem *". I'm not > > > quite sure what the right thing to do is here: having it as u8 to make > > > the pointer arithmetic work seems reasonable to me, > > > > A u8 null pointer is still a null pointer. Are you sure you are quoting > > the right warning? AFAICS, PCI_IOBASE is not a null pointer. > > Null pointer has nothing to do with this. The warning is about arithmetic > operation on void*, which is undefined behavior. Wait, I am dumb. It is about null pointer. Pretend you didn't see my email. > > Compilers usually do arithmetic on void* the same way as for u8*, but that > is not defined by C. > > Best regards, > Nam
On Mon, 27 May 2024 00:44:13 PDT (-0700), schwab@suse.de wrote: > On Mai 26 2024, Palmer Dabbelt wrote: > >> I recently started noticing warnings along the lines of >> >> include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] >> insw(addr, buffer, count); >> ^~~~~~~~~~~~~~~~~~~~~~~~~ >> arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' >> #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) >> >> which are triggered by having PCI_IOBASE be a "void __iomem *". I'm not >> quite sure what the right thing to do is here: having it as u8 to make >> the pointer arithmetic work seems reasonable to me, > > A u8 null pointer is still a null pointer. Are you sure you are quoting > the right warning? AFAICS, PCI_IOBASE is not a null pointer. Ya, sorry, I just saw the void* UB and didn't even read to the "null"...
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h index 1c5c641075d2..a2d2d10c48cf 100644 --- a/arch/riscv/include/asm/io.h +++ b/arch/riscv/include/asm/io.h @@ -27,7 +27,7 @@ */ #ifdef CONFIG_MMU #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1) -#define PCI_IOBASE ((void __iomem *)PCI_IO_START) +#define PCI_IOBASE ((u8 __iomem *)PCI_IO_START) #endif /* CONFIG_MMU */ /*
I recently started noticing warnings along the lines of include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] insw(addr, buffer, count); ^~~~~~~~~~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw' #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count) which are triggered by having PCI_IOBASE be a "void __iomem *". I'm not quite sure what the right thing to do is here: having it as u8 to make the pointer arithmetic work seems reasonable to me, but a bunch of other ports still have it as "void __iomem *". Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> --- Not sure if I just started noticing these, but a bunch show up when merging Linus' master from this afternoon. Having some sort of fix here seems reasonable, as "void *" arithmetic is undefined. I didn't check if the other ports are suffering from this too, I figured I'd just send a patch so I don't forget about it. --- arch/riscv/include/asm/io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)