rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.59k stars 12.74k forks source link

What should we guarantee regarding "sort-of unused" extern statics #54388

Closed pnkfelix closed 6 years ago

pnkfelix commented 6 years ago

Spawned off of https://github.com/rust-lang/rust/pull/54188#issuecomment-423172455

Right now we have a "test" that is auto-generated (via rustfix) that looks like this:

https://github.com/rust-lang/rust/blob/f7f4c500b46603386e940f116b469c7adc043a6d/src/test/ui/extern/extern-const.fixed#L12-L25

Note that there aren't any definitions provided for that extern "C" { static C: u8; }; we haven't linked in any code that gives a definition for it.

Today, rustc will compile that code without complaint (as long as you don't add the -g to the command line). And you can even run it.

Likewise, cargo build --release will also compile without complaint: play

but cargo build (developer aka debug mode) says this (play):

   Compiling extern_const v0.1.0 (/tmp/extern_const)
     Running `rustc --edition=2018 --crate-name extern_const src/main.rs --color never --crate-type bin --emit=dep-info,link -C codegen-units=1 -C debuginfo=2 -C metadata=85ca601d97244906 -C extra-filename=-85ca601d972449\
06 --out-dir /tmp/extern_const/target/debug/deps -C incremental=/tmp/extern_const/target/debug/incremental -L dependency=/tmp/extern_const/target/debug/deps`
error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/tmp/extern_const/target/debug/deps/extern_c\
onst-85ca601d97244906.1grotb0ctcuur2a3.rcgu.o" "/tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906.2nfhy5xytludulsp.rcgu.o" "/tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906.4otyorm6idcpq6f\
0.rcgu.o" "/tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906.5faf0o72xf8hr32x.rcgu.o" "/tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906.9rjqxqcpfdxtjec.rcgu.o" "/tmp/extern_const/target/de\
bug/deps/extern_const-85ca601d97244906.uxgx6titm1bphcw.rcgu.o" "-o" "/tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906" "/tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906.2q6157o69h749896.r\
cgu.o" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro" "-Wl,-znow" "-nodefaultlibs" "-L" "/tmp/extern_const/target/debug/deps" "-L" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-l\
inux-gnu/lib" "-Wl,--start-group" "-Wl,-Bstatic" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-3eb03432d1a557ee.rlib" "/home/pnkfelix/.rustup/toolchain\
s/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-77521309be1d38a9.rlib" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/l\
ib/liballoc_jemalloc-cba7d140ba86aeeb.rlib" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-2e1ca0447d72aed7.rlib" "/home/pnkfelix/.rustup/toolchains/\
nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc_system-ad6b32d9e8e7bb10.rlib" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib\
/liblibc-fb58cc2ace508689.rlib" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-5f62eb2f1b8adf62.rlib" "/home/pnkfelix/.rustup/toolchains/nightly-x86_6\
4-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-8c2c57d12c24c7a1.rlib" "-Wl,--end-group" "/home/pnkfelix/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/li\
bcompiler_builtins-2d2930074aa4f43b.rlib" "-Wl,-Bdynamic" "-ldl" "-lrt" "-lpthread" "-lpthread" "-lgcc_s" "-lc" "-lm" "-lrt" "-lpthread" "-lutil" "-lutil"
  = note: /tmp/extern_const/target/debug/deps/extern_const-85ca601d97244906.9rjqxqcpfdxtjec.rcgu.o: In function `extern_const::main':
          /tmp/extern_const/src/main.rs:5: undefined reference to `C'
          collect2: error: ld returned 1 exit status

So my hypothesis is that LLVM is managing to optimize the code enough that it doesn't notice the missing extern symbol definiton.

(But for some reason, after applying #54188, in some scenarios it does start to notice the missing extern symbol definition.)

pnkfelix commented 6 years ago

My current assumptions:

pnkfelix commented 6 years ago

nominating for discussion at compiler team meeting.

nagisa commented 6 years ago

IMO not-a-bug.

It is possible to make this sort of case behave consistently regardless of the compilation mode (to make it consistently succeed, a weak definition could be provided, but that seems kind of insane thing to do; to make it consistently fail a non-optimisable reference to the declaration would have to be generated).

However, the current behaviour is already fairly consistent across all the toolchains I’ve used so far and has its own benefits as well (not having to generate extra code for example)

pnkfelix commented 6 years ago

discussed at T-compiler meeting

But it was at the end of the meeting and I decided, based on the opinions put forward, we should not attempt to reach any final conclusions in a five minute discussion

pnkfelix commented 6 years ago

@nagisa I just want to clarify: when you say "not-a-bug", you are saying the current behavior of rustc is not a bug in rustc, right?

(As opposed to, e.g., "the test is not buggy")

That is my reading of the paragraphs you wrote after the "not-a-bug", but I just wanted to double-check.

nagisa commented 6 years ago

Your interpretation is correct.

On Thu, Sep 20, 2018, 18:51 Felix S Klock II notifications@github.com wrote:

@nagisa https://github.com/nagisa I just want to clarify: when you say "not-a-bug", you are saying the current behavior of rustc is not a bug in rustc, right?

(As opposed to, e.g., "the test is not buggy")

That is my reading of the paragraphs you wrote after the "not-a-bug", but I just wanted to double-check.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/54388#issuecomment-423233895, or mute the thread https://github.com/notifications/unsubscribe-auth/AApc0gCeHRWRGbK7R7OTU4u8bi-qF4hjks5uc7lvgaJpZM4WyJPW .

pnkfelix commented 6 years ago

based on the radio silence over past month, I'm concluding that the compiler's behavior here is not-buggy, and that the test itself should be revised in some manner (perhaps as outlined above) to avoid tripping on this.