Auto merge of #3856 - jder:mac-native-libs, r=RalfJung

bors · bors · commit 989f63bc531d · 2024-09-03T14:23:04.000Z
Enable native libraries on macOS Fixes #3595 by using `-fvisibility=hidden` and the visibility attribute supported by both gcc and clang rather than the previous gcc-only mechanism for symbol hiding. Also brings over cfg changes from #3594 which enable native-lib functionality on all unixes. Thanks for taking a look, feedback very welcome! cc `@RalfJung`
diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml
@@ -37,8 +37,6 @@ features = ['unprefixed_malloc_on_supported_platforms']
 
 [target.'cfg(unix)'.dependencies]
 libc = "0.2"
-
-[target.'cfg(target_os = "linux")'.dependencies]
 libffi = "3.2.0"
 libloading = "0.8"
 
diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md
@@ -383,7 +383,7 @@ to Miri failing to detect cases of undefined behavior in a program.
   file descriptors will be mixed up.
   This is **work in progress**; currently, only integer arguments and return values are
   supported (and no, pointer/integer casts to work around this limitation will not work;
-  they will fail horribly). It also only works on Linux hosts for now.
+  they will fail horribly). It also only works on Unix hosts for now.
 * `-Zmiri-measureme=<name>` enables `measureme` profiling for the interpreted program.
    This can be used to find which parts of your program are executing slowly under Miri.
    The profile is written out to a file inside a directory called `<name>`, and can be processed
diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs
@@ -535,9 +535,9 @@ pub struct MiriMachine<'tcx> {
     pub(crate) basic_block_count: u64,
 
     /// Handle of the optional shared object file for native functions.
-    #[cfg(target_os = "linux")]
+    #[cfg(unix)]
     pub native_lib: Option<(libloading::Library, std::path::PathBuf)>,
-    #[cfg(not(target_os = "linux"))]
+    #[cfg(not(unix))]
     pub native_lib: Option<!>,
 
     /// Run a garbage collector for BorTags every N basic blocks.
@@ -678,7 +678,7 @@ impl<'tcx> MiriMachine<'tcx> {
             report_progress: config.report_progress,
             basic_block_count: 0,
             clock: Clock::new(config.isolated_op == IsolatedOp::Allow),
-            #[cfg(target_os = "linux")]
+            #[cfg(unix)]
             native_lib: config.native_lib.as_ref().map(|lib_file_path| {
                 let target_triple = layout_cx.tcx.sess.opts.target_triple.triple();
                 // Check if host target == the session target.
@@ -700,9 +700,9 @@ impl<'tcx> MiriMachine<'tcx> {
                     lib_file_path.clone(),
                 )
             }),
-            #[cfg(not(target_os = "linux"))]
+            #[cfg(not(unix))]
             native_lib: config.native_lib.as_ref().map(|_| {
-                panic!("loading external .so files is only supported on Linux")
+                panic!("calling functions from native libraries via FFI is only supported on Unix")
             }),
             gc_interval: config.gc_interval,
             since_gc: 0,
diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs
@@ -226,7 +226,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         let this = self.eval_context_mut();
 
         // First deal with any external C functions in linked .so file.
-        #[cfg(target_os = "linux")]
+        #[cfg(unix)]
         if this.machine.native_lib.as_ref().is_some() {
             use crate::shims::native_lib::EvalContextExt as _;
             // An Ok(false) here means that the function being called was not exported
diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs
@@ -2,7 +2,7 @@
 
 mod alloc;
 mod backtrace;
-#[cfg(target_os = "linux")]
+#[cfg(unix)]
 mod native_lib;
 mod unix;
 mod wasi;
diff --git a/src/tools/miri/tests/native-lib/fail/function_not_in_so.rs b/src/tools/miri/tests/native-lib/fail/function_not_in_so.rs
@@ -1,4 +1,6 @@
-//@only-target-linux
+// Only works on Unix targets
+//@ignore-target-windows
+//@ignore-target-wasm
 //@only-on-host
 //@normalize-stderr-test: "OS `.*`" -> "$$OS"
 
diff --git a/src/tools/miri/tests/native-lib/fail/private_function.rs b/src/tools/miri/tests/native-lib/fail/private_function.rs
@@ -0,0 +1,15 @@
+// Only works on Unix targets
+//@ignore-target-windows
+//@ignore-target-wasm
+//@only-on-host
+//@normalize-stderr-test: "OS `.*`" -> "$$OS"
+
+extern "C" {
+    fn not_exported();
+}
+
+fn main() {
+    unsafe {
+        not_exported(); //~ ERROR: unsupported operation: can't call foreign function `not_exported`
+    }
+}
diff --git a/src/tools/miri/tests/native-lib/fail/private_function.stderr b/src/tools/miri/tests/native-lib/fail/private_function.stderr
@@ -0,0 +1,15 @@
+error: unsupported operation: can't call foreign function `not_exported` on $OS
+  --> $DIR/private_function.rs:LL:CC
+   |
+LL |         not_exported();
+   |         ^^^^^^^^^^^^^^ can't call foreign function `not_exported` on $OS
+   |
+   = help: if this is a basic API commonly used on this target, please report an issue with Miri
+   = help: however, note that Miri does not aim to support every FFI function out there; for instance, we will not support APIs for things such as GUIs, scripting languages, or databases
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/private_function.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+
diff --git a/src/tools/miri/tests/native-lib/native-lib.map b/src/tools/miri/tests/native-lib/native-lib.map
diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs
@@ -1,4 +1,6 @@
-//@only-target-linux
+// Only works on Unix targets
+//@ignore-target-windows
+//@ignore-target-wasm
 //@only-on-host
 
 fn main() {
diff --git a/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs b/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs
@@ -1,4 +1,6 @@
-//@only-target-linux
+// Only works on Unix targets
+//@ignore-target-windows
+//@ignore-target-wasm
 //@only-on-host
 
 extern "C" {
diff --git a/src/tools/miri/tests/native-lib/ptr_read_access.c b/src/tools/miri/tests/native-lib/ptr_read_access.c
@@ -1,8 +1,11 @@
 #include <stdio.h>
 
+// See comments in build_native_lib()
+#define EXPORT __attribute__((visibility("default")))
+
 /* Test: test_pointer */
 
-void print_pointer(const int *ptr) {
+EXPORT void print_pointer(const int *ptr) {
   printf("printing pointer dereference from C: %d\n", *ptr);
 }
 
@@ -12,7 +15,7 @@ typedef struct Simple {
   int field;
 } Simple;
 
-int access_simple(const Simple *s_ptr) {
+EXPORT int access_simple(const Simple *s_ptr) {
   return s_ptr->field;
 }
 
@@ -24,7 +27,7 @@ typedef struct Nested {
 } Nested;
 
 // Returns the innermost/last value of a Nested pointer chain.
-int access_nested(const Nested *n_ptr) {
+EXPORT int access_nested(const Nested *n_ptr) {
   // Edge case: `n_ptr == NULL` (i.e. first Nested is None).
   if (!n_ptr) { return 0; }
 
@@ -42,6 +45,6 @@ typedef struct Static {
     struct Static *recurse;
 } Static;
 
-int access_static(const Static *s_ptr) {
+EXPORT int access_static(const Static *s_ptr) {
   return s_ptr->recurse->recurse->value;
 }
diff --git a/src/tools/miri/tests/native-lib/scalar_arguments.c b/src/tools/miri/tests/native-lib/scalar_arguments.c
@@ -1,27 +1,35 @@
 #include <stdio.h>
 
-int add_one_int(int x) {
+// See comments in build_native_lib()
+#define EXPORT __attribute__((visibility("default")))
+
+EXPORT int add_one_int(int x) {
   return 2 + x;
 }
 
-void printer() {
+EXPORT void printer(void) {
   printf("printing from C\n");
 }
 
 // function with many arguments, to test functionality when some args are stored
 // on the stack
-int test_stack_spill(int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l) {
+EXPORT int test_stack_spill(int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l) {
   return a+b+c+d+e+f+g+h+i+j+k+l;
 }
 
-unsigned int get_unsigned_int() {
+EXPORT unsigned int get_unsigned_int(void) {
   return -10;
 }
 
-short add_int16(short x) {
+EXPORT short add_int16(short x) {
   return x + 3;
 }
 
-long add_short_to_long(short x, long y) {
+EXPORT long add_short_to_long(short x, long y) {
   return x + y;
 }
+
+// To test that functions not marked with EXPORT cannot be called by Miri.
+int not_exported(void) {
+  return 0;
+}
diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs
@@ -36,20 +36,21 @@ fn build_native_lib() -> PathBuf {
     // Create the directory if it does not already exist.
     std::fs::create_dir_all(&so_target_dir)
         .expect("Failed to create directory for shared object file");
-    let so_file_path = so_target_dir.join("native-lib.so");
+    // We use a platform-neutral file extension to avoid having to hard-code alternatives.
+    let native_lib_path = so_target_dir.join("native-lib.module");
     let cc_output = Command::new(cc)
         .args([
             "-shared",
+            "-fPIC",
+            // We hide all symbols by default and export just the ones we need
+            // This is to future-proof against a more complex shared object which eg defines its own malloc
+            // (but we wouldn't want miri to call that, as it would if it was exported).
+            "-fvisibility=hidden",
             "-o",
-            so_file_path.to_str().unwrap(),
+            native_lib_path.to_str().unwrap(),
             // FIXME: Automate gathering of all relevant C source files in the directory.
             "tests/native-lib/scalar_arguments.c",
             "tests/native-lib/ptr_read_access.c",
-            // Only add the functions specified in libcode.version to the shared object file.
-            // This is to avoid automatically adding `malloc`, etc.
-            // Source: https://anadoxin.org/blog/control-over-symbol-exports-in-gcc.html/
-            "-fPIC",
-            "-Wl,--version-script=tests/native-lib/native-lib.map",
             // Ensure we notice serious problems in the C code.
             "-Wall",
             "-Wextra",
@@ -64,7 +65,7 @@ fn build_native_lib() -> PathBuf {
             String::from_utf8_lossy(&cc_output.stderr),
         );
     }
-    so_file_path
+    native_lib_path
 }
 
 /// Does *not* set any args or env vars, since it is shared between the test runner and
@@ -300,7 +301,7 @@ fn main() -> Result<()> {
         WithDependencies,
         tmpdir.path(),
     )?;
-    if cfg!(target_os = "linux") {
+    if cfg!(unix) {
         ui(Mode::Pass, "tests/native-lib/pass", &target, WithoutDependencies, tmpdir.path())?;
         ui(
             Mode::Fail { require_patterns: true, rustfix: RustfixMode::Disabled },
