Skip to content

[5.5] Enable checking for -no-toolchain-stdlib-rpath and use it on this repo's binaries before installing #738

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

Merged
merged 1 commit into from
Jul 26, 2021

Conversation

finagolfin
Copy link
Member

@finagolfin finagolfin commented Jun 30, 2021

Cherry-pick of #732 and #767

  • Explanation: This matches the upstream driver and removes the build toolchain's rpath for this shipping swift-driver executable itself on ELF platforms.
  • Scope: Adds behavior to match upstream and remove an extraneous rpath from the swift-driver and other executables from this repo.
  • Risk: None
  • Testing: Passes all tests, as indicated in the upstream pull, and should now pass more tests from the compiler test suite.
  • Reviewer: @artemcm

@finagolfin finagolfin requested a review from a team as a code owner June 30, 2021 23:24
@finagolfin
Copy link
Member Author

@varungandhi-apple, would you run the CI on this?

@artemcm
Copy link
Contributor

artemcm commented Jul 1, 2021

@swift-ci please test

@finagolfin
Copy link
Member Author

Passed CI, ready to merge.

@finagolfin
Copy link
Member Author

@DougGregor, would be good to get this in as it fixes a security issue on linux. The latest official prebuilt Swift 5.5 snapshot toolchains use swift-driver by default, which has this ELF RPATH:

> readelf -d swift-5.5-DEVELOPMENT-SNAPSHOT-2021-07-03-a-centos8/usr/bin/swift-driver | ag runpath
0x000000000000001d (RUNPATH)            Library runpath: [/home/build-user/swift-nightly-install/usr/lib/swift/linux:$ORIGIN:$ORIGIN/../lib/swift/linux]

This pull will remove that first /home/build-user/ path.

@finagolfin
Copy link
Member Author

Ping, would be good to get this in.

@finagolfin
Copy link
Member Author

@artemcm, anything holding this up?

@artemcm
Copy link
Contributor

artemcm commented Jul 20, 2021

@buttaface, we will try and get this in; but, before we do, could you please add an XCTest for this change and then include it in the cherry-pick? Despite the compiler's validation suite, we would still like to have the driver's own test-suite capture these things in XCTests to be more stand-alone w.r.t. the compiler.
I apologize, I really should have pointed that out on the original review.

@finagolfin
Copy link
Member Author

No problem, submitted for trunk in #767, will cherry-pick here once it's in.

@finagolfin
Copy link
Member Author

Cherry-picked the test too.

@finagolfin
Copy link
Member Author

Anything holding this up? I think it just needs to be run on the CI with the new test and then we can get it in.

@artemcm
Copy link
Contributor

artemcm commented Jul 26, 2021

@swift-ci please test

@tkremenek tkremenek merged commit 1422de0 into swiftlang:release/5.5 Jul 26, 2021
@finagolfin finagolfin deleted the elf-rpath branch July 26, 2021 21:32
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.

3 participants