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

[Bug]: C compilation breaks when using gcc 12 #107

Open
1 task
ahans opened this issue Jan 5, 2023 · 3 comments
Open
1 task

[Bug]: C compilation breaks when using gcc 12 #107

ahans opened this issue Jan 5, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@ahans
Copy link

ahans commented Jan 5, 2023

What happened?

I'm working on integrating support for gcc 12.2.0. I added the relevant entries to _TOOLCHAINS in toolchain/defs.bzl and built an updated sysroot, also updating _SYSROOTS in the same file to point to my local archive. Things work mostly as expected, however, there is one issue when compiling a C source file that includes stdatomic.h. Since gcc 12.2.0 adds (some) support for C++23, there is a file stdatomic.h as part of the C++ headers. Since all directories are added to the gcc call via -isystem, a C-only built finds that header as well, and due to the preprocessor guards in there it does nothing. Digging a bit further and asking on the libstdc++ mailinglist, I found out that a C compilation is not supposed to see that header in the first place. When using only --sysroot and dropping the -nostdinc and -isystem arguments, gcc actually does the right thing. So it looks like the natural fix for this is to do exactly that: use --sysroot and drop the -nostdinc and -isystem arguments. Unfortunately, it's not entirely straightforward, because gcc uses stddef.h and stdarg.h from the toolchain archive and then those paths are made absolute ones in the .d file, making Bazel complain about undeclared includes.

The easiest workaround is patching the C++23 stdatomic.h, so it does #include_next when included from C code. However, it looks like the better solution here is to properly use --sysroot. I'm not sure why gcc makes some paths absolute in the .d file, but I can imagine that using a single archive with compiler and sysroot would make things much easier.

Why do you have the custom sysroot and explicitly remove the one from the Bootlin toolchain? Also, when you build gcc yourself as part of the sysroot, why do you even use the Bootlin toolchain?

Version

Development (host) and target OS/architectures:

Output of bazel --version:
bazel 5.3.1

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:
patched main

Language(s) and/or frameworks involved:
C, C++

How to reproduce

I can put up an example to repro. But understanding the rationale behind the custom sysroot would be very helpful as a start.

Any other information?

No response

Fund our work

  • Sponsor our open source work by donating a bug bounty
@ahans ahans added the bug Something isn't working label Jan 5, 2023
@ahans
Copy link
Author

ahans commented Jan 10, 2023

I have made some progress here. We need to use the absolute path with --sysroot for the linker to find all required files. That is probably the reason why the wrapper script makes the path absolute. Using -isysroot additionally with a relative path we can make some paths in main.d relative, so Bazel is happy. However, it looks like some more -isystem is still required to make gcc output only relative paths.

Splitting the C and C++ includes is probably the way to go. I ended up with hard-coding most things now. A generic solution could look like this:

  1. Change includes() from sysroot/flags.bzl to support two lists: one for C only, another one for C++. Could also make sense to have to separate functions here.
  2. Split cc_toolchain_config's includes into c_includes and cxx_includes. When invoking cc_toolchain_config(), pass the results from functions edited in the previous step.
  3. Instead of using a single include_feature, split it into two, one for C only (with C compile actions), one for C++ (with C++ compile actions).

I'm still unsure why you use a custom sysroot. My guess is that you don't like the glibc that comes with the default Buildroot Bootlin ones. But I understand now that this part is separate from the issue with gcc 12.

@f0rmiga
Copy link
Owner

f0rmiga commented Feb 1, 2023

Hi @ahans, thanks for the detailed report! You are right that we have a separate sysroot because of glibc. There's some thought behind it that I put here: https://blog.aspect.dev/hermetic-c-toolchain. The major goal is to produce binaries that are linked against old symbols in order to make them more portable.

We have this mix of sysroot and bootlin compilers because of historical reasons. We initially used bootlin only to avoid the trouble of using a custom-built solution. It helped up to a certain extent. I don't think the current state of this repository is the final state I envision. To also support macOS (#93), we will have to dump the bootlin toolchains in any case.

Having said that, I'd be open to take a patch for what you described, but eventually we will ditch bootlin, so not sure if it's worth your time.

@gregmagolan gregmagolan moved this to 📋 Backlog in Open Source Feb 6, 2023
@gfrankliu
Copy link

The reason you saw those errors was due to the gcc version mismatch. You need to update this line and recreate the sysroot to match your GCC version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants