TEXTREL in i686 since 1.41.0

[Cogitri]

Hello,

I'm in the process of upgrading Rust to 1.41.0 on Alpine Linux and it appears that a TEXTREL has been introduced in 1.41.0:

TEXTREL /usr/lib/librustc_macros-4cbed8a36ab240e8.so

The full build log is located here if it's of interest: https://gitlab.alpinelinux.org/Cogitri/aports/-/jobs/44396

[jonas-schievink]

This apparently breaks Android builds, since that forbids text relocations

https://users.rust-lang.org/t/cant-link-rust-code-on-i686-linux-android-with-rust-1-41/37807

[awakecoding]

+1 I am one of the unfortunate winner of a release-blocking CI build failure for Android now that we've upgraded to 1.41.0. We've disabled android-x86 builds for now :(

[brainlock]

I added a minimal repro to my repository here: https://github.com/brainlock/rust-1.41-android-linker-bug-repro, in the hope that it can help debugging this (and because I was curious to dig into this myself :-) ).

You can clone the repo, then

docker build -t repro .

docker run --rm -it repro bash

In the container you can run ./repro-min.sh.

I bisected it using rustup and nightly releases, 2012-12-09 is good, 2019-12-10 is bad:

root@dea5c1fdc0d5:/usr/src/repro# rustup default nightly-2019-12-09
info: using existing install for 'nightly-2019-12-09-x86_64-unknown-linux-gnu'
info: default toolchain set to 'nightly-2019-12-09-x86_64-unknown-linux-gnu'

  nightly-2019-12-09-x86_64-unknown-linux-gnu unchanged - rustc 1.41.0-nightly (59947fcae 2019-12-08)

root@dea5c1fdc0d5:/usr/src/repro# ./repro-min.sh
   Compiling repro v0.1.0 (/usr/src/repro)
    Finished release [optimized] target(s) in 0.20s
root@dea5c1fdc0d5:/usr/src/repro# rustup default nightly-2019-12-10
info: using existing install for 'nightly-2019-12-10-x86_64-unknown-linux-gnu'
info: default toolchain set to 'nightly-2019-12-10-x86_64-unknown-linux-gnu'

  nightly-2019-12-10-x86_64-unknown-linux-gnu unchanged - rustc 1.41.0-nightly (76a252ea9 2019-12-09)

root@dea5c1fdc0d5:/usr/src/repro# ./repro-min.sh
   Compiling repro v0.1.0 (/usr/src/repro)
    Finished release [optimized] target(s) in 0.20s
ld: target/i686-unknown-linux-gnu/release/librepro.a(std-c3eaafedbae89cb4.std.52ue7rpt-cgu.0.rcgu.o): warning: relocation against `__rust_probestack' in read-only section `.text._ZN3std3sys4unix2fs4copy17h783e034bf468da7dE'
ld: warning: creating a DT_TEXTREL in a shared object
root@dea5c1fdc0d5:/usr/src/repro#

When using nightly-2019-12-10:

root@28bb5be323b6:/usr/src/repro# rustc --version
rustc 1.41.0-nightly (76a252ea9 2019-12-09)

[Mark-Simulacrum]

Conveniently, no rollups!

76a252e Auto merge of #67110 - tmandry:bump-compiler-builtins, r=alexcrichton
7de9402 Auto merge of #67096 - mark-i-m:fix-rustc-guide-2, r=ehuss
3ff17e7 Auto merge of #67016 - lqd:placeholder_loans, r=matthewjasper
dbbe4f1 Auto merge of #67004 - estebank:issue-66958, r=eddyb
2b0e6d2 Auto merge of #67003 - cjgillot:corrida, r=Mark-Simulacrum
14195e1 Auto merge of #66984 - GuillaumeGomez:move-clean-types, r=kinnison

I would personally suspect #67110.

Can you try checking this via https://crates.io/crates/rustup-toolchain-install-master/?

Specifically, comparing 76a252e and 7de9402?

[brainlock]

While rustup-toolchain-install-master compiles ( 😅 ): I see that https://github.com/rust-lang/rust/pull/67110 upgrades compiler-builtins. I had a look at the commits in compiler-builtins which touch __rust_probestack, and I think we can likely narrow it down to rust-lang/compiler-builtins@2566aa6.

At this point I'm too much out of my depth, this needs people more knowledgeable than me 😉 . I hope I could help debugging this, and will continue watching the issue as I'm learning a great deal just by investigating this.

Besides the actual fix, there are two additional things I'd love to understand:

1. why doesn't scanelf -qT std-c3eaafedbae89cb4.std.52ue7rpt-cgu.0.rcgu.o detect this? It looks like the obj file is not marked in any way has having a TEXTREL, but maybe the inline asm from that commit defines __rust_probestack in such a way that relocating it needs touching the .text section (wild guess)

2. how would we go at testing this? Where in the rust project would I add a test for this? And given that scanelf can't detect this, maybe a good way to "integration test" this would be to build a minimal rust static lib, and try to link it into a shared object with --warn-shared-textrel --fatal-warnings like I'm doing in the repro code. Do I understand correctly that "all code produced by rustc should be relocatable" is a guaranteed property (thus worth testing), or am I completely mistaken?

[brainlock]

Meanwhile, I can confirm that we got the right commit (76a252e):

albe@myo ~/code/repro [master] % docker build -t repro .
[...]
albe@myo ~/code/repro [master] % docker run --rm -it repro bash
root@c74a4c84b096:/usr/src/repro# rustup default 76a252ea9e7be93a61ffdf33b3533e24a9cf459d
info: default toolchain set to '76a252ea9e7be93a61ffdf33b3533e24a9cf459d'
root@c74a4c84b096:/usr/src/repro# rustc --version
rustc 1.41.0-nightly (76a252ea9 2019-12-09)
root@c74a4c84b096:/usr/src/repro# ./repro-min.sh
ld: target/i686-unknown-linux-gnu/release/librepro.a(std-c3eaafedbae89cb4.std.52ue7rpt-cgu.0.rcgu.o): warning: relocation against `__rust_probestack' in read-only section `.text._ZN3std3sys4unix2fs4copy17h783e034bf468da7dE'
ld: warning: creating a DT_TEXTREL in a shared object
root@c74a4c84b096:/usr/src/repro#
root@c74a4c84b096:/usr/src/repro#
root@c74a4c84b096:/usr/src/repro# rustup default 7de9402b77ded0d8ec9e1c554521b2121449ef2b
info: default toolchain set to '7de9402b77ded0d8ec9e1c554521b2121449ef2b'
root@c74a4c84b096:/usr/src/repro# rustc --version
rustc 1.41.0-nightly (7de9402b7 2019-12-09)
root@c74a4c84b096:/usr/src/repro# ./repro-min.sh
root@c74a4c84b096:/usr/src/repro# echo $?
0
root@c74a4c84b096:/usr/src/repro#

[pnkfelix]

triage: P-high. Assigning to self. Leaving nominated because I'd like to discuss this at today's meeting.

[nikomatsakis]

cc @tmandry, who did the rollup of compiler-builtins, do we know which of the following PRs may have caused the problem

• probestack: add frame pointers for easier traceback compiler-builtins#306 (seems pretty clearly ok)
• Support deterministic builds by remapping the __FILE__ prefix if the … compiler-builtins#309 (remapping __FILE__? seems ok)
• Modernize the testcrate slighty compiler-builtins#310 (modernize test crate, seems ok)
• Implement LLVM's elementwise unordered atomic memory intrinsics compiler-builtins#311 (atomic intrinsics? seems ok)
• Tidy up unordered elementwise atomic memory intrinsics compiler-builtins#312 (atomic intrinsics? seems ok)
• Lokathor tries to use the .gitmodules compiler-builtins#313 (gitmodules? probably ok)
• Implement bcmp compiler-builtins#315 (bcmp? seems ok)
• Emit _fltused on uefi targets as a short-term workaround compiler-builtins#317 (eliminate __fltused on uefi targets?)
• Allow FFI-unsafe warnings for u128/i128 compiler-builtins#323 (FFI unsafe warnings for i128/u128)
• Gate atomic intrinsics on presence of instructions compiler-builtins#324
• Add control flow information to __rust_probestack compiler-builtins#328

@pnkfelix hypothesizes rust-lang/compiler-builtins#328, do you know that this could be the cause, @tmandry?

[tmandry]

It could be. Likely the difference is in the attributes of the probestack function (before that change it was a #[naked] rust function, afterward it was defined in raw assembly).

@brianlock could you try adding the line .hidden __rust_probestack around here?

If that fixes it, it may break it in other environments (e.g. when generating code for a shared vs static library). We'd probably need to thread some new cfg flags through, but I'm not sure if that's actually possible.

The other way is going back to defining the assembly in a naked function, but a bug, probably in rustc, prevented me from doing this the first time.

cc @alexcrichton

[Mark-Simulacrum]

cc @Amanieu (though they're on vacation this week I believe) on the inline assembly options as potentially relevant for RFC and if they have thoughts on how to fix it for this case

[alexcrichton]

I would agree with the conclusion that rust-lang/compiler-builtins#328 is the most likely candidate here for a cause of the regression. That being said I have no idea what TEXTREL is, what's happening there to cause it, or how we might fix this.

[Amanieu]

Looking at the objdump output, you can see the difference between the 2019-12-09 and 2019-12-10 nightlies:

2019-12-09:00000000 g     F .text.__rust_probestack	0000002f .hidden __rust_probestack
2019-12-10:00000000 g     F .text.__rust_probestack	00000031 __rust_probestack

The textrel happens because the __rust_probestack has global visibility and can therefore (in theory) be overridden at runtime, e.g. with LD_PRELOAD. This means that the dynamic linker will need to patch the .text section at load time to fix up the call instruction.

As @tmandry suggested, adding .hidden __rust_probestack should fix the problem since the linker will then resolve references to the symbol within the shared library itself.

[tmandry]

As @tmandry suggested, adding .hidden __rust_probestack should fix the problem since the linker will then resolve references to the symbol within the shared library itself.

That's probably the correct fix then, I'm just not 100% sure it will work on all targets . If it fails we should see it in CI, though.

[Cogitri]

Will this be released in a 1.41.1, or should distros just apply this downstream? We'd very much like Rust 1.41.0 without TEXTRELs on x86.

[Cogitri]

Also, thanks for the quick fix of course :)

[Mark-Simulacrum]

I would not currently wait for a 1.41.1 release. I hope that it will make it into 1.42 though.

[Cogitri]

Alright, thanks for the info.

[brainlock]

Thanks for fixing this! I verified that this solves the issue on our end, and opened a PR with a simple regression test. The test fails on i686-unknown-linux-gnu when I revert compiler-builtins to 0.1.24.

[brainlock]

I'm not sure this is the right place to bring it up, but without this fix it is impossible to deploy a rust static library to platforms that enforce properly-relocatable code (x86 android). It didn't make the cut for 1.41.1, is there any chance for 1.42? Can we help to make this happen?

Thanks!

[mati865]

#69086 is beta nominated, it will be backported soon and included in 1.42 release.

[brainlock]

#69086 is beta nominated, it will be backported soon and included in 1.42 release.

Fantastic, thanks. Do you think it would be worth it to also include the regression test (#69168) in the release?

[mati865]

Do you think it would be worth it to also include the regression test (#69168) in the release?

CCing the reviewers @tmandry @Mark-Simulacrum

[awakecoding]

awesome, I am waiting for a release with this fix. Any idea when 1.42 is planned?

[Cogitri]

Stable releases usually occur every 6 weeks, so it should land in ~2 weeks.

[Mark-Simulacrum]

I've nominated the regression test as well!