Skip to content
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

LTO=thin appears broken on nightly rustc? #110256

Closed
allight opened this issue Apr 12, 2023 · 6 comments · Fixed by #110535
Closed

LTO=thin appears broken on nightly rustc? #110256

allight opened this issue Apr 12, 2023 · 6 comments · Fixed by #110535
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@allight
Copy link

allight commented Apr 12, 2023

When building https://github.com/alacritty/alacritty with cargo build -r on nightly rust the resulting binary will segfault. This does not occur when built with stable rust.

This behavior change appears to be gated by lto="thin".

Code

I tried this code:

rustup default nightly
rustup update
git clone https://github.com/alacritty/alacritty
cd alacritty
cargo build -r
./target/release/alacritty

I expected to see this happen: alacritty to startup as occurs with rustup default stable

Instead, this happened: compiled program segfaults

Version it worked on

stable-x86_64-unknown-linux-gnu unchanged - rustc 1.68.2 (9eb3afe 2023-03-27)

Version with regression

nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.70.0-nightly (9df3a39 2023-04-11)

rustc --version --verbose:

rustc 1.70.0-nightly (9df3a39fb 2023-04-11)
binary: rustc
commit-hash: 9df3a39fb30575d808e70800f9fad5362aac57a2
commit-date: 2023-04-11
host: x86_64-unknown-linux-gnu
release: 1.70.0-nightly
LLVM version: 16.0.2

This appears to be triggered by lto being set to "thin" in Cargo.toml

See alacritty issues: alacritty/alacritty#6818 and alacritty/alacritty#6854 for investigation.

@allight allight added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Apr 12, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 12, 2023
@allight
Copy link
Author

allight commented Apr 12, 2023

NB This also appears to require that optimization is turned on to reproduce.

@matthiaskrgr
Copy link
Member

Hm fun, I can reproduce this with cargo build --release in alacritty git on x86

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Apr 12, 2023

I tried to bisect this

# 04.11:  bad
# 04.08 : bad

# 04.01: bad 
# 03.27: bad  
# 03.26: bad
# 03.25: good
# 03.20: good 

and t looks like this regressed in nightly-2023-03-26, so probably llvm upgrade? rip :(

@matthiaskrgr matthiaskrgr added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Apr 12, 2023
@nikic nikic added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Apr 13, 2023
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 13, 2023
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 13, 2023
@DianQK
Copy link
Member

DianQK commented Apr 16, 2023

Upstream candidate patch: D148456.
Here is a reproduction example.

fn foo(_b: &[u8]) {}

fn main() {
    let b = Vec::<u8>::new();
    let b = std::borrow::Cow::Borrowed(&b);
    foo(&b);
}
rustc +nightly ./main.rs -C opt-level=1 -C panic=abort -C codegen-units=1
./main
# [1]    619598 illegal hardware instruction (core dumped)  ./main

If you change from &Vec<u8> to &[u8] this will work fine.

diff --git a/alacritty/src/renderer/text/atlas.rs b/alacritty/src/renderer/text/atlas.rs
index 9d4cb401..f106c8be 100644
--- a/alacritty/src/renderer/text/atlas.rs
+++ b/alacritty/src/renderer/text/atlas.rs
@@ -157,6 +157,7 @@ impl Atlas {
             // Load data into OpenGL.
             let (format, buffer) = match &glyph.buffer {
                 BitmapBuffer::Rgb(buffer) => {
+                    let buffer: &[u8] = buffer;
                     multicolor = false;
                     // Gles context doesn't allow uploading RGB data into RGBA texture, so need
                     // explicit copy.
@@ -175,6 +176,7 @@ impl Atlas {
                 },
                 BitmapBuffer::Rgba(buffer) => {
                     multicolor = true;
+                    let buffer: &[u8] = buffer;
                     (gl::RGBA, Cow::Borrowed(buffer))
                 },
             };

@bors bors closed this as completed in dc73052 Apr 20, 2023
@jyn514 jyn514 added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Apr 20, 2023
@DianQK
Copy link
Member

DianQK commented Aug 10, 2023

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority regression-untriaged Untriaged performance or correctness regression. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants