Repository navigation
warnings generated by retain attribute in sel4test library header #97
Description
When support for lld was added in #75, this retain attribute was added. However this is causing warnings now when compiling with gcc-12:
/tmp/tmp.BhrhA5NZPe/projects/seL4_libs/libsel4test/include/sel4test/test.h:145:12: error: 'retain' attribute ignored [-Werror=attributes]
145 | struct testcase TEST_ ## _name = { \
| ^~~~~~~~
/tmp/tmp.BhrhA5NZPe/projects/seL4_libs/libsel4test/include/sel4test/test.h:155:73: note: in expansion of macro 'DEFINE_TEST_WITH_TYPE'
155 | #define DEFINE_TEST_BOOTSTRAP(_name, _description, _function, _enabled) DEFINE_TEST_WITH_TYPE(_name, _description, _function, BOOTSTRAP, _enabled)
| ^~~~~~~~~~~~~~~~~~~~~
/tmp/tmp.BhrhA5NZPe/projects/sel4test/apps/sel4test-driver/src/tests/syscall.c:237:5: note: in expansion of macro 'DEFINE_TEST_BOOTSTRAP'
237 | DEFINE_TEST_BOOTSTRAP(_test, "Basic " #_syscall "() testing", test_ ## _syscall, _enabled)
| ^~~~~~~~~~~~~~~~~~~~~
/tmp/tmp.BhrhA5NZPe/projects/sel4test/apps/sel4test-driver/src/tests/syscall.c:239:5: note: in expansion of macro 'GENERATE_SYSCALL_TEST_MAYBE'
239 | GENERATE_SYSCALL_TEST_MAYBE(_test, _syscall, _code, true)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/tmp.BhrhA5NZPe/projects/sel4test/apps/sel4test-driver/src/tests/syscall.c:245:1: note: in expansion of macro 'GENERATE_SYSCALL_TEST'
245 | GENERATE_SYSCALL_TEST(SYSCALL0000, seL4_Yield,
| ^~~~~~~~~~~~~~~~~~~~~
/tmp/tmp.BhrhA5NZPe/projects/seL4_libs/libsel4test/include/sel4test/test.h:145:12: error: 'retain' attribute ignored [-Werror=attributes]
145 | struct testcase TEST_ ## _name = { \
| ^~~~~~~~
There doesn't seem to be a way to reliably test if gcc supports the retain attribute as __has_attribute(retain) returns 1 anyway.
Is there an alternative approach here? (ping @heshamelmatary)
Activity
Indanz commentedon Feb 27, 2025
binutils 2.36 or newer is required according to this, otherwise you get that warning. There is a bug report by the same llvm dev for gcc 11 here with some more info: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99587
heshamelmatary commentedon Feb 28, 2025
@kent-mcleod I believe @Indanz is right. I also left a comment back then to address this issue that might solve those warnings? seL4/seL4_tools#173 (comment)
Please let me know if this issue still holds
midnightveil commentedon Mar 19, 2025
This makes the current sel4test manifest fail to build now, when building with
arm-gnu-toolchain-12.2.rel1-x86_64-aarch64-none-elf, because of-Werror.Reverting 12d0d97 (in seL4_libs) and be8277a84c0c59b472909877540d55d53739001e. (in seL4_tools) allows this to build.
Indanz commentedon Mar 19, 2025
We have multiple options:
I have no opinion about the way forward.
12.2 was released in 2022, do any newer versions have the same problem? Current release is 14.2.
kent-mcleod commentedon Mar 19, 2025
It works on the versions of the toolchains provided by the docker containers, which is why the changes haven't been reverted yet.
If there's an easy way to conditionally compile based on lld then that would be good, but I'm not sure what the condition would be. Unless it's a conditional on clang vs gcc.
We could add some cmake tests that checks if the toolchain supports retain and then conditionally use it based on that. But that's more maintenance effort.
The documentation on the
usedattribute 1, explains that for ELF targets the retain attribute is needed to prevent unused sections from being stripped during linker section GC.heshamelmatary commentedon Mar 19, 2025
We could probably, in cmake, check for the binutils version and conditionally add
-z nostart-stop-gcfor older versionsIvan-Velickovic commentedon May 21, 2025
Tried with
aarch64-none-elf-gcc (GCC) 14.2.1 20250322andGNU ld (GNU Binutils) 2.44but still the same issue.Indanz commentedon May 21, 2025
Considering the problems this gives with gcc, I propose we remove the newly added retain keyword altogether and just use
-z nostart-stop-gcto fix the original llvm problem. Any downsides to that?kent-mcleod commentedon May 22, 2025
Alternatively we could unwind the previous commit and make the retain keyword conditional of the compiler-linker selection that's known in CMake scope. In the draft PR i just conditioned on Clang or !Clang because I don't have toolchains to test with in front of me: #102
Won't we then have to detect and handle cases where nostart-stop-gc isn't supported by the linker?
Indanz commentedon May 23, 2025
That seems the best short-term fix, yes. Except if we can find a better solution.
Yes, but if the linker ignores unknown options it's an improvement. We'd add it only for llvm, until gcc also enables LTO by default. Anyway, limiting
retainto Clang is probably the better solution, but this is something we need to keep in mind.This post is very illuminating.
Network manager had the same issue and they solved it by wrapping retain around a __clang__check too.
midnightveil commentedon May 30, 2025
Just for the record: elfloader-tool also needs to be fixed https://github.com/seL4/seL4_tools/blob/34bb2ca78418f3347e7b3aad7e4b5035339f8a5d/elfloader-tool/include/drivers/common.h#L46