native-lib/trace: Intercept allocating calls, but do nothing by nia-e · Pull Request #4792 · rust-lang/miri

nia-e · 2025-12-27T17:02:02Z

This is part 1 of #4791; it adds the bits needed to intercept allocating/deallocating calls to libc, but silently ignores them for now. In a follow-up PR this will forward them to shims we control.

rustbot · 2025-12-27T17:02:05Z

Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two.
Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it.

rustbot · 2026-01-02T18:22:34Z

☔ The latest upstream changes (possibly 4b59d7e) made this pull request unmergeable. Please resolve the merge conflicts.

RalfJung

Thanks for the PR! I can already see I have to brace myself for some more abstraction-breaking shenanigans.^^ :)

Here's a first round of comments. As you said there's also conflicts to be resolved and CI failures to be looked into.

@rustbot author

View changes since this review

RalfJung · 2026-01-22T20:38:41Z

 nix = { version = "0.30.1", features = ["mman", "ptrace", "signal"], optional = true }
 ipc-channel = { version = "0.20.0", optional = true }
 capstone = { version = "0.13", optional = true }
+proc-maps = { version = "0.4.0", optional = true }


That's a huge pile of new dependencies. :/ and some duplicated ones, like libloading.

How long does this build script take? We just managed to finally get rid of the capstone overhead for check builds, I don't want to add back any new slow build scripts.

Is it? It seems like the only dependency on linux is libc: https://github.com/rbspy/proc-maps/blob/master/Cargo.toml

The lockfile has a load of new stuff in it -- that has to come from somewhere?

The lockfile has a load of new stuff in it -- that has to come from somewhere?

I believe the additional lockfile stuff comes from this windows dependency. While it's not compiled or downloaded in non-windows hosts, the lockfile tracks all the different targets I believe.

Since this is a Linux-only feature if I understand correctly, the procfs crate without any default features would work too. It pulls some other dependencies and it's slightly larger but if in the future miri needs other linux only data it could be useful

(nix:nix-shell-env) $ cargo tree proc-maps v0.1.0 (/home/fhonduvillac/src/miri-experiments/proc-maps) └── proc-maps v0.5.0 └── libc v0.2.189 (nix:nix-shell-env) $ ls -lah target/release/proc-maps -rwxrwxr-x 2 fhonduvillac fhonduvillac 470K Jul 28 09:42 target/release/proc-maps

(nix:nix-shell-env) $ cargo tree procfs v0.1.0 (/home/fhonduvillac/src/miri-experiments/procfs) └── procfs v0.18.0 ├── bitflags v2.13.1 ├── procfs-core v0.18.0 │ ├── bitflags v2.13.1 │ └── hex v0.4.3 └── rustix v1.1.4 ├── bitflags v2.13.1 └── linux-raw-sys v0.12.1 (nix:nix-shell-env) $ ls -lah target/release/procfs -rwxrwxr-x 2 fhonduvillac fhonduvillac 519K Jul 28 09:42 target/release/procfs

RalfJung · 2026-01-22T20:39:50Z

-        let alloc = ();

-        trace::Supervisor::do_ffi(alloc, || {
+        let ty_is_sized = dest.layout.ty.is_sized(*this.tcx, this.typing_env());


We do not support unsized return types for FFI calls.^^ So what is this about?

EDIT: Ah, it is factoring out the RawPtr check below -- but this is incorrect, it checks sizedness of the wrong thing. Anyway this logic looks quite different on master so hopefully a rebase will make this all go away.

RalfJung · 2026-01-22T20:40:13Z

                    return interp_ok(ImmTy::uninit(dest.layout));
                }
-                ty::RawPtr(ty, ..) if ty.is_sized(*this.tcx, this.typing_env()) => {
+                ty::RawPtr(ty, ..) if ty_is_sized => {


This changes what is being checked. Previously it checked whether the pointee is sized, now you are checking of the pointer is sized (which obviously it always is).

RalfJung · 2026-01-22T20:40:42Z


        if tracing {
-            this.tracing_apply_accesses(maybe_memevents.unwrap())?;
+            let mm = maybe_memevents.unwrap();


What's mm for? After unwrapping they are not "maybe" any more.

RalfJung · 2026-01-22T20:42:27Z


+        // Save the machine pointer to a location where the libc interceptors can use it,
+        // since we can't pass in arguments.
+        super::parent::MACHINE_PTR.store(machine_ptr.cast(), std::sync::atomic::Ordering::Relaxed);


This seems very similar to native_lib_ecx_interchange which has been added by a concurrent PR -- we don't need both of those, do we?

RalfJung · 2026-01-22T20:53:29Z

+            // TODO: This should probably only log writes & not reads, since reads
+            // in these functions will never expose provenance to the rest of the native
+            // code. However, these functions likely won't even do any reads, so...
+            wait_for_signal(Some(pid), signal::SIGTRAP, InitialCont::Yes, Some(catch_segfaults))
+                .unwrap();


What are we even doing here? Who's logging stuff? What does that have to do with exposing anything? I am confused.

The commit message says "Handle accesses during libc fn calls also". Is this something we have to do for some reason, or something you thought would be better to do but it's not mandatory?

RalfJung · 2026-01-22T20:53:50Z

+            wait_for_signal(Some(pid), signal::SIGTRAP, InitialCont::Yes, Some(catch_segfaults))
+                .unwrap();
+
+            // Unset the breakpoint stuff and move the ip back an instruction to compensate.


As in, undo the ret_addr editing we did above? Right?

RalfJung · 2026-01-22T20:55:54Z

+        ptrace::write(
+            pid,
+            libc::malloc as *mut _,
+            (libc::malloc as *mut libc::c_long).read_volatile(),
+        )
+        .unwrap();


Lol you're just copying the bytes from the parent (where we didn't change them) back to the child? 😭 😂

RalfJung · 2026-01-22T20:57:12Z


+/// Set up SIGTRAPs on the first few bytes of malloc/free/etc.
+#[expect(clippy::as_conversions)]
+fn trap_libc(pid: unistd::Pid) {


I think it would be good to consistently use debugger terminology here: we are setting and unsetting breakpoints.

RalfJung · 2026-01-22T20:57:59Z

+            // Override the return address to give us another sigtrap
+            // (but save the original bytes).


This is another case of setting (and later unsetting) a breakpoint, right?

javierhonduco

Thank you so much for working on this, I am very interested in this PR! Left some drive-by comments, hope that's okay but feel free to ignore them.

I maintain a bunch of Rust libraries with a significant amount of ffi code, so very excited to have better support for native code in the future 🎉

View changes since this review

javierhonduco · 2026-07-28T13:50:11Z

 nix = { version = "0.30.1", features = ["mman", "ptrace", "signal"], optional = true }
 ipc-channel = { version = "0.20.0", optional = true }
 capstone = { version = "0.13", optional = true }
+proc-maps = { version = "0.4.0", optional = true }


The lockfile has a load of new stuff in it -- that has to come from somewhere?

I believe the additional lockfile stuff comes from this windows dependency. While it's not compiled or downloaded in non-windows hosts, the lockfile tracks all the different targets I believe.

Since this is a Linux-only feature if I understand correctly, the procfs crate without any default features would work too. It pulls some other dependencies and it's slightly larger but if in the future miri needs other linux only data it could be useful

(nix:nix-shell-env) $ cargo tree proc-maps v0.1.0 (/home/fhonduvillac/src/miri-experiments/proc-maps) └── proc-maps v0.5.0 └── libc v0.2.189 (nix:nix-shell-env) $ ls -lah target/release/proc-maps -rwxrwxr-x 2 fhonduvillac fhonduvillac 470K Jul 28 09:42 target/release/proc-maps

(nix:nix-shell-env) $ cargo tree procfs v0.1.0 (/home/fhonduvillac/src/miri-experiments/procfs) └── procfs v0.18.0 ├── bitflags v2.13.1 ├── procfs-core v0.18.0 │ ├── bitflags v2.13.1 │ └── hex v0.4.3 └── rustix v1.1.4 ├── bitflags v2.13.1 └── linux-raw-sys v0.12.1 (nix:nix-shell-env) $ ls -lah target/release/procfs -rwxrwxr-x 2 fhonduvillac fhonduvillac 519K Jul 28 09:42 target/release/procfs

javierhonduco · 2026-07-28T13:50:57Z

+        // since we can't pass in arguments.
+        super::parent::MACHINE_PTR.store(machine_ptr.cast(), std::sync::atomic::Ordering::Relaxed);
+        // Give the libc interceptors the event channel.
+        let mut e_rx = super::parent::EVT_RX.lock().unwrap();


nit: Is there a reason why this error is not being handled? Just curious!

native-lib/trace: Intercept allocating calls, but do nothing

066aa78

rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Dec 27, 2025

native-lib/trace: Handle accesses during libc fn calls also

966775e

nia-e force-pushed the alloc-tracing-libc-part-1 branch from 428bee0 to 966775e Compare December 28, 2025 13:02

RalfJung reviewed Jan 22, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Jan 22, 2026

javierhonduco reviewed Jul 28, 2026

View reviewed changes

		// Override the return address to give us another sigtrap
		// (but save the original bytes).

Uh oh!

Conversation

nia-e commented Dec 27, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 27, 2025

Uh oh!

rustbot commented Jan 2, 2026

Uh oh!

RalfJung left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javierhonduco left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nia-e commented Dec 27, 2025 •

edited by rustbot

Loading

RalfJung left a comment •

edited

Loading

RalfJung Jan 22, 2026 •

edited

Loading

javierhonduco left a comment •

edited by rustbot

Loading