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

Fails when LTO is enabled #186

Closed
e00E opened this issue Jul 11, 2023 · 17 comments · Fixed by #188
Closed

Fails when LTO is enabled #186

e00E opened this issue Jul 11, 2023 · 17 comments · Fixed by #188

Comments

@e00E
Copy link

e00E commented Jul 11, 2023

a.zip

./b/Cargo.toml:
[package]
name = "b"
version = "0.1.0"
edition = "2021"

[dependencies]
a = { path = "../a" }

[profile.release]
lto = "thin"

./b/src/main.rs:
fn main() {
    a::foo();
}

./a/src/lib.rs:
pub fn foo() {}

./a/Cargo.toml:
[package]
name = "a"
version = "0.1.0"
edition = "2021"

[dependencies]

Enter the b directory. cargo check works. cargo asm fails with a long error that ends with:

... -Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/a/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/home/a/b/target/release/deps/b-a3de2d2ccfd03dbf" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-nodefaultlibs"
= note: /usr/bin/ld: /home/a/b/target/release/deps/liba-ea018d38ebe9b07c.rlib: error adding symbols: file format not recognized
collect2: error: ld returned 1 exit status

Succeeds with lto = "off", lto = false. Fails with lto = "thin", lto = "fat". See the Cargo documentation for more information on LTO.

@e00E e00E changed the title Errors with thin lto Fails when lto is enabled Jul 11, 2023
@e00E e00E changed the title Fails when lto is enabled Fails when LTO is enabled Jul 11, 2023
@pacak
Copy link
Owner

pacak commented Jul 11, 2023

Related ticket is this #146

As far as I remember lto means generating multiple .s files and rustc is not exposing enough information for cargo-show-asm to tell what exactly was generating this time. I guess it's time to revisit the file detection one more time :)

@e00E
Copy link
Author

e00E commented Jul 12, 2023

The state of the other issues was unclear to me. Wasn't sure if this was accepted behavior or needed a reproduction so I made this one. If you don't want to support LTO then I would like the Readme to mention that.

Thanks for working on this project btw.

@pacak
Copy link
Owner

pacak commented Jul 12, 2023

I don't mind supporting LTO, just the way it works now it asks rustc to generate asm/lto/whatever, before LTO runs and that previous ticket was mostly about making sure it still compiles and it was compiling for me with all the lto options I tried. Will try to fix, not sure how long it will take.

@zeroflaw
Copy link

@e00E try version 0.2.14, seems to work better for me.

@pacak
Copy link
Owner

pacak commented Jul 16, 2023

Removing LTO hack seems to fix the problem and both lto=thin and lto=fat, it just works on 1.71.0... 🤔 And still does not work on 1.67.1... So probably 1.67.1 was just a temporary regression of some sort.

It probably makes sense to drop the hack and proclaim that 1.67.1 + LTO of any sort is not supported...

Decisions, decisions... 🤔

@pacak
Copy link
Owner

pacak commented Jul 16, 2023

@e00E I pushed a branch called lto, you can install it using something like cargo install --git https://github.com/pacak/cargo-show-asm --branch lto. Can you check if it fixes your problem?

@e00E
Copy link
Author

e00E commented Jul 17, 2023

That fixes it.

@pacak pacak mentioned this issue Jul 17, 2023
@zeroflaw
Copy link

upgrade from 0.2.14 all good here too.

@V0ldek
Copy link

V0ldek commented Oct 9, 2023

I'm still getting an issue on 0.2.21 that seems related. With thin LTO when trying to disassemble a --bin target I get this:

~/rsonpath> cargo asm -p rsonpath --bin rq
warning: ignoring emit path because multiple .s files were produced

warning: `rsonpath` (bin "rq") generated 1 warning
    Finished release [optimized + debuginfo] target(s) in 0.03s

Error: Cannot locate the path to the asm file

With LTO disabled it works.

As far as I remember lto means generating multiple .s files and rustc is not exposing enough information for cargo-show-asm to tell what exactly was generating this time.

I'm thinking it's this issue, judging by "warning: ignoring emit path because multiple .s files were produced`?

@pacak
Copy link
Owner

pacak commented Oct 9, 2023

I'm still getting an issue on 0.2.21 that seems related. With thin LTO when trying to disassemble a --bin target I get this:

What version of the compiler are you using?

@V0ldek
Copy link

V0ldek commented Oct 9, 2023

What version of the compiler are you using?

Latest 1.73.0: rustc 1.73.0 (cc66ad468 2023-10-03)
Fails on current nightly as well: rustc 1.75.0-nightly (bf9a1c8a1 2023-10-08)

@pacak
Copy link
Owner

pacak commented Oct 9, 2023

Hmm... Is the repo publicly available?

@V0ldek
Copy link

V0ldek commented Oct 9, 2023

Yup, it's https://github.com/V0ldek/rsonpath.

The exact command I'm running is cargo asm -p rsonpath --bin rq in the project root.

@pacak
Copy link
Owner

pacak commented Oct 10, 2023

Interestingly enough cargo asm -p rsonpath --bin rq --profile distribution works...

@V0ldek
Copy link

V0ldek commented Oct 10, 2023

May it be because codegen-units = 1?

@pacak
Copy link
Owner

pacak commented Oct 10, 2023

Most likely, but I'm passing codegen-units = 1 regardless so it probably means rustc ignores this with lto = thin 🤔

@pacak
Copy link
Owner

pacak commented Oct 10, 2023

I guess the best way to deal with LTO is to implement #174 after all. I'll look into that once I'm done messing with bpaf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants