Bug 277333 - Mk/Features/lto.mk: passing -C lto=no breaks LTO_UNSAFE ports
Summary: Mk/Features/lto.mk: passing -C lto=no breaks LTO_UNSAFE ports
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Piotr Kubaj
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-26 16:07 UTC by Jan Beich
Modified: 2024-03-26 18:45 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2024-02-26 16:07:32 UTC
Regressed by ports 33b550f03016 + ports 1b286170da8f. Consumers with unconditional LTO_UNSAFE break on FreeBSD 13.2 due to LLVM mismatch e.g.,

https://pkg-status.freebsd.org/beefy16/data/132amd64-default/3229bb06d7d7/logs/errors/veloren-weekly-s20240215_1.log

  = note: ld: error: /wrkdirs/usr/ports/games/veloren-weekly/work/target/x86_64-unknown-freebsd/release/deps/libveloren_common_frontend-69e7ee9b38f43147.rlib(veloren_common_frontend-69e7ee9b38f43147.veloren_common_frontend.bdf2b8bf52e7e608-cgu.05.rcgu.o): Opaque pointers are only supported in -opaque-pointers mode (Producer: 'LLVM17.0.6-rust-1.75.0-stable' Reader: 'LLVM 14.0.5')

https://pkg-status.freebsd.org/beefy16/data/132amd64-default/3229bb06d7d7/logs/errors/jumpy-0.10.0_1.log
https://pkg-status.freebsd.org/ampere3/data/132arm64-default/e9c9c73181b5/logs/errors/jumpy-0.10.0.log

  = note: ld: error: /wrkdirs/usr/ports/games/jumpy/work/target/x86_64-unknown-freebsd/release/deps/libhumantime_serde-fff4eb01f7a1da96.rlib(humantime_serde-fff4eb01f7a1da96.humantime_serde.af9df457ca654d80-cgu.0.rcgu.o): Opaque pointers are only supported in -opaque-pointers mode (Producer: 'LLVM17.0.6-rust-1.76.0-stable' Reader: 'LLVM 14.0.5')
  = note: ld: error: /wrkdirs/usr/ports/games/jumpy/work/target/aarch64-unknown-freebsd/release/deps/libhumantime_serde-e707afda9551c5e2.rlib(humantime_serde-e707afda9551c5e2.humantime_serde.cd5b947a98481e3d-cgu.0.rcgu.o): Opaque pointers are only supported in -opaque-pointers mode (Producer: 'LLVM17.0.6-rust-1.75.0-stable' Reader: 'LLVM 14.0.5')

https://pkg-status.freebsd.org/beefy16/data/132amd64-default/3229bb06d7d7/logs/errors/texlab-4.2.0_15.log

  = note: ld: error: /wrkdirs/usr/ports/devel/texlab/work/target/x86_64-unknown-freebsd/release/deps/libfern-f198c6d179095779.rlib(fern-f198c6d179095779.fern.468fa5bf8ac90584-cgu.00.rcgu.o): Opaque pointers are only supported in -opaque-pointers mode (Producer: 'LLVM17.0.6-rust-1.76.0-stable' Reader: 'LLVM 14.0.5')
Comment 3 Gleb Popov freebsd_committer freebsd_triage 2024-03-26 12:58:24 UTC
Sorry, but what's "-C lto=no"?
Comment 4 Jan Beich freebsd_committer freebsd_triage 2024-03-26 13:06:45 UTC
(In reply to Gleb Popov from comment #3)
Added in ports 1b286170da8f and supposed to override lto=true in Cargo.toml but unlike CARGO_PROFILE_RELEASE_LTO=false has the opposite effect.

See also
https://doc.rust-lang.org/rustc/codegen-options/index.html#lto
https://doc.rust-lang.org/cargo/reference/profiles.html#lto
Notice, Cargo has different behavior between "false" and "off".lto=no
Comment 5 Gleb Popov freebsd_committer freebsd_triage 2024-03-26 13:09:59 UTC
USES=cargo should arrange flags in such way that LTO is disabled by default.
lto.mk should append/change flags so that LTO gets enabled.

I don't quite see why that .else case containing -C lto=no is needed.
Comment 6 Piotr Kubaj freebsd_committer freebsd_triage 2024-03-26 14:52:01 UTC
-C lto=no was added because some ports fail to build due to running out of memory on 32-bits. Setting LTO_UNSAFE on those ports didn't make a difference, because upstreams of those ports explicitly enabled LTO.

If you have a better fix for such a case, patches welcome.
Comment 7 Gleb Popov freebsd_committer freebsd_triage 2024-03-26 14:59:15 UTC
Generally, <FEATURE>_UNSAFE should completely disable a feature. I still don't get how wrapping everything LTO-related into

.if !defined(LTO_UNSAFE)
.endif

doesn't fix the issue.
Comment 8 Gleb Popov freebsd_committer freebsd_triage 2024-03-26 15:00:54 UTC
(In reply to Piotr Kubaj from comment #6)
Ah, you mean that LTO is enabled somewhere inside the project's cargo file?
Comment 9 Piotr Kubaj freebsd_committer freebsd_triage 2024-03-26 15:01:27 UTC
(In reply to Gleb Popov from comment #8)
Indeed, that is the case.
Comment 10 Jan Beich freebsd_committer freebsd_triage 2024-03-26 15:49:40 UTC
(In reply to Piotr Kubaj from comment #6)
> some ports fail

Which ports on what release/architecture tuples? Your commits failed to document this but here you have documented regressions that may justify backout before 2024Q2 branches.

> If you have a better fix for such a case, patches welcome.

Why not consistently use CARGO_PROFILE_*_LTO for both default/WITH_LTO and LTO_UNSAFE/WITHOUT_LTO ? Ports without USES=cargo likely don't respect RUSTFLAGS unless passed via MAKE_ENV, anyway.
Comment 11 Gleb Popov freebsd_committer freebsd_triage 2024-03-26 16:05:07 UTC
There should really be only two cases: WITH_LTO=yes and WITHOUT_LTO=yes

Since LTO is an off-by-default feature, Uses/cargo.mk should arrange things in such way that any Rust program would be compiled without LTO. That means overriding cargo.toml knobs too.
Comment 12 Piotr Kubaj freebsd_committer freebsd_triage 2024-03-26 16:14:55 UTC
I currently don't remember which ports, but looking at my commits, it would be textproc/typst on powerpc / 14.0-RELEASE.
Comment 13 Jan Beich freebsd_committer freebsd_triage 2024-03-26 16:34:10 UTC
(In reply to Gleb Popov from comment #11)
> Since LTO is an off-by-default feature

Not in USES=cargo after ports 967022fd812c. The ports affected here explicitly enable LTO via lto=true in Cargo.toml but define LTO_UNSAFE due to incompatibility with CARGO_PROFILE_RELEASE_PANIC set by Mk/Features/lto.mk, leading to -C lto=no causing desync between cargo and rustc.
Comment 14 Jan Beich freebsd_committer freebsd_triage 2024-03-26 16:52:01 UTC
Consumers cannot drop LTO_UNSAFE (to match Cargo.toml) and override CARGO_PROFILE_RELEASE_PANIC due to Mk/Features/lto.mk appending CARGO_ENV after (the last assignment wins) ports' own CARGO_ENV. For example, the following restores the regression (see inline comment) introduced by ports 967022fd812c.

$ make -V CARGO_ENV:M\*PANIC\*
CARGO_PROFILE_RELEASE_PANIC=unwind CARGO_PROFILE_RELEASE_PANIC="abort"

diff --git a/devel/texlab/Makefile b/devel/texlab/Makefile
index 5ae2bf759953..3cc9eab3cd51 100644
--- a/devel/texlab/Makefile
+++ b/devel/texlab/Makefile
@@ -17,7 +17,7 @@ GH_ACCOUNT=	latex-lsp
 
 # Fixes: error: the linked panic runtime `panic_unwind` is not compiled with
 # this crate's panic strategy `abort`
-LTO_UNSAFE=	yes
+CARGO_ENV=	CARGO_PROFILE_RELEASE_PANIC=unwind
 
 PLIST_FILES=	bin/${PORTNAME} \
 		share/man/man1/${PORTNAME}.1.gz
Comment 15 Jan Beich freebsd_committer freebsd_triage 2024-03-26 17:37:21 UTC
WITHOUT_<FEATURE> in ports' Makefile hasn't been made illegal yet, so ports 2b74e8c0ebd7 used it as a workaround due to 2024Q2 branching soon. As long as you don't break my ports I ultimately don't care how this is fixed.

Alternatively, lang/rust needs to enable PORT_LLVM by default to help plain LTO like here or cross-LTO in www/firefox and graphics/mesa-devel. Doing so is a lot of work: backport LLVM fixes to devel/llvm* ports, bump LLVM_DEFAULT (or pin to avoid downgrade), prepare for extra maintenance on upgrades and split devel/llvm* into subpackages (Rust only needs LLVM but llvm* packages also include Clang/LLD/LLDB/Flang/etc which take more time to build).
Comment 16 Gleb Popov freebsd_committer freebsd_triage 2024-03-26 17:51:59 UTC
Why can't we unconditionally pass -C lto=no in cargo.mk and then override it with -C lto=something_else in lto.mk?
Comment 17 Jan Beich freebsd_committer freebsd_triage 2024-03-26 18:08:38 UTC
To summarize, WITH_LTO looks like crap to me:
- LTO_UNSAFE with inconsistent (CARGO_PROFILE_* vs. RUSTFLAGS) and unexpected semantics (LTO_UNSAFE vs. WITHOUT_LTO)
- Modifies RUSTFLAGS without passing to MAKE_ENV (for ports *without* USES=cargo)
- Overrides individual CARGO_ENV assignemnets in consumers
- Fakes LTO in USES=cabal (-Wl,--gc-sections is a different thing and common in C/C++ world)

I may be wrong but you can easily test via devel/texlab which is fast to build.
Comment 18 Jan Beich freebsd_committer freebsd_triage 2024-03-26 18:45:56 UTC
(In reply to Gleb Popov from comment #16)
USES=cargo is not supposed to desync cargo vs. rustc, leading to an undefined behavior. If a Rust feature is exposed via Cargo then use Cargo variables to control it. Passing -C lto=no leads to some files built with LTO and some without LTO, likely causing rustc to leak LLVM bitcode (skipping optimization step) to the system linker (LTO-unaware or incompatible LLVM version).

See also review D29548 which tried to move rustc bits into a separate USES.