casaroli opened a new pull request, #19600:
URL: https://github.com/apache/nuttx/pull/19600

   ## Summary
   
   ARM PIC uses r10 as the base register, but the tree has never been 
consistent about it. `arch/arm/src/common/Toolchain.defs` gives 
`CONFIG_BUILD_PIC` `-mpic-register=r9` at line 594 and `CONFIG_PIC` 
`-mpic-register=r10` at line 620, and `arm_initialstate.c` sets `REG_R9` from 
inline assembly under one and `REG_PIC` under the other, with a comment that 
reads *"Set the PIC base register (probably R10)"*. This settles it on r9 for 
all of PIC: NXFLAT, ELF PIC and `CONFIG_BUILD_PIC` alike.
   
   r9 is the right register rather than an arbitrary one. It is the AAPCS 
platform register, the "static base", and it is what GCC itself selects for 
`-msingle-pic-base` on an EABI target; r10 is the non-EABI default.
   
   It also fixes a configuration that cannot build today. Stack checking adds 
`-ffixed-r10` in `armv7-m/Toolchain.defs:149` and `armv8-m/Toolchain.defs:168` 
while `CONFIG_PIC` adds `-mpic-register=r10`, and GCC rejects the pair with 
*"unable to use 'r10' for PIC register"*. The comment above `REG_PIC` has 
always said the register "can be R9 if stack checking is enabled", but the 
definition was unconditionally `REG_R10`, so it would have named the wrong 
register even if the build had succeeded.
   
   ## Why the NXFLAT thunk generator comes with it
   
   Moving the register on its own breaks NXFLAT, silently. Import thunks are 
generated by `mknxflat`, which had the register baked into its template as `add 
ip,ip,sl`, so a module compiled `-mpic-register=r9` reaches its data through r9 
while its import stubs still add r10, and it branches to a wild address on its 
first call into the base firmware.
   
   `mknxflat` lives outside this repository, in the buildroot NXFLAT toolchain, 
so the kernel and the tool could drift with nothing to detect it. The first 
commit therefore brings it in-tree as `tools/nxflat/`, with libbfd replaced by 
reading the ELF symbol table directly — libbfd was only ever used to open the 
file and enumerate symbols, and it is GPL, which we cannot depend on. The 
thunks are then generated from a single `NXFLAT_PIC_REG` on every build, so the 
two cannot drift.
   
   That commit changes no output. Against the upstream tool, for both ARM and 
Thumb-2, with and without `-w`, over modules exercising the plain, weak and 
non-returning thunk paths, the generated thunk files are byte-identical.
   
   ## Modules built before this change
   
   They are refused at load rather than left to crash.
   
   The NXFLAT header cannot carry a version: `h_magic` is written by 
`ldnxflat`, which is GPL, derived from elf2flt, and stays out of this 
repository, so it can never be changed in step with the loader. The import 
table can, because both of its ends are in-tree — `mknxflat` emits it and 
`nxflat_bindimports()` reads it — and `ldnxflat` passes it through untouched. 
Every module now imports `__nxflat_abi_v2`, the base firmware defines it, and a 
module that does not import it is refused with `-ENOEXEC`.
   
   Making the marker a real exported symbol rather than a name the loader 
special-cases keeps it out of the build system's way: a board's symbol table 
picks it up exactly as it picks up `printf`, so `mksymtab.sh` and its 
equivalents need no change. It also diagnoses the reverse direction for free — 
a module built against a newer ABI than its firmware fails with `Exported 
symbol "__nxflat_abi_v2" not found`.
   
   ## Impact
   
   Most of the churn is boards restating a default. `ARCHPICFLAGS` is a `?=` 
default so that a board only speaks up when it differs, and twenty-six were 
assigning the value the default already had; those lines are removed rather 
than edited. `MKNXFLAT` gets the same treatment: thirteen boards named the same 
tool, and the only thing that varies is ARM versus Thumb-2, which falls out of 
`CONFIG_ARM_THUMB`. `LDNXFLAT` gains a default too — it stays an out-of-tree 
PATH lookup, but naming it centrally fixes boards that never assigned it, where 
it expanded to nothing and handed make a recipe beginning `-e`, whose leading 
dash make ate as "ignore errors".
   
   The non-ARM boards carrying `-mpic-register=r10` lose it. It is an ARM-only 
option, reachable only through `CPICFLAGS`, which is only used to build NXFLAT 
modules, and no non-ARM board enables NXFLAT; six other RISC-V boards already 
omitted it.
   
   `mknxflat` is built only when `CONFIG_NXFLAT` is set, following the 
`CONFIG_BOARD_ETC_ROMFS_PASSWD_ENABLE` precedent in `tools/Unix.mk`.
   
   **Breaking change for NXFLAT users.** A prebuilt NXFLAT module, or one built 
with an out-of-tree `mknxflat`, will no longer load. It is rejected cleanly 
with `-ENOEXEC` rather than crashing, and the fix is to rebuild it against this 
tree. Nothing else is affected: `CONFIG_PIC` without `CONFIG_NXFLAT` needs no 
action, and configurations with neither are untouched.
   
   ## Licensing
   
   `tools/nxflat/mknxflat.c` and the two `.def` templates are BSD-3-Clause, 
copyright Gregory Nutt and Cadenux LLC, imported from the NuttX buildroot 
NXFLAT toolchain with their headers intact. The containing repository's 
`COPYING` is GPL-2.0, but these particular files carry their own BSD-3 grant, 
name only Nutt and Cadenux as copyright holders, and descend from XFLAT's 
`ldelflib` rather than from elf2flt. @gregory-nutt for confirmation.
   
   `ldnxflat` is deliberately not imported. It is GPL-2.0-or-later and derives 
from elf2flt, with roughly ten copyright holders reaching back to 1996 — it 
cannot be relicensed and cannot live in an Apache repository. NXFLAT continues 
to need it from the buildroot toolchain, exactly as today.
   
   ## Testing
   
   Host: macOS 26.5.1 on arm64, Arm GNU Toolchain 14.2.Rel1, QEMU 11.0.3.
   
   `lm3s6965-ek:qemu-nxflat` under QEMU, configured and built with no 
overrides. The `nxflat` example runs the errno, hello and struct modules with 
output identical to the same configuration built from master:
   
   ```
   * Executing errno      -> Hello, World on stdout / stderr, errno is 2
   * Executing hello      -> Hello, world!  It has been said.
   * Executing struct     -> End-of-Test.. Exit-ing
   ```
   
   The same firmware, with the modules rebuilt by the old out-of-tree 
generator, refuses all three:
   
   ```
   ERROR: exec(errno) failed: 8
   ERROR: exec(hello) failed: 8
   ```
   
   Before the ABI marker that same combination locked up in a HardFault with no 
console output, which is what the marker is for.
   
   `mps3-an547:picostest`, which is `CONFIG_PIC` without `CONFIG_NXFLAT`, 
builds clean and does not build the thunk generator.
   
   `tools/checkpatch.sh -f` passes on every changed C file and header.
   
   Not covered: ELF PIC has no runtime coverage here. `mps3-an547:picostest` 
builds and boots identically to master, but NSH takes no console input in that 
configuration under QEMU — on master as well — so no module is actually 
executed. The modules were confirmed statically to use r9 as their PIC base.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to