casaroli opened a new pull request, #19508:
URL: https://github.com/apache/nuttx/pull/19508
## Summary
`arch/arm/src/common/Toolchain.defs` adds `--fixed-r10` to `CFLAGS` when
`CONFIG_PIC` is enabled. Nearly every board `Make.defs` includes that file
and
then assigns
```make
CFLAGS := $(ARCHCFLAGS) $(ARCHOPTIMIZATION) $(ARCHCPUFLAGS) ...
```
with `:=`, which discards it. 346 of the tree's board files contain that
exact
`CPICFLAGS = $(ARCHPICFLAGS) $(CFLAGS)` idiom, and every one of them assigns
`CFLAGS` this way first.
That flag is what stops the base firmware from allocating r10 — the register
a
PIC module reaches its own data through. When it is lost, a base firmware
routine that calls back into module code (`qsort()` with a module comparison
function is the standard case) arrives with r10 holding whatever the firmware
last put there, and the module reads its data through a bad base pointer.
The failure is silent and remote from its cause: the build succeeds, every
module loads, and only the callback misbehaves.
`mps3-an547:picostest` is the only defconfig in the tree that sets
`CONFIG_PIC=y`, so it is the only board changed here. It re-applies the flag
after the `:=` and filters it back out of `CPICFLAGS` and `CELFFLAGS`,
because
GCC rejects `--fixed-r10` alongside the `-mpic-register=r10` those carry once
`-fpic` is in effect:
```
cc1: error: unable to use 'r10' for PIC register
```
That filtering is also why the flag cannot simply move to `ARCHCPUFLAGS` to
survive the `:=` — the module flags derive from `CFLAGS`, so it would land in
exactly that rejected combination. `Toolchain.defs` now records this, since
the
next board to enable `CONFIG_PIC` will otherwise hit the same trap.
## Impact
- **Users:** affects `CONFIG_PIC` configurations only. `mps3-an547:picostest`
now genuinely reserves r10 in the base firmware.
- **Build process:** no change for the other 345 boards — they get a comment
in
`Toolchain.defs` and nothing else. No new warnings.
- **Hardware:** none directly; changes generated code for `CONFIG_PIC`
firmware
only (one fewer allocatable register, which is the point).
- **Documentation / security / compatibility:** unaffected.
- **Not fixed here:** any board that enables `CONFIG_PIC` in future still
needs
the same three lines. A tree-wide mechanical change to all 346 files would
fix it once and for all; I did not attempt that, and would rather a
maintainer say whether it is wanted before 346 files are touched.
## Testing
Board: `mps3-an547:picostest` — the only defconfig with `CONFIG_PIC=y`.
Because the failure is a lost compiler flag, the test is what the build
system
computes for `CFLAGS`. Reproduce with:
```sh
./tools/configure.sh mps3-an547:picostest
cat > /tmp/show.mk <<'EOF'
include $(TOPDIR)/Make.defs
show:
@echo "CONFIG_PIC = $(CONFIG_PIC)"
@echo "CFLAGS has --fixed-r10 : $(if $(filter
--fixed-r10,$(CFLAGS)),YES,NO)"
@echo "CPICFLAGS has --fixed-r10 : $(if $(filter
--fixed-r10,$(CPICFLAGS)),YES,NO)"
@echo "CELFFLAGS has --fixed-r10 : $(if $(filter
--fixed-r10,$(CELFFLAGS)),YES,NO)"
EOF
make -f /tmp/show.mk TOPDIR=$PWD show
```
Before (master `7df7c6ee`) — the flag is gone:
```
CONFIG_PIC = y
CFLAGS has --fixed-r10 : NO
CPICFLAGS has --fixed-r10 : NO
CELFFLAGS has --fixed-r10 : NO
```
After this patch — firmware reserves r10, module flags do not:
```
CONFIG_PIC = y
CFLAGS has --fixed-r10 : YES
CPICFLAGS has --fixed-r10 : NO
CELFFLAGS has --fixed-r10 : NO
```
The conflict that makes the filtering necessary, shown directly:
```
$ arm-none-eabi-gcc -mcpu=cortex-m55 -mthumb -fpic -msingle-pic-base \
--fixed-r10 -mpic-register=r10 -c t.c -o t.o
cc1: error: unable to use 'r10' for PIC register
$ arm-none-eabi-gcc -mcpu=cortex-m55 -mthumb -fpic -msingle-pic-base \
-mpic-register=r10 -c t.c -o t.o # accepted
```
**Limitation, stated plainly:** I could not produce a full link of
`mps3-an547:picostest`, and it is not for want of a toolchain — I repeated
this
with the Arm GNU Toolchain 15.2.Rel1 (full newlib). The Makefile build of
this
board does not complete **on unmodified master either**, for reasons
unrelated
to this patch:
```
$ ./tools/configure.sh mps3-an547:picostest && make
arm-none-eabi-gcc: error: missing argument to '-Wstack-usage='
```
`CONFIG_STACK_USAGE_WARNING` is unset for this config, and `Toolchain.defs`
only omits `-Wstack-usage=` when the value is exactly `0`, so an empty value
is
passed through. Setting it to `0` gets further, and then:
```
make[1]: *** No rule to make target `arm_exception.S', needed by `.depend'.
Stop.
make: *** [pass2dep] Error 2
```
Both failures reproduce identically on master with and without this patch, so
they are pre-existing and out of scope here — but they do mean the one board
in
the tree that enables `CONFIG_PIC` currently has no working Makefile build,
which is probably why this flag went missing unnoticed. The flag-level
evidence
above is therefore the strongest available; a maintainer with a working
`CONFIG_PIC` build is very welcome to add a link log.
The same mechanism is exercised end-to-end in a downstream tree of mine on
RP2350 hardware, where the equivalent register (r9, for FDPIC modules) being
lost to this same `:=` caused a firmware `qsort()` callback into module code
to
read garbage, and re-applying the flag fixed it.
--
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]
