Enable combining +crt-static and relocation-model=pic on x86_64-unknown-linux-gnu

[bossmc]

Modern gcc versions support -static-pie, and rustc will already fall-back to -static if the local gcc is too old (and hence this change is optimistic rather than absolute). This brings the -musl and -gnu targets to feature compatibility (albeit with different default settings).

Of note a -static or -static-pie binary based on glibc that uses NSS-backed functions (gethostbyname or getpwuid etc.) need to have access to the libnss_X.so.2 libraries and any of their dynamic dependencies.

I wasn't sure about the # only/# ignore changes (I've not got a gnux32 toolchain to test with hence not also enabling -static-pie there).

[rust-highfive]

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

[bossmc]

The build failure appears to be the fall-back logic from -static-pie to -static kicking in because (I assume) the linker doesn't support that flag.

I don't know what linker is being used in that build, if it's clang-10 that supports the flag (it's been supported since clang-9) and if it's the standard gcc (which by default is gcc-9 on focal) that also supports the flag (it's been supported since gcc-8).

The test as written assumes that the linker supports -static-pie because it's fairly tricky to dynamically detect support (need to work out what linker rustc will use, and then work out if it supports the flag).

Is there a possibility to either upgrade the linker in the tests? Or to provide a hint to the test scripts so they can expect the fall-back behaviour if appropriate?

[JohnCSimon]

Triage:
@bossmc - this has sat idle for a few weeks - maybe try pinging the appropriate people again?

[bossmc]

I'm not sure who's the appropriate people for this change, I was trusting Liz Lemon that @estebank was a good choice, and I don't know if there's anyone else who would also be a sensible choice.

[crlf0710]

@bossmc Triage: I'd suggest you reach the t-compiler people on Zulip to move this forward. In the mean time, i'll convert this pr to draft to remove this from triage cycle.

[estebank]

Apologies for the unresponsiveness, I'd made a point of coming back to it and forgot.

The code changes look reasonable, but I am not the right person to help with the test failure.

Could someone in @rust-lang/compiler take a look?

[eddyb]

r? @nagisa

[nagisa]

Is there a possibility to either upgrade the linker in the tests? Or to provide a hint to the test scripts so they can expect the fall-back behaviour if appropriate?

Rust currently supports fairly ancient GCC and clang both for building rustc itself as well as a toolchain. Bumping to fairly recent versions of gcc/clang would likely be a MCP-level material if not an entire RFC.

cc @rust-lang/release for opinions though.

[nagisa]

(in practice we've been okay with conditional support for certain toolchain features in the past, I don't see why we wouldn't do that here as well, after adding some support to compiletest to check if the linker supports the necessary features)

[bossmc]

Just to clarify - this rustc change doesn't require a newer linker, it just enables taking advantage of the linker's -static-pie feature if it's available.

If we want CI to build with a minimal linker version then the test becomes a regression test rather than a feature test (testing that if the compiler is asked for static pie but the linker doesn't support it then it builds a static (non-pie) binary instead). It might also be useful to have a newer linker in CI (as well?) to test the function when the linker does support it? At the very least we'd need to not break x.py test for people who's native toolchain does support the function.

(your follow-up message has overlapped me writing this) I agree that this would be best with conditional support in compiletest - I'm happy to try and implement that as part of this MR if people agree that's the right direction to go.

[nagisa]

Adjusting CI to somehow allow for multiple linker versions might be an option too, but I don't know enough about our CI set up to tell for sure. @Mark-Simulacrum may have a better idea?

[Mark-Simulacrum]

CI for x86_64 tests with a Ubuntu 20.04 base image today (https://github.com/rust-lang/rust/blob/master/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile). I believe many other platforms do the same or similar; indeed, I'm not actually confident we run tests on the more ancient builders we use for dist jobs.

It sounds like we need a more modern gcc/clang for the test suite to pass; I think likely it makes sense to do this:

• Add support for something like a needs-gcc-version flag to compiletest (we already do this for LLVM, for example). Ideally we'd make sure that the test can run if either clang or gcc of the appropriate minimum is available. Since this is a make test it'd also be fine with me to add the support within the script, just exiting early from the test if we don't see a new enough gcc/clang available.
• Add a test that confirms the ambient environment 'works' with this PR (i.e., we successfully fallback to not supporting).

I can't really comment on the change itself, so not sure if it makes sense, but in terms of verifying it's support this seems like the reasonable approach. The second step there can plausibly be skipped, depending on how the code is laid out -- @nagisa or whoever reviews this can decide that :) Generally though progressively enhancing seems good; ideally we'd provide some signal to the user that they can get this combination working if they get a more modern linker. Not sure we have any nice way of doing that though, so may not be worth it -- eventually it'll just work for folks.

[bors]

☔ The latest upstream changes (presumably #89652) made this pull request unmergeable. Please resolve the merge conflicts.

[bossmc]

@Mark-Simulacrum thanks for those notes, just coming back to this now I've got a moment. I've rebased the change and now the CI is passing - presumably because the CI test image has been updated and now has a new-enough gcc (or maybe clang).

I've looked into adding either a compiletest header filter, or simply filtering in the Makefile but in both cases I can' t see a way to tell which linker is in use (I assume we do this test with both gcc and clang, and maybe even directly with ld?) - is there something I'm missing? Even if I work that out it's not clear how I would write a header filter (I'd need something like (linker is gcc && linker version is >= 8.0.0) || (linker is clang && linker version is >= 9.0.0) which is the kind of complex filter that isn't supported yet in compiletest as far as I can see.

[Mark-Simulacrum]

Hm. I think probably the easiest thing for now is to have a run-make test, not integrated into compiletest, that runs some command to get the linker version -- I'm not too familiar with how to get this information out of gcc/clang. We should be able to set the linker for rustc though with -Clinker and -Clinker-flavor, which I hope is enough that we can get that test working. Once the version is detected we can bail out (exit 0 or so) if it's too old.

The important thing will be to make sure that:

• The test runs in CI
• The test doesn't fail with older toolchains (running locally / other CI)

But I don't think we need to ensure it runs on both gcc/clang or some such, we can just keep it to CI-only for the time being. Our CI doesn't currently have anyway to verify that a test actually ran at least once across all our builders, but we don't need to try and tackle that in this PR.

[bossmc]

(As a note to myself, but possibly interesting to others)

You can get the versions of gcc/clang with the --version flag, except that's very "human-oriented":

$ gcc --version
gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ clang --version
Ubuntu clang version 13.0.1-++20211027043036+08e3a5ccd952-1~exp1~20211027163603.15
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

So instead, one can use the "magic #define variables" that the two compilers use (and then abuse the pre-processor to get the values of those macros):

$ echo __GNUC__ __GNUC_MINOR__ __GNUC_PATCHLEVEL__ | gcc -x c -E - | grep -ve '^#'
9 3 0
$ echo __clang_major__ __clang_minor__ __clang_patchlevel__ | clang -x c -E - | grep -ve '^#'
13 0 1

In fact, for this test we just need to check the major versions (8 for gcc and 9 for clang) so this seems pretty easy to do in the Makefile (or more likely in a script that the Makefile calls).

[bossmc]

@Mark-Simulacrum I've added a first pass at what we've been discussing (I'm pretty sure I can commonize the check_X_version scripts but I wanted to check that you're happy with the direction this is going before I go down a rabbithole).

[bossmc]

https://github.com/rust-lang/rust/runs/4289582821?check_suite_focus=true is a build failing due to a deliberately added exit 4 just after we've confirmed that (a) the linker in the CI is new enough to support -static-pie and (b) we've checked that rustc has built a relocatable, static ELF executable (i.e. just after the test has passed in the meaningful way).

I'll revert that change now so this MR is mergeable again.

[nagisa]

Seems reasonable to me, could you please squash the commit history to remove the testing commits?

r=me after.

[bossmc]

@nagisa - done, thanks!

[rust-log-analyzer]

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

running 165 tests
...................................i............

command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/i686-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/mir-opt" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/mir-opt" "--stage-id" "stage2-i686-unknown-linux-gnu" "--suite" "mir-opt" "--mode" "mir-opt" "--target" "i686-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-12/bin/FileCheck" "--nodejs" "/usr/bin/node" "--linker" "cc" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0  -Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "12.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hellonew hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit orcshared orctargetprocess passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--channel" "nightly" "--color" "always"
expected success, got: signal: 11 (core dumped)

Build completed unsuccessfully in 0:02:14

[JohnCSimon]

triage: looks like this is ready for review

@rustbot label: -S-waiting-on-author +S-waiting-on-review

[nagisa]

@bors r+

[bors]

📌 Commit 6a88311 has been approved by nagisa