linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] pending bug fixes for nolibc
@ 2023-01-09  7:54 Willy Tarreau
  2023-01-09  7:54 ` [PATCH 1/6] nolibc: fix fd_set type Willy Tarreau
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Willy Tarreau, Warner Losh, Sven Schnelle

Hello Paul,

please consider the current patch series for merging into your fixes queue.
The intent is to get them before 6.2, then backported where relevant.

It addresses the following bugs:
  - fd_set was incorrectly defined as arrays of u32 instead of long,
    which breaks BE64. Fix courtesy of Sven Schnelle.

  - S_ISxxx macros were incorrectly testing the bits after applying them
    instead of applying S_ISFMT to the value. Fix from Warner Losh.

  - the mips code was randomly broken due to an unprotected "noreorder"
    directive in the _start code that would prevent the assembler from
    filling delayed slots, and randomly leaving other instructions there

  - since the split of the single include file into multiple files, we're
    implicitly refraining from including some which are not explicitly
    added in the code. It causes build failures when such files contain
    definitions for functions that may be used e.g. by libgcc, such as
    raise() or memset(), which are often called only by a few archs at
    certain optimization levels only.

  - gcc 11.3 in ARM thumb2 mode at -O2 was able to recognize a memset()
    construction inside the memset() definition, and it replaced it with
    a call to... memset(). We cannot impose to userland to build with
    -ffreestanding so the introduction of an empty asm() statement in
    the loop was enough to stop this.

  - most of the O_* macros were wrong on RISCV because their octal value
    was used as a hexadecimal one when the platform was introduced. This
    was revealed by the selftest breaking in getdents64().

The series was tested on x86_64, i386, armv5, armv7, thumb1, thumb2,
mips and riscv, all at -O0, -Os and -O3. This is based on the "nolibc"
branch of your linux-rcu tree. Do not hesitate to let me know if you
prefer that I rebase it on a different one.

Thank you!
Willy

---
Sven Schnelle (1):
  nolibc: fix fd_set type

Warner Losh (1):
  tools/nolibc: Fix S_ISxxx macros

Willy Tarreau (4):
  tools/nolibc: restore mips branch ordering in the _start block
  tools/nolibc: fix missing includes causing build issues at -O0
  tools/nolibc: prevent gcc from making memset() loop over itself
  tools/nolibc: fix the O_* fcntl/open macro definitions for riscv

 tools/include/nolibc/arch-mips.h  |  2 +
 tools/include/nolibc/arch-riscv.h | 14 +++----
 tools/include/nolibc/ctype.h      |  3 ++
 tools/include/nolibc/errno.h      |  3 ++
 tools/include/nolibc/signal.h     |  3 ++
 tools/include/nolibc/stdio.h      |  3 ++
 tools/include/nolibc/stdlib.h     |  3 ++
 tools/include/nolibc/string.h     |  8 +++-
 tools/include/nolibc/sys.h        |  2 +
 tools/include/nolibc/time.h       |  3 ++
 tools/include/nolibc/types.h      | 70 ++++++++++++++++++-------------
 tools/include/nolibc/unistd.h     |  3 ++
 12 files changed, 79 insertions(+), 38 deletions(-)

-- 
2.35.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] nolibc: fix fd_set type
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
@ 2023-01-09  7:54 ` Willy Tarreau
  2023-01-09  7:54 ` [PATCH 2/6] tools/nolibc: Fix S_ISxxx macros Willy Tarreau
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Sven Schnelle, Willy Tarreau

From: Sven Schnelle <svens@linux.ibm.com>

The kernel uses unsigned long for the fd_set bitmap,
but nolibc use u32. This works fine on little endian
machines, but fails on big endian. Convert to unsigned
long to fix this.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/types.h | 53 ++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 959997034e55..300e0ff1cd58 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -89,39 +89,46 @@
 #define EXIT_SUCCESS 0
 #define EXIT_FAILURE 1
 
+#define FD_SETIDXMASK (8 * sizeof(unsigned long))
+#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
+
 /* for select() */
 typedef struct {
-	uint32_t fd32[(FD_SETSIZE + 31) / 32];
+	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
 } fd_set;
 
-#define FD_CLR(fd, set) do {                                            \
-		fd_set *__set = (set);                                  \
-		int __fd = (fd);                                        \
-		if (__fd >= 0)                                          \
-			__set->fd32[__fd / 32] &= ~(1U << (__fd & 31)); \
+#define FD_CLR(fd, set) do {						\
+		fd_set *__set = (set);					\
+		int __fd = (fd);					\
+		if (__fd >= 0)						\
+			__set->fds[__fd / FD_SETIDXMASK] &=		\
+				~(1U << (__fd & FX_SETBITMASK));	\
 	} while (0)
 
-#define FD_SET(fd, set) do {                                            \
-		fd_set *__set = (set);                                  \
-		int __fd = (fd);                                        \
-		if (__fd >= 0)                                          \
-			__set->fd32[__fd / 32] |= 1U << (__fd & 31);    \
+#define FD_SET(fd, set) do {						\
+		fd_set *__set = (set);					\
+		int __fd = (fd);					\
+		if (__fd >= 0)						\
+			__set->fds[__fd / FD_SETIDXMASK] |=		\
+				1 << (__fd & FD_SETBITMASK);		\
 	} while (0)
 
-#define FD_ISSET(fd, set) ({                                                  \
-		fd_set *__set = (set);                                        \
-		int __fd = (fd);                                              \
-		int __r = 0;                                                  \
-		if (__fd >= 0)                                                \
-			__r = !!(__set->fd32[__fd / 32] & 1U << (__fd & 31)); \
-		__r;                                                          \
+#define FD_ISSET(fd, set) ({						\
+			fd_set *__set = (set);				\
+			int __fd = (fd);				\
+		int __r = 0;						\
+		if (__fd >= 0)						\
+			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
+1U << (__fd & FD_SET_BITMASK));						\
+		__r;							\
 	})
 
-#define FD_ZERO(set) do {                                               \
-		fd_set *__set = (set);                                  \
-		int __idx;                                              \
-		for (__idx = 0; __idx < (FD_SETSIZE+31) / 32; __idx ++) \
-			__set->fd32[__idx] = 0;                         \
+#define FD_ZERO(set) do {						\
+		fd_set *__set = (set);					\
+		int __idx;						\
+		int __size = (FD_SETSIZE+FD_SETBITMASK) / FD_SETIDXMASK;\
+		for (__idx = 0; __idx < __size; __idx++)		\
+			__set->fds[__idx] = 0;				\
 	} while (0)
 
 /* for poll() */
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] tools/nolibc: Fix S_ISxxx macros
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
  2023-01-09  7:54 ` [PATCH 1/6] nolibc: fix fd_set type Willy Tarreau
@ 2023-01-09  7:54 ` Willy Tarreau
  2023-01-09  7:54 ` [PATCH 3/6] tools/nolibc: restore mips branch ordering in the _start block Willy Tarreau
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Warner Losh, Willy Tarreau

From: Warner Losh <imp@bsdimp.com>

The mode field has the type encoded as an value in a field, not as a bit
mask. Mask the mode with S_IFMT instead of each type to test. Otherwise,
false positives are possible: eg S_ISDIR will return true for block
devices because S_IFDIR = 0040000 and S_IFBLK = 0060000 since mode is
masked with S_IFDIR instead of S_IFMT. These macros now match the
similar definitions in tools/include/uapi/linux/stat.h.

Signed-off-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/types.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 300e0ff1cd58..f1d64fca7cf0 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -26,13 +26,13 @@
 #define S_IFSOCK       0140000
 #define S_IFMT         0170000
 
-#define S_ISDIR(mode)  (((mode) & S_IFDIR)  == S_IFDIR)
-#define S_ISCHR(mode)  (((mode) & S_IFCHR)  == S_IFCHR)
-#define S_ISBLK(mode)  (((mode) & S_IFBLK)  == S_IFBLK)
-#define S_ISREG(mode)  (((mode) & S_IFREG)  == S_IFREG)
-#define S_ISFIFO(mode) (((mode) & S_IFIFO)  == S_IFIFO)
-#define S_ISLNK(mode)  (((mode) & S_IFLNK)  == S_IFLNK)
-#define S_ISSOCK(mode) (((mode) & S_IFSOCK) == S_IFSOCK)
+#define S_ISDIR(mode)  (((mode) & S_IFMT) == S_IFDIR)
+#define S_ISCHR(mode)  (((mode) & S_IFMT) == S_IFCHR)
+#define S_ISBLK(mode)  (((mode) & S_IFMT) == S_IFBLK)
+#define S_ISREG(mode)  (((mode) & S_IFMT) == S_IFREG)
+#define S_ISFIFO(mode) (((mode) & S_IFMT) == S_IFIFO)
+#define S_ISLNK(mode)  (((mode) & S_IFMT) == S_IFLNK)
+#define S_ISSOCK(mode) (((mode) & S_IFMT) == S_IFSOCK)
 
 /* dirent types */
 #define DT_UNKNOWN     0x0
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] tools/nolibc: restore mips branch ordering in the _start block
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
  2023-01-09  7:54 ` [PATCH 1/6] nolibc: fix fd_set type Willy Tarreau
  2023-01-09  7:54 ` [PATCH 2/6] tools/nolibc: Fix S_ISxxx macros Willy Tarreau
@ 2023-01-09  7:54 ` Willy Tarreau
  2023-01-09  7:54 ` [PATCH 4/6] tools/nolibc: fix missing includes causing build issues at -O0 Willy Tarreau
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Willy Tarreau

Depending on the compiler used and the optimization options, the sbrk()
test was crashing, both on real hardware (mips-24kc) and in qemu. One
such example is kernel.org toolchain in version 11.3 optimizing at -Os.

Inspecting the sys_brk() call shows the following code:

  0040047c <sys_brk>:
    40047c:       24020fcd        li      v0,4045
    400480:       27bdffe0        addiu   sp,sp,-32
    400484:       0000000c        syscall
    400488:       27bd0020        addiu   sp,sp,32
    40048c:       10e00001        beqz    a3,400494 <sys_brk+0x18>
    400490:       00021023        negu    v0,v0
    400494:       03e00008        jr      ra

It is obviously wrong, the "negu" instruction is placed in beqz's
delayed slot, and worse, there's no nop nor instruction after the
return, so the next function's first instruction (addiu sip,sip,-32)
will also be executed as part of the delayed slot that follows the
return.

This is caused by the ".set noreorder" directive in the _start block,
that applies to the whole program. The compiler emits code without the
delayed slots and relies on the compiler to swap instructions when this
option is not set. Removing the option would require to change the
startup code in a way that wouldn't make it look like the resulting
code, which would not be easy to debug. Instead let's just save the
default ordering before changing it, and restore it at the end of the
_start block. Now the code is correct:

  0040047c <sys_brk>:
    40047c:       24020fcd        li      v0,4045
    400480:       27bdffe0        addiu   sp,sp,-32
    400484:       0000000c        syscall
    400488:       10e00002        beqz    a3,400494 <sys_brk+0x18>
    40048c:       27bd0020        addiu   sp,sp,32
    400490:       00021023        negu    v0,v0
    400494:       03e00008        jr      ra
    400498:       00000000        nop

Fixes: 66b6f755ad45 ("rcutorture: Import a copy of nolibc") #5.0
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/arch-mips.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index 5fc5b8029bff..7380093ba9e7 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -192,6 +192,7 @@ struct sys_stat_struct {
 __asm__ (".section .text\n"
     ".weak __start\n"
     ".set nomips16\n"
+    ".set push\n"
     ".set    noreorder\n"
     ".option pic0\n"
     ".ent __start\n"
@@ -210,6 +211,7 @@ __asm__ (".section .text\n"
     "li $v0, 4001\n"              // NR_exit == 4001
     "syscall\n"
     ".end __start\n"
+    ".set pop\n"
     "");
 
 #endif // _NOLIBC_ARCH_MIPS_H
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] tools/nolibc: fix missing includes causing build issues at -O0
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
                   ` (2 preceding siblings ...)
  2023-01-09  7:54 ` [PATCH 3/6] tools/nolibc: restore mips branch ordering in the _start block Willy Tarreau
@ 2023-01-09  7:54 ` Willy Tarreau
  2023-01-09  7:54 ` [PATCH 5/6] tools/nolibc: prevent gcc from making memset() loop over itself Willy Tarreau
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Willy Tarreau

After the nolibc includes were split to facilitate portability from
standard libcs, programs that include only what they need may miss
some symbols which are needed by libgcc. This is the case for raise()
which is needed by the divide by zero code in some architectures for
example.

Regardless, being able to include only the apparently needed files is
convenient.

Instead of trying to move all exported definitions to a single file,
since this can change over time, this patch takes another approach
consisting in including the nolibc header at the end of all standard
include files. This way their types and functions are already known
at the moment of inclusion, and including any single one of them is
sufficient to bring all the required ones.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/ctype.h  | 3 +++
 tools/include/nolibc/errno.h  | 3 +++
 tools/include/nolibc/signal.h | 3 +++
 tools/include/nolibc/stdio.h  | 3 +++
 tools/include/nolibc/stdlib.h | 3 +++
 tools/include/nolibc/string.h | 3 +++
 tools/include/nolibc/sys.h    | 2 ++
 tools/include/nolibc/time.h   | 3 +++
 tools/include/nolibc/types.h  | 3 +++
 tools/include/nolibc/unistd.h | 3 +++
 10 files changed, 29 insertions(+)

diff --git a/tools/include/nolibc/ctype.h b/tools/include/nolibc/ctype.h
index e3000b2992d7..6f90706d0644 100644
--- a/tools/include/nolibc/ctype.h
+++ b/tools/include/nolibc/ctype.h
@@ -96,4 +96,7 @@ int ispunct(int c)
 	return isgraph(c) && !isalnum(c);
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_CTYPE_H */
diff --git a/tools/include/nolibc/errno.h b/tools/include/nolibc/errno.h
index 06893d6dfb7a..9dc4919c769b 100644
--- a/tools/include/nolibc/errno.h
+++ b/tools/include/nolibc/errno.h
@@ -24,4 +24,7 @@ static int errno;
  */
 #define MAX_ERRNO 4095
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_ERRNO_H */
diff --git a/tools/include/nolibc/signal.h b/tools/include/nolibc/signal.h
index ef47e71e2be3..137552216e46 100644
--- a/tools/include/nolibc/signal.h
+++ b/tools/include/nolibc/signal.h
@@ -19,4 +19,7 @@ int raise(int signal)
 	return sys_kill(sys_getpid(), signal);
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_SIGNAL_H */
diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
index a3cebc4bc3ac..96ac8afc5aee 100644
--- a/tools/include/nolibc/stdio.h
+++ b/tools/include/nolibc/stdio.h
@@ -303,4 +303,7 @@ void perror(const char *msg)
 	fprintf(stderr, "%s%serrno=%d\n", (msg && *msg) ? msg : "", (msg && *msg) ? ": " : "", errno);
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_STDIO_H */
diff --git a/tools/include/nolibc/stdlib.h b/tools/include/nolibc/stdlib.h
index 92378c4b9660..a24000d1e822 100644
--- a/tools/include/nolibc/stdlib.h
+++ b/tools/include/nolibc/stdlib.h
@@ -419,4 +419,7 @@ char *u64toa(uint64_t in)
 	return itoa_buffer;
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_STDLIB_H */
diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index ad97c0d522b8..0932db3817d2 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -285,4 +285,7 @@ char *strrchr(const char *s, int c)
 	return (char *)ret;
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_STRING_H */
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index ce3ee03aa679..78473d34e27c 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -1243,5 +1243,7 @@ ssize_t write(int fd, const void *buf, size_t count)
 	return ret;
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
 
 #endif /* _NOLIBC_SYS_H */
diff --git a/tools/include/nolibc/time.h b/tools/include/nolibc/time.h
index d18b7661fdd7..84655361b9ad 100644
--- a/tools/include/nolibc/time.h
+++ b/tools/include/nolibc/time.h
@@ -25,4 +25,7 @@ time_t time(time_t *tptr)
 	return tv.tv_sec;
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_TIME_H */
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index f1d64fca7cf0..fbbc0e68c001 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -209,4 +209,7 @@ struct stat {
 })
 #endif
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_TYPES_H */
diff --git a/tools/include/nolibc/unistd.h b/tools/include/nolibc/unistd.h
index 1c25e20ee360..1cfcd52106a4 100644
--- a/tools/include/nolibc/unistd.h
+++ b/tools/include/nolibc/unistd.h
@@ -51,4 +51,7 @@ int tcsetpgrp(int fd, pid_t pid)
 	return ioctl(fd, TIOCSPGRP, &pid);
 }
 
+/* make sure to include all global symbols */
+#include "nolibc.h"
+
 #endif /* _NOLIBC_UNISTD_H */
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] tools/nolibc: prevent gcc from making memset() loop over itself
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
                   ` (3 preceding siblings ...)
  2023-01-09  7:54 ` [PATCH 4/6] tools/nolibc: fix missing includes causing build issues at -O0 Willy Tarreau
@ 2023-01-09  7:54 ` Willy Tarreau
  2023-01-09  7:54 ` [PATCH 6/6] tools/nolibc: fix the O_* fcntl/open macro definitions for riscv Willy Tarreau
  2023-01-09 19:11 ` [PATCH 0/6] pending bug fixes for nolibc Paul E. McKenney
  6 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Willy Tarreau

When building on ARM in thumb mode with gcc-11.3 at -O2 or -O3,
nolibc-test segfaults during the select() tests. It turns out that at
this level, gcc recognizes an opportunity for using memset() to zero
the fd_set, but it miscompiles it because it also recognizes a memset
pattern as well, and decides to call memset() from the memset() code:

  000122bc <memset>:
     122bc:       b510            push    {r4, lr}
     122be:       0004            movs    r4, r0
     122c0:       2a00            cmp     r2, #0
     122c2:       d003            beq.n   122cc <memset+0x10>
     122c4:       23ff            movs    r3, #255        ; 0xff
     122c6:       4019            ands    r1, r3
     122c8:       f7ff fff8       bl      122bc <memset>
     122cc:       0020            movs    r0, r4
     122ce:       bd10            pop     {r4, pc}

Simply placing an empty asm() statement inside the loop suffices to
avoid this.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/string.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index 0932db3817d2..fffdaf6ff467 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -88,8 +88,11 @@ void *memset(void *dst, int b, size_t len)
 {
 	char *p = dst;
 
-	while (len--)
+	while (len--) {
+		/* prevent gcc from recognizing memset() here */
+		asm volatile("");
 		*(p++) = b;
+	}
 	return dst;
 }
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] tools/nolibc: fix the O_* fcntl/open macro definitions for riscv
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
                   ` (4 preceding siblings ...)
  2023-01-09  7:54 ` [PATCH 5/6] tools/nolibc: prevent gcc from making memset() loop over itself Willy Tarreau
@ 2023-01-09  7:54 ` Willy Tarreau
  2023-01-09 19:11 ` [PATCH 0/6] pending bug fixes for nolibc Paul E. McKenney
  6 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-09  7:54 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Willy Tarreau

When RISCV port was imported in 5.2, the O_* macros were taken with
their octal value and written as-is in hex, resulting in the getdents64()
to fail in nolibc-test.

Fixes: 582e84f7b779 ("tool headers nolibc: add RISCV support") #5.2
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/arch-riscv.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index ba04771cb3a3..a3bdd9803f8c 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -11,13 +11,13 @@
 #define O_RDONLY            0
 #define O_WRONLY            1
 #define O_RDWR              2
-#define O_CREAT         0x100
-#define O_EXCL          0x200
-#define O_NOCTTY        0x400
-#define O_TRUNC        0x1000
-#define O_APPEND       0x2000
-#define O_NONBLOCK     0x4000
-#define O_DIRECTORY  0x200000
+#define O_CREAT          0x40
+#define O_EXCL           0x80
+#define O_NOCTTY        0x100
+#define O_TRUNC         0x200
+#define O_APPEND        0x400
+#define O_NONBLOCK      0x800
+#define O_DIRECTORY   0x10000
 
 struct sys_stat_struct {
 	unsigned long	st_dev;		/* Device.  */
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/6] pending bug fixes for nolibc
  2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
                   ` (5 preceding siblings ...)
  2023-01-09  7:54 ` [PATCH 6/6] tools/nolibc: fix the O_* fcntl/open macro definitions for riscv Willy Tarreau
@ 2023-01-09 19:11 ` Paul E. McKenney
  2023-01-10  7:28   ` Willy Tarreau
  6 siblings, 1 reply; 9+ messages in thread
From: Paul E. McKenney @ 2023-01-09 19:11 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: linux-kernel, Warner Losh, Sven Schnelle

On Mon, Jan 09, 2023 at 08:54:36AM +0100, Willy Tarreau wrote:
> Hello Paul,
> 
> please consider the current patch series for merging into your fixes queue.
> The intent is to get them before 6.2, then backported where relevant.
> 
> It addresses the following bugs:
>   - fd_set was incorrectly defined as arrays of u32 instead of long,
>     which breaks BE64. Fix courtesy of Sven Schnelle.
> 
>   - S_ISxxx macros were incorrectly testing the bits after applying them
>     instead of applying S_ISFMT to the value. Fix from Warner Losh.
> 
>   - the mips code was randomly broken due to an unprotected "noreorder"
>     directive in the _start code that would prevent the assembler from
>     filling delayed slots, and randomly leaving other instructions there
> 
>   - since the split of the single include file into multiple files, we're
>     implicitly refraining from including some which are not explicitly
>     added in the code. It causes build failures when such files contain
>     definitions for functions that may be used e.g. by libgcc, such as
>     raise() or memset(), which are often called only by a few archs at
>     certain optimization levels only.
> 
>   - gcc 11.3 in ARM thumb2 mode at -O2 was able to recognize a memset()
>     construction inside the memset() definition, and it replaced it with
>     a call to... memset(). We cannot impose to userland to build with
>     -ffreestanding so the introduction of an empty asm() statement in
>     the loop was enough to stop this.
> 
>   - most of the O_* macros were wrong on RISCV because their octal value
>     was used as a hexadecimal one when the platform was introduced. This
>     was revealed by the selftest breaking in getdents64().
> 
> The series was tested on x86_64, i386, armv5, armv7, thumb1, thumb2,
> mips and riscv, all at -O0, -Os and -O3. This is based on the "nolibc"
> branch of your linux-rcu tree. Do not hesitate to let me know if you
> prefer that I rebase it on a different one.

"81 test(s) passed", so queued at urgent-nolibc.2023.01.09a, thank you!

Also, thank you for the detailed cover letter, which I co-opted into the
signed tag.  But please check to make sure that my wordsmithing didn't
break anything.

If all goes well, I will send the pull request to Linus before the end
of this week.

							Thanx, Paul

> Thank you!
> Willy
> 
> ---
> Sven Schnelle (1):
>   nolibc: fix fd_set type
> 
> Warner Losh (1):
>   tools/nolibc: Fix S_ISxxx macros
> 
> Willy Tarreau (4):
>   tools/nolibc: restore mips branch ordering in the _start block
>   tools/nolibc: fix missing includes causing build issues at -O0
>   tools/nolibc: prevent gcc from making memset() loop over itself
>   tools/nolibc: fix the O_* fcntl/open macro definitions for riscv
> 
>  tools/include/nolibc/arch-mips.h  |  2 +
>  tools/include/nolibc/arch-riscv.h | 14 +++----
>  tools/include/nolibc/ctype.h      |  3 ++
>  tools/include/nolibc/errno.h      |  3 ++
>  tools/include/nolibc/signal.h     |  3 ++
>  tools/include/nolibc/stdio.h      |  3 ++
>  tools/include/nolibc/stdlib.h     |  3 ++
>  tools/include/nolibc/string.h     |  8 +++-
>  tools/include/nolibc/sys.h        |  2 +
>  tools/include/nolibc/time.h       |  3 ++
>  tools/include/nolibc/types.h      | 70 ++++++++++++++++++-------------
>  tools/include/nolibc/unistd.h     |  3 ++
>  12 files changed, 79 insertions(+), 38 deletions(-)
> 
> -- 
> 2.35.3
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/6] pending bug fixes for nolibc
  2023-01-09 19:11 ` [PATCH 0/6] pending bug fixes for nolibc Paul E. McKenney
@ 2023-01-10  7:28   ` Willy Tarreau
  0 siblings, 0 replies; 9+ messages in thread
From: Willy Tarreau @ 2023-01-10  7:28 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: linux-kernel, Warner Losh, Sven Schnelle

Hello Paul,

On Mon, Jan 09, 2023 at 11:11:41AM -0800, Paul E. McKenney wrote:
> On Mon, Jan 09, 2023 at 08:54:36AM +0100, Willy Tarreau wrote:
> > Hello Paul,
> > 
> > please consider the current patch series for merging into your fixes queue.
> > The intent is to get them before 6.2, then backported where relevant.
> > 
> > It addresses the following bugs:
> >   - fd_set was incorrectly defined as arrays of u32 instead of long,
> >     which breaks BE64. Fix courtesy of Sven Schnelle.
> > 
> >   - S_ISxxx macros were incorrectly testing the bits after applying them
> >     instead of applying S_ISFMT to the value. Fix from Warner Losh.
> > 
> >   - the mips code was randomly broken due to an unprotected "noreorder"
> >     directive in the _start code that would prevent the assembler from
> >     filling delayed slots, and randomly leaving other instructions there
> > 
> >   - since the split of the single include file into multiple files, we're
> >     implicitly refraining from including some which are not explicitly
> >     added in the code. It causes build failures when such files contain
> >     definitions for functions that may be used e.g. by libgcc, such as
> >     raise() or memset(), which are often called only by a few archs at
> >     certain optimization levels only.
> > 
> >   - gcc 11.3 in ARM thumb2 mode at -O2 was able to recognize a memset()
> >     construction inside the memset() definition, and it replaced it with
> >     a call to... memset(). We cannot impose to userland to build with
> >     -ffreestanding so the introduction of an empty asm() statement in
> >     the loop was enough to stop this.
> > 
> >   - most of the O_* macros were wrong on RISCV because their octal value
> >     was used as a hexadecimal one when the platform was introduced. This
> >     was revealed by the selftest breaking in getdents64().
> > 
> > The series was tested on x86_64, i386, armv5, armv7, thumb1, thumb2,
> > mips and riscv, all at -O0, -Os and -O3. This is based on the "nolibc"
> > branch of your linux-rcu tree. Do not hesitate to let me know if you
> > prefer that I rebase it on a different one.
> 
> "81 test(s) passed", so queued at urgent-nolibc.2023.01.09a, thank you!
> 
> Also, thank you for the detailed cover letter, which I co-opted into the
> signed tag.

You're welcome!

> But please check to make sure that my wordsmithing didn't
> break anything.

It all looks perfect to me.

> If all goes well, I will send the pull request to Linus before the end
> of this week.

Great, thank you!
Willy

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-01-10  7:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-09  7:54 [PATCH 0/6] pending bug fixes for nolibc Willy Tarreau
2023-01-09  7:54 ` [PATCH 1/6] nolibc: fix fd_set type Willy Tarreau
2023-01-09  7:54 ` [PATCH 2/6] tools/nolibc: Fix S_ISxxx macros Willy Tarreau
2023-01-09  7:54 ` [PATCH 3/6] tools/nolibc: restore mips branch ordering in the _start block Willy Tarreau
2023-01-09  7:54 ` [PATCH 4/6] tools/nolibc: fix missing includes causing build issues at -O0 Willy Tarreau
2023-01-09  7:54 ` [PATCH 5/6] tools/nolibc: prevent gcc from making memset() loop over itself Willy Tarreau
2023-01-09  7:54 ` [PATCH 6/6] tools/nolibc: fix the O_* fcntl/open macro definitions for riscv Willy Tarreau
2023-01-09 19:11 ` [PATCH 0/6] pending bug fixes for nolibc Paul E. McKenney
2023-01-10  7:28   ` Willy Tarreau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).