search

chettrick / discobsd

2.11BSD-based Unix-like OS for STM32 and PIC32 Microcontrollers
http://www.DiscoBSD.org
BSD 3-Clause "New" or "Revised" License
182 stars 16 forks source link

sys/kernel: Check elf machine type based on architecture #12

Closed amarkee closed 1 year ago

amarkee commented 1 year ago

Elf checking code would always check for MIPS architecture, even on ARM platforms.

Compiler macro check is potentially not portable to all compilers.

I only tested this building with gcc 9.4.0 on an ubuntu 20.04.4 system and running on the f411renucleo platform.

chettrick commented 1 year ago

The ELF exec code for stm32 needs a whole rework. As you can see in file /sys/stm32/elf_machdep.h and its commit log (use --follow for full history), it is a clone of the pic32 file from when stm32 was first bootstrapped. Since I have been running only a.out binaries for the base system, I am sure there are many more issues with getting ELF binaries working well. Also, most of the binutils-type tools in the base system only work for a.out binaries.

This can go in, though, with some changes.

For the #ifdef or #if defined() we should be using the __thumb2__ macro for Cortex-M3/4/7 devices and __thumb__ for Cortex-M0+. __arm__ is defined for Cortex-M, but it is also defined for the Cortex-A family and other legacy Arm devices like ARM7TDMI, which are not supported at all.

/sys/kern only uses #ifdef, but #if defined() is used in /sys/stm32 and /sys/pic32.

Have any #if defined() or #ifdef macros in the first column - don't indent them. See /sys/kern/exec_subr.c for an example.

amarkee commented 1 year ago

Thank you for the feedback. I applied the formatting changes and made the error message a bit more descriptive.

What code formatting style do you prefer for this project? OpenBSD's? I'd like to save some work on reviews in the future.

I converted this back into a draft as I'm now going to review /sys/stm32/elf_machdep.h, make any necessary changes and conduct some more thorough testing.

amarkee commented 1 year ago

After reviewing the elf_machdep.h files and comparing them to newer netBSD code (the original appeared to come from there) I came up with a solution I like more. Rather than using pre-processor conditionals in the architecture independent code, a machine specific header is included with ELF32_MACHDEP_ID defined. I then updated the elf_machdep.h files from netBSD.

Most of the macros in elf_machdep.h are unused and are there because they were copied from netBSD. Would you prefer the unused portions be removed or kept for the future?

BTW: thanks for sharing this cool project!