Message ID | 110083dce6019a1b98bda674d2e19acef4ed1dda.1591201405.git.alistair.francis@wdc.com |
---|---|
State | Committed |
Headers |
Return-Path: <libc-alpha-bounces@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 CA766388C021; Wed, 3 Jun 2020 16:34:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CA766388C021 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1591202066; bh=XFCNmasIZACVBsLwWYHZTCo/eCVq4oQiPlKlaoQp5Mw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=UMKyA4E7jbx0fHh6WhPFAiB5/fhpgNfDHDdRUDbWqG7Sd3+7Ksb9OU6vDYqAkavQ8 kHWCu9dglwbKbLaT44dpC8B76kMjRcvD0nFHktD1dVq08wE+LxfbC1VIZH5u53wUoe 9MM98UcJD5r81pzEOBYgl29Ipbd5DvDY1pSbep18= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from esa3.hgst.iphmx.com (esa3.hgst.iphmx.com [216.71.153.141]) by sourceware.org (Postfix) with ESMTPS id AB04A388C016 for <libc-alpha@sourceware.org>; Wed, 3 Jun 2020 16:34:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AB04A388C016 IronPort-SDR: nkaPjdlv8iLfOrwciVjVuKKZ/0v+MPucbUwuZXDXk/zMTgxEDIDS55hpaEsVafRM2zoS7R1oFg nDopj3vjH99m105ayNxf/p/iyI1zdSQHh5nB5gazxKGFuT+5+RP2wruLQErvjgm39rd6+5LU0b 1yifNZMNU6vPJUNedZN36wuJVHtgqi2fBKzP5cO6xEPL0a4zXtkisUsm2BEEmxg1cObAs/AIGY G+JBxGEoVYN9k99CAY1WvFhtrMvhaXm3QaNtBuWc1JYMoKXDK7K3hVWMLkP3OdgroMYUfx+SWK fk8= X-IronPort-AV: E=Sophos;i="5.73,468,1583164800"; d="scan'208";a="143452245" Received: from uls-op-cesaip01.wdc.com (HELO uls-op-cesaep01.wdc.com) ([199.255.45.14]) by ob1.hgst.iphmx.com with ESMTP; 04 Jun 2020 00:34:24 +0800 IronPort-SDR: vECvpmyGqLFVMGfnWHHKV9paAxvGX6jGBgNu/OizCxHSjHJxavLaApb6yM8L5ej1w9RAZV4CR3 Gyb0Tiy4RTwaDhAiOtLT/bpwQN5HPoUSo= Received: from uls-op-cesaip02.wdc.com ([10.248.3.37]) by uls-op-cesaep01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Jun 2020 09:24:01 -0700 IronPort-SDR: kYGKL5jU7d2zMROmwkIzMKPQMT9U+QPUU1iHL3RlhMI41/221UtH4HmzgsI65K34BkbEUnw+gn tXYuTyuvebiw== WDCIronportException: Internal Received: from cne220230.ad.shared (HELO risc6-mainframe.hgst.com) ([10.86.57.144]) by uls-op-cesaip02.wdc.com with ESMTP; 03 Jun 2020 09:34:23 -0700 To: libc-alpha@sourceware.org Subject: [PATCH v2 05/18] RISC-V: Add path of library directories for the 32-bit Date: Wed, 3 Jun 2020 09:25:37 -0700 Message-Id: <110083dce6019a1b98bda674d2e19acef4ed1dda.1591201405.git.alistair.francis@wdc.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <cover.1591201405.git.alistair.francis@wdc.com> References: <cover.1591201405.git.alistair.francis@wdc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-16.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <http://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <http://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Alistair Francis via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Alistair Francis <alistair.francis@wdc.com> Cc: alistair.francis@wdc.com Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
glibc port for 32-bit RISC-V (RV32)
|
|
Commit Message
Alistair Francis
June 3, 2020, 4:25 p.m. UTC
From: Zong Li <zongbox@gmail.com>
For the recommand of 64 bit version, we add the libraries path of 32 bit
in this patch.
The status of RV32 binaries under RV64 kernels is that the ISA
optionally supports having different XLEN for user and supervisor modes,
but AFAIK there's no silicon that implements this feature, and the Linux
kernel doesn't support it yet.
For the recommand of 64 bit version, we add the libraries path of 32 bit
in this patch. This includes a fix to avoid an out of bound array check
when building with GCC 8.2.
---
sysdeps/unix/sysv/linux/riscv/dl-cache.h | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
Comments
On Wed, 3 Jun 2020, Alistair Francis via Libc-alpha wrote: > From: Zong Li <zongbox@gmail.com> > > For the recommand of 64 bit version, we add the libraries path of 32 bit > in this patch. I have troubles comprehending this sentence; also it's repeated literally below. Please rewrite an remove the duplication. > The status of RV32 binaries under RV64 kernels is that the ISA > optionally supports having different XLEN for user and supervisor modes, > but AFAIK there's no silicon that implements this feature, and the Linux > kernel doesn't support it yet. I'm not sure if this note is relevant here. > For the recommand of 64 bit version, we add the libraries path of 32 bit > in this patch. This includes a fix to avoid an out of bound array check > when building with GCC 8.2. Where exactly is that fix? Shouldn't it be applied as a separate preparatory change? > diff --git a/sysdeps/unix/sysv/linux/riscv/dl-cache.h b/sysdeps/unix/sysv/linux/riscv/dl-cache.h > index c297dfe84f..60fc172edb 100644 > --- a/sysdeps/unix/sysv/linux/riscv/dl-cache.h > +++ b/sysdeps/unix/sysv/linux/riscv/dl-cache.h [...] > @@ -49,9 +51,16 @@ > do \ > { \ > size_t len = strlen (dir); \ > - char path[len + 9]; \ > + char path[len + 10]; \ > memcpy (path, dir, len + 1); \ > - if (len >= 12 && ! memcmp(path + len - 12, "/lib64/lp64d", 12)) \ > + if (len >= 13 && ! memcmp(path + len - 13, "/lib32/ilp32d", 13)) \ > + { \ > + len -= 9; \ > + path[len] = '\0'; \ > + } \ > + if (len >= 12 \ > + && (! memcmp(path + len - 12, "/lib32/ilp32", 12) \ > + || ! memcmp(path + len - 12, "/lib64/lp64d", 12))) \ Please correct indentation above and use tabs throughout. I think this code should use `else' clauses. While the truncation of the string will cause subsequent `memcmp' calls to fail, they'll still be executed, `len' permitting, making this ugly. Though frankly with the growing number of entries I would rewrite this sequence of conditionals entirely, as a loop over a (static) array of the strings processed, cutting the insane number of error-prone hardcoded string lengths while at it too. It is only ever used in `ldconfig', and then invoked there at most twice, so it is not that it is critical enough for performance to justify illegibility, and making additional `strlen' calls shouldn't be a big deal. > @@ -64,6 +73,10 @@ > add_dir (path); \ > if (len >= 4 && ! memcmp(path + len - 4, "/lib", 4)) \ > { \ > + memcpy (path + len, "32/ilp32d", 10); \ > + add_dir (path); \ > + memcpy (path + len, "32/ilp32", 9); \ > + add_dir (path); \ Then the same array of strings could be used here (skipping over the "/lib" prefix throughout). Maciej
On Wed, Jul 8, 2020 at 11:43 AM Maciej W. Rozycki via Libc-alpha <libc-alpha@sourceware.org> wrote: > > On Wed, 3 Jun 2020, Alistair Francis via Libc-alpha wrote: > > > From: Zong Li <zongbox@gmail.com> > > > > For the recommand of 64 bit version, we add the libraries path of 32 bit > > in this patch. > > I have troubles comprehending this sentence; also it's repeated literally > below. Please rewrite an remove the duplication. > > > The status of RV32 binaries under RV64 kernels is that the ISA > > optionally supports having different XLEN for user and supervisor modes, > > but AFAIK there's no silicon that implements this feature, and the Linux > > kernel doesn't support it yet. > > I'm not sure if this note is relevant here. > > > For the recommand of 64 bit version, we add the libraries path of 32 bit > > in this patch. This includes a fix to avoid an out of bound array check > > when building with GCC 8.2. > > Where exactly is that fix? Shouldn't it be applied as a separate > preparatory change? I have re-written the commit message to be clearer. > > > diff --git a/sysdeps/unix/sysv/linux/riscv/dl-cache.h b/sysdeps/unix/sysv/linux/riscv/dl-cache.h > > index c297dfe84f..60fc172edb 100644 > > --- a/sysdeps/unix/sysv/linux/riscv/dl-cache.h > > +++ b/sysdeps/unix/sysv/linux/riscv/dl-cache.h > [...] > > @@ -49,9 +51,16 @@ > > do \ > > { \ > > size_t len = strlen (dir); \ > > - char path[len + 9]; \ > > + char path[len + 10]; \ > > memcpy (path, dir, len + 1); \ > > - if (len >= 12 && ! memcmp(path + len - 12, "/lib64/lp64d", 12)) \ > > + if (len >= 13 && ! memcmp(path + len - 13, "/lib32/ilp32d", 13)) \ > > + { \ > > + len -= 9; \ > > + path[len] = '\0'; \ > > + } \ > > + if (len >= 12 \ > > + && (! memcmp(path + len - 12, "/lib32/ilp32", 12) \ > > + || ! memcmp(path + len - 12, "/lib64/lp64d", 12))) \ > > Please correct indentation above and use tabs throughout. Fixed. > > I think this code should use `else' clauses. While the truncation of the > string will cause subsequent `memcmp' calls to fail, they'll still be > executed, `len' permitting, making this ugly. > > Though frankly with the growing number of entries I would rewrite this > sequence of conditionals entirely, as a loop over a (static) array of the > strings processed, cutting the insane number of error-prone hardcoded > string lengths while at it too. It is only ever used in `ldconfig', and > then invoked there at most twice, so it is not that it is critical enough > for performance to justify illegibility, and making additional `strlen' > calls shouldn't be a big deal. I have converted this to a loop Alistair > > > @@ -64,6 +73,10 @@ > > add_dir (path); \ > > if (len >= 4 && ! memcmp(path + len - 4, "/lib", 4)) \ > > { \ > > + memcpy (path + len, "32/ilp32d", 10); \ > > + add_dir (path); \ > > + memcpy (path + len, "32/ilp32", 9); \ > > + add_dir (path); \ > > Then the same array of strings could be used here (skipping over the > "/lib" prefix throughout). > > Maciej
diff --git a/sysdeps/unix/sysv/linux/riscv/dl-cache.h b/sysdeps/unix/sysv/linux/riscv/dl-cache.h index c297dfe84f..60fc172edb 100644 --- a/sysdeps/unix/sysv/linux/riscv/dl-cache.h +++ b/sysdeps/unix/sysv/linux/riscv/dl-cache.h @@ -34,6 +34,8 @@ RISC-V, libraries can be found in paths ending in: - /lib64/lp64d - /lib64/lp64 + - /lib32/ilp32d + - /lib32/ilp32 - /lib (only ld.so) so this will add all of those paths. @@ -49,9 +51,16 @@ do \ { \ size_t len = strlen (dir); \ - char path[len + 9]; \ + char path[len + 10]; \ memcpy (path, dir, len + 1); \ - if (len >= 12 && ! memcmp(path + len - 12, "/lib64/lp64d", 12)) \ + if (len >= 13 && ! memcmp(path + len - 13, "/lib32/ilp32d", 13)) \ + { \ + len -= 9; \ + path[len] = '\0'; \ + } \ + if (len >= 12 \ + && (! memcmp(path + len - 12, "/lib32/ilp32", 12) \ + || ! memcmp(path + len - 12, "/lib64/lp64d", 12))) \ { \ len -= 8; \ path[len] = '\0'; \ @@ -64,6 +73,10 @@ add_dir (path); \ if (len >= 4 && ! memcmp(path + len - 4, "/lib", 4)) \ { \ + memcpy (path + len, "32/ilp32d", 10); \ + add_dir (path); \ + memcpy (path + len, "32/ilp32", 9); \ + add_dir (path); \ memcpy (path + len, "64/lp64d", 9); \ add_dir (path); \ memcpy (path + len, "64/lp64", 8); \