casaroli commented on PR #20711:
URL: https://github.com/apache/nuttx/pull/20711#issuecomment-6288540638
Thank you, this fixes the two open parts of #19509. I built
`mps3-an547:picostest` with CMake on master e29a0b2262c and on this PR
(arm-gnu-toolchain 13.2.Rel1) and checked `compile_commands.json`:
| Objects | master | this PR |
|---|---|---|
| firmware (1152) | no `-ffixed-r9` | `-ffixed-r9` |
| ELF applications hello + ostest (35) | `-mpic-register=r9` only |
`PICFLAGS -mpic-register=r9`, no `-ffixed-r9` |
| `crt0.c` | `-mpic-register=r9` only | `PICFLAGS -mpic-register=r9`, no
`-ffixed-r9` |
| "useless without -fpic" warnings | 36 | 0 |
The ELF `hello` now has only `R_ARM_GOT_BREL` relocations, as a Make PIC
module has.
Note for the test: on apps master, `examples/hello` and `testing/ostest`
have no `MODULE` in their CMakeLists, so with CMake they are built into the
firmware, and no ELF application gets these flags at all. I used
apache/nuttx-apps#3883 for hello and added `MODULE ${CONFIG_TESTING_OSTEST}` to
ostest locally. Also, the CMake build of `picostest` does not link on master or
with this PR (`undefined reference to g_exports` from `nsh_init.c`,
`CONFIG_NSH_SYMTAB`); that is not caused by this PR.
Three requests:
1. `PICFLAGS` does not need the cache entry or the fallback. `elf.cmake`
already sees the normal variable from `gcc.cmake`: I removed `CACHE INTERNAL`
and replaced the fallback with `message(FATAL_ERROR)`, and the configure passed
with the same flags. With Clang the fallback gives GCC-only options
(`-mno-pic-data-is-text-relative`; Clang also has no `-mpic-register`). Please
keep the one `set(PICFLAGS ...)` in `gcc.cmake` as it was.
2. `ELF_BINARY` marks more targets than the ELF applications:
`nuttx_add_module()` sets it on kernel modules, and `nuttx_add_application()`
sets it on `DYNLIB` shared libraries too. So these lose `-ffixed-r9`. The Make
build keeps `--fixed-r9` for them (`CMODULEFLAGS` comes from `CFLAGS`, which
has `ARCHCFLAGS += --fixed-r9`). Also, with `CONFIG_MODULES` a `DYNLIB` library
gets `NUTTX_ELF_APP_COMPILE_OPTIONS`, so it now gets `PICFLAGS` with
`-msingle-pic-base` and would read its data through the r9 of the program that
calls it; Make compiles shared libraries with `CMODULEFLAGS`, without
`PICFLAGS`. Could you use a separate target property for "compiled as a PIC ELF
application" (set for the ELF application targets and for `STARTUP_OBJS`) and
key the generator expression on it?
3. That also avoids `ELF_BINARY TRUE` on `STARTUP_OBJS`: everywhere else
`ELF_BINARY` holds the path of the ELF file (`nuttx_add_symtab()` passes it to
`nm`), so a boolean there is easy to misread.
Could you also add to "Testing" what you checked (the flags per kind of
object, and the warning count)?
--
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]
