macports-base icon indicating copy to clipboard operation
macports-base copied to clipboard

As of gcc 10, g++ supports setting the c++ runtime via -stdlib

Open cjones051073 opened this issue 3 years ago • 36 comments

cjones051073 avatar Jul 13 '22 13:07 cjones051073

gcc11 has supported this for a while now too (came out Dec 2020)

https://github.com/gcc-mirror/gcc/commit/662b9c55cf06d3df6682ef865fb2b685820317a9

and I’ll check gcc10;as I think it does too.

kencu avatar Jul 14 '22 13:07 kencu

gcc11 has supported this for a while now too (came out Dec 2020)

gcc-mirror/gcc@662b9c5

and I’ll check gcc10;as I think it does too.

yes, I just realised this myself. Currently testing some updates to gcc10/11 that bring in patches generated from Iain's forks. I've enabled this option there as well...

cjones051073 avatar Jul 14 '22 13:07 cjones051073

it might be wise to just use Iains forks instead of selectively bringing in patches?

kencu avatar Jul 14 '22 13:07 kencu

it might be wise to just use Iains forks instead of selectively bringing in patches?

What I am doing is directly equivalent, in that I generate the patch file myself from his repo by comparing the branch to the release tag.

git diff --no-prefix releases/gcc-11.3.0 gcc-11.3-darwin-r2

I prefer this for a number of reasons, and in the end the result is exactly the same, by construction.

cjones051073 avatar Jul 14 '22 13:07 cjones051073

He extensively tests them, far more than we ever will or could, and if we are not using his complete fork but some subset of it, how can he be expected to support us? Food for thought…

kencu avatar Jul 14 '22 13:07 kencu

ok, if you’re using the entire fork, that should be fine then. I had the impression it was otherwise.

Great!

kencu avatar Jul 14 '22 13:07 kencu

He extensively tests them, far more than we ever will or could, and if we are not using his complete fork but some subset of it, how can he be expected to support us? Food for thought…

I am using his fork, just via a patch file rather than directly. The end result is exactly the same, and I personally prefer it for a number of reasons. Its much simpler to roll back to the stock release, by just removing the use of the patch. Its also much easier to see what is changed w.r.t. the stock release, by just looking at the patch.

cjones051073 avatar Jul 14 '22 13:07 cjones051073

ok, if you’re using the entire fork, that should be fine then. I had the impression it was otherwise.

Great!

Sorry, perhaps due to my uses of the term patches in pural. Its one patch per gcc version, that is exactly what is needed to take the stock release to a specific tag in Iain's repo...

cjones051073 avatar Jul 14 '22 13:07 cjones051073

one last thing to ponder before pulling the trigger on the gcc updates.

Iain has rpath turned on for 10.11 and up, where it is required. Systems < 10.11 still default to full path names in libraries (where DYLD_LIBRARY_PATH still works).

However, we could turn rpath library names on optionally for 10.5 and up with a configure argument. Then all the systems would be the same, except for 10.4, which we can ignore for the moment I think.

This has some advantages -- install_name_tool changes, etc, would be consistent across the 10.5 - current spectrum. I can't really think of any major disadvantages, given that we have to bite the rpath bullet anyway.

What do you think?

kencu avatar Jul 14 '22 16:07 kencu

well, one thing to ponder - we override libstdc++ in legacysupport when it has the broken ODR issue... I have (so far) never tried overriding DYLD_LIBRARY_PATH when using rpath library names. Maybe it works as expected.

@MarcusCalhoun-Lopez -- any input ?

kencu avatar Jul 14 '22 16:07 kencu

I would leave things as they are for now on the older systems, where legacy support is needed. If you want you can then experiment with enabling rpath in a follow up update.

cjones051073 avatar Jul 14 '22 16:07 cjones051073

Iain has rpath turned on for 10.11 and up, where it is required

Why is rpath required on 10.11 and up? Apologies, I feel like I've asked this 12 times before but I don't remember the answer.

ryandesign avatar Jul 14 '22 18:07 ryandesign

when running the gcc test suite, using /bin/sh, MacOS does not allow overriding the library locations using DYLD_LIBRARY_PATH as of 10.11.

So to build and test gcc on 10.11 and up, Iain uses rpath library names instead, which can be directed to the test libraries instead of the installed libraries.

Up to now, he has been working around this issue by installing the libraries and then running the tests, but that is no longer sustainable for him, and it's rpath or no gcc on Darwin.

kencu avatar Jul 14 '22 18:07 kencu

I know -- you would think there would be some other way to get past this -- but apparently no, he says.

kencu avatar Jul 14 '22 18:07 kencu

Note, I resisted suggesting this till now, but there is a way to remove the rpath entries ourselves post destroot. It probably would only be a block of code < 10 lines long to find all dylibs in the destroot and replace all @rpath's with a fixed path.

I've resisted doing this as I prefer to stick to what upstream does as much as we can, but if it proves too problematic it would not be that complicated to do..

cjones051073 avatar Jul 14 '22 18:07 cjones051073

Yes, I suggested that option about 6 months ago as a possibility in the libgcc ports. But, it is just better, I think, to get this done, rather than work around Iain forever, I finally concluded...

we could still rewrite the install names in libgcc if we really wanted to -- we'd have to do them all, and maintain them forever -- ...

kencu avatar Jul 14 '22 18:07 kencu

Yeah , I tend to agree in so far as the closer we stick to what Iain expects to see, the easier we will get support from him, which is definitely a worth wild consideration.

So I propose we see how bad the rpath fallout is, and if it proves too much we then think about replacing them ourselves post-destroot ?

cjones051073 avatar Jul 14 '22 18:07 cjones051073

sounds like the best option to me too. There are many library references embedded in the gcc binaries and libraries, we might need to find and rewrite all those too, during the gcc build? Once you start down that path, where it winds up is always an adventure it seems...

kencu avatar Jul 14 '22 19:07 kencu

when running the gcc test suite, using /bin/sh, MacOS does not allow overriding the library locations using DYLD_LIBRARY_PATH as of 10.11.

Ah right. Due to SIP.

I know -- you would think there would be some other way to get past this -- but apparently no, he says.

Well there absolutely is another way. Sounds like the test suite was designed to set DYLD_LIBRARY_PATH somewhere at a top level and relied on the fact that it was automatically passed down to the subprocesses that eventually needed this value. DYLD_LIBRARY_PATH was, prior to SIP, affecting all processes, even for example the /bin/sh process which did not need it; DYLD_LIBRARY_PATH could even have detrimental effects for /bin/sh. So the workaround would be to use a different variable name at the top level—one that does not begin with DYLD_—and then only set DYLD_LIBRARY_PATH to that other variable's value at those specific places where it is needed. For example instead of:

DYLD_LIBRARY_PATH="$PWD/libs" ./testrunner

with testrunner containing something like:

#!/bin/sh
./mytest

where ./mytest is a test program that links with a library that's in $PWD/libs, you could instead use:

MY_LIBRARY_PATH="$PWD/libs" ./testrunner

and then testrunner would contain:

#!/bin/sh
DYLD_LIBRARY_PATH="$MY_LIBRARY_PATH" ./mytest

If the developers of gcc choose not to restructure their test system to do this that is up to them, but it is not right to say that rpath is needed because of a macOS restriction; it is only needed because of an outdated assumption being made by the gcc test system.

ryandesign avatar Jul 14 '22 20:07 ryandesign

@kencu As you have a good relationship with Iain could you forward the above to him to see what he says, as it does make sense to me (I also never fully brought the argument why rpath was necessary).

cjones051073 avatar Jul 14 '22 21:07 cjones051073

@ryandesign I guess one argument against your proposal is the gcc build system is not just for darwin, so embedding a direct reference to DYLD_LIBRARY_PATH which is macOS specific, in what I imagine might be a bunch of places, might not be something upstream would allow..

cjones051073 avatar Jul 14 '22 21:07 cjones051073

B.t.w. We have kinda highjacked this PR to discuss the gcc rpath issue, which has nothing to do with the changes here. @jmroot thanks for your comments, I will get to them sometime soon…

cjones051073 avatar Jul 14 '22 21:07 cjones051073

I don't have personal experience with Linux but I imagine the same issue would apply there and on other UNIX operating systems that use LIBRARY_PATH, they just don't have SIP to protect them from the adverse effects that setting such a variable at a greater scope than needed could represent. Changing their build/test system to set the appropriate variable only when needed would thus not only fix the test problem on OS X with SIP but also increase the security of all systems. This needn't involve copy/pasting a bunch of "if Darwin then DYLD_LIBRARY_PATH else LIBRARY_PATH" code all over the place; it can be done once centrally and the name of the library path variable can be placed into another variable which is passed down through the environment:

#!/bin/sh
# top-level testing script
case `uname -s` in
    Darwin) library_path_var=DYLD_LIBRARY_PATH;;
    *)      library_path_var=LIBRARY_PATH;;
esac
library_path="$PWD/libs"
export library_path
export library_path_var
./testrunner
#!/bin/sh
# test runner
eval "$library_path_var=\$library_path"
export "$library_path_var"
./mytest1
./mytest2

ryandesign avatar Jul 14 '22 21:07 ryandesign

@kencu As you have a good relationship with Iain could you forward the above to him to see what he says, as it does make sense to me (I also never fully brought the argument why rpath was necessary).

To paraphrase Erdos, iain's mind is now closed to this issue. He has reached his final conclusion about it.

I think we can make it work.

kencu avatar Jul 15 '22 03:07 kencu

most of the time (almost all the time?) you probably don’t want gcc to build against libc++, even if it technically could, I think, as that pathway is very untested at present.

with this PR, if I read it right, you will be using it almost all the time, passing -stdlib=libc++. Are you sure that is what you would like base to do?

we mostly use gcc to compile fortran code… I have not as yet tried doing that while passing -stdlib=libc++ ; maybe it works? dunno.

also, gcc doesn’t understand -stdlib=macports-libstdc++.

I’m not sure if gcc understands -stdlib=libstdc++; perhaps it does, but even so, that would choose wrong for macports even if it did.

kencu avatar Jul 15 '22 10:07 kencu

-stdlib=libc++ isn't used for c or fortran code, only c++ so its not an issue for those use cases.

For c++, no port should currently be using g++ to build, due to the runtime issues. Yes, support is experimental at the moment, but the changes here make it feasible to consider using macports g++ to build c++ ports. The current fallbacks for c++ compilers do not consider adding g++ to the lists, and I am not proposing we do that, so in practise the support here will only be used when ports, or users, manually opt in to trying it.

cjones051073 avatar Jul 15 '22 10:07 cjones051073

To answer one other question -stdlib=libstdc++ works just fine (its basically the default)

Oberon ~/cernbox/MacPorts/c++17 > g++-mp-12 -std=c++17 -stdlib=libstdc++ ./test.cpp
Oberon ~/cernbox/MacPorts/c++17 > otool -L ./a.out 
./a.out:
	@rpath/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.30.0)
	@rpath/libgcc_s.1.1.dylib (compatibility version 1.0.0, current version 1.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)

cjones051073 avatar Jul 15 '22 10:07 cjones051073

To answer one other question -stdlib=libstdc++ works just fine (its basically the default)

Oberon ~/cernbox/MacPorts/c++17 > g++-mp-12 -std=c++17 -stdlib=libstdc++ ./test.cpp
Oberon ~/cernbox/MacPorts/c++17 > otool -L ./a.out 
./a.out:
	@rpath/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.30.0)
	@rpath/libgcc_s.1.1.dylib (compatibility version 1.0.0, current version 1.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)

that is not what macports expects the stdlib to be with that option though, Chris

kencu avatar Jul 15 '22 12:07 kencu

-stdlib=libc++ isn't used for c or fortran code, only c++ so its not an issue for those use cases.

we can see where it creeps in I suppose, linker invocations, etc. I suspect it will be an issue, but we’ll know soon enough.

For c++, no port should currently be using g++ to build, due to the runtime issues. Yes, support is experimental at the moment, but the changes here make it feasible to consider using macports g++ to build c++ ports. The current fallbacks for c++ compilers do not consider adding g++ to the lists, and I am not proposing we do that, so in practise the support here will only be used when ports, or users, manually opt in to trying it.

nobody should be suggesting we add g++ to the fallback compilers.

I’m just more conserative in my moves, I guess. Defaulting every macports gcc compiler to now use libc++ is not what I would do (will do), but it all shakes out in the end.

kencu avatar Jul 15 '22 13:07 kencu

To answer one other question -stdlib=libstdc++ works just fine (its basically the default)

Oberon ~/cernbox/MacPorts/c++17 > g++-mp-12 -std=c++17 -stdlib=libstdc++ ./test.cpp
Oberon ~/cernbox/MacPorts/c++17 > otool -L ./a.out 
./a.out:
	@rpath/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.30.0)
	@rpath/libgcc_s.1.1.dylib (compatibility version 1.0.0, current version 1.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)

that is not what macports expects the stdlib to be with that option though, Chris

I disagree. its exactly what you get if you do not specify stdlib at all, which is what I would expect

    Oberon ~/cernbox/MacPorts/c++17 > g++-mp-12 -std=c++17 ./test.cpp
    Oberon ~/cernbox/MacPorts/c++17 > otool -L ./a.out 
    ./a.out:
    	@rpath/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.30.0)
    	@rpath/libgcc_s.1.1.dylib (compatibility version 1.0.0, current version 1.1.0)
    	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)

cjones051073 avatar Jul 15 '22 13:07 cjones051073