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
From: Warner Losh <[email protected]>
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 <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
---
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
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 <[email protected]>
---
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
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
>
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