tectonic-typesetting / tectonic

A modernized, complete, self-contained TeX/LaTeX engine, powered by XeTeX and TeXLive.
https://tectonic-typesetting.github.io/
Other
3.98k stars 161 forks source link

Tectonic fails to build when link time optimization is enabled #795

Open inglor opened 3 years ago

inglor commented 3 years ago

LTO flag was added as a choice flag when building packages in Archlinux recently. When enabled, build fails during build.

From makepkg.conf man page: https://man.archlinux.org/man/makepkg.conf.5

lto
  Enable building packages using link time optimization. Adds -flto to both CFLAGS and CXXFLAGS.

Full logs with and without LTO attached. out-no-lto.log out.log

pkgw commented 3 years ago

Hmm, it looks like this might be related to the interdependencies amongst the various supporting C libraries that combine to create the tectonic executable. I bet there's some magic linker flag that will get everything working again, but not sure how hard it will be to figure out what it is ...

inglor commented 2 years ago

@pkgw At the moment since the LTO flag is now the default in Archlinux the package fails. The easiest fix is to add to the PKGBUILD:

option=("!lto")

Alternatively I managed to get some progress with linking the tectonic executable with the below, but fails on test link:

if [[ $CFLAGS = *-flto* ]]; then
   local rustflags=(
     -C linker=clang
     -C linker-plugin-lto
     -C link-args="$CFLAGS -fuse-ld=lld"
   )
   IFS=$'\x1f'
   export CARGO_ENCODED_RUSTFLAGS="${rustflags[*]}"
   unset IFS
   export CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1
   export CARGO_PROFILE_RELEASE_LTO=fat
   export CC=clang
   export CXX=clang++
fi
cargo build --release --frozen

Note the use of clang is mandatory to properly handle LTO.

ratmice commented 2 years ago

I suspect, since there has been a number of ld bugs in the past involving the combination of LTO, and -Wl,--as-needed, that this is one of those linker bugs. Could you try it out with -C link-arg=-Wl,--no-as-needed.

Also maybe note which version of ld the error is coming from can probably pass -C link-arg=-Wl,--version or --verbose may be better

inglor commented 2 years ago

@ratmice tried with the below PKGBUILD and it fails during testing

diff --git a/PKGBUILD b/PKGBUILD
index 2d5bae9..87e805d 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -18,18 +18,34 @@ pkgdesc='Modernized, complete, self-contained TeX/LaTeX engine, powered by XeTeX
 url=https://tectonic-typesetting.github.io/
 license=('MIT')
 depends=('fontconfig' 'harfbuzz-icu' 'openssl')
-makedepends=('cargo' 'pkg-config')
+makedepends=('cargo' 'pkg-config' 'clang' 'lld')
 source=("$pkgname-$pkgver.tar.gz::https://crates.io/api/v1/crates/$pkgname/$pkgver/download")
 sha512sums=('4750b32621f5557bd4ef885445898b1b555461bc30cf8476a9fb2a76b008d7378664465f4cd6b1b8ddc7305cd24bda4a6d865ec925fad64edcd9e4a302d70b21')

 build() {
        cd $pkgname-$pkgver
-       cargo build --release --locked --features external-harfbuzz
+       if [[ $CFLAGS = *-flto* ]]; then
+               local rustflags=(
+                       -C linker=clang
+                       -C linker-plugin-lto
+                       -C link-args="$CFLAGS -fuse-ld=lld"
+                       -C link-arg=-Wl,--no-as-needed
+                       -C link-arg=-Wl,--version
+               )
+               IFS=$'\x1f'
+               export CARGO_ENCODED_RUSTFLAGS="${rustflags[*]}"
+               unset IFS
+               export CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1
+               export CARGO_PROFILE_RELEASE_LTO=fat
+               export CC=clang
+               export CXX=clang++
+       fi
+       cargo build --release --locked --verbose --features external-harfbuzz
 }

 check() {
        cd $pkgname-$pkgver
-       cargo test --release --locked --features external-harfbuzz
+       cargo test --release --locked --verbose --features external-harfbuzz
 }

 package() {

makepkg-lto-verbose.log

Side-note: during tests one of them creates a .pdf and opens it - I have to manually close it to continue, is this expected?

ratmice commented 2 years ago

yeah sorry I should have noted -Wl,--version is definitely seems like it should fail, as it just prints the version and exits, without actually doing anything, I was just hoping to see the version of the linker the compiler is picking up.

Unhelpfully it seems like cargo filtered out linker output and just output the command line, I think you may need cargo build -vv to get the actual linker output..

ratmice commented 2 years ago

The other thing I should note is that -Wl,--no-as-needed seems to be showing up after -Wl,-as-needed libs... -Wl-no-as-needed, and I think it only affects libraries on the command line after the setting.

So setting it may not be having the effect. I think we need to look into rustc to see if there is a setting to disable it maybe.

ratmice commented 2 years ago

Some bugs and various issues, regarding getting rustc to disable this, If i understand correctly, that there is an implementation you might be able to use that rfc to add the flag on a unstable compiler. https://github.com/rust-lang/rust/issues/57837 https://github.com/rust-lang/rfcs/blob/master/text/2951-native-link-modifiers.md https://github.com/rust-lang/rust/issues/81490

inglor commented 2 years ago

If helpful here's the log of -vv and without the -Wl,--version.

makepkg-lto-vv.log