Hi,
I'm seeing a failure building ISL using uclibc-ng and GCC 13.2
[ALL ] /home/ctng/x-tools/x86_64-multilib-linux-uclibc/lib/gcc/x86_64-multilib-linux-uclibc/13.2.0/../../../../x86_64-multilib-linux-uclibc/bin/ld.bfd:
isl_test2.o: non-canonical reference to canonical protected function
`__pthread_key_create' in
/home/ctng/x-tools/x86_64-multilib-linux-uclibc/x86_64-multilib-linux-uclibc/sysroot/lib64/libc.so.1
[ALL ] /home/ctng/x-tools/x86_64-multilib-linux-uclibc/lib/gcc/x86_64-multilib-linux-uclibc/13.2.0/../../../../x86_64-multilib-linux-uclibc/bin/ld.bfd:
failed to set dynamic section sizes: bad value
I don't really understand all the ins and outs but it looks like some
libstdc++ APIs used by ISLs test code boils down to a call to
__pthread_key_create which is marked protected so ld complains.
I can successfully build the same code with glibc (haven't tried
musl). So I'm reaching out to uclibc-ng rather than binutils or GCC.
Any thoughts on what might be the problem and what the solution might be?
Hello
here are my last patches
Mit freundlichem Gruß,
With best regards,
Ramin Seyed-Moussavi
Lead Software Developer
Phone: +49 30 3499834 0
Fax: +49 30 3499834 28
[cid:image001.png@01D9C916.916A8140]
Gustav-Meyer-Allee 25
Haus 12.2
D - 13355 Berlin
Amtsgericht Berlin, Charlottenburg HRB 79187
USt.-IdNr: DE814030948
Geschäftsführer: M.Sc. Markus Nigg
YACOUB im Internet: www.yacoub.de<http://www.yacoub.de/>
The information contained in this email is intended only for its addressee and may contain confidential and/or legally proteced information. If the reader of this email is not the intended recipient, you are hereby notified that saving, distribution or use of the content of this email in any way is prohibited. If you have received this email in error, please notify the sender and delete the email. We do not accept any responsibility for any damages caused by email transmitted.
Setting signal handler in the kernel and then updating sighandler[sig]
results in a crash if a signal which handler is being changed from
SIG_DFL to a non-default was pending. Improve the race a little and
update the sighandler[sig] before the sigaction syscall. It doesn't
eliminate the race entirely, but fixes this particular failing case.
E.g. this fixes the 100% reproducible segfault in the busybox hush shell
built with FEATURE_EDITING_WINCH on ssh client's terminal window resize,
but in that case there's one more even bigger issue: busybox calls
sigaction with both old and new signal pointers pointing to the same
structure instance, as a result act->sa_handler after the sigaction
syscall is not what the user requested, but the previous handler.
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
Changes v1 -> v2:
- initialize 'save' with NULL to avoid compiler warning. The code
cannot use uninitialized 'save' value, so no logic change is needed.
libpthread/linuxthreads/signals.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/libpthread/linuxthreads/signals.c b/libpthread/linuxthreads/signals.c
index 0c0f2b6b1d2a..0cde54a16d27 100644
--- a/libpthread/linuxthreads/signals.c
+++ b/libpthread/linuxthreads/signals.c
@@ -134,6 +134,7 @@ int sigaction(int sig, const struct sigaction * act,
{
struct sigaction newact;
struct sigaction *newactp;
+ void *save = NULL;
#ifdef DEBUG_PT
printf(__FUNCTION__": pthreads wrapper!\n");
@@ -142,6 +143,8 @@ printf(__FUNCTION__": pthreads wrapper!\n");
sig == __pthread_sig_cancel ||
(sig == __pthread_sig_debug && __pthread_sig_debug > 0))
return EINVAL;
+ if (sig > 0 && sig < NSIG)
+ save = sighandler[sig].old;
if (act)
{
newact = *act;
@@ -154,22 +157,24 @@ printf(__FUNCTION__": pthreads wrapper!\n");
newact.sa_handler = (__sighandler_t) pthread_sighandler;
}
newactp = &newact;
+ if (sig > 0 && sig < NSIG)
+ sighandler[sig].old = (arch_sighandler_t) act->sa_handler;
}
else
newactp = NULL;
if (__libc_sigaction(sig, newactp, oact) == -1)
- return -1;
+ {
+ if (act && sig > 0 && sig < NSIG)
+ sighandler[sig].old = save;
+ return -1;
+ }
#ifdef DEBUG_PT
printf(__FUNCTION__": sighandler installed, sigaction successful\n");
#endif
if (sig > 0 && sig < NSIG)
{
if (oact != NULL)
- oact->sa_handler = (__sighandler_t) sighandler[sig].old;
- if (act)
- /* For the assignment is does not matter whether it's a normal
- or real-time signal. */
- sighandler[sig].old = (arch_sighandler_t) act->sa_handler;
+ oact->sa_handler = save;
}
return 0;
}
--
2.30.2
Setting signal handler in the kernel and then updating sighandler[sig]
results in a crash if a signal which handler is being changed from
SIG_DFL to a non-default was pending. Improve the race a little and
update the sighandler[sig] before the sigaction syscall. It doesn't
eliminate the race entirely, but fixes this particular failing case.
E.g. this fixes the 100% reproducible segfault in the busybox hush shell
built with FEATURE_EDITING_WINCH on ssh client's terminal window resize,
but in that case there's one more even bigger issue: busybox calls
sigaction with both old and new signal pointers pointing to the same
structure instance, as a result act->sa_handler after the sigaction
syscall is not what the user requested, but the previous handler.
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
libpthread/linuxthreads/signals.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/libpthread/linuxthreads/signals.c b/libpthread/linuxthreads/signals.c
index 0c0f2b6b1d2a..0cde54a16d27 100644
--- a/libpthread/linuxthreads/signals.c
+++ b/libpthread/linuxthreads/signals.c
@@ -134,6 +134,7 @@ int sigaction(int sig, const struct sigaction * act,
{
struct sigaction newact;
struct sigaction *newactp;
+ void *save;
#ifdef DEBUG_PT
printf(__FUNCTION__": pthreads wrapper!\n");
@@ -142,6 +143,8 @@ printf(__FUNCTION__": pthreads wrapper!\n");
sig == __pthread_sig_cancel ||
(sig == __pthread_sig_debug && __pthread_sig_debug > 0))
return EINVAL;
+ if (sig > 0 && sig < NSIG)
+ save = sighandler[sig].old;
if (act)
{
newact = *act;
@@ -154,22 +157,24 @@ printf(__FUNCTION__": pthreads wrapper!\n");
newact.sa_handler = (__sighandler_t) pthread_sighandler;
}
newactp = &newact;
+ if (sig > 0 && sig < NSIG)
+ sighandler[sig].old = (arch_sighandler_t) act->sa_handler;
}
else
newactp = NULL;
if (__libc_sigaction(sig, newactp, oact) == -1)
- return -1;
+ {
+ if (act && sig > 0 && sig < NSIG)
+ sighandler[sig].old = save;
+ return -1;
+ }
#ifdef DEBUG_PT
printf(__FUNCTION__": sighandler installed, sigaction successful\n");
#endif
if (sig > 0 && sig < NSIG)
{
if (oact != NULL)
- oact->sa_handler = (__sighandler_t) sighandler[sig].old;
- if (act)
- /* For the assignment is does not matter whether it's a normal
- or real-time signal. */
- sighandler[sig].old = (arch_sighandler_t) act->sa_handler;
+ oact->sa_handler = save;
}
return 0;
}
--
2.30.2
The current definition of __WCHAR_MIN and __WCHAR_MAX are only correct
when wchar_t is an int. This is not the case on ARM/AArch64 where
wchar_t is an unsigned int, or some other architectures where wchar_t
is a long.
The current incorrect definition causes a build issue for example when
building mpd, which uses boost, with gcc 12.x:
In file included from /home/thomas/buildroot/aarch64/host/aarch64-buildroot-linux-uclibc/sysroot/usr/include/boost/integer.hpp:20,
from /home/thomas/buildroot/aarch64/host/aarch64-buildroot-linux-uclibc/sysroot/usr/include/boost/crc.hpp:42,
from ../src/storage/StorageState.cxx:43:
/home/thomas/buildroot/aarch64/host/aarch64-buildroot-linux-uclibc/sysroot/usr/include/boost/integer_traits.hpp:105:69: error: narrowing conversion of ‘-2147483648’ from ‘int’ to ‘wchar_t’ [-Wnarrowing]
105 | public detail::integer_traits_base<wchar_t, WCHAR_MIN, WCHAR_MAX>
| ^
This issue was fixed in glibc in 2013, see bug report
https://sourceware.org/bugzilla/show_bug.cgi?id=15036, and upstream
commit
https://sourceware.org/git/?p=glibc.git;a=commit;h=052aff95782fefe9c6356647….
Since the i386-specific definition of __WCHAR_MIN and __WCHAR_MAX was
also removed at the same time in glibc, we do the same as part of this
commit.
Reported-by: Clément Ramirez <clement.ramirez(a)bootlin.com>
With-some-useful-help-from: Paul Kocialkowski <paul.kocialkowski(a)bootlin.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni(a)bootlin.com>
---
libc/sysdeps/linux/common/bits/wchar.h | 32 ++++++++++++++++++++++----
libc/sysdeps/linux/i386/bits/wchar.h | 25 --------------------
2 files changed, 28 insertions(+), 29 deletions(-)
delete mode 100644 libc/sysdeps/linux/i386/bits/wchar.h
diff --git a/libc/sysdeps/linux/common/bits/wchar.h b/libc/sysdeps/linux/common/bits/wchar.h
index a3ff5319e..50c7ef37b 100644
--- a/libc/sysdeps/linux/common/bits/wchar.h
+++ b/libc/sysdeps/linux/common/bits/wchar.h
@@ -1,5 +1,5 @@
/* wchar_t type related definitions.
- Copyright (C) 2000 Free Software Foundation, Inc.
+ Copyright (C) 2000-2022 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -14,12 +14,36 @@
You should have received a copy of the GNU Lesser General Public
License along with the GNU C Library; if not, see
- <http://www.gnu.org/licenses/>. */
+ <https://www.gnu.org/licenses/>. */
#ifndef _BITS_WCHAR_H
#define _BITS_WCHAR_H 1
-#define __WCHAR_MIN (-2147483647 - 1)
-#define __WCHAR_MAX (2147483647)
+/* The fallback definitions, for when __WCHAR_MAX__ or __WCHAR_MIN__
+ are not defined, give the right value and type as long as both int
+ and wchar_t are 32-bit types. Adding L'\0' to a constant value
+ ensures that the type is correct; it is necessary to use (L'\0' +
+ 0) rather than just L'\0' so that the type in C++ is the promoted
+ version of wchar_t rather than the distinct wchar_t type itself.
+ Because wchar_t in preprocessor #if expressions is treated as
+ intmax_t or uintmax_t, the expression (L'\0' - 1) would have the
+ wrong value for WCHAR_MAX in such expressions and so cannot be used
+ to define __WCHAR_MAX in the unsigned case. */
+
+#ifdef __WCHAR_MAX__
+# define __WCHAR_MAX __WCHAR_MAX__
+#elif L'\0' - 1 > 0
+# define __WCHAR_MAX (0xffffffffu + L'\0')
+#else
+# define __WCHAR_MAX (0x7fffffff + L'\0')
+#endif
+
+#ifdef __WCHAR_MIN__
+# define __WCHAR_MIN __WCHAR_MIN__
+#elif L'\0' - 1 > 0
+# define __WCHAR_MIN (L'\0' + 0)
+#else
+# define __WCHAR_MIN (-__WCHAR_MAX - 1)
+#endif
#endif /* bits/wchar.h */
diff --git a/libc/sysdeps/linux/i386/bits/wchar.h b/libc/sysdeps/linux/i386/bits/wchar.h
deleted file mode 100644
index b94fc7a3f..000000000
--- a/libc/sysdeps/linux/i386/bits/wchar.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/* wchar_t type related definitions.
- Copyright (C) 2000 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <http://www.gnu.org/licenses/>. */
-
-#ifndef _BITS_WCHAR_H
-#define _BITS_WCHAR_H 1
-
-#define __WCHAR_MIN (-2147483647l - 1l)
-#define __WCHAR_MAX (2147483647l)
-
-#endif /* bits/wchar.h */
--
2.41.0
From: Yann Sionneau <ysionneau(a)kalrayinc.com>
Without this patch, since https://github.com/torvalds/linux/commit/dcd46d897adb70d63e025f175a00a89797…
this test runs in infinite loop.
Signed-off-by: Yann Sionneau <ysionneau(a)kalrayinc.com>
---
test/unistd/exec-null.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/test/unistd/exec-null.c b/test/unistd/exec-null.c
index 3df99f3..0f79b3d 100644
--- a/test/unistd/exec-null.c
+++ b/test/unistd/exec-null.c
@@ -4,7 +4,12 @@
int main(int argc, char *argv[])
{
- if (argc == 0)
+ /* since Linux https://github.com/torvalds/linux/commit/dcd46d897adb70d63e025f175a00a89797…
+ * kernel forces an empty first arg if execve is called
+ * with argv == NULL.
+ * so we need to handle argc == 1 for newer kernel as well
+ */
+ if (argc == 0 || argc == 1)
return 0;
char *exec_argv[1], *exec_envp[1];
--
2.17.1
On 2023/06/07 23:57, Yann Sionneau wrote:
> Hello uClibc-ng hackers, Damien, Rich,
>
> I am sending this email to discuss the possibility to revert
> https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/commit/?id=08d46f1ce21e4e…
>
> I think this change is wrong, I have been tracking an issue for a few
> days and I am pretty sure the issue comes from this commit.
>
> Basically what happens is that with this commit, tst-cancel18 test is
> failing.
>
> The test is available over there:
> https://github.com/wbx-github/uclibc-ng-test/blob/master/test/nptl/tst-canc…
>
> The reason for this fail is that clock_nanosleep code from libc is badly
> compiled:
> https://elixir.bootlin.com/uclibc-ng/latest/source/librt/clock_nanosleep.c#…
>
> Because SINGLE_THREAD_P is actually defined, in case your libc build
> supports NPTL threads (which is, I think, the majority of configs,
> except very old systems using LINUXTHREADS or NOMMU maybe).
>
> In my case (kvx arch):
>
> #define SINGLE_THREAD_P __builtin_expect (THREAD_GETMEM (THREAD_SELF,
> header.multiple_threads) == 0, 1)
>
> See:
> https://elixir.bootlin.com/uclibc-ng/latest/source/libpthread/nptl/sysdeps/…
>
> This results to clock_nanosleep code being compiled to a very simple
> version that directly calls the syscall and does not change at all the
> thread cancellability to asynchronous before calling the syscall.
>
> Therefore, when the test main thread calls pthread_cancel(), no signal
> is sent (see
> https://elixir.bootlin.com/uclibc-ng/latest/source/libpthread/nptl/pthread_…)
> and therefore the nanosleep syscall is never interrupted and the test fails.
>
> clock_nanosleep is defined to be a mandatory cancellation point by
> POSIX, it must be interruptible in all cases by pthread_cancel, whether
> in synchronous or asynchronous mode.
>
> I think the fix for the NOMMU case should be different, maybe checking
> for if LIBC_CANCEL_ASYNC is defined ?
>
> Damien do you have time to work on a new fix for the NOMMU case? Are you
> OK with the revert?
Not OK with a revert as that will bring back the compilation errors for RISC-V
NOMMU case.
Instead, please work on a fix for your case on top of the current code. If I
recall correctly, the code around this is rather messy with lots of ifdefs,
which may be why this regression was introduced. Cleaning up the code first to
avoid that may help avoiding further issues in the future.
Note that to test patches/regressions for risc-v NOMMU builds, you can simply
use buildroot and run:
make sipeed_maix_bit_defconfig
make
To check that it builds.
>
> Can someone confirm my analysis here?
>
> Thanks!
>
> Best regards,
>
--
Damien Le Moal
Western Digital Research