[EXT] Re: Broken RISC-V code in newlib

[Christian Herber (OSS)]

I am a bit surprised that this slipped in. Are there no regression test run before applying patches?

> -----Original Message-----
> From: Eric Salem <ericsalem@gmail.com>
> Sent: Tuesday, 25 March 2025 15:35
> To: Kito Cheng <kito.cheng@gmail.com>
> Cc: Jeff Law <jlaw@ventanamicro.com>; newlib@sourceware.org
> Subject: [EXT] Re: Broken RISC-V code in newlib
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi Kito,
> 
> On 3/25/25 9:23 AM, Kito Cheng wrote:
> > Hi Eric:
> >
> >> diff --git a/newlib/libc/machine/riscv/sys/asm.h
> >> b/newlib/libc/machine/riscv/sys/asm.h
> >> index 0a354b220517..a9792e964ebc 100644
> >> --- a/newlib/libc/machine/riscv/sys/asm.h
> >> +++ b/newlib/libc/machine/riscv/sys/asm.h
> >> @@ -12,7 +12,9 @@
> >>  #ifndef _SYS_ASM_H
> >>  #define _SYS_ASM_H
> >>
> >> +#if !__ASSEMBLER__
> >>  #include <stdint.h>
> >> +#endif
> >>
> >>  /*
> >>   * Macros to handle different pointer/register sizes for 32/64-bit
> >> code @@ -22,13 +24,17 @@  # define SZREG 8  # define REG_S sd  #
> >> define REG_L ld
> >> +#if !__ASSEMBLER__
> >>  typedef uint64_t uintxlen_t;
> >> +#endif
> >>  #elif __riscv_xlen == 32
> >>  # define PTRLOG 2
> >>  # define SZREG 4
> >>  # define REG_S sw
> >>  # define REG_L lw
> >> +#if !__ASSEMBLER__
> >>  typedef uint32_t uintxlen_t;
> >> +#endif
> >>  #else
> >>  # error __riscv_xlen must equal 32 or 64  #endif
> >
> > Seems like sys/asm.h is not the right place to put those C typedef and
> > also including #include <stdint.h>
> >
> > Could you just create a xlenint.h in newlib/libc/machine/riscv/ then
> > include that by #include "xlenint.h" in those places you want to use?
> >
> > Since uintxlen_t is not defined in either standard C or RISC-V
> > conventions, so I would like to prevent exposing that to users if
> > possible :)
> 
> Sure, I can do that.
> 
> I think the system headers must be getting included in my case, and the local
> headers in Jeff's case, because I don't see how else I wouldn't be getting the
> compilation errors.
> 
> Eric