Wilfred / difftastic

a structural diff that understands syntax 🟥🟩
https://difftastic.wilfred.me.uk/
MIT License
20.52k stars 330 forks source link

MUSL build: 'difft --version' terminated by signal SIGSEGV #563

Open tmysik opened 1 year ago

tmysik commented 1 year ago

I just tried the latest version (0.51.1), the MUSL build but unfortunately, it does not work for me. Let me know how can I provide more details, thanks

~ difft --version                                                                                                                                                                                                                                                                                                                          
fish: Job 1, 'difft --version' terminated by signal SIGSEGV (Address boundary error)

My OS is Oracle Linux 8.8.

Wilfred commented 1 year ago

Hmm, I can reproduce on my system too. gdb doesn't show anything useful:

$ gdb ./difft
Reading symbols from ./difft...
(No debugging symbols found in ./difft)
(gdb) run --version
Starting program: /tmp/difft --version

Program received signal SIGSEGV, Segmentation fault.
0x00000000000a56a6 in ?? ()
(gdb) bt
#0  0x00000000000a56a6 in ?? ()
#1  0x00007ffff43845a5 in ?? ()
#2  0x00007ffff3e69fbb in ?? ()
#3  0x00007ffff3e6ba30 in ?? ()
#4  0x00007ffff4573061 in ?? ()
#5  0x00007ffff3f3c390 in ?? ()
#6  0x00007ffff4573086 in ?? ()
#7  0x00007ffff4573069 in ?? ()
#8  0x0000000000000000 in ?? ()

although that 0x0 in #8 does look suspicious.

Wilfred commented 1 year ago

Possibly related: https://github.com/rust-lang/rust/issues/95926

Maybe musl builds should use rust 1.61 to avoid that issue.

Wilfred commented 1 year ago

https://github.com/rust-lang/rust/issues/74757 also shows a very similar backtrace.

Wilfred commented 1 year ago

What's really bizarre here is that the musl builds are dynamically linked. This is wrong, and differs from the rust default (musl targets are statically linked).

$ ldd difft
    linux-vdso.so.1 (0x00007fff1a376000)
    libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x00007f6283400000)
    libm.so.6 => /usr/lib/libm.so.6 (0x00007f6287ae0000)
    libc.so.6 => /usr/lib/libc.so.6 (0x00007f6283000000)
    /usr/lib64/ld-linux-x86-64.so.2 (0x00007f6287c01000)
    libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f6287abb000)

This is slightly different from the normal linux-gnu target, which also has libpthread and libdl, but the musl build is definitely dynamically linked for some reason.

agateau commented 1 year ago

This is slightly different from the normal linux-gnu target, which also has libpthread and libdl, but the musl build is definitely dynamically linked for some reason.

For what it's worth: I built Clyde, which is Rust-based and provides a musl static binary for Linux. To make it statically linked I had to add this to the build script:

export RUSTFLAGS='-C target-feature=+crt-static'
Wilfred commented 1 year ago

Thanks, I'll try that. I don't understand why I'm not getting static linking, rust should statically link musl builds by default I believe:

$ cargo new demo      
     Created binary (application) `demo` package

$ cd demo       
$ cargo build --release --target x86_64-unknown-linux-musl                                                                       
   Compiling demo v0.1.0 (/tmp/demo)
    Finished release [optimized] target(s) in 0.32s

$ ldd target/x86_64-unknown-linux-musl/release/demo      
    statically linked
agateau commented 1 year ago

That's odd indeed. Maybe the behavior is different in your example because the demo project does not have any dependency?

Wilfred commented 11 months ago
$ cross build --release --target x86_64-unknown-linux-musl

$ ldd target/x86_64-unknown-linux-musl/release/difft
    statically linked

$ ./target/x86_64-unknown-linux-musl/release/difft --version
Difftastic 0.52.0 (6805939c6 2023-09-28)

I can't replicate this with cross locally. I'm inclined to blame the upload-rust-binary action. It might also be easier to do more work in shell scripts in this repo, as it's much easier to reproduce issues than re-running GiHub actions over and over.

Wilfred commented 11 months ago

I'm wondering if something is picking up an old version of cross. https://github.com/cross-rs/cross/issues/902 looks relevant.

Wilfred commented 11 months ago

Probably worth filing an issue on upload-rust-binary-action to see if I've missed anything.

nekopsykose commented 11 months ago

This is slightly different from the normal linux-gnu target, which also has libpthread and libdl, but the musl build is definitely dynamically linked for some reason.

that's actually not correct anymore. glibc 2.34 removed dynamic libdl.so and libpthread.so, so for new binaries you don't see them anymore- there is only a stub empty .a. see https://developers.redhat.com/articles/2021/12/17/why-glibc-234-removed-libpthread for some details

what that means is, that ldd list means the binary has all the libs you would expect for a -crt-static(dynamic) *-linux-gnu(glibc) triplet build. i.e. it's not linked to musl at all (or probably it somehow ends up creating a frankenstein binary that on the surface looks like it links glibc; like having all the glibc libs in NEEDED but actually the musl static binary startup code). perhaps something went wrong with the configured rustup toolchain in your case? i haven't seen this specific failure case before, so it's probably related to some default target or the cross setup.

nekopsykose commented 11 months ago

ah, yeah that cross issue above looks very related

Wilfred commented 9 months ago

Reproduces on CI now, fortunately: https://github.com/Wilfred/difftastic/actions/runs/6986277695/job/19011557098 (after 6051f0519bcef18d86ff043da15deac2a4274361).