newlib: riscv: Fix build

Message ID 20250325192003.220010-1-ericsalem@gmail.com
State New
Headers
Series newlib: riscv: Fix build |

Commit Message

Eric Salem March 25, 2025, 7:20 p.m. UTC
  The sys/asm.h header file is included for certain assembly files, so
move the typedef to a separate header file due to the build breaking on
some systems. Also include the port's string.h header file instead of
the system's version.

Addresses: https://sourceware.org/pipermail/newlib/2025/021591.html
Fixes: c3b9bb173c8c ("newlib: riscv: Add XLEN typedef and clean up types")
Reported-by: Jeff Law <jlaw@ventanamicro.com>
Suggested-by: Kito Cheng <kito.cheng@gmail.com>
Signed-off-by: Eric Salem <ericsalem@gmail.com>
---
 newlib/libc/machine/riscv/stpcpy.c      |  2 +-
 newlib/libc/machine/riscv/strcpy.c      |  2 +-
 newlib/libc/machine/riscv/strlen.c      |  4 ++--
 newlib/libc/machine/riscv/sys/asm.h     |  4 ----
 newlib/libc/machine/riscv/sys/string.h  |  2 +-
 newlib/libc/machine/riscv/sys/xlenint.h | 14 ++++++++++++++
 6 files changed, 19 insertions(+), 9 deletions(-)
 create mode 100644 newlib/libc/machine/riscv/sys/xlenint.h
  

Comments

Kito Cheng March 26, 2025, 1:39 a.m. UTC | #1
Just one more comment, don't put xlenint.h in
newlib/libc/machine/riscv/sys, please move that into
newlib/libc/machine/riscv
The difference between the two is that the former will be installed
into the toolchain, but the latter one won't.

Otherwise LGTM, also verified on my side :)


On Wed, Mar 26, 2025 at 3:20 AM Eric Salem <ericsalem@gmail.com> wrote:
>
> The sys/asm.h header file is included for certain assembly files, so
> move the typedef to a separate header file due to the build breaking on
> some systems. Also include the port's string.h header file instead of
> the system's version.
>
> Addresses: https://sourceware.org/pipermail/newlib/2025/021591.html
> Fixes: c3b9bb173c8c ("newlib: riscv: Add XLEN typedef and clean up types")
> Reported-by: Jeff Law <jlaw@ventanamicro.com>
> Suggested-by: Kito Cheng <kito.cheng@gmail.com>
> Signed-off-by: Eric Salem <ericsalem@gmail.com>
> ---
>  newlib/libc/machine/riscv/stpcpy.c      |  2 +-
>  newlib/libc/machine/riscv/strcpy.c      |  2 +-
>  newlib/libc/machine/riscv/strlen.c      |  4 ++--
>  newlib/libc/machine/riscv/sys/asm.h     |  4 ----
>  newlib/libc/machine/riscv/sys/string.h  |  2 +-
>  newlib/libc/machine/riscv/sys/xlenint.h | 14 ++++++++++++++
>  6 files changed, 19 insertions(+), 9 deletions(-)
>  create mode 100644 newlib/libc/machine/riscv/sys/xlenint.h
>
> diff --git a/newlib/libc/machine/riscv/stpcpy.c b/newlib/libc/machine/riscv/stpcpy.c
> index 9243457b25a2..0c545623ba9e 100644
> --- a/newlib/libc/machine/riscv/stpcpy.c
> +++ b/newlib/libc/machine/riscv/stpcpy.c
> @@ -1,5 +1,5 @@
> -#include <string.h>
>  #include <stdbool.h>
> +#include "sys/string.h"
>
>  char *stpcpy(char *dst, const char *src)
>  {
> diff --git a/newlib/libc/machine/riscv/strcpy.c b/newlib/libc/machine/riscv/strcpy.c
> index f770493fbc2d..856b66ebc801 100644
> --- a/newlib/libc/machine/riscv/strcpy.c
> +++ b/newlib/libc/machine/riscv/strcpy.c
> @@ -9,8 +9,8 @@
>     http://www.opensource.org/licenses.
>  */
>
> -#include <string.h>
>  #include <stdbool.h>
> +#include "sys/string.h"
>
>  char *strcpy(char *dst, const char *src)
>  {
> diff --git a/newlib/libc/machine/riscv/strlen.c b/newlib/libc/machine/riscv/strlen.c
> index 7e5d41617eac..398c4b426676 100644
> --- a/newlib/libc/machine/riscv/strlen.c
> +++ b/newlib/libc/machine/riscv/strlen.c
> @@ -9,9 +9,9 @@
>     http://www.opensource.org/licenses.
>  */
>
> -#include <string.h>
> +#include <stddef.h>
>  #include <stdint.h>
> -#include "sys/asm.h"
> +#include "sys/string.h"
>
>  size_t strlen(const char *str)
>  {
> diff --git a/newlib/libc/machine/riscv/sys/asm.h b/newlib/libc/machine/riscv/sys/asm.h
> index 0a354b220517..8c8aeb3ae775 100644
> --- a/newlib/libc/machine/riscv/sys/asm.h
> +++ b/newlib/libc/machine/riscv/sys/asm.h
> @@ -12,8 +12,6 @@
>  #ifndef _SYS_ASM_H
>  #define _SYS_ASM_H
>
> -#include <stdint.h>
> -
>  /*
>   * Macros to handle different pointer/register sizes for 32/64-bit code
>   */
> @@ -22,13 +20,11 @@
>  # define SZREG 8
>  # define REG_S sd
>  # define REG_L ld
> -typedef uint64_t uintxlen_t;
>  #elif __riscv_xlen == 32
>  # define PTRLOG 2
>  # define SZREG 4
>  # define REG_S sw
>  # define REG_L lw
> -typedef uint32_t uintxlen_t;
>  #else
>  # error __riscv_xlen must equal 32 or 64
>  #endif
> diff --git a/newlib/libc/machine/riscv/sys/string.h b/newlib/libc/machine/riscv/sys/string.h
> index b65635cb6cb6..f72ffa6caac1 100644
> --- a/newlib/libc/machine/riscv/sys/string.h
> +++ b/newlib/libc/machine/riscv/sys/string.h
> @@ -13,7 +13,7 @@
>  #define _SYS_STRING_H
>
>  #include <stdbool.h>
> -#include "asm.h"
> +#include "xlenint.h"
>
>  #if __riscv_zbb
>    #include <riscv_bitmanip.h>
> diff --git a/newlib/libc/machine/riscv/sys/xlenint.h b/newlib/libc/machine/riscv/sys/xlenint.h
> new file mode 100644
> index 000000000000..37f0ac81f307
> --- /dev/null
> +++ b/newlib/libc/machine/riscv/sys/xlenint.h
> @@ -0,0 +1,14 @@
> +#ifndef _SYS_XLENINT_H
> +#define _SYS_XLENINT_H
> +
> +#include <stdint.h>
> +
> +#if __riscv_xlen == 64
> +typedef uint64_t uintxlen_t;
> +#elif __riscv_xlen == 32
> +typedef uint32_t uintxlen_t;
> +#else
> +# error __riscv_xlen must equal 32 or 64
> +#endif
> +
> +#endif /* sys/xlenint.h */
> --
> 2.49.0
>
  
Eric Salem March 26, 2025, 2:15 a.m. UTC | #2
Hi Kito,

On 3/25/25 8:39 PM, Kito Cheng wrote:
> Just one more comment, don't put xlenint.h in
> newlib/libc/machine/riscv/sys, please move that into
> newlib/libc/machine/riscv
> The difference between the two is that the former will be installed
> into the toolchain, but the latter one won't.
> 
> Otherwise LGTM, also verified on my side :)

I'm trying to move the file to that directory, but now the problem I'm
facing is sys/string.h in riscv can no longer find the file. Newlib
copies the header files to the targ-include directory when building,
but only those that are in the machine and sys directories. So when I go
to build, it can't find xlenint.h anymore.

Any suggestions? Perhaps I'm doing something wrong, but looking at the
file layout of other ports, this seems intentional by Newlib, where the
libc/machine/<port> directories contain C and assembly files, and all
header files go in subdirectories.

Eric
  
Eric Salem March 26, 2025, 1:28 p.m. UTC | #3
On 3/25/25 9:15 PM, Eric Salem wrote:
> Hi Kito,
> 
> On 3/25/25 8:39 PM, Kito Cheng wrote:
>> Just one more comment, don't put xlenint.h in
>> newlib/libc/machine/riscv/sys, please move that into
>> newlib/libc/machine/riscv
>> The difference between the two is that the former will be installed
>> into the toolchain, but the latter one won't.
>>
>> Otherwise LGTM, also verified on my side :)
> 
> I'm trying to move the file to that directory, but now the problem I'm
> facing is sys/string.h in riscv can no longer find the file. Newlib
> copies the header files to the targ-include directory when building,
> but only those that are in the machine and sys directories. So when I go
> to build, it can't find xlenint.h anymore.
> 
> Any suggestions? Perhaps I'm doing something wrong, but looking at the
> file layout of other ports, this seems intentional by Newlib, where the
> libc/machine/<port> directories contain C and assembly files, and all
> header files go in subdirectories.
> 
> Eric

It looks like if I add the path to Makefile.am, this could work. I'll dig
into it today.
  
Eric Salem March 26, 2025, 9:22 p.m. UTC | #4
On 3/26/25 8:28 AM, Eric Salem wrote:
> On 3/25/25 9:15 PM, Eric Salem wrote:
>> Hi Kito,
>>
>> On 3/25/25 8:39 PM, Kito Cheng wrote:
>>> Just one more comment, don't put xlenint.h in
>>> newlib/libc/machine/riscv/sys, please move that into
>>> newlib/libc/machine/riscv
>>> The difference between the two is that the former will be installed
>>> into the toolchain, but the latter one won't.
>>>
>>> Otherwise LGTM, also verified on my side :)
>>
>> I'm trying to move the file to that directory, but now the problem I'm
>> facing is sys/string.h in riscv can no longer find the file. Newlib
>> copies the header files to the targ-include directory when building,
>> but only those that are in the machine and sys directories. So when I go
>> to build, it can't find xlenint.h anymore.
>>
>> Any suggestions? Perhaps I'm doing something wrong, but looking at the
>> file layout of other ports, this seems intentional by Newlib, where the
>> libc/machine/<port> directories contain C and assembly files, and all
>> header files go in subdirectories.
>>
>> Eric
> 
> It looks like if I add the path to Makefile.am, this could work. I'll dig
> into it today.

I'm a little concerned about making this change. If you run the following
find command in newlib/libc/machine:

find -maxdepth 2 -iname "*.h"

You'll see all the header files contained in each newlib/libc/machine/<port>
directory:

./aarch64/asmdefs.h
./amdgcn/exit-value.h
./arc/asm.h
./arm/arm_asm.h
./h8300/defines.h
./h8300/setarch.h
./hppa/DEFS.h
./i386/i386mach.h
./m68k/m68kasm.h
./microblaze/mb_endian.h
./powerpc/fix64.h
./sh/asm.h
./spu/c99ppe.h
./spu/ea_internal.h
./spu/spu_timer_internal.h
./spu/straddr.h
./spu/strcpy.h
./spu/strncmp.h
./spu/vec_literal.h
./visium/memcpy.h
./visium/memset.h
./x86_64/x86_64mach.h
./xtensa/xtensa-asm.h
./z8k/args.h

If Makefile.am is updated to copy all header files in <port> to targ-include,
I don't know what the consequences would be for each port, and there's no
practical way for me to thoroughly test this.

To reiterate, it's not a C or assembly file in <port> that's including a header
file that would also be located in <port>. It's a header file (in this case)
in <port>/sys that needs to also include a header file currently located in
<port>/sys (currently how the patch would work).

What does everyone think? I think keeping the new xlenint.h header file in
<port>/sys would be the safer option.

Eric
  
Eric Salem March 27, 2025, 2:58 p.m. UTC | #5
On 3/26/25 4:22 PM, Eric Salem wrote:
> On 3/26/25 8:28 AM, Eric Salem wrote:
>> On 3/25/25 9:15 PM, Eric Salem wrote:
>>> Hi Kito,
>>>
>>> On 3/25/25 8:39 PM, Kito Cheng wrote:
>>>> Just one more comment, don't put xlenint.h in
>>>> newlib/libc/machine/riscv/sys, please move that into
>>>> newlib/libc/machine/riscv
>>>> The difference between the two is that the former will be installed
>>>> into the toolchain, but the latter one won't.
>>>>
>>>> Otherwise LGTM, also verified on my side :)
>>>
>>> I'm trying to move the file to that directory, but now the problem I'm
>>> facing is sys/string.h in riscv can no longer find the file. Newlib
>>> copies the header files to the targ-include directory when building,
>>> but only those that are in the machine and sys directories. So when I go
>>> to build, it can't find xlenint.h anymore.
>>>
>>> Any suggestions? Perhaps I'm doing something wrong, but looking at the
>>> file layout of other ports, this seems intentional by Newlib, where the
>>> libc/machine/<port> directories contain C and assembly files, and all
>>> header files go in subdirectories.
>>>
>>> Eric
>>
>> It looks like if I add the path to Makefile.am, this could work. I'll dig
>> into it today.
> 
> I'm a little concerned about making this change. If you run the following
> find command in newlib/libc/machine:
> 
> find -maxdepth 2 -iname "*.h"
> 
> You'll see all the header files contained in each newlib/libc/machine/<port>
> directory:
> 
> ./aarch64/asmdefs.h
> ./amdgcn/exit-value.h
> ./arc/asm.h
> ./arm/arm_asm.h
> ./h8300/defines.h
> ./h8300/setarch.h
> ./hppa/DEFS.h
> ./i386/i386mach.h
> ./m68k/m68kasm.h
> ./microblaze/mb_endian.h
> ./powerpc/fix64.h
> ./sh/asm.h
> ./spu/c99ppe.h
> ./spu/ea_internal.h
> ./spu/spu_timer_internal.h
> ./spu/straddr.h
> ./spu/strcpy.h
> ./spu/strncmp.h
> ./spu/vec_literal.h
> ./visium/memcpy.h
> ./visium/memset.h
> ./x86_64/x86_64mach.h
> ./xtensa/xtensa-asm.h
> ./z8k/args.h
> 
> If Makefile.am is updated to copy all header files in <port> to targ-include,
> I don't know what the consequences would be for each port, and there's no
> practical way for me to thoroughly test this.
> 
> To reiterate, it's not a C or assembly file in <port> that's including a header
> file that would also be located in <port>. It's a header file (in this case)
> in <port>/sys that needs to also include a header file currently located in
> <port>/sys (currently how the patch would work).
> 
> What does everyone think? I think keeping the new xlenint.h header file in
> <port>/sys would be the safer option.
> 
> Eric

Doing some more digging, when I run:

find -ipath "*/sys/*.h"

in newlib/lib/machine, I get some interesting results for spu:

./spu/sys/custom_file.h
./spu/sys/linux_syscalls.h

I don't think either of these are supposed to be in the sys directory.
However, Christian pointed out:

https://sourceware.org/pipermail/libc-alpha/2017-June/081918.html

mentions __libc_detect_null() shouldn't be present in a public header. So
what I'm going to do is move both sys/string.h and sys/xlenint.h one
directory above, update the includes for the various C and assembly files,
and send a v2 patch.

Jeff, if you don't mind, could you test the patch once it's ready? Just
want to ensure it doesn't break any of your builds.

Thanks,

Eric
  
Corinna Vinschen March 27, 2025, 3:15 p.m. UTC | #6
[Adding Jjohnston]

Hey Jeff,

can you please take a look?  THis concernes the build system...


Thanks,
Corinna


On Mar 27 09:58, Eric Salem wrote:
> On 3/26/25 4:22 PM, Eric Salem wrote:
> > On 3/26/25 8:28 AM, Eric Salem wrote:
> >> On 3/25/25 9:15 PM, Eric Salem wrote:
> >>> Hi Kito,
> >>>
> >>> On 3/25/25 8:39 PM, Kito Cheng wrote:
> >>>> Just one more comment, don't put xlenint.h in
> >>>> newlib/libc/machine/riscv/sys, please move that into
> >>>> newlib/libc/machine/riscv
> >>>> The difference between the two is that the former will be installed
> >>>> into the toolchain, but the latter one won't.
> >>>>
> >>>> Otherwise LGTM, also verified on my side :)
> >>>
> >>> I'm trying to move the file to that directory, but now the problem I'm
> >>> facing is sys/string.h in riscv can no longer find the file. Newlib
> >>> copies the header files to the targ-include directory when building,
> >>> but only those that are in the machine and sys directories. So when I go
> >>> to build, it can't find xlenint.h anymore.
> >>>
> >>> Any suggestions? Perhaps I'm doing something wrong, but looking at the
> >>> file layout of other ports, this seems intentional by Newlib, where the
> >>> libc/machine/<port> directories contain C and assembly files, and all
> >>> header files go in subdirectories.
> >>>
> >>> Eric
> >>
> >> It looks like if I add the path to Makefile.am, this could work. I'll dig
> >> into it today.
> > 
> > I'm a little concerned about making this change. If you run the following
> > find command in newlib/libc/machine:
> > 
> > find -maxdepth 2 -iname "*.h"
> > 
> > You'll see all the header files contained in each newlib/libc/machine/<port>
> > directory:
> > 
> > ./aarch64/asmdefs.h
> > ./amdgcn/exit-value.h
> > ./arc/asm.h
> > ./arm/arm_asm.h
> > ./h8300/defines.h
> > ./h8300/setarch.h
> > ./hppa/DEFS.h
> > ./i386/i386mach.h
> > ./m68k/m68kasm.h
> > ./microblaze/mb_endian.h
> > ./powerpc/fix64.h
> > ./sh/asm.h
> > ./spu/c99ppe.h
> > ./spu/ea_internal.h
> > ./spu/spu_timer_internal.h
> > ./spu/straddr.h
> > ./spu/strcpy.h
> > ./spu/strncmp.h
> > ./spu/vec_literal.h
> > ./visium/memcpy.h
> > ./visium/memset.h
> > ./x86_64/x86_64mach.h
> > ./xtensa/xtensa-asm.h
> > ./z8k/args.h
> > 
> > If Makefile.am is updated to copy all header files in <port> to targ-include,
> > I don't know what the consequences would be for each port, and there's no
> > practical way for me to thoroughly test this.
> > 
> > To reiterate, it's not a C or assembly file in <port> that's including a header
> > file that would also be located in <port>. It's a header file (in this case)
> > in <port>/sys that needs to also include a header file currently located in
> > <port>/sys (currently how the patch would work).
> > 
> > What does everyone think? I think keeping the new xlenint.h header file in
> > <port>/sys would be the safer option.
> > 
> > Eric
> 
> Doing some more digging, when I run:
> 
> find -ipath "*/sys/*.h"
> 
> in newlib/lib/machine, I get some interesting results for spu:
> 
> ./spu/sys/custom_file.h
> ./spu/sys/linux_syscalls.h
> 
> I don't think either of these are supposed to be in the sys directory.
> However, Christian pointed out:
> 
> https://sourceware.org/pipermail/libc-alpha/2017-June/081918.html
> 
> mentions __libc_detect_null() shouldn't be present in a public header. So
> what I'm going to do is move both sys/string.h and sys/xlenint.h one
> directory above, update the includes for the various C and assembly files,
> and send a v2 patch.
> 
> Jeff, if you don't mind, could you test the patch once it's ready? Just
> want to ensure it doesn't break any of your builds.
> 
> Thanks,
> 
> Eric
  
Eric Salem March 29, 2025, 11:19 p.m. UTC | #7
Hi Jeff and Kito,

On 3/27/25 9:58 AM, Eric Salem wrote:
> I don't think either of these are supposed to be in the sys directory.
> However, Christian pointed out:
> 
> https://sourceware.org/pipermail/libc-alpha/2017-June/081918.html
> 
> mentions __libc_detect_null() shouldn't be present in a public header. So
> what I'm going to do is move both sys/string.h and sys/xlenint.h one
> directory above, update the includes for the various C and assembly files,
> and send a v2 patch.
> 
> Jeff, if you don't mind, could you test the patch once it's ready? Just
> want to ensure it doesn't break any of your builds.
> 
> Thanks,
> 
> Eric

I made the aforementioned changes. I moved both xlenint.h and string.h
(also renamed the latter to avoid any confusion) to one directory above.
Now the __libc_* functions defined in that file are no longer being included
in a public header.

Could you please test your build systems again with the patch below? If it
works, I'll send this one as v2.

Thanks,

Eric

diff --git a/newlib/libc/machine/riscv/sys/string.h b/newlib/libc/machine/riscv/rv_string.h
similarity index 97%
rename from newlib/libc/machine/riscv/sys/string.h
rename to newlib/libc/machine/riscv/rv_string.h
index b65635cb6cb6..362f66a024bf 100644
--- a/newlib/libc/machine/riscv/sys/string.h
+++ b/newlib/libc/machine/riscv/rv_string.h
@@ -9,11 +9,11 @@
    http://www.opensource.org/licenses.
 */
 
-#ifndef _SYS_STRING_H
-#define _SYS_STRING_H
+#ifndef _RV_STRING_H
+#define _RV_STRING_H
 
 #include <stdbool.h>
-#include "asm.h"
+#include "xlenint.h"
 
 #if __riscv_zbb
   #include <riscv_bitmanip.h>
@@ -121,4 +121,4 @@ static __inline char *__libc_strcpy(char *dst, const char *src, bool ret_start)
 }
 
 
-#endif
+#endif /* _RV_STRING_H */
diff --git a/newlib/libc/machine/riscv/stpcpy.c b/newlib/libc/machine/riscv/stpcpy.c
index 9243457b25a2..14c32221179b 100644
--- a/newlib/libc/machine/riscv/stpcpy.c
+++ b/newlib/libc/machine/riscv/stpcpy.c
@@ -1,5 +1,5 @@
-#include <string.h>
 #include <stdbool.h>
+#include "rv_string.h"
 
 char *stpcpy(char *dst, const char *src)
 {
diff --git a/newlib/libc/machine/riscv/strcpy.c b/newlib/libc/machine/riscv/strcpy.c
index f770493fbc2d..a05ec1c0f8e7 100644
--- a/newlib/libc/machine/riscv/strcpy.c
+++ b/newlib/libc/machine/riscv/strcpy.c
@@ -9,8 +9,8 @@
    http://www.opensource.org/licenses.
 */
 
-#include <string.h>
 #include <stdbool.h>
+#include "rv_string.h"
 
 char *strcpy(char *dst, const char *src)
 {
diff --git a/newlib/libc/machine/riscv/strlen.c b/newlib/libc/machine/riscv/strlen.c
index 7e5d41617eac..9bfd2a136753 100644
--- a/newlib/libc/machine/riscv/strlen.c
+++ b/newlib/libc/machine/riscv/strlen.c
@@ -11,7 +11,7 @@
 
 #include <string.h>
 #include <stdint.h>
-#include "sys/asm.h"
+#include "rv_string.h"
 
 size_t strlen(const char *str)
 {
diff --git a/newlib/libc/machine/riscv/sys/asm.h b/newlib/libc/machine/riscv/sys/asm.h
index 0a354b220517..8c8aeb3ae775 100644
--- a/newlib/libc/machine/riscv/sys/asm.h
+++ b/newlib/libc/machine/riscv/sys/asm.h
@@ -12,8 +12,6 @@
 #ifndef _SYS_ASM_H
 #define _SYS_ASM_H
 
-#include <stdint.h>
-
 /*
  * Macros to handle different pointer/register sizes for 32/64-bit code
  */
@@ -22,13 +20,11 @@
 # define SZREG	8
 # define REG_S sd
 # define REG_L ld
-typedef uint64_t uintxlen_t;
 #elif __riscv_xlen == 32
 # define PTRLOG 2
 # define SZREG	4
 # define REG_S sw
 # define REG_L lw
-typedef uint32_t uintxlen_t;
 #else
 # error __riscv_xlen must equal 32 or 64
 #endif
diff --git a/newlib/libc/machine/riscv/xlenint.h b/newlib/libc/machine/riscv/xlenint.h
new file mode 100644
index 000000000000..86363a80655f
--- /dev/null
+++ b/newlib/libc/machine/riscv/xlenint.h
@@ -0,0 +1,14 @@
+#ifndef _XLENINT_H
+#define _XLENINT_H
+
+#include <stdint.h>
+
+#if __riscv_xlen == 64
+typedef uint64_t uintxlen_t;
+#elif __riscv_xlen == 32
+typedef uint32_t uintxlen_t;
+#else
+# error __riscv_xlen must equal 32 or 64
+#endif
+
+#endif /* _XLENINT_H */
  
Jeff Law April 2, 2025, 12:20 a.m. UTC | #8
On 3/29/25 5:19 PM, Eric Salem wrote:
> Hi Jeff and Kito,
> 
> On 3/27/25 9:58 AM, Eric Salem wrote:
>> I don't think either of these are supposed to be in the sys directory.
>> However, Christian pointed out:
>>
>> https://sourceware.org/pipermail/libc-alpha/2017-June/081918.html
>>
>> mentions __libc_detect_null() shouldn't be present in a public header. So
>> what I'm going to do is move both sys/string.h and sys/xlenint.h one
>> directory above, update the includes for the various C and assembly files,
>> and send a v2 patch.
>>
>> Jeff, if you don't mind, could you test the patch once it's ready? Just
>> want to ensure it doesn't break any of your builds.
>>
>> Thanks,
>>
>> Eric
> 
> I made the aforementioned changes. I moved both xlenint.h and string.h
> (also renamed the latter to avoid any confusion) to one directory above.
> Now the __libc_* functions defined in that file are no longer being included
> in a public header.
> 
> Could you please test your build systems again with the patch below? If it
> works, I'll send this one as v2.
Worked fine in my builder once I moved the older version out of the way ;-)

jeff
  
Eric Salem April 2, 2025, 1:56 a.m. UTC | #9
On 4/1/25 7:20 PM, Jeff Law wrote:
> On 3/29/25 5:19 PM, Eric Salem wrote:
>> Could you please test your build systems again with the patch below? If it
>> works, I'll send this one as v2.
> Worked fine in my builder once I moved the older version out of the way ;-)
> 
> jeff

:) Thanks, Jeff! I'll get v2 out shortly.
  

Patch

diff --git a/newlib/libc/machine/riscv/stpcpy.c b/newlib/libc/machine/riscv/stpcpy.c
index 9243457b25a2..0c545623ba9e 100644
--- a/newlib/libc/machine/riscv/stpcpy.c
+++ b/newlib/libc/machine/riscv/stpcpy.c
@@ -1,5 +1,5 @@ 
-#include <string.h>
 #include <stdbool.h>
+#include "sys/string.h"
 
 char *stpcpy(char *dst, const char *src)
 {
diff --git a/newlib/libc/machine/riscv/strcpy.c b/newlib/libc/machine/riscv/strcpy.c
index f770493fbc2d..856b66ebc801 100644
--- a/newlib/libc/machine/riscv/strcpy.c
+++ b/newlib/libc/machine/riscv/strcpy.c
@@ -9,8 +9,8 @@ 
    http://www.opensource.org/licenses.
 */
 
-#include <string.h>
 #include <stdbool.h>
+#include "sys/string.h"
 
 char *strcpy(char *dst, const char *src)
 {
diff --git a/newlib/libc/machine/riscv/strlen.c b/newlib/libc/machine/riscv/strlen.c
index 7e5d41617eac..398c4b426676 100644
--- a/newlib/libc/machine/riscv/strlen.c
+++ b/newlib/libc/machine/riscv/strlen.c
@@ -9,9 +9,9 @@ 
    http://www.opensource.org/licenses.
 */
 
-#include <string.h>
+#include <stddef.h>
 #include <stdint.h>
-#include "sys/asm.h"
+#include "sys/string.h"
 
 size_t strlen(const char *str)
 {
diff --git a/newlib/libc/machine/riscv/sys/asm.h b/newlib/libc/machine/riscv/sys/asm.h
index 0a354b220517..8c8aeb3ae775 100644
--- a/newlib/libc/machine/riscv/sys/asm.h
+++ b/newlib/libc/machine/riscv/sys/asm.h
@@ -12,8 +12,6 @@ 
 #ifndef _SYS_ASM_H
 #define _SYS_ASM_H
 
-#include <stdint.h>
-
 /*
  * Macros to handle different pointer/register sizes for 32/64-bit code
  */
@@ -22,13 +20,11 @@ 
 # define SZREG	8
 # define REG_S sd
 # define REG_L ld
-typedef uint64_t uintxlen_t;
 #elif __riscv_xlen == 32
 # define PTRLOG 2
 # define SZREG	4
 # define REG_S sw
 # define REG_L lw
-typedef uint32_t uintxlen_t;
 #else
 # error __riscv_xlen must equal 32 or 64
 #endif
diff --git a/newlib/libc/machine/riscv/sys/string.h b/newlib/libc/machine/riscv/sys/string.h
index b65635cb6cb6..f72ffa6caac1 100644
--- a/newlib/libc/machine/riscv/sys/string.h
+++ b/newlib/libc/machine/riscv/sys/string.h
@@ -13,7 +13,7 @@ 
 #define _SYS_STRING_H
 
 #include <stdbool.h>
-#include "asm.h"
+#include "xlenint.h"
 
 #if __riscv_zbb
   #include <riscv_bitmanip.h>
diff --git a/newlib/libc/machine/riscv/sys/xlenint.h b/newlib/libc/machine/riscv/sys/xlenint.h
new file mode 100644
index 000000000000..37f0ac81f307
--- /dev/null
+++ b/newlib/libc/machine/riscv/sys/xlenint.h
@@ -0,0 +1,14 @@ 
+#ifndef _SYS_XLENINT_H
+#define _SYS_XLENINT_H
+
+#include <stdint.h>
+
+#if __riscv_xlen == 64
+typedef uint64_t uintxlen_t;
+#elif __riscv_xlen == 32
+typedef uint32_t uintxlen_t;
+#else
+# error __riscv_xlen must equal 32 or 64
+#endif
+
+#endif /* sys/xlenint.h */