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

Harden dummy:dummy binutils #2228

Merged
merged 1 commit into from
Dec 16, 2015
Merged

Conversation

wpoely86
Copy link
Member

By adding the system library path using the rpath option, the
resulting binaries should be 'harden' to work in an environment with a
other libstdc++ etc. The rpath will make sure it always finds the
correct library.

Fix for #2188

By adding the system library path using the `rpath` option, the
resulting binaries should be 'harden' to work in an environment with a
other libstdc++ etc. The rpath will make sure it always finds the
correct library.
@wpoely86
Copy link
Member Author

Test report by @wpoely86
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux SL 6.7, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.6.6
See https://gist.github.com/b854fb1a1211da7fcce9 for a full test report.

@wpoely86
Copy link
Member Author

Test report by @wpoely86
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux SL 6.7, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.6.6
See https://gist.github.com/a2d490feff7db105af82 for a full test report.

@wpoely86
Copy link
Member Author

Verification by objdump -x <binary>:

Dynamic Section:
  NEEDED               libdl.so.2
  NEEDED               libc.so.6
  RPATH                /lib64:/usr/lib64:/lib:/usr/lib

@wpoely86
Copy link
Member Author

Test report by @wpoely86
SUCCESS
Build succeeded for 2 out of 2 (1 easyconfigs in this PR)
Linux debian 8.1, Intel(R) Xeon(R) CPU 5130 @ 2.00GHz, Python 2.7.9
See https://gist.github.com/536c3bc22998030be340 for a full test report.

@hpcugentbot
Copy link
Contributor

Easyconfigs unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyconfigs-pr-builder/5353/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

prebuildopts = 'LIBS="$EBROOTZLIB/lib/libz.a"'
# further, add the system library path in the rpath: this should 'harden' the
# resulting binutils to bootstrap GCC (no trouble when other libstdc++ is build etc)
preconfigopts = 'LIBS="$EBROOTZLIB/lib/libz.a -Wl,-rpath=/lib64 -Wl,-rpath=/usr/lib64 -Wl,-rpath=/lib -Wl,-rpath=/usr/lib"'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can't combine all the rpath chunks in a single one?

-Wl,-rpath=/lib64:/usr/lib64:/lib:/usr/lib"

Haven't checked, but I would fine it weird if this is not supported.

This is screaming for support in framework btw. ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shortest form I can find is:

-Wl,-rpath=/lib64,-rpath=/usr/lib64,-rpath=/lib,-rpath=/usr/lib

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the framework support, follow up in easybuilders/easybuild-framework#651

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wpoely86: let's leave it as is for now, since this is tested thoroughly, and should be good to merge

We can clean it up later.

I'd love to get feedback from @geimer on this though, before merging it...

@boegel boegel added this to the v2.5.0 milestone Dec 15, 2015
@boegel
Copy link
Member

boegel commented Dec 15, 2015

I'm not sure whether it's a good idea to include something like this so close to the release...
Especially since this is waaaaay down in the stack.

On the other hand, I guess just building this + GCC on top of its is going to tell us whether it causes any problems?

@geimer, @ocaisa: do you think something like this may have fallout in one way or another?

@boegel
Copy link
Member

boegel commented Dec 15, 2015

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
Linux centos linux 7.1.1503, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/e65fef3cbe0db35e570c for a full test report.

@boegel
Copy link
Member

boegel commented Dec 16, 2015

built on top of this binutils (all easyconfigs that use a dummy-built binutils): https://gist.github.com/ea03dd15f457abefdc3f

@boegel
Copy link
Member

boegel commented Dec 16, 2015

Just made very sure that this doesn't "trickle up" to stuff build on top of this binutils, it doesn't (seem to):

-bash-4.2$ objdump -x $EASYBUILD_PREFIX/software/binutils/2.25/bin/ld| egrep 'Dynamic section|NEEDED|RPATH'
  NEEDED               libdl.so.2
  NEEDED               libc.so.6
  RPATH                /lib64:/usr/lib64:/lib:/usr/lib
-bash-4.2$ objdump -x $EASYBUILD_PREFIX/software/GCC/4.9.2-binutils-2.25/bin/gcc  | egrep 'Dynamic section|NEEDED|RPATH'
  NEEDED               libm.so.6
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2

cc @ocaisa

@ocaisa
Copy link
Member

ocaisa commented Dec 16, 2015

Given the way the mainline toolchains (foss, intel) are constructed in the next release (with a new binutils built on top of the compiled GCC) I can't see any problem for them. Also since the dummy binutils libs are not dynamic there's no issue there. I guess the only impact would be if this somehow affected the behaviour of the linker...but from GCC at least that doesn't seem to be the case.

@boegel
Copy link
Member

boegel commented Dec 16, 2015

I rebuilt more than just GCC on top of this, see https://gist.github.com/ea03dd15f457abefdc3f .

So, should be good to go, I'm merging this in, so @geimer can come back to us later to tell us this is the worst idea ever. ;-)

@boegel
Copy link
Member

boegel commented Dec 16, 2015

Thanks @wpoely86!

boegel added a commit that referenced this pull request Dec 16, 2015
@boegel boegel merged commit 8bc4483 into easybuilders:develop Dec 16, 2015
@wpoely86 wpoely86 deleted the rpathbinutils branch December 16, 2015 10:10
boegel added a commit to boegel/easybuild-easyconfigs that referenced this pull request Dec 16, 2015
…ention OpenFOAM 3.0.0 in v2.5.0 release notes
@geimer
Copy link
Contributor

geimer commented Dec 17, 2015

This relies on libstdc++.so residing in (/usr)?/lib(64)?, right? If so, it may work on Fedora, but may break on Debian and derivatives where libstdc++.so lives in /usr/lib/gcc/x86_64-linux-gnu/<system_gcc_version>. In any case, rpath'ing system library directories somehow feels wrong to me...

But to be honest, I also can't think of a better alternative at the moment.

@wpoely86
Copy link
Member Author

@geimer yes, I'm aware. I've been playing with -static-libgcc -static-libstdc++ but that relies on the fact that the static libraries are installed. It would be cool if we could add the rpath after compiling and just for the libraries that are used.

And, it's indeed quite dirty. But this binutils should only be used once for building GCC and then thrown away.

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.

5 participants