Rust: Miscompilation when building firefox on debug mode.

[emilio]

We've come across a memory corruption issue that happens during the Gecko build process on debug builds, see https://bugzilla.mozilla.org/show_bug.cgi?id=1420301.

After a bit of fiddling, the fix is https://github.com/KyleMayes/clang-sys/pull/69/commits/2226f78af3debb6655f98968ae68de596bb2a203, so something looks fishy on rustcs side.

I'm bisecting now.

[emilio]

Beta is affected fwiw.

[kennytm]

(Aside: symbol.map(|s| *s) is the same as symbol.cloned() assuming s is Copy, though I'm not sure if that will reintroduce the mis-compilation)

[emilio]

First bad:

rustc 1.23.0-nightly (33374fa9d 2017-11-20)

Last good:

rustc 1.23.0-nightly (5041b3bb3 2017-11-19)

Regression range: https://github.com/rust-lang/rust/compare/5041b3bb3...33374fa9d

@SimonSapin points out that the workaround is not exactly the same (the inferred type for the bogus case is Symbol<extern fn> while in mine is Symbol<Option<extern fn>>. None of the functions are null though, doing some more experimentation now.

[SimonSapin]

@kennytm This option contains a libloading::Symbol<_> which is Deref, not a &_.

[kennytm]

@SimonSapin huh okay. cc @eddyb #45225 the most likely target in the range.

[emilio]

Just to make clear what's happening and making type inference easier to follow:

Crashes (this is equivalent to the current code):

diff --git a/src/link.rs b/src/link.rs
index 543a5a5..bc81d96 100644
--- a/src/link.rs
+++ b/src/link.rs
@@ -23,7 +23,10 @@ macro_rules! link {
     (@LOAD: #[cfg($cfg:meta)] fn $name:ident($($pname:ident: $pty:ty), *) $(-> $ret:ty)*) => (
         #[cfg($cfg)]
         pub fn $name(library: &mut super::SharedLibrary) {
-            let symbol = unsafe { library.library.get(stringify!($name).as_bytes()) }.ok();
+            use libloading::Symbol;
+            use super::*;
+            let symbol: Option<Symbol<unsafe extern fn($($pty,)*) -> _>> =
+                unsafe { library.library.get(stringify!($name).as_bytes()) }.ok();
             library.functions.$name = symbol.map(|s| *s);
         }

Doesn't crash:

diff --git a/src/link.rs b/src/link.rs
index 543a5a5..bdf7962 100644
--- a/src/link.rs
+++ b/src/link.rs
@@ -23,7 +23,16 @@ macro_rules! link {
     (@LOAD: #[cfg($cfg:meta)] fn $name:ident($($pname:ident: $pty:ty), *) $(-> $ret:ty)*) => (
         #[cfg($cfg)]
         pub fn $name(library: &mut super::SharedLibrary) {
-            let symbol = unsafe { library.library.get(stringify!($name).as_bytes()) }.ok();
+            use libloading::Symbol;
+            use super::*;
+
+            fn gimme_a_ref<T>(_: &T) {}
+
+            let symbol: Option<Symbol<unsafe extern fn($($pty,)*) -> _>> =
+                unsafe { library.library.get(stringify!($name).as_bytes()) }.ok();
+
+            gimme_a_ref(&symbol);
+
             library.functions.$name = symbol.map(|s| *s);
         }

Also doesn't crash:

diff --git a/src/link.rs b/src/link.rs
index 543a5a5..03f37b6 100644
--- a/src/link.rs
+++ b/src/link.rs
@@ -23,8 +23,11 @@ macro_rules! link {
     (@LOAD: #[cfg($cfg:meta)] fn $name:ident($($pname:ident: $pty:ty), *) $(-> $ret:ty)*) => (
         #[cfg($cfg)]
         pub fn $name(library: &mut super::SharedLibrary) {
-            let symbol = unsafe { library.library.get(stringify!($name).as_bytes()) }.ok();
-            library.functions.$name = symbol.map(|s| *s);
+            use libloading::Symbol;
+            use super::*;
+            let symbol: Option<Symbol<Option<unsafe extern fn($($pty,)*) -> _>>> =
+                unsafe { library.library.get(stringify!($name).as_bytes()) }.ok();
+            library.functions.$name = symbol.and_then(|s| *s);
         }

         #[cfg(not($cfg))]

[emilio]

Turns out that FF compiles some dependencies with -C opt-level=1.

I put a more minimal repro here: https://github.com/emilio/rustc-46239-repro

You can repro with RUSTFLAGS="-C opt-level=1" cargo run

[eddyb]

Minimal (still only at opt-level=1) reproduction without taking apart libloading:

extern crate libloading;

fn main() {
    unsafe {
        let library = libloading::Library::new("libc.so.6").unwrap();
        let symbol: Option<libloading::Symbol<extern fn(i32) -> !>> =
            library.get("exit".as_bytes()).ok();
        symbol.map(|s| *s).unwrap()(0);
    }
}

[nagisa]

Following is the LLVM-IR that when optimised with -O1 results in IR that passes uninitialised alloca to a call. Can be reproduced by running opt -dse unoptimised.ll.

Unoptimised

; ModuleID = '<stdin>'
source_filename = "test0-6309762d5a7d9b368daa0ebe527434c3.rs"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: norecurse readnone uwtable
define internal noalias nonnull align 8 dereferenceable(8) void (i32)** @0(i8** noalias readonly dereferenceable(8)) unnamed_addr #0 {
  %2 = bitcast i8** %0 to void (i32)**
  ret void (i32)** %2
}

; Function Attrs: norecurse readonly uwtable
define nonnull void (i32)* @"_ZN4test4main28_$u7b$$u7b$closure$u7d$$u7d$17hc0fea9cc3707b635E"(i8*) unnamed_addr #1 {
  %2 = alloca i8*, align 8
  store i8* %0, i8** %2
  %3 = call noalias align 8 dereferenceable(8) void (i32)** @0(i8** noalias readonly dereferenceable(8) %2)
  %4 = load void (i32)*, void (i32)** %3, !nonnull !1
  ret void (i32)* %4
}

attributes #0 = { norecurse readnone uwtable "probe-stack"="__rust_probestack" }
attributes #1 = { norecurse readonly uwtable "probe-stack"="__rust_probestack" }

!llvm.module.flags = !{!0}

!0 = !{i32 7, !"PIE Level", i32 2}
!1 = !{}

Optimised

; ModuleID = '<stdin>'
source_filename = "test0-6309762d5a7d9b368daa0ebe527434c3.rs"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: norecurse readnone uwtable
define internal noalias nonnull align 8 dereferenceable(8) void (i32)** @0(i8** noalias readonly dereferenceable(8)) unnamed_addr #0 {
  %2 = bitcast i8** %0 to void (i32)**
  ret void (i32)** %2
}

; Function Attrs: norecurse readonly uwtable
define nonnull void (i32)* @"_ZN4test4main28_$u7b$$u7b$closure$u7d$$u7d$17hc0fea9cc3707b635E"(i8*) unnamed_addr #1 {
  %2 = alloca i8*, align 8
  %3 = call noalias align 8 dereferenceable(8) void (i32)** @0(i8** noalias readonly dereferenceable(8) %2)
  %4 = load void (i32)*, void (i32)** %3, !nonnull !1
  ret void (i32)* %4
}

attributes #0 = { norecurse readnone uwtable "probe-stack"="__rust_probestack" }
attributes #1 = { norecurse readonly uwtable "probe-stack"="__rust_probestack" }

!llvm.module.flags = !{!0}

!0 = !{i32 7, !"PIE Level", i32 2}
!1 = !{}

[eddyb]

Reduction to builtin types and safe code only, compile with rustc -C opt-level=1:

fn project<T>(x: &(T,)) -> &T { &x.0 }

fn dummy() {}

fn main() {
    let f = (dummy as fn(),);
    (*project(&f))();
}