Miri fails to run a binary targeting Android

[landaire]

I'm attempting to run tests targeting an Android application under Miri and it fails to get past the std::rt::init() routine. The error is as follows:

error: unsupported operation: `extern` static DefId(1:15149 ~ std[2a7e]::sys::unix::android::signal::{extern#1}::signal) is not supported by Miri
  --> /Users/landerb/.rustup/toolchains/nightly-2022-03-03-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/android.rs:75:5
   |
75 |     weak!(fn signal(c_int, sighandler_t) -> sighandler_t);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `extern` static DefId(1:15149 ~ std[2a7e]::sys::unix::android::signal::{extern#1}::signal) is not supported by Miri
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

   = note: inside `std::sys::unix::android::signal` at /Users/landerb/.rustup/toolchains/nightly-2022-03-03-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/weak.rs:40:38
   = note: inside `std::sys::unix::init::reset_sigpipe` at /Users/landerb/.rustup/toolchains/nightly-2022-03-03-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/mod.rs:123:19
   = note: inside `std::sys::unix::init` at /Users/landerb/.rustup/toolchains/nightly-2022-03-03-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/unix/mod.rs:66:5
   = note: inside `std::rt::init` at /Users/landerb/.rustup/toolchains/nightly-2022-03-03-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:78:9
   = note: inside closure at /Users/landerb/.rustup/toolchains/nightly-2022-03-03-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:127:42

The issue seems to be with how the std::sys::unix::android::signal() function is implemented with weak linking: https://github.com/rust-lang/rust/blob/1eb72580d076935a3e590deb6e5813a5aef3eca4/library/std/src/sys/unix/android.rs#L74-L81

We could init the extern static for Android, but I'm genuinely not sure how to go about looking up the implementation pointer it should be linked to or if we should implement some other workaround for this.

[RalfJung]

Yeah, I never tested any Android targets, so there are probably quite a few things missing. I'd be open to adding it as a "best effort" kind of target (where we would disable tests if it turns out things break in non-trivial-to-fix-ways). We don't have an official target support policy but de-facto it's "Linux, macOS, and Windows" (and quite a few OS shims are macOS/Linux only, like file system access).

Regarding this concrete problem, if you add signal to the list you found (which target_os does Android have? It might also need a new match arm) then it will be created as a zero-initialized static. For weak symbols on Linux that's enough, as the comment explains.

Though looking at the libstd code you linked, that will just mean we panic. So that's no good. Looks like for the first time Miri might have to actually implement a weak symbol. I imagine this should work similarly to our existing dlsym support. We have this Dlsym enum that we use to generate function pointers here; the same approach should work when initializing our extern statics. If you add a new variant to the enum Rust will tell you where you have to add the corresponding implementation for what happens when that pointer is called. Seeing as on Linux, signal does nothing, the same is probably appropriate here.

[landaire]

I'd be open to adding it as a "best effort" kind of target (where we would disable tests if it turns out things break in non-trivial-to-fix-ways). We don't have an official target support policy but de-facto it's "Linux, macOS, and Windows" (and quite a few OS shims are macOS/Linux only, like file system access).

Completely understand and this sounds reasonable to me. I was actually pleasantly surprised that things even barely worked :)

Was something like this what you had in mind for the implementation (this is pseudocode -- I don't have a build environment set up yet)?

Expand to view diff

diff --git a/src/machine.rs b/src/machine.rs
index bb43cb95..87d2128c 100644
--- a/src/machine.rs
+++ b/src/machine.rs
@@ -253,6 +253,19 @@ impl MemoryExtra {
                 this.write_scalar(Scalar::from_u8(0), &place.into())?;
                 Self::add_extern_static(this, "_tls_used", place.ptr);
             }
+            "android" =>
+                for symbol_name in &[b"signal"] {
+                    let dlsym = Dlsym::from_str(symbol_name, &this.tcx.sess.target.os)?
+                        .unwrap_or_else(|| {
+                            panic!("hardcoded `extern static` symbol {} has no dlsym handler")
+                        });
+                    let ptr = this.memory.create_fn_alloc(FnVal::Other(dlsym));
+                    Self::add_extern_static(
+                        this,
+                        std::str::from_utf8(symbol_name).expect("failed to convert symbol name to str"),
+                        place.ptr,
+                    );
+                },
             _ => {} // No "extern statics" supported on this target
         }
         Ok(())
diff --git a/src/shims/posix/linux/dlsym.rs b/src/shims/posix/linux/dlsym.rs
index 1b7ac275..20e71406 100644
--- a/src/shims/posix/linux/dlsym.rs
+++ b/src/shims/posix/linux/dlsym.rs
@@ -3,7 +3,11 @@ use rustc_middle::mir;
 use crate::*;
 
 #[derive(Debug, Copy, Clone)]
-pub enum Dlsym {}
+#[allow(non_camel_case_types)]
+pub enum Dlsym {
+    /// for android only
+    signal,
+}
 
 impl Dlsym {
     // Returns an error for unsupported symbols, and None if this symbol
@@ -13,6 +17,7 @@ impl Dlsym {
             "__pthread_get_minstack" => None,
             "getrandom" => None, // std falls back to syscall(SYS_getrandom, ...) when this is NULL.
             "statx" => None,     // std falls back to syscall(SYS_statx, ...) when this is NULL.
+            "signal" => Some(Dlsym::signal),
             _ => throw_unsup_format!("unsupported Linux dlsym: {}", name),
         })
     }
@@ -27,9 +32,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         ret: Option<(&PlaceTy<'tcx, Tag>, mir::BasicBlock)>,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
-        let (_dest, _ret) = ret.expect("we don't support any diverging dlsym");
-        assert!(this.tcx.sess.target.os == "linux");
+        let (dest, ret) = ret.expect("we don't support any diverging dlsym");
+        assert!(matches!(this.tcx.sess.target.os, "linux" | "android"));
+
+        match dlsym {
+            Dlsym::signal => {
+                let &[ref _sig, ref _func] = check_arg_count(args)?;
+                this.write_null(dest)?;
+            }
+        }
 
-        match dlsym {}
+        trace!("{:?}", this.dump_place(**dest));
+        this.go_to_block(ret);
+        Ok(())
     }
 }

If this is going down the right path then I could probably start experimenting tomorrow... One note: I know you said to add another enum variant and presumably that'd be adding another variant here for Android? If that's what you meant, does it still make sense since we'd be duplicating mostly everything between the two platform modules?

[RalfJung]

I was originally thinking the Dlsym value could be hardcoded, but using from_str also works of course -- no strong opinion here.

One note: I know you said to add another enum variant and presumably that'd be adding another variant here for Android? If that's what you meant, does it still make sense since we'd be duplicating mostly everything between the two platform modules?

Right so that is something that is hard for me to judge -- should we treat Android as yet another POSIX system, next to Linux and macOS, or treat it like a variant of Linux? Given its target_os value, making it separate from Linux seems like the natural choice, but I also don't know how much overlap there is with Linux -- i.e., how much closer to Linux it is compared to macOS.

Given that Miri is basically working at the level of the libc here, and Android has its own, my inclination is to treat it like its own variant of POSIX. So maybe we could start with that, and if there turns out to be too much code duplication with Linux we can reevaluate?

So, that would mean adding posix/android/dlsym.rs and adding the signal variant there, and then adding an Android variant in posix/dlsym.rs.
(And maybe we should rename "posix" to "unix" to match the Rust standard library? That is a separate question though.)