cc: remove unneeded linker flag when building with GCC on Linux

[danielnachun]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?
• Have you successfully run brew man locally and committed any changes?

---

This pull request should fix a bug on Linux when attempting to use a newer gcc when the brewed gcc@5 is installed and is linked into $HOMEBREW_PREFIX/lib. The determine_library_paths method in extend/ENV/super.rb adds $HOMEBREW_PREFIX/lib to the list of the library paths, which results in -L$HOMEBREW_PREFIX/lib being add as a linker flag to the superenv. This creates a problem when using newer of versions of gcc, because this flag causes the linker to use $HOMEBREW_PREFIX/lib/libstdc++.so.6 and the other run time libraries from gcc-5 instead of the versions of those libraries that come with newer version of gcc which we are trying to use.

This solution resolves this by inserting an additional linker flag for the directory which contains the runtime libraries for the newer gcc before $HOMEBREW_PREFIX/lib, so that the linker will always find the newer gcc runtime libraries before finding the older copies of those libraries in $HOMEBREW_PREFIX/lib.

This fix should not be merged until gcc on Linux points to gcc@10. Once that is completed, we will migrate all formulae which need a newer gcc from whatever version they are currently using to depends_on gcc. In the unlikely event that we have formulae which need a gcc version between 5 and 10, we'd have to make equivalent changes for those versions of gcc as well. This is because the directory with the run time libraries includes the gcc version, and therefore differs for each version.

The other solution that was mentioned was to remove $HOMEBREW_PREFIX/lib from the list of library paths added by determine_library_paths, but I don't know what kind of breakage this could create, so this solution is safer. However feedback on this decision would be great as there may be other opinions about how best to handle this.

[BrewTestBot]

Review period will end on 2021-02-15 at 07:38:15 UTC.

[iMichka]

gcc formulae provide a spec file, did you check if this is used by the linker, and if we could add this flag to the respective spec files?

If not, this code needs more logic to work for any version of gcc, not gcc@10 only. Because if someone updates gcc to gcc@11 in homebrew-core, this will break this part of the code.

[MikeMcQuaid]

The other solution that was mentioned was to remove $HOMEBREW_PREFIX/lib from the list of library paths added by determine_library_paths, but I don't know what kind of breakage this could create, so this solution is safer. However feedback on this decision would be great as there may be other opinions about how best to handle this.

Another option still is to ensure there's no GCC files in that directory at all. I really dislike the idea of hardcoding a GCC version in the shims here.

[danielnachun]

I agree with both of you that we should not hardcode the GCC version here. The easiest way to avoid that would be to programmatically determine the GCC version and then replace "10" with the value of the variable containing the version.

Regarding GCC specs, I think the way those specs are currently used is that they are passed in as args, "cleaned" through refurbish_args, and then added before ldflags. If we can get the path -L#{Formula["gcc@VERSION"].opt_lib}/gcc/VERSION into the specs when linking, that should fix the problem. The GCC specs files are rather cryptic so I'll have to look into how to add this.

Regarding the removal of the GCC files in $HOMEBREW_PREFIX/lib, this was the solution I was originally exploring, but it wouldn't be as simple as removing them. I think the reason those files are there is so that binaries built with the CI host gcc-5 can find libstdc++ and other GCC runtime libraries on systems which have brewed gcc-5. If you run brew unlink gcc@5 on a system with brewed gcc-5 (which will delete the symlinks to the GCC files in $HOMEBREW_PREFIX/lib), binaries built with gcc-5 will not be able to find libstdc++.

I think we could circumvent that by moving the symlinks for the GCC files to a subdirectory like $HOMEBREW_PREFIX/lib/gcc (-L$HOMBREW_PREFIX/lib is not recursive). That directory could then be added during keg relocation if the bottle does not depend on a newer version of gcc. On systems that don't use brewed gcc-5 this wouldn't have any affect as $HOMEBREW_PREFIX/lib/gcc would be non-existent or empty.

I think if I can get the linker flags into the GCC spec files, that would be the "cleanest" solution. If it's okay I'd like to leave this open since it's a WIP, but I'll definitely close it if I am successful with the other two solutions.

[BrewTestBot]

Review period ended.

[danielnachun]

@iMichka @sjackman I was digging around the specs for gcc-10 trying to see where I should try to add the linker flag for GCC runtime libs, and I found this very interesting line:

*link_libgcc:
-nostdlib -L/home/dnachun/.linuxbrew/Cellar/gcc@10/10.2.0_1/bin/../lib/gcc/10/gcc/x86_64-pc-linux-gnu/10.2.0 -L/home/dnachun/.linuxbrew/lib/gcc/10 -L/home/dnachun/.linuxbrew/lib

As you can see, -L$HOMEBREW_PREFIX/lib is already being added here, and is after -L$HOMEBREW_PREFIX/lib/gcc/10, exactly as we would want it if were were trying to avoid linking to the gcc-5 runtime libraries.

I believe the reason we are getting these linker errors is because we are adding -L$HOMEBREW_PREFIX/lib in the superenv and this preempts the linker flags added in the specs. However, we don't actually need to add -L$HOMEBREW_PREFIX/lib in the superenv when building with gcc in Linux, because the specs already add it for us! I'm guessing it was added to the superenv for building with clang on macOS (it might still be needed when building with clang under Linux too).

To test this, I tried three different scenarios:

1. Remove -L$HOMEBREW_PREFIX/lib from superenv and the specs file
   Result: gcc breaks completely
2. Remove -L$HOMEBREW_PREFIX/lib from specs file, but not the superenv
   Result: gcc compiles the formula successfully, but I get the same linker error as before because it links to the gcc-5 runtime libs instead of gcc-10
3. Remove -L$HOMEBREW_PREFIX/lib from the superenv, but not the specs file
   Result: gcc compiles the formula successfully, and there is no linker error!

The solution here is very simple - we just remove -L$HOMEBREW_PREFIX/lib from the ldflags on Linux unless we are using clang. I don't think clang will have the same issue because they bump the soname of their runtime libraries with every new version, unlike GCC. And it doesn't affect GCC on macOS either, because we don't need to link the GCC runtime libs to $HOMEBREW_PREFIX/lib like we do on Linux.

I've renamed the issue since the change for this PR is different now, but I think this is a much simpler solution than what I've tried before.

[iMichka]

Makes sense to me. The spec files are currently only installed on Linux (we still need to backport that code to homebrew-core but it's coming soon). It's a good way to handle this, as each formula provides their linker flags and it avoids conflicts.

Can you maybe just check one final thing: what happens with mixed dependency trees? (I mean with for example gcc@6 and gcc@7 being dependencies of a formula). Are both paths added, and in which order?

Also, are the lib flags for system gcc5 still added? Or does it also use the spec file from system gcc?

[MikeMcQuaid]

If you run brew unlink gcc@5 on a system with brewed gcc-5 (which will delete the symlinks to the GCC files in $HOMEBREW_PREFIX/lib), binaries built with gcc-5 will not be able to find libstdc++.

I appreciate this means that in the short-term we can't just unlink this formula but long-term it feels like relying on opt/gcc@5/lib is going to be a better strategy that's more resistant to user behaviour (as, presumably, a user can still run brew unlink gcc@5 and have everything start to explode which is explicitly something we try to avoid with linking).

I think we could circumvent that by moving the symlinks for the GCC files to a subdirectory like $HOMEBREW_PREFIX/lib/gcc (-L$HOMBREW_PREFIX/lib is not recursive).

If we're going to move them anyway (in a way that doesn't break existing binary packages which I'm not sure is possible): I'd suggest using the existing optlink path.

I'm guessing it was added to the superenv for building with clang on macOS (it might still be needed when building with clang under Linux too).

Yes, most probably.

3. but not the specs file

Stupid question: what's the "specs file" reference here?

Makes sense to me. The spec files are currently only installed on Linux (we still need to backport that code to homebrew-core but it's coming soon). It's a good way to handle this, as each formula provides their linker flags and it avoids conflicts.

Would there be any value/benefit in doing it here before we ship this PR?

[MikeMcQuaid]

A comment explaining why this isn't needed for clang would be good. Have you tested compiling with Clang on Linux and ensured this doesn't break things?

[danielnachun]

@iMichka Do we currently have a way to use the clang superenv on Linux? I'm not even sure if we officially support building with clang on Linux right now. I'll update this with a comment to explain my understanding of why it is not needed, and I will also test clang if possible. I think we should leave $HOMEBREW_PREFIX/lib in the ldflags because clang doesn't have an equivalent to the specs file that tells it to look there for libraries.

[iMichka]

@MikeMcQuaid see https://github.com/Homebrew/linuxbrew-core/blob/master/Formula/gcc.rb#L224-L270 for the specs file. This code is in the post-install step of each gcc formula provided by linuxbrew-core. It allows to defined the different include and lib paths for each gcc version.

[MikeMcQuaid]

@MikeMcQuaid see https://github.com/Homebrew/linuxbrew-core/blob/master/Formula/gcc.rb#L224-L270 for the specs file. This code is in the post-install step of each gcc formula provided by linuxbrew-core. It allows to defined the different include and lib paths for each gcc version.

That looks pretty heavy for formulae code. Is it just use in a single GCC or multiple? Why does it have to be in post-install rather than install? Would be good to get something (tested, too!) into Homebrew/brew if it's used on more than a single formula.

[iMichka]

There are some steps to keep a backup of the original specs file, but we might just drop that. And it's in the post install step as the content depends on the version of glibc that is installed (though I do not know the details here). I agree that we could move that code to Homebrew/brew as it is used by each gcc formula in the same ways.

[MikeMcQuaid]

I agree that we could move that code to Homebrew/brew as it is used by each gcc formula in the same ways.

Sounds good in that case, yeh.

[danielnachun]

If you run brew unlink gcc@5 on a system with brewed gcc-5 (which will delete the symlinks to the GCC files in $HOMEBREW_PREFIX/lib), binaries built with gcc-5 will not be able to find libstdc++.

I appreciate this means that in the short-term we can't just unlink this formula but long-term it feels like relying on opt/gcc@5/lib is going to be a better strategy that's more resistant to user behaviour (as, presumably, a user can still run brew unlink gcc@5 and have everything start to explode which is explicitly something we try to avoid with linking).

I think we could circumvent that by moving the symlinks for the GCC files to a subdirectory like $HOMEBREW_PREFIX/lib/gcc (-L$HOMBREW_PREFIX/lib is not recursive).

If we're going to move them anyway (in a way that doesn't break existing binary packages which I'm not sure is possible): I'd suggest using the existing optlink path.

Using the opt_lib path sounds like the best long term solution to me as well. Ideally brew unlink shouldn't be able to break anything, especially not anything poured from bottles. We could add the opt_lib for gcc-5 (or whatever the host gcc version is in CI) to the RPATH when pouring bottles if the user has the brewed gcc-5 installed and the formula does not depend, even indirectly, on a newer version of GCC. We'd have to do a lot of checking to make sure nothing breaks by doing this but I believe it would improve the user experience if we remove the possibility of breaking things with brew unlink.

[danielnachun]

Makes sense to me. The spec files are currently only installed on Linux (we still need to backport that code to homebrew-core but it's coming soon). It's a good way to handle this, as each formula provides their linker flags and it avoids conflicts.

Can you maybe just check one final thing: what happens with mixed dependency trees? (I mean with for example gcc@6 and gcc@7 being dependencies of a formula). Are both paths added, and in which order?

I should clarify that when I look at the build logs for a formula using a newer version of GCC, I don't actually see any of those linker flags in the GCC call. I think the arguments from the specs file are "invisibly" added to the GCC call after all the arguments provided to it on the command line.

When you say mixed dependency do you mean a situation with depends_on gcc@6 and depends_on gcc@7 in the same formula? Or one where formula A depends on formula B, and formula A also has a runtime dependency on gcc-7, while formula B has a runtime dependency on gcc-6?

If you're talking about the latter case, I think the arguments from the specs are added based on whichever version of gcc is actually called by the formula. If the formula uses gcc-7 and depends on a formula which has a runtime dependency on gcc-6, this shouldn't be an issue - the linker will use the libstdc++.so.6 from the gcc-7 specs which is backwards compatible with gcc-6 (and gcc-5).

Where we'd run into problems is if we had the opposite situation - the formula uses gcc-6 but depends on a formula which has a runtime dependency gcc-7. The libstdc++.so.6 from gcc-6 is not forwards compatible with the one from gcc-7 (I think GCC should bump the soname of libstdc++ with each major release like LLVM does, but we don't control that).

I think the only solution there is to forbid formulae on Linux from using a version of gcc that is older than the newest version used by anything in the formula's dependency tree. The easiest way to achieve this is by just using the latest version of GCC (currently gcc-10) whenever the CI host gcc (currently gcc-5) is too old. However we could add an of audit to check for this automatically in PRs.

Sorry for such a long answer, but this is a rather complex problem! I'll try to find some test cases to verify what I've said here.

[danielnachun]

Also, are the lib flags for system gcc5 still added? Or does it also use the spec file from system gcc?

I think the lib flags from the system GCC spec are only used when we actually call the system GCC. We use the system GCC in CI or whenever the host GCC is version 5 or greater. The first argument in the specs is -nostdlib, which should remove the standard lib paths fro the linker search path, effectively "hiding" the system GCC from brewed GCC.