Message ID | 20250325192003.220010-1-ericsalem@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <newlib-bounces~patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A1326385770B for <patchwork@sourceware.org>; Tue, 25 Mar 2025 19:20:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A1326385770B Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=TeTL/8eQ X-Original-To: newlib@sourceware.org Delivered-To: newlib@sourceware.org Received: from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com [IPv6:2607:f8b0:4864:20::72a]) by sourceware.org (Postfix) with ESMTPS id 001FD3858283 for <newlib@sourceware.org>; Tue, 25 Mar 2025 19:20:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 001FD3858283 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 001FD3858283 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::72a ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1742930406; cv=none; b=kkrUYJNBvQKnxz/AOQzgIby5/waak+mEPqsxYWAyWHolB0USjkyfgDZAAhvcY5FbER9K2wXrf93goDmwCkCVeV2YmJqPxvoaugF0UhYDP6fjl6PeD450YPGjlJ+/IH1GIo6oWMEy3GJ1lWNdWTYvhLghoiIXEIgOEgMgEjmbH/Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1742930406; c=relaxed/simple; bh=2BWLjq56OMZOmJ2r9cZChQFazc1saOFXdSLCKXfVaiQ=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=LeTS0IDfl3EpB4x+KgJs6CnXpKSD/ehws0ZjNXH6KTcoyWG9bvD8BvYfSBlop9LtE0nfHMA3KvuWCudrQGx+VnxpXdvTEuSTaNN7vufVi9k3DiUTRSXvLfJj59r7OWo2TzrGhi/xHxFjFdO6XKIe//REDReTFu/GoOVKquLFETw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 001FD3858283 Received: by mail-qk1-x72a.google.com with SMTP id af79cd13be357-7c5e1b40f68so70444185a.1 for <newlib@sourceware.org>; Tue, 25 Mar 2025 12:20:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1742930405; x=1743535205; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=DRZ/pzjngaHOQgutZeIevSSgmhPXY3FX5rLlrDfengQ=; b=TeTL/8eQHBCtRVxCX9Ep6AT3/Qgcvwtzf091BYCthw8iSctY+andzAkXKin2Tby91C tA2iWJG/l88MmUu2NpEZxX0UvVAHtRA/0FdpvV56VvZy7CY7auz0dYtD4E7bG5nEklnP KYeyU5Pm+znYSc27yK/ywLth5ddQgzrJm6w1ckDJM6nFymG7Np2bJITVmwv3qnOjpz9o UynbetDhFKZgHxu5ffcjobAkPNZQBX2OAoy8lwiK0Ko4JbxayT1CavRPvFLnCfkp6BWV 9JhB4EPWFqdcucY+FvKhS4tx1LtwiLdfS4+3fH0S0mGDJedDYY99+BOaAiczq3QGA4tk wbEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742930405; x=1743535205; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DRZ/pzjngaHOQgutZeIevSSgmhPXY3FX5rLlrDfengQ=; b=v4DoQdgQaU22qTl9VHtq1rfMqY067RxghK2GNsT0WO52H4R7341k8zkwDW0ncYPdPW JBAfm9XycesNIyx15OmFBlOWy2nFRAKpUsd8y7vApxVtCueZT0W0dxmyqTqjjkS9nwXB zrVD+6bmEh6EX3S6PNd5LnxEytWUcZQQixNOgpXiakgU9AyLSGy4Ixxspc/syrbe/vvf 271wQoMy6Fpqqfd0pSqyLyt9cX6lOvEl9qgvFhfdA5oJIVVVyMk8hkWLYVHkVfCmg6Is hb9C5usCAICpQEglSWH2xZ263ZfmqW8s5mzb7WvY5W2WnPepZoM/J+FfyJe++Z03VYKd 87ng== X-Gm-Message-State: AOJu0YwQuY+5l/8Aq7xAVS/hg6mSUZUzuj3lRknoivbMVquvq+Jddv3C ZESauO300cLidzUyTmaY9RLWKlyy48WIjugUp6gPThAM22ASoS61sa7ggLNz X-Gm-Gg: ASbGncuHpoyYmN6v3xMI+QZ0pByfjXIaSjL1UMWNUhEce1AOE58htqBeVZM7+SpLupJ Rd//nGVJkCD3M5OHSs7wf2nUMla/twPXM+N49/P/7tzr+i7hASvgweT02NBeXTo8EYl0pJHMg+q WqBSLYm2f1PtEbVUwbijkjBefVzUyRzFO0fyqHlyAGimJTnIVa3qzfvTNyWvbXPp7HfTz46E1TM JBi4hyqQ9repEZPmEfEoJEAn3GLTqkb8+MFFIBVx7xj4A4AoYo0X4leRAupLgVW9OaDSNqGjEiJ 6aFTaOw37kA4lSQaNwLvMlpgTQSIwSKXpL1D5/cxGbg4ky0+L/QKGr0h+PNeM30C3x/gAb6gFLD 4gw== X-Google-Smtp-Source: AGHT+IGerpUv3agG5jC5QF/RnKO3UXyjp5MWPvaea1QWrvJV6Lr88SJaE72aiUMb2A4qfCk3W5+yHA== X-Received: by 2002:a05:620a:170d:b0:7c5:6410:3a6 with SMTP id af79cd13be357-7c5ba17d196mr2716405385a.27.1742930404951; Tue, 25 Mar 2025 12:20:04 -0700 (PDT) Received: from fedora.. (c-73-176-204-61.hsd1.il.comcast.net. [73.176.204.61]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7c5b92b827csm671925685a.14.2025.03.25.12.20.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Mar 2025 12:20:04 -0700 (PDT) From: Eric Salem <ericsalem@gmail.com> To: newlib@sourceware.org Cc: Christian Herber <christian.herber@oss.nxp.com>, Jeff Law <jlaw@ventanamicro.com>, Kito Cheng <kito.cheng@gmail.com> Subject: [PATCH] newlib: riscv: Fix build Date: Tue, 25 Mar 2025 14:20:03 -0500 Message-ID: <20250325192003.220010-1-ericsalem@gmail.com> X-Mailer: git-send-email 2.49.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Newlib mailing list <newlib.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/newlib>, <mailto:newlib-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/newlib/> List-Post: <mailto:newlib@sourceware.org> List-Help: <mailto:newlib-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/newlib>, <mailto:newlib-request@sourceware.org?subject=subscribe> Errors-To: newlib-bounces~patchwork=sourceware.org@sourceware.org |
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
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 >
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
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.
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
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
[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
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 */
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
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.
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 */