Set of patches for few problems found when trying to build project. Also enable some additional checks and also fix issues found.
Amadeusz Sławiński (7): adb_kbd.c: Fix function key definitions Change malloc to take size_t as argument Label fallthroughs in switch()es Makefile.target: Enable few additional warnings Disable stack protector on all architectures bootstrap: Check result of fread Tell linker that stack is not executable
Makefile.target | 2 ++ arch/amd64/lib.c | 2 +- arch/ppc/build.xml | 8 ++++---- arch/ppc/ofmem.c | 2 +- arch/ppc/qemu/ofmem.c | 2 +- arch/sparc32/lib.c | 2 +- arch/sparc64/lib.c | 2 +- arch/x86/lib.c | 2 +- config/scripts/switch-arch | 14 +++++++------- drivers/adb_bus.c | 2 +- drivers/adb_kbd.c | 2 +- include/libc/stdlib.h | 2 +- kernel/bootstrap.c | 6 +++++- libc/vsprintf.c | 1 + 14 files changed, 28 insertions(+), 21 deletions(-)
Fix typo, where last definition should have been F16 instead of F15. Found with -Woverride-init
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- drivers/adb_kbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/adb_kbd.c b/drivers/adb_kbd.c index dec8366..9d67162 100644 --- a/drivers/adb_kbd.c +++ b/drivers/adb_kbd.c @@ -88,7 +88,7 @@ static const char *ADB_sequences[] = { [KEY_F13] = "~25[", [KEY_F14] = "~26[", [KEY_F15] = "~28[", - [KEY_F15] = "~29[", + [KEY_F16] = "~29[", };
/* ADB US keyboard translation map */
Should not affect behaviour as it just calls ofmem_malloc which takes size_t as argument anyway. Reported when build with -Wbuiltin-declaration-mismatch.
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- arch/amd64/lib.c | 2 +- arch/ppc/ofmem.c | 2 +- arch/ppc/qemu/ofmem.c | 2 +- arch/sparc32/lib.c | 2 +- arch/sparc64/lib.c | 2 +- arch/x86/lib.c | 2 +- include/libc/stdlib.h | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/amd64/lib.c b/arch/amd64/lib.c index f04458e..9efbe68 100644 --- a/arch/amd64/lib.c +++ b/arch/amd64/lib.c @@ -39,7 +39,7 @@ static char memory[MEMSIZE]; static void *memptr=memory; static int memsize=MEMSIZE;
-void *malloc(int size) +void *malloc(size_t size) { void *ret=(void *)0; if(memsize>=size) { diff --git a/arch/ppc/ofmem.c b/arch/ppc/ofmem.c index c9b066e..5da641b 100644 --- a/arch/ppc/ofmem.c +++ b/arch/ppc/ofmem.c @@ -131,7 +131,7 @@ void ofmem_arch_map_pages(ucell phys, ucell virt, ucell size, ucell mode) /************************************************************************/
void * -malloc( int size ) +malloc( size_t size ) { return ofmem_malloc(size); } diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index 4ff0803..3a21d25 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -233,7 +233,7 @@ pa2va(phys_addr_t pa) }
void * -malloc(int size) +malloc(size_t size) { return ofmem_malloc(size); } diff --git a/arch/sparc32/lib.c b/arch/sparc32/lib.c index 727929c..ad27c7c 100644 --- a/arch/sparc32/lib.c +++ b/arch/sparc32/lib.c @@ -85,7 +85,7 @@ pa2va(phys_addr_t pa) }
void * -malloc(int size) +malloc(size_t size) { return ofmem_malloc(size); } diff --git a/arch/sparc64/lib.c b/arch/sparc64/lib.c index e9fff28..1a5c9f1 100644 --- a/arch/sparc64/lib.c +++ b/arch/sparc64/lib.c @@ -55,7 +55,7 @@ pa2va(phys_addr_t pa) return pa; }
-void *malloc(int size) +void *malloc(size_t size) { return ofmem_malloc(size); } diff --git a/arch/x86/lib.c b/arch/x86/lib.c index eeb901b..6618e5c 100644 --- a/arch/x86/lib.c +++ b/arch/x86/lib.c @@ -39,7 +39,7 @@ static char memory[MEMSIZE]; static void *memptr=memory; static int memsize=MEMSIZE;
-void *malloc(int size) +void *malloc(size_t size) { void *ret=(void *)0; if(memsize>=size) { diff --git a/include/libc/stdlib.h b/include/libc/stdlib.h index ef08838..3784eb4 100644 --- a/include/libc/stdlib.h +++ b/include/libc/stdlib.h @@ -16,7 +16,7 @@ #ifndef _H_STDLIB #define _H_STDLIB
-extern void *malloc( int size ); +extern void *malloc( size_t size ); extern void free( void *ptr ); extern void *realloc( void *ptr, size_t size );
There is few places where code wants to fallthrough, label them so it is known that it is expected. Detected with -Wimplicit-fallthrough
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- drivers/adb_bus.c | 2 +- libc/vsprintf.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/adb_bus.c b/drivers/adb_bus.c index 1aa442a..acd59b3 100644 --- a/drivers/adb_bus.c +++ b/drivers/adb_bus.c @@ -248,7 +248,7 @@ int adb_cmd (adb_dev_t *dev, uint8_t cmd, uint8_t reg, break; case ADB_LISTEN: memcpy(adb_send + 1, buf, len); - /* No break here */ + /* fallthrough */ case ADB_TALK: adb_send[0] = (dev->addr << 4) | cmd | reg; break; diff --git a/libc/vsprintf.c b/libc/vsprintf.c index 29ec7b9..ed4d38a 100644 --- a/libc/vsprintf.c +++ b/libc/vsprintf.c @@ -333,6 +333,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
case 'X': flags |= LARGE; + /* fallthrough */ case 'x': base = 16; break;
Add -Wbuiltin-declaration-mismatch -Wimplicit-fallthrough -Woverride-init -Wno-maybe-uninitialized to HOSTCFLAGS and CFLAGS.
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- Makefile.target | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/Makefile.target b/Makefile.target index 4c54105..6f2d264 100644 --- a/Makefile.target +++ b/Makefile.target @@ -12,6 +12,7 @@ HOSTCFLAGS+= -O2 -g -DFCOMPILER -DBOOTSTRAP $(CROSSCFLAGS) HOSTCFLAGS+= -Wall -Wredundant-decls -Wshadow -Wpointer-arith HOSTCFLAGS+= -Wstrict-prototypes -Wmissing-declarations -Wundef -Wendif-labels HOSTCFLAGS+= -Wstrict-aliasing -Wwrite-strings -Wmissing-prototypes -Wnested-externs +HOSTCFLAGS+= -Wbuiltin-declaration-mismatch -Wimplicit-fallthrough -Woverride-init -Wno-maybe-uninitialized HOSTCFLAGS+= -W # Flags for dependency generation HOSTCFLAGS+= -MMD -MP -MT $@ -MF '$(*D)/$(*F).d' @@ -29,6 +30,7 @@ CFLAGS+= -Os -g -DNATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH -USWAP_ENDIANNESS CFLAGS+= -Wall -Wredundant-decls -Wshadow -Wpointer-arith CFLAGS+= -Wstrict-prototypes -Wmissing-declarations -Wundef -Wendif-labels CFLAGS+= -Wstrict-aliasing -Wwrite-strings -Wmissing-prototypes -Wnested-externs +CFLAGS+= -Wbuiltin-declaration-mismatch -Wimplicit-fallthrough -Woverride-init -Wno-maybe-uninitialized CFLAGS+= -Werror # Flags for dependency generation CFLAGS+= -MMD -MP -MT $@ -MF '$(*D)/$(*F).d'
OpenBIOS currently fails to link with stack protector enabled, as it is unlikely needed in boot loader anyway, just make sure it is disabled on all targets.
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- config/scripts/switch-arch | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/config/scripts/switch-arch b/config/scripts/switch-arch index b5acc6c..f4da5c6 100755 --- a/config/scripts/switch-arch +++ b/config/scripts/switch-arch @@ -258,7 +258,7 @@ for ARCH in $arch_list; do case $ARCH in amd64) select_prefix x86_64 - CFLAGS="-fno-builtin" + CFLAGS="-fno-builtin -fno-stack-protector" AS_FLAGS= ;;
@@ -266,10 +266,10 @@ for ARCH in $arch_list; do select_prefix powerpc powerpc64 if [ "$unix" = "no" ]; then # 604 cpu includes support for PReP as well as Mac - CFLAGS="-m32 -mcpu=604 -msoft-float -fno-builtin-bcopy -fno-builtin-log2" + CFLAGS="-m32 -mcpu=604 -msoft-float -fno-builtin-bcopy -fno-builtin-log2 -fno-stack-protector" AS_FLAGS="-m32" else - CFLAGS="-fno-builtin" + CFLAGS="-fno-builtin -fno-stack-protector" AS_FLAGS= fi ;; @@ -278,25 +278,25 @@ for ARCH in $arch_list; do select_prefix powerpc64
# 970 cpu is used in all 64-bit Macs but disable altivec - CFLAGS="-mcpu=970 -mno-altivec -Wa,-a64 -m64 -msoft-float -fno-builtin" + CFLAGS="-mcpu=970 -mno-altivec -Wa,-a64 -m64 -msoft-float -fno-builtin -fno-stack-protector" AS_FLAGS="-Wa,-a64" ;;
sparc32) select_prefix sparc sparc64 - CFLAGS="-Wa,-xarch=v8 -Wa,-32 -m32 -mcpu=supersparc -fno-builtin" + CFLAGS="-Wa,-xarch=v8 -Wa,-32 -m32 -mcpu=supersparc -fno-builtin -fno-stack-protector" AS_FLAGS="-Wa,-xarch=v8 -Wa,-32" ;;
sparc64) select_prefix sparc64 - CFLAGS="-Wa,-xarch=v9b -Wa,-64 -m64 -mcpu=ultrasparc -mcmodel=medany -fno-builtin" + CFLAGS="-Wa,-xarch=v9b -Wa,-64 -m64 -mcpu=ultrasparc -mcmodel=medany -fno-builtin -fno-stack-protector" AS_FLAGS="-Wa,-xarch=v9b -Wa,-64" ;;
x86) select_prefix i486 x86_64 - CFLAGS="-fno-builtin -m32" + CFLAGS="-fno-builtin -m32 -fno-stack-protector" AS_FLAGS="-Wa,-32" ;; esac
Removes -Wunused-result warning.
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- kernel/bootstrap.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/bootstrap.c b/kernel/bootstrap.c index b7658ab..9bc94bb 100644 --- a/kernel/bootstrap.c +++ b/kernel/bootstrap.c @@ -1004,7 +1004,11 @@ encode_file( const char *name ) if (verbose) { printk("\nEncoding %s [%d bytes]\n", name, size ); } - fread( dict + dicthead, size, 1, file ); + if (fread( dict + dicthead, size, 1, file ) != 1) { + printk("panic: Can't read dictionary.\n"); + fclose(file); + exit(1); + } PUSH( pointer2cell(dict + dicthead) ); PUSH( size ); dicthead += size;
Eliminates following warning from linker: powerpc-unknown-linux-gnu-ld: warning: crtsavres.o: missing .note.GNU-stack section implies executable stack powerpc-unknown-linux-gnu-ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
Signed-off-by: Amadeusz Sławiński amade@asmblr.net --- arch/ppc/build.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/ppc/build.xml b/arch/ppc/build.xml index e606313..9415376 100644 --- a/arch/ppc/build.xml +++ b/arch/ppc/build.xml @@ -146,7 +146,7 @@
<executable name="openbios-briq.elf" target="target" condition="BRIQ"> <rule> - $(call quiet-command,$(LD) -g -Ttext=0x01e01000 -Bstatic $^ $(shell $(CC) -print-libgcc-file-name) -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") + $(call quiet-command,$(LD) -g -z noexecstack -Ttext=0x01e01000 -Bstatic $^ $(shell $(CC) -print-libgcc-file-name) -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") $(call quiet-command,$(NM) $@.nostrip | sort > $(ODIR)/openbios-briq.syms," GEN $(TARGET_DIR)$@.syms") $(call quiet-command,$(STRIP) $@.nostrip -o $@," STRIP $(TARGET_DIR)$@")</rule> <object source="start.S"/> @@ -162,7 +162,7 @@
<executable name="openbios-pearpc.elf" target="target" condition="PEARPC"> <rule> - $(call quiet-command,$(LD) -g -Ttext=0x01e01000 -Bstatic $^ $(shell $(CC) -print-libgcc-file-name) -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") + $(call quiet-command,$(LD) -g -z noexecstack -Ttext=0x01e01000 -Bstatic $^ $(shell $(CC) -print-libgcc-file-name) -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") $(call quiet-command,$(NM) $@.nostrip | sort > $(ODIR)/openbios-pearpc.syms," GEN $(TARGET_DIR)$@.syms") $(call quiet-command,$(STRIP) $@.nostrip -o $@," STRIP $(TARGET_DIR)$@")</rule> <object source="start.S"/> @@ -178,7 +178,7 @@
<executable name="openbios-qemu.elf" target="target" condition="QEMU"> <rule> - $(call quiet-command,$(LD) --warn-common -N -T $(SRCDIR)/arch/$(ARCH)/qemu/ldscript -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") + $(call quiet-command,$(LD) --warn-common -z noexecstack -N -T $(SRCDIR)/arch/$(ARCH)/qemu/ldscript -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") $(call quiet-command,$(NM) $@.nostrip | sort > $(ODIR)/openbios-qemu.syms," GEN $(TARGET_DIR)$@.syms") $(call quiet-command,$(STRIP) $@.nostrip -o $@," STRIP $(TARGET_DIR)$@")</rule> <object source="qemu/start.S"/> @@ -197,7 +197,7 @@
<executable name="openbios-mol.elf" target="target" condition="MOL"> <rule> - $(call quiet-command,$(LD) -g -Ttext=0x01e01000 -Bstatic $^ $(shell $(CC) -print-libgcc-file-name) -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") + $(call quiet-command,$(LD) -g -z noexecstack -Ttext=0x01e01000 -Bstatic $^ $(shell $(CC) -print-libgcc-file-name) -o $@.nostrip --whole-archive $^," LINK $(TARGET_DIR)$@") $(call quiet-command,$(NM) $@.nostrip | sort > $(ODIR)/openbios-mol.syms," GEN $(TARGET_DIR)$@.syms") $(call quiet-command,$(STRIP) $@.nostrip -o $@," STRIP $(TARGET_DIR)$@")</rule> <object source="start.S"/>
On 24/02/2024 18:25, Amadeusz Sławiński via OpenBIOS wrote:
Set of patches for few problems found when trying to build project. Also enable some additional checks and also fix issues found.
Amadeusz Sławiński (7): adb_kbd.c: Fix function key definitions Change malloc to take size_t as argument Label fallthroughs in switch()es Makefile.target: Enable few additional warnings Disable stack protector on all architectures bootstrap: Check result of fread Tell linker that stack is not executable
Makefile.target | 2 ++ arch/amd64/lib.c | 2 +- arch/ppc/build.xml | 8 ++++---- arch/ppc/ofmem.c | 2 +- arch/ppc/qemu/ofmem.c | 2 +- arch/sparc32/lib.c | 2 +- arch/sparc64/lib.c | 2 +- arch/x86/lib.c | 2 +- config/scripts/switch-arch | 14 +++++++------- drivers/adb_bus.c | 2 +- drivers/adb_kbd.c | 2 +- include/libc/stdlib.h | 2 +- kernel/bootstrap.c | 6 +++++- libc/vsprintf.c | 1 + 14 files changed, 28 insertions(+), 21 deletions(-)
Thanks for this series, and apologies it has taken me a while to look at it. In general this looks good and also coincides with some other build fixes I've been looking at with newer compilers reported by the Debian packagers.
We're nearly at the end of QEMU freeze so once that's over and I've validated these changes against my additional local build fixes, I'll apply these to git.
ATB,
Mark.
On 13/04/2024 12:43, Mark Cave-Ayland wrote:
On 24/02/2024 18:25, Amadeusz Sławiński via OpenBIOS wrote:
Set of patches for few problems found when trying to build project. Also enable some additional checks and also fix issues found.
Amadeusz Sławiński (7): adb_kbd.c: Fix function key definitions Change malloc to take size_t as argument Label fallthroughs in switch()es Makefile.target: Enable few additional warnings Disable stack protector on all architectures bootstrap: Check result of fread Tell linker that stack is not executable
Makefile.target | 2 ++ arch/amd64/lib.c | 2 +- arch/ppc/build.xml | 8 ++++---- arch/ppc/ofmem.c | 2 +- arch/ppc/qemu/ofmem.c | 2 +- arch/sparc32/lib.c | 2 +- arch/sparc64/lib.c | 2 +- arch/x86/lib.c | 2 +- config/scripts/switch-arch | 14 +++++++------- drivers/adb_bus.c | 2 +- drivers/adb_kbd.c | 2 +- include/libc/stdlib.h | 2 +- kernel/bootstrap.c | 6 +++++- libc/vsprintf.c | 1 + 14 files changed, 28 insertions(+), 21 deletions(-)
Thanks for this series, and apologies it has taken me a while to look at it. In general this looks good and also coincides with some other build fixes I've been looking at with newer compilers reported by the Debian packagers.
We're nearly at the end of QEMU freeze so once that's over and I've validated these changes against my additional local build fixes, I'll apply these to git.
Pushed to master, thanks!
ATB,
Mark.
-
Amadeusz Sławiński -
Mark Cave-Ayland