| Message ID | 20210303041211.26945-3-marek.behun@nic.cz |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | U-Boot LTO (Sandbox + ARM Nokia RX-51) | expand |
On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > When building with LTO, the system libc's `errno` variable used in > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > lib/errno.c) with the following error: > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > section .tbss mismatches non-TLS reference in > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o Do you know if this is the expected behavior when enabling LTO on the compiler? > > To avoid this conflict use different asm label for this variable when > CONFIG_SANDBOX is enabled. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > --- > include/errno.h | 8 +++++++- > lib/errno.c | 4 +++- > 2 files changed, 10 insertions(+), 2 deletions(-) Regards, Bin
On Fri, 5 Mar 2021 11:00:45 +0800 Bin Meng <bmeng.cn@gmail.com> wrote: > On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > > > When building with LTO, the system libc's `errno` variable used in > > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > > lib/errno.c) with the following error: > > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > > section .tbss mismatches non-TLS reference in > > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > Do you know if this is the expected behavior when enabling LTO on the compiler? I don't, but this is a bug anyway. The symbol clashes with the symbol from glibc. Does somebody know whether the usage of this symbol in os.c does really use glibc's version or U-Boot's one?
Hi Marek, On Fri, 5 Mar 2021 at 08:37, Marek Behun <marek.behun@nic.cz> wrote: > > On Fri, 5 Mar 2021 11:00:45 +0800 > Bin Meng <bmeng.cn@gmail.com> wrote: > > > On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > > > > > When building with LTO, the system libc's `errno` variable used in > > > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > > > lib/errno.c) with the following error: > > > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > > > section .tbss mismatches non-TLS reference in > > > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > > > Do you know if this is the expected behavior when enabling LTO on the compiler? > > I don't, but this is a bug anyway. The symbol clashes with the symbol > from glibc. Does somebody know whether the usage of this symbol in os.c > does really use glibc's version or U-Boot's one? It is intended to use glibc's version. In fact I don't think U-Boot should have an errno. We return errors in each case, as does Linux. Regards, Simon
On Fri, 5 Mar 2021 09:39:53 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi Marek, > > On Fri, 5 Mar 2021 at 08:37, Marek Behun <marek.behun@nic.cz> wrote: > > > > On Fri, 5 Mar 2021 11:00:45 +0800 > > Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > > > > > > > When building with LTO, the system libc's `errno` variable used in > > > > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > > > > lib/errno.c) with the following error: > > > > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > > > > section .tbss mismatches non-TLS reference in > > > > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > > > > > Do you know if this is the expected behavior when enabling LTO on the compiler? > > > > I don't, but this is a bug anyway. The symbol clashes with the symbol > > from glibc. Does somebody know whether the usage of this symbol in os.c > > does really use glibc's version or U-Boot's one? > > It is intended to use glibc's version. In fact I don't think U-Boot > should have an errno. We return errors in each case, as does Linux. The problem is that libc defines errno as a thread-local variable or, in older version, as a macro expading to a function dereference, i.e. #define errno (*__get_threads_errno()) But U-Boot usis the errno symbol defined in include/errno.h as a symbol. So in order for these two symbols not to clash (in case libc is using thread-local symbol with name errno), we need to rename the U-Boot errno variable's symbol name.
Hi Marek, On Fri, 5 Mar 2021 at 09:50, Marek Behun <marek.behun@nic.cz> wrote: > > On Fri, 5 Mar 2021 09:39:53 -0700 > Simon Glass <sjg@chromium.org> wrote: > > > Hi Marek, > > > > On Fri, 5 Mar 2021 at 08:37, Marek Behun <marek.behun@nic.cz> wrote: > > > > > > On Fri, 5 Mar 2021 11:00:45 +0800 > > > Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > > > > > > > > > When building with LTO, the system libc's `errno` variable used in > > > > > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > > > > > lib/errno.c) with the following error: > > > > > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > > > > > section .tbss mismatches non-TLS reference in > > > > > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > > > > > > > Do you know if this is the expected behavior when enabling LTO on the compiler? > > > > > > I don't, but this is a bug anyway. The symbol clashes with the symbol > > > from glibc. Does somebody know whether the usage of this symbol in os.c > > > does really use glibc's version or U-Boot's one? > > > > It is intended to use glibc's version. In fact I don't think U-Boot > > should have an errno. We return errors in each case, as does Linux. > > The problem is that libc defines errno as a thread-local variable or, > in older version, as a macro expading to a function dereference, i.e. > #define errno (*__get_threads_errno()) > But U-Boot usis the errno symbol defined in include/errno.h as a symbol. > > So in order for these two symbols not to clash (in case libc is using > thread-local symbol with name errno), we need to rename the U-Boot > errno variable's symbol name. Rename is OK, but can we delete it instead? I really don't think it should be there. Regards, Simon
On 05.03.21 16:37, Marek Behun wrote: > On Fri, 5 Mar 2021 11:00:45 +0800 > Bin Meng <bmeng.cn@gmail.com> wrote: > >> On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: >>> >>> When building with LTO, the system libc's `errno` variable used in >>> arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in >>> lib/errno.c) with the following error: >>> .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 >>> section .tbss mismatches non-TLS reference in >>> /tmp/u-boot.EQlEXz.ltrans0.ltrans.o >> >> Do you know if this is the expected behavior when enabling LTO on the compiler? > > I don't, but this is a bug anyway. The symbol clashes with the symbol > from glibc. Does somebody know whether the usage of this symbol in os.c > does really use glibc's version or U-Boot's one? > Hello Marek, Why do you resort to assembler in your patch instead of simply using: #define errno __uboot_errno to substitute the symbol? Why explicitly set errno = 0? Globals are automatically initialized to zero. @Bin: Here is an example demonstrating that glibc's errno is used in os.c: => host ls hostfs errno = 9 readdir: Bad file descriptor double free or corruption (top) Aborted caused by the change below: diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 3d8af0a52b..5b45296c47 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -456,9 +456,12 @@ int os_dirent_ls(const char *dirname, struct os_dirent_node **headp) for (node = head = NULL;; node = next) { errno = 0; + closedir(dir); entry = readdir(dir); if (!entry) { ret = errno; + printf("errno = %d\n", errno); + perror("readdir"); break; } next = malloc(sizeof(*node) + strlen(entry->d_name) + 1); Best regards Heinrich
On 05.03.21 17:58, Simon Glass wrote: > Hi Marek, > > On Fri, 5 Mar 2021 at 09:50, Marek Behun <marek.behun@nic.cz> wrote: >> >> On Fri, 5 Mar 2021 09:39:53 -0700 >> Simon Glass <sjg@chromium.org> wrote: >> >>> Hi Marek, >>> >>> On Fri, 5 Mar 2021 at 08:37, Marek Behun <marek.behun@nic.cz> wrote: >>>> >>>> On Fri, 5 Mar 2021 11:00:45 +0800 >>>> Bin Meng <bmeng.cn@gmail.com> wrote: >>>> >>>>> On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: >>>>>> >>>>>> When building with LTO, the system libc's `errno` variable used in >>>>>> arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in >>>>>> lib/errno.c) with the following error: >>>>>> .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 >>>>>> section .tbss mismatches non-TLS reference in >>>>>> /tmp/u-boot.EQlEXz.ltrans0.ltrans.o >>>>> >>>>> Do you know if this is the expected behavior when enabling LTO on the compiler? >>>> >>>> I don't, but this is a bug anyway. The symbol clashes with the symbol >>>> from glibc. Does somebody know whether the usage of this symbol in os.c >>>> does really use glibc's version or U-Boot's one? >>> >>> It is intended to use glibc's version. In fact I don't think U-Boot >>> should have an errno. We return errors in each case, as does Linux. >> >> The problem is that libc defines errno as a thread-local variable or, >> in older version, as a macro expading to a function dereference, i.e. >> #define errno (*__get_threads_errno()) >> But U-Boot usis the errno symbol defined in include/errno.h as a symbol. >> >> So in order for these two symbols not to clash (in case libc is using >> thread-local symbol with name errno), we need to rename the U-Boot >> errno variable's symbol name. > > Rename is OK, but can we delete it instead? I really don't think it > should be there. What makes you think so? fs/fs.c:614: errno = -ret; Best regards Heinrich
On Fri, 5 Mar 2021 09:58:34 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi Marek, > > On Fri, 5 Mar 2021 at 09:50, Marek Behun <marek.behun@nic.cz> wrote: > > > > On Fri, 5 Mar 2021 09:39:53 -0700 > > Simon Glass <sjg@chromium.org> wrote: > > > > > Hi Marek, > > > > > > On Fri, 5 Mar 2021 at 08:37, Marek Behun <marek.behun@nic.cz> wrote: > > > > > > > > On Fri, 5 Mar 2021 11:00:45 +0800 > > > > Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > > > > > > > > > > > When building with LTO, the system libc's `errno` variable used in > > > > > > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > > > > > > lib/errno.c) with the following error: > > > > > > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > > > > > > section .tbss mismatches non-TLS reference in > > > > > > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > > > > > > > > > Do you know if this is the expected behavior when enabling LTO on the compiler? > > > > > > > > I don't, but this is a bug anyway. The symbol clashes with the symbol > > > > from glibc. Does somebody know whether the usage of this symbol in os.c > > > > does really use glibc's version or U-Boot's one? > > > > > > It is intended to use glibc's version. In fact I don't think U-Boot > > > should have an errno. We return errors in each case, as does Linux. > > > > The problem is that libc defines errno as a thread-local variable or, > > in older version, as a macro expading to a function dereference, i.e. > > #define errno (*__get_threads_errno()) > > But U-Boot usis the errno symbol defined in include/errno.h as a symbol. > > > > So in order for these two symbols not to clash (in case libc is using > > thread-local symbol with name errno), we need to rename the U-Boot > > errno variable's symbol name. > > Rename is OK, but can we delete it instead? I really don't think it > should be there. We can't simply delete it. The whole u-boot is using the errno symbol from include/errno.h and if we want the whole u-boot to use libc's symbol we need to code include/errno.h to declare it in the same way as libc, which may be different for different libcs.
Hi Marek, On Fri, 5 Mar 2021 at 10:24, Marek Behun <marek.behun@nic.cz> wrote: > > On Fri, 5 Mar 2021 09:58:34 -0700 > Simon Glass <sjg@chromium.org> wrote: > > > Hi Marek, > > > > On Fri, 5 Mar 2021 at 09:50, Marek Behun <marek.behun@nic.cz> wrote: > > > > > > On Fri, 5 Mar 2021 09:39:53 -0700 > > > Simon Glass <sjg@chromium.org> wrote: > > > > > > > Hi Marek, > > > > > > > > On Fri, 5 Mar 2021 at 08:37, Marek Behun <marek.behun@nic.cz> wrote: > > > > > > > > > > On Fri, 5 Mar 2021 11:00:45 +0800 > > > > > Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > > > > > > > > > > > > > > When building with LTO, the system libc's `errno` variable used in > > > > > > > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > > > > > > > lib/errno.c) with the following error: > > > > > > > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > > > > > > > section .tbss mismatches non-TLS reference in > > > > > > > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > > > > > > > > > > > Do you know if this is the expected behavior when enabling LTO on the compiler? > > > > > > > > > > I don't, but this is a bug anyway. The symbol clashes with the symbol > > > > > from glibc. Does somebody know whether the usage of this symbol in os.c > > > > > does really use glibc's version or U-Boot's one? > > > > > > > > It is intended to use glibc's version. In fact I don't think U-Boot > > > > should have an errno. We return errors in each case, as does Linux. > > > > > > The problem is that libc defines errno as a thread-local variable or, > > > in older version, as a macro expading to a function dereference, i.e. > > > #define errno (*__get_threads_errno()) > > > But U-Boot usis the errno symbol defined in include/errno.h as a symbol. > > > > > > So in order for these two symbols not to clash (in case libc is using > > > thread-local symbol with name errno), we need to rename the U-Boot > > > errno variable's symbol name. > > > > Rename is OK, but can we delete it instead? I really don't think it > > should be there. > > We can't simply delete it. The whole u-boot is using the errno symbol > from include/errno.h and if we want the whole u-boot to use libc's > symbol we need to code include/errno.h to declare it in the same way as > libc, which may be different for different libcs. OK... Heinrich I don't think the fs needs to use errno, or perhaps it should have its own local version. It's just not nice to have a global error number IMO. Anyway, this is for future discussion, not for Marek to worry about. I am fine with Marek's solution. Regards, Simon
On Fri, 5 Mar 2021 18:21:51 +0100 Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 05.03.21 16:37, Marek Behun wrote: > > On Fri, 5 Mar 2021 11:00:45 +0800 > > Bin Meng <bmeng.cn@gmail.com> wrote: > > > >> On Wed, Mar 3, 2021 at 12:13 PM Marek Behún <marek.behun@nic.cz> wrote: > >>> > >>> When building with LTO, the system libc's `errno` variable used in > >>> arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > >>> lib/errno.c) with the following error: > >>> .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > >>> section .tbss mismatches non-TLS reference in > >>> /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > >> > >> Do you know if this is the expected behavior when enabling LTO on the compiler? > > > > I don't, but this is a bug anyway. The symbol clashes with the symbol > > from glibc. Does somebody know whether the usage of this symbol in os.c > > does really use glibc's version or U-Boot's one? > > > > Hello Marek, > > Why do you resort to assembler in your patch instead of simply using: > > #define errno __uboot_errno > > to substitute the symbol? Meeeeeh. :D That would just make error messages from gcc more complicated, if suddenly the compiler spat out 2 more lines, saying "in expansion of macro...". I think that using attributes, static inline functions and everything else the compiler provides instead of macros is better. > Why explicitly set errno = 0? Globals are automatically initialized to zero. I just added the symbol renaming part, the = 0 assignment was already there. I don't think this commit should remove it. If we want that, we can make it in another commit. Marek
On Tue, 2 Mar 2021 at 21:12, Marek Behún <marek.behun@nic.cz> wrote: > > When building with LTO, the system libc's `errno` variable used in > arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in > lib/errno.c) with the following error: > .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 > section .tbss mismatches non-TLS reference in > /tmp/u-boot.EQlEXz.ltrans0.ltrans.o > > To avoid this conflict use different asm label for this variable when > CONFIG_SANDBOX is enabled. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > --- > include/errno.h | 8 +++++++- > lib/errno.c | 4 +++- > 2 files changed, 10 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
diff --git a/include/errno.h b/include/errno.h index 3af539b9e9..652ad67306 100644 --- a/include/errno.h +++ b/include/errno.h @@ -8,7 +8,13 @@ #include <linux/errno.h> -extern int errno; +#ifdef __SANDBOX__ +#define __errno_asm_label asm("__u_boot_errno") +#else +#define __errno_asm_label +#endif + +extern int errno __errno_asm_label; #define __set_errno(val) do { errno = val; } while (0) diff --git a/lib/errno.c b/lib/errno.c index 8330a8fd14..ca0c756bd9 100644 --- a/lib/errno.c +++ b/lib/errno.c @@ -1 +1,3 @@ -int errno = 0; +#include <errno.h> + +int errno __errno_asm_label = 0;
When building with LTO, the system libc's `errno` variable used in arch/sandbox/cpu/os.c conflicts with U-Boot's `errno` (defined in lib/errno.c) with the following error: .../ld: errno@@GLIBC_PRIVATE: TLS definition in /lib64/libc.so.6 section .tbss mismatches non-TLS reference in /tmp/u-boot.EQlEXz.ltrans0.ltrans.o To avoid this conflict use different asm label for this variable when CONFIG_SANDBOX is enabled. Signed-off-by: Marek Behún <marek.behun@nic.cz> --- include/errno.h | 8 +++++++- lib/errno.c | 4 +++- 2 files changed, 10 insertions(+), 2 deletions(-)