seL4 / sel4test

Test suite for seL4.
http://sel4.systems
Other
25 stars 63 forks source link

seL4test fails to compile with a `x86_64-unknown-none-elf` compiler #127

Closed wucke13 closed 1 month ago

wucke13 commented 1 month ago

For historical reasons, the x86-*-elf compiler target treats a single forward slash (/) as beginning of comment. Now, the following test uses a single forward slash / (albeit in a pre-processor macro, this still triggers):

https://github.com/seL4/sel4test/blob/fe4839eb4a59c9b9d02bb7eb626d5fbea653c29b/apps/sel4test-tests/src/arch/x86/tests/alignment_asm.S#L7

The simple hotfix is to use a x86_64-unknown-linux-* compiler, which does not exert this behavior (as communicated back '05 on the gnu mailing lists). An alternative is to pass the flag --divide passed to x86_64-unknown-none-elf-as, to disable this behavior and treat a / int he middle of a pre-processor line as a true division.

Now, for most practical reason using a x86_64-unknown-linux-gnu targeted gcc likely is fine. But, conceptually this is unclean, one really does not need the Linux headers etc. to compile seL4, and on all other target architectures (except for the armv7 eabi stuff) the -elf works fine (as it should be).

Indanz commented 1 month ago

That line was introduced by commit 5c3be68.

Seems it might be fixed by adding the --divide command line option to AS.

Or we can rewrite the code to not trigger this. Does it trigger if you write CONFIG_WORD_SIZE/8?

wucke13 commented 1 month ago

Or we can rewrite the code to not trigger this. Does it trigger if you write CONFIG_WORD_SIZE/8?

No, but then the test is semantically altered, because the compiler only sees CONFIG_WORD_SIZE (everything past the first / in a line is discarded/treated as comment)

Seems it might be fixed by adding the --divide command line option to AS.

I'm currently tinkering with that. An issues I found is, that the build scripts invokes x86_64-unknown-none-elf-gcc to generate alignment_asm.S.obj. So, if I pass the --divide to ASMFLAGS, it gets passed to x86_64-unknown-none-elf-gcc which does not understand that flag. Only x86_64-unknown-none-elf-as does recognize it.

Just in case, yes I do have the AS environment variable set to x86_64-unknown-none-elf-as, so that is not the reason why *-gcc is used instead of -as to assemble the asm to object code.

wucke13 commented 1 month ago

So, in Nix I can use the NIX_CFLAGS_COMPILE environment variable to inject arbitrary flags to the compiler x86_64-unknown-none-elf-gcc. Using that, I can pass -Wa,--divide, which in term tells gcc to pass --divide to the assembler. With that, it works for me. So, emitting the -Wa,--divide as additional flag to the compiler via the CMake scripts might fix this.

Indanz commented 1 month ago

I don't think any of us can be bothered writing a special check to the CMake scripts to check specifically for this issue, but feel free to send a patch.

It seems reasonable to expect people to add toolchain specific flags to their build environment when they use an unsupported toolchain, so closing this issue for now.

To be clear, I agree that x86_64-unknown-none-elf-gcc should work and that it's unfortunate that there's a stupid issue like this. But AFAIK, officially only the Linux toolchains are supported, even for Arm.

Smattr commented 1 month ago

Alternatively, you could rewrite the expression to CONFIG_WORD_SIZE >> 3.

wucke13 commented 4 weeks ago

@Indanz

But AFAIK, officially only the Linux toolchains are supported, even for Arm.

Where would I read more about this? I kept wondering what toolchains are officially blessed, and I heared chatter (maybe from @Ivan-Velickovic ?) that the -linux toolchains are purely used because of availability in Debian repos. Authoritative information on the matter (that goes beyond a "just apt-get install whatever arm toolchains current Debian ships") would be appreciated from me!

Indanz commented 4 weeks ago

Not sure, I guess it was based on: https://docs.sel4.systems/projects/buildsystem/host-dependencies.html

I remember trying to use aarch64-unknown-none-elf-gcc and it didn't work for some reason. Non-Microkit based systems follow the SYSV ABI for ELF files, maybe that's where the -linux toolchain requirement comes from. But if you have it working for sel4test then it may have been something else.

Indanz commented 4 weeks ago

To elaborate, I would consider what's tested by CI as "officially supported". Anything deviating from that may not work for whatever stupid reason, which can be fixed if people send patches of course.