casaroli opened a new pull request, #19510:
URL: https://github.com/apache/nuttx/pull/19510
## Summary
Three commits, each self-contained.
**1. `arch/arm: stop boards silently discarding --fixed-r10`**
`Toolchain.defs` adds `--fixed-r10` to `CFLAGS` under `CONFIG_PIC`, but
nearly
every board `Make.defs` includes that file and then assigns
```make
CFLAGS := $(ARCHCFLAGS) $(ARCHOPTIMIZATION) $(ARCHCPUFLAGS) ...
```
with `:=`, which discards it. 266 of the 269 ARM board files assign
`CFLAGS` that way; the remaining three delegate to a shared makefile that
does
the same thing.
The flag is what stops the base firmware allocating r10 — the register a PIC
module reaches its own data through. Losing it is silent and the symptom is
remote from the cause: the build succeeds, and only a callback from firmware
into module code (`qsort()` with a module comparison function is the standard
case) misbehaves, reading its data through a register the firmware has since
reused.
Moving it to `ARCHCFLAGS` puts it on the far side of that `:=`, which
re-expands `ARCHCFLAGS`. No board changes are needed.
**2. `arch/arm: hoist the duplicated PIC module flags into Toolchain.defs`**
Every ARM board carried the same three lines — 258, 267 and 267 copies:
```make
ARCHPICFLAGS = -fpic -msingle-pic-base -mpic-register=r10
CPICFLAGS = $(ARCHPICFLAGS) $(CFLAGS)
CXXPICFLAGS = $(ARCHPICFLAGS) $(CXXFLAGS)
```
They move to `arch/arm/src/common/Toolchain.defs`. `ARCHPICFLAGS` uses `?=`
and the derived flags use deferred `=`, so a board can still override or
append after including the file, and `CFLAGS` is whatever the board finally
set it to.
This is also what makes commit 1 safe in general. A module is the other side
of the `--fixed-r10` contract: it gets r10 via `-mpic-register=r10`, and GCC
rejects both on one command line with *"unable to use 'r10' for PIC
register"*. Because `CPICFLAGS`, `CXXPICFLAGS`, `CELFFLAGS` and `CXXELFFLAGS`
all derive from `CFLAGS`, the flag now gets filtered back out in one place
instead of each board having to do it.
Three boards keep a definition because they genuinely differ: `am67` adds
`-ffixed-r10`, `tiva` conditionally adds `-mno-pic-data-is-text-relative`,
and
`tlsr82` wants only `-fpic`. `tlsr82` previously appended to an unset
variable, so it becomes a plain assignment to preserve its result.
**3. `arch/arm: add the missing space in CXXELFFLAGS`**
```make
CXXELFFLAGS = $(CXXFLAGS)-fvisibility=hidden -mlong-calls
```
runs the last token of `CXXFLAGS` into the first of the additions. On
`stm32f4discovery:nsh` that yields the single token
`-DNDEBUG-fvisibility=hidden`, so a C++ ELF module gets neither a usable
`NDEBUG` nor the `-fvisibility=hidden` it was meant to be built with. Which
token is mangled depends on what `CXXFLAGS` ends with, so it varies by board.
## Impact
- **Boards that defined the standard three lines (the large majority):** no
change in flags at all — verified below.
- **Boards that referenced `ARCHPICFLAGS` without ever defining it** — the
`mps2`/`mps3`, `qemu-armv7a`/`armv7r`, `fvp` and `mcx-nxxx` families, seven
in total: their `CPICFLAGS` previously carried no PIC flags whatsoever, so
any PIC module they built could not have worked. They now get the standard
set. **This is the one behavioural change in the series.**
- **`CONFIG_PIC` configurations:** the base firmware now genuinely reserves
r10, and module flags no longer carry the contradictory `--fixed-r10`.
`mps3-an547:picostest` is the only defconfig in the tree with
`CONFIG_PIC=y`.
- **C++ ELF modules:** gain a correct `NDEBUG` and `-fvisibility=hidden`
instead of one malformed token.
- **Board maintainers:** a new board no longer needs to copy the three lines,
and a board enabling `CONFIG_PIC` no longer needs to know about the
`--fixed-r10` interaction.
- **Diff size:** 269 files, +91/−1084. All but three of the board changes are
pure deletions.
- Documentation, security and non-ARM architectures are unaffected. `arm64`
has its own `Toolchain.defs` and is untouched.
## Testing
Host:
```
macOS 26.5.1 (Darwin 25.5.0, arm64)
Arm GNU Toolchain 15.2.Rel1 (arm-none-eabi-gcc 15.2.1)
GNU make
```
### Flag equivalence, before and after
The risk in a change this wide is silently altering a board's flags, so
`ARCHPICFLAGS`, `CPICFLAGS`, `CXXPICFLAGS` and `CFLAGS` were dumped for a
spread of boards on `master` and on this branch and compared token by token.
Boards were chosen to cover the standard case, each of the three that differ,
and the ones that never defined `ARCHPICFLAGS`:
| Board | Result |
|---|---|
| `stm32f4discovery:nsh` | identical |
| `lm3s6965-ek:nsh` (conditional append) | identical |
| `tlsr8278adk80d:nsh` (`-fpic` only) | identical |
| `t3-gem-o1:nsh` (`-ffixed-r10`) | identical |
| `nucleo-l552ze:nsh` | identical |
| `mps3-an547:picostest` | `+[-fpic -msingle-pic-base -mpic-register=r10]`,
`CPICFLAGS -[--fixed-r10]` |
| `mps2-an521:nsh` | `+[-fpic -msingle-pic-base -mpic-register=r10]` |
| `qemu-armv7a:nsh` | `+[-fpic -msingle-pic-base -mpic-register=r10]` |
The three non-identical boards are exactly the ones that referenced
`ARCHPICFLAGS` without defining it, as described under Impact.
### The flag actually reaching the compiler
```
$ ./tools/configure.sh mps3-an547:picostest
$ make -f /tmp/show.mk TOPDIR=$PWD show # prints $(CFLAGS) etc.
```
On master:
```
CONFIG_PIC = y
CFLAGS has --fixed-r10 : NO
```
On this branch:
```
CONFIG_PIC = y
CFLAGS has --fixed-r10 : YES
CPICFLAGS has --fixed-r10 : NO
CELFFLAGS has --fixed-r10 : NO
```
### A PIC module compiles
Compiling a file with the exact `CPICFLAGS` the build computes, on
`mps3-an547:picostest`:
```
$ arm-none-eabi-gcc $CPICFLAGS -c t.c -o t.o
cc1: error: unable to use 'r10' for PIC register # commit 1 alone
# commit 2: compiles
```
That error is what makes commits 1 and 2 a pair. Applying the `ARCHCFLAGS`
move on its own is fine for `mps3-an547` as it stands today (it has no
`-mpic-register=r10` to clash with), but becomes a build failure the moment
that board gains the standard `ARCHPICFLAGS` — which commit 2 gives it.
### CXXELFFLAGS
`stm32f4discovery:nsh`, before:
```
-DNDEBUG-fvisibility=hidden
```
after:
```
-DNDEBUG
-fvisibility=hidden
```
### Builds
```
stm32f4discovery:nsh make exit=0
mps3-an547:picostest (CONFIG_PIC=y) make exit=0
```
`lm3s6965-ek:nsh` was not built: its configuration selects the
`arm-nuttx-elf-` buildroot toolchain, which is not installed on this host
(`arm-nuttx-elf-gcc failed: 127`). Its flags are covered by the equivalence
check above.
### Tree checks
```
$ ./tools/checkpatch.sh -c -u -m -g master..HEAD
Used config files:
1: .codespellrc
✔️ All checks pass.
```
Commit 1 also adds one line to `.codespell-ignore-lines`: the pre-existing
`"*.siz"` gsize comment in `Toolchain.defs`, which codespell reads as a
misspelling of "size". It fires for any patch touching that file, because
checkpatch scans whole files rather than changed lines.
--
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]
