Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit assembler debug level to improve toolchain compatability #79

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

nspin
Copy link
Member

@nspin nspin commented Oct 25, 2023

More recent versions of aarch64-none-elf-as don't support passing a debug level to -g.

Specifically, GNU Binutils 2.40 (associated with GCC 12.3) doesn't support this.

@nspin nspin force-pushed the pr/improve-toolchain-compatibility branch from b03dc95 to ea36717 Compare October 25, 2023 18:01
@lsf37 lsf37 requested a review from Ivan-Velickovic October 25, 2023 22:06
@Ivan-Velickovic
Copy link
Collaborator

I guess the reason we pin a specific version of the toolchain is to avoid issues like this.

I'm not hesitant to upgrade the toolchain version if there's some specific reason for it, but the README does mention a specific toolchain version for a reason.

@nspin
Copy link
Member Author

nspin commented Oct 25, 2023

I noticed that the debug level parameter to -g in as is undocumented even in Binutils 2.35.1, the version included in the toolchain pinned in the Microkit README, which accepts the paramter. I took a quick look at as.c in Binutils 2.35.1 and Binutils 2.40, the version I was using that does not accept that parameter, along with the commit responsible for this change, which confirmed my suspicion that the debug level parameter doesn't actually do anything in the as version that does accept it.

So, there is no cost to dropping the debug level parameter to -g.

@Ivan-Velickovic
Copy link
Collaborator

I noticed that the debug level parameter to -g in as is undocumented even in Binutils 2.35.1, the version included in the toolchain pinned in the Microkit README, which accepts the paramter. I took a quick look at as.c in Binutils 2.35.1 and Binutils 2.40, the version I was using that does not accept that parameter, along with the commit responsible for this change, which confirmed my suspicion that the debug level parameter doesn't actually do anything in the as version that does accept it.

So, there is no cost to dropping the debug level parameter to -g.

Thanks for looking into it, I have come across this issue before and I did find documentation on it here https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html (search for -glevel).

I know that the toolchain provided by ARM for GCC 12 and above does not allow specifying the level and for adding macOS support to the SDK we would need to upgrade the toolchain version anyways as the macOS ARM64 host toolchain is only available for 12 and above.

This kind of inconsistency between C compilers is quite... annoying. What I might do is take your patch and see if the hash of all the ELFs is the same, that way we'll know if specifying the level is doing anything.

@nspin
Copy link
Member Author

nspin commented Oct 26, 2023

GCC supports a debug level parameter, but the issue here is the assembler ($(TOOLCHAIN)-as) not supporting it.

@Ivan-Velickovic
Copy link
Collaborator

Ah my bad, I thought you were removing -g3 from GCC as well. Apologies.

More recent versions of aarch64-none-elf-as don't support passing a
debug level to -g.

Signed-off-by: Nick Spinale <[email protected]>
@Ivan-Velickovic Ivan-Velickovic force-pushed the pr/improve-toolchain-compatibility branch from ea36717 to c7c6112 Compare November 21, 2023 04:32
@Ivan-Velickovic Ivan-Velickovic merged commit ff8d4a8 into seL4:main Nov 21, 2023
8 checks passed
@Ivan-Velickovic
Copy link
Collaborator

No difference in the binaries. Merged in this case because we don't have to do any special handling to get both the ARM provided toolchain and the Nix ARM toolchain working and I want to upgrade to GCC 12 soon anyways for macOS support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants