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

dep-info doesn't contain statically linked libraries #58393

Closed
glandium opened this issue Feb 12, 2019 · 11 comments
Closed

dep-info doesn't contain statically linked libraries #58393

glandium opened this issue Feb 12, 2019 · 11 comments
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@glandium
Copy link
Contributor

Consider the following dummy crate:

$ cargo new foo
$ cd foo
$ cat >> Cargo.toml <<EOF
[build-dependencies]
cc = "1.0"
EOF
$ cat > foo.c <<EOF
#include <stdio.h>
void foo() {
    printf("foo\n");
}
EOF
$ cat > build.rs <<EOF
extern crate cc;

fn main() {
    cc::Build::new()
        .file("foo.c")
        .compile("foo");
}
EOF
$ cat > src/main.rs <<EOF
extern {
    fn foo();
}

fn main() {
    unsafe { foo() };
}
EOF

Now run cargo run. It will work as expected, printing foo.

The problem is that foo-*.d doesn't contain anything to do with libfoo.a, which is obviously a dependency. One consequence of this is that sccache relies on this dep-info for its caching, and changes to libfoo.a are completely ignored. One real-world consequence is that on Firefox CI, jobs that build C/C++ code with clang 3.9 still get a libgkrust.a where C code was built with clang 7 (and vice-versa, depending which cached first).

Cc: @alexcrichton @luser @froydnj

@alexcrichton
Copy link
Member

This may be the same as #57717?

@luser
Copy link
Contributor

luser commented Feb 12, 2019

Yes, this sounds like the same issue. I don't think this actually impacts sccache though--we explicitly include static libraries/rlibs in the hash calculation:
https://github.com/mozilla/sccache/blob/12008f60bc8b5335e8e8fa878e3bdc2749e50756/src/compiler/rust.rs#L889
https://github.com/mozilla/sccache/blob/12008f60bc8b5335e8e8fa878e3bdc2749e50756/src/compiler/rust.rs#L984

...however testing locally shows that this isn't working as expected, so there may be an sccache bug here.

@luser
Copy link
Contributor

luser commented Feb 12, 2019

After fixing a few assumptions (sccache won't cache the bin compilation in your example crate, and by default cargo will use incremental compilation which sccache also won't cache) I realize that mozilla/sccache#169 makes it impossible to validate this on Mac currently.

@glandium
Copy link
Contributor Author

This may be the same as #57717?

It looks like so.

Yes, this sounds like the same issue. I don't think this actually impacts sccache though--we explicitly include static libraries/rlibs in the hash calculation:
https://github.com/mozilla/sccache/blob/12008f60bc8b5335e8e8fa878e3bdc2749e50756/src/compiler/rust.rs#L889
https://github.com/mozilla/sccache/blob/12008f60bc8b5335e8e8fa878e3bdc2749e50756/src/compiler/rust.rs#L984

... except static libraries are not necessarily on the command line. The real usecase is libgkrust, and liblmdb.a is definitely not on the command line when it's built.

@luser
Copy link
Contributor

luser commented Feb 13, 2019

... except static libraries are not necessarily on the command line. The real usecase is libgkrust, and liblmdb.a is definitely not on the command line when it's built.

Sure, it must be built into a crate that gets built into libgkrust, right? At some point it will be listed on a rustc command line and sccache will notice that, and the rlib that results from that compilation will be different, which feeds into the hash of any downstream crates.

From your example above (I tweaked the crate to also have a lib.rs because sccache wouldn't cache compiling main.rs):

$ ar t target/debug/libfoo.rlib 
__.SYMDEF
foo-a909ce61422d161f.foo.8wed3dm3-cgu.0.rcgu.o
foo.o
rust.metadata.bin
foo-a909ce61422d161f.foo.8wed3dm3-cgu.0.rcgu.bc.z

That foo.o is the object file compiled via build.rs.

@glandium
Copy link
Contributor Author

Mmmm I wasn't looking at rlibs but it doesn't matter.

Check out https://queue.taskcluster.net/v1/task/eQ-_VIhVTQ-atFQQ4xY2CA/runs/0/artifacts/public/build/sccache.log, which is the sccache log for https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunning%2Cpending%2Crunnable&revision=952b928f1605ad5676ae4ccfd41612b34523bae5&searchStr=base%2Ctoolchain%2Cclang

You'll find that libgkrust comes from http://taskcluster-level-3-sccache-us-east-1.s3.amazonaws.com/7/6/d/76d95e1dadd71176f87a3ed0833e2aa98aeeaec8a8908975022ea38f5cf6ad666eb3be85da656557337ebacfae56752eef06552a094fd31ae7a184fe62408d4a . You can extract libgkrust from that file, and extract mdb.o from that libgkrust, and its sha1 is c3596962e998a2e297557b527447ec2105250083.

The only library on the rustc command line apart libmozilla_central_workspace_hack is libgkrust_shared which you can find coming from http://taskcluster-level-3-sccache-us-east-1.s3.amazonaws.com/9/6/5/96558c40f1012a4e6191830725f3c864d59caab1c6634d030b8c8c8d244e747d8d442b6fda6aa5831cd27968a10a29d23944694cc7cdad46d8adb189ccf6cfa1
Downloading that and extracting libgkrust_shared from there reveals it doesn't contain mdb.o.

For good measure, you can also get libmozilla_central_workspace_hack from http://taskcluster-level-3-sccache-us-east-1.s3.amazonaws.com/0/3/5/03580fb14fe7ff2c655ad6edaf21d368a1181d4b91a0a68a462cad2904ba00a6e7e25dbc91e23692c58eea0b090c1fbc774edb0c69c6dbcc134cba6529a65c99 and it doesn't contain mdb.o either.

As a matter of fact, if you dig in the sccache log, you won't find any command with a lmdb-sys rlib.

You will however find that lmdb-sys was compiled locally because of cache miss, and was uploaded to http://taskcluster-level-3-sccache-us-east-1.s3.amazonaws.com/8/4/2/84261d4d450e73958498fb067498de30b2186f51050c574e2365b2d72dc615e91b962014eedbf9061e0f854ddb97dd595162f0bb785a943c57c885a22e9b949a which, if you download, extract, and further extract mdb.o reveals a sha1 of 6da395323dc70937ca235d9a6eef0fca700653e1.

That mdb.o is what should be in libgkrust, but isn't!

@luser
Copy link
Contributor

luser commented Feb 19, 2019

I did a local Linux build from current central and looked in the cargo output dir and found:

/build/debug-mozilla-central/x86_64-unknown-linux-gnu/debug/build/lmdb-rkv-sys-5adf33664f0da0c9/out/liblmdb.a
/build/debug-mozilla-central/x86_64-unknown-linux-gnu/debug/build/lmdb-rkv-sys-5adf33664f0da0c9/out/mdb.o

The mdb.o there is contained in liblmdb.a. It gets placed into liblmdb_rkv_sys.rlib:

$ ar t /build/debug-mozilla-central/x86_64-unknown-linux-gnu/debug/deps/liblmdb_rkv_sys-f7b0ebb3c9c5ffa5.rlib
lmdb_rkv_sys-f7b0ebb3c9c5ffa5.lmdb_rkv_sys.a092og8o-cgu.0.rcgu.o
mdb.o
midl.o
rust.metadata.bin
lmdb_rkv_sys-f7b0ebb3c9c5ffa5.lmdb_rkv_sys.a092og8o-cgu.0.rcgu.bc.z

Looking at a recent inbound build log shows -l static=lmdb on the rustc commandline when building the lmdb-rkv-sys crate:

[task 2019-02-19T19:05:14.213Z] 19:05:14     INFO -       Running `/builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/rustc/bin/rustc --crate-name lmdb_rkv_sys /builds/worker/workspace/build/src/third_party/rust/lmdb-rkv-sys/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=2 -C panic=abort -C codegen-units=1 -C metadata=868d3183b14563f1 -C extra-filename=-868d3183b14563f1 --out-dir /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/release/deps --extern libc=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/deps/liblibc-930a7bf71af2ebf7.rlib --cap-lints warn -C opt-level=2 -C debuginfo=2 -Dwarnings -L native=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/build/lmdb-rkv-sys-fda4b8b1c2a675c1/out -l static=lmdb`

From your build log I can see lmdb-sys including -l static=lmdb:

[task 2019-02-12T01:39:20.320Z] 01:39:20     INFO -       Running `/builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/rustc/bin/rustc --crate-name lmdb_sys /builds/worker/workspace/build/src/third_party/rust/lmdb-sys/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=1 -C panic=abort -C debuginfo=2 -C debug-assertions=on -C metadata=e4b94618a8e59df1 -C extra-filename=-e4b94618a8e59df1 --out-dir /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/debug/deps --extern libc=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps/liblibc-50993e3425e943f1.rlib --cap-lints warn -C opt-level=2 -C debuginfo=2 -Dwarnings -L native=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/build/lmdb-sys-acded252dc3be516/out -l static=lmdb`

And then the lmdb crate references lmdb-sys as an extern:

[task 2019-02-12T01:39:20.360Z] 01:39:20     INFO -       Running `/builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/rustc/bin/rustc --crate-name lmdb /builds/worker/workspace/build/src/third_party/rust/lmdb-rkv/src/lib.rs --color never --crate-type lib --emit=dep-info,link -C opt-level=1 -C panic=abort -C debuginfo=2 -C debug-assertions=on -C metadata=d332f6813fc7ced5 -C extra-filename=-d332f6813fc7ced5 --out-dir /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/debug/deps --extern bitflags=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps/libbitflags-59f5ca3d9c98e954.rlib --extern libc=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps/liblibc-50993e3425e943f1.rlib --extern lmdb_sys=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/deps/liblmdb_sys-e4b94618a8e59df1.rlib --cap-lints warn -C opt-level=2 -C debuginfo=2 -Dwarnings -L native=/builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/build/lmdb-sys-acded252dc3be516/out`

So the hash ought to be included transitively since we hash externs for rustc compiles. Unfortunately there are a lot of moving pieces here so it's hard to tell where exactly things have gone wrong.

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 6, 2019
@glandium
Copy link
Contributor Author

glandium commented Dec 6, 2019

This is happening again in Firefox, in a more insidious way. This time it involves ASAN, and we can't really brush the problem away. A reproducer for this new instance, to show the extent of the problem, looks like the following:

$ cargo new --lib foo
$ cat >> foo/Cargo.toml <<EOF
[build-dependencies]
cc = "1.0"
EOF
$ cat > foo/foo.c <<EOF
#include <stdio.h>
void foo() {
    printf("foo\n");
}
EOF
$ cat > foo/build.rs <<EOF
extern crate cc;

fn main() {
    cc::Build::new()
        .file("foo.c")
        .compile("foo");
}
EOF
$ cat > foo/src/lib.rs <<EOF
extern {
    fn foo();
}

#[no_mangle]
pub extern "C" fn rust_foo() {
    unsafe { foo() };
}
EOF
$ cargo new --lib bar
$ cat >> bar/Cargo.toml <<EOF
foo = { path = "../foo" }
EOF
$ cat > bar/src/lib.rs << EOF
extern crate foo;
EOF
$ cargo new --lib qux
$ cat >> qux/Cargo.toml <<EOF
> bar = { path = "../bar" }

[lib]
crate-type = ["staticlib"]
EOF
$ cat > qux/src/lib.rs << EOF
extern crate bar;
EOF
$ cd qux
$ cargo build -v

What you'll see from the cargo build -v output is that there's only one command with -l static=foo in, and it's the one for the foo crate. And if you look at libfoo-.rlib, it contains foo.o, while libbar-.rlib doesn't. But the final libqux.a does.

So, as far as sccache is involved, when libqux.a is built, all it knows is about libbar-.rlib, and since that doesn't contain foo.o, libbar-.rlib doesn't vary when foo.o varies, and sccache doesn't know that libqux.a varies when foo.o varies.

A workaround for this is to add a dependency on foo in qux...

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Dec 6, 2019
@chmanchester
Copy link
Contributor

Using the example from #58393 (comment) I can see that sccache knows bar depends on foo via --extern, but looking at libfoo.rmeta doesn't tell us when foo.o changes (looking at libfoo.rlib might). @alexcrichton, is there a reliable way for sccache to get from libfoo.rmeta to foo.o in this case?

gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Dec 7, 2019
…. r=chmanchester

See rust-lang/rust#58393 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D56129

UltraBlame original commit: b515b0637e5eaf851af4a5aa1f8668c02d1a38ab
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Dec 7, 2019
…. r=chmanchester

See rust-lang/rust#58393 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D56129

UltraBlame original commit: b515b0637e5eaf851af4a5aa1f8668c02d1a38ab
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Dec 7, 2019
…. r=chmanchester

See rust-lang/rust#58393 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D56129

UltraBlame original commit: b515b0637e5eaf851af4a5aa1f8668c02d1a38ab
@alexcrichton
Copy link
Member

@chmanchester I don't think there's away to get this dependency information right now unfortunately, this would require changes to rustc.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 5, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 5, 2020
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Aug 6, 2020
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Aug 6, 2020
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Aug 6, 2020
…t dependencies for sccache. r=glandium

See rust-lang/rust#58393

Differential Revision: https://phabricator.services.mozilla.com/D85713

UltraBlame original commit: 060254b3ad73855146cd3d6147e50fbc514e1cdc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Aug 6, 2020
…t dependencies for sccache. r=glandium

See rust-lang/rust#58393

Differential Revision: https://phabricator.services.mozilla.com/D85713

UltraBlame original commit: 513d01d2b20cb2d485c30b7144a6bd83fd31b22c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Aug 6, 2020
…t dependencies for sccache. r=glandium

See rust-lang/rust#58393

Differential Revision: https://phabricator.services.mozilla.com/D85713

UltraBlame original commit: 060254b3ad73855146cd3d6147e50fbc514e1cdc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Aug 6, 2020
…t dependencies for sccache. r=glandium

See rust-lang/rust#58393

Differential Revision: https://phabricator.services.mozilla.com/D85713

UltraBlame original commit: 513d01d2b20cb2d485c30b7144a6bd83fd31b22c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Aug 6, 2020
…t dependencies for sccache. r=glandium

See rust-lang/rust#58393

Differential Revision: https://phabricator.services.mozilla.com/D85713

UltraBlame original commit: 060254b3ad73855146cd3d6147e50fbc514e1cdc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Aug 6, 2020
…t dependencies for sccache. r=glandium

See rust-lang/rust#58393

Differential Revision: https://phabricator.services.mozilla.com/D85713

UltraBlame original commit: 513d01d2b20cb2d485c30b7144a6bd83fd31b22c
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 7, 2024
…x-build-system-reviewers,sergesanspaille

It was disabled in bug 1525760 because of rust-lang/rust#58393
but since then we actually implemented a workaround (in bug 1601704)

That workaround, however, is also needed on the newly added
crashreporter crate.

Ironically, if these tasks had had sccache enabled in the first place,
bug 1911513 wouldn't have been caught.

Differential Revision: https://phabricator.services.mozilla.com/D218611
aosmond pushed a commit to aosmond/gecko that referenced this issue Aug 7, 2024
…x-build-system-reviewers,sergesanspaille

It was disabled in bug 1525760 because of rust-lang/rust#58393
but since then we actually implemented a workaround (in bug 1601704)

That workaround, however, is also needed on the newly added
crashreporter crate.

Ironically, if these tasks had had sccache enabled in the first place,
bug 1911513 wouldn't have been caught.

Differential Revision: https://phabricator.services.mozilla.com/D218611
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Aug 8, 2024
…x-build-system-reviewers,sergesanspaille

It was disabled in bug 1525760 because of rust-lang/rust#58393
but since then we actually implemented a workaround (in bug 1601704)

That workaround, however, is also needed on the newly added
crashreporter crate.

Ironically, if these tasks had had sccache enabled in the first place,
bug 1911513 wouldn't have been caught.

Differential Revision: https://phabricator.services.mozilla.com/D218611
@Enselic
Copy link
Member

Enselic commented Nov 12, 2024

Triage: There seems to be agreement that this is a duplicate to #57717, so let's close as duplicate.

@Enselic Enselic closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants