Freebsd-target-support #2221

b-ncMN · 2022-06-09T14:06:46Z

Implement freebsd as a target for miri

diff

src/shims/unix/freebsd/foreign_items.rs

src/shims/unix/freebsd/mod.rs

src/shims/unix/macos/dlsym.rs

tests/pass/libc.rs

src/shims/env.rs

RalfJung · 2022-06-10T16:22:02Z

Both PRs need that function - so landing it first is the best way to avoid conflicts later. (This was in response to #2221 (comment) but Github reply-by-email is not that great.)

RalfJung · 2022-06-10T19:42:51Z

Thanks for the PR! Looks like our target support can grow quite a bit in the near future. :)

I think for this to work we need to make sure that these targets can actually remain supported as the Rust standard library evolves. So if/when we add freebsd to CI and the supported target in the README, then we should also list there who is actually going to maintain that target -- so that we have someone to ping when there is trouble with that target (the Rust standard library will inevitably change and that might require adjustments in Miri so that the target remains supported). We shouldn't have support for targets that nobody is going to maintain. And I don't want to be on the hook for maintaining them all. :)

So @InfRandomness do you want to be listed as maintaining freebsd support in Miri?

b-ncMN · 2022-06-10T20:45:13Z

Thanks for the PR! Looks like our target support can grow quite a bit in the near future. :)

I think for this to work we need to make sure that these targets can actually remain supported as the Rust standard library evolves. So if/when we add freebsd to CI and the supported target in the README, then we should also list there who is actually going to maintain that target -- so that we have someone to ping when there is trouble with that target (the Rust standard library will inevitably change and that might require adjustments in Miri so that the target remains supported). We shouldn't have support for targets that nobody is going to maintain. And I don't want to be on the hook for maintaining them all. :)

So @InfRandomness do you want to be listed as maintaining freebsd support in Miri?

Sure !

bors · 2022-06-12T00:32:59Z

☔ The latest upstream changes (presumably #2226) made this pull request unmergeable. Please resolve the merge conflicts.

b-ncMN · 2022-06-13T20:37:58Z

tests/pass/hello.rs seems to pass successfully but the other tests don't seem like they do

dtolnay · 2022-06-21T05:17:16Z

Thank you InfRandomness, this PR was already super helpful to me in rust-lang/libc#2813 (review) for validating a FreeBSD codepath in the libc crate.

src/shims/unix/freebsd/foreign_items.rs

RalfJung · 2022-06-26T20:37:20Z

tests/pass/libc.rs

+    unsafe {
+        assert_eq!(libc::getpid(), std::process::id());
+    }
+}


This function is not even called; seems to be some rebase mistake?
Please just remove it. :)

RalfJung · 2022-06-26T20:38:18Z

This looks good overall! I just left some minor comments.
Please pull before pushing again; I have rebased your branch and adjusted CI to only run some minimal tests for FreeBSD. That should make CI pass, hopefully. :)

- Merge pthread_attr_getstack shim to unix/foreign_items.rs

src/shims/unix/foreign_items.rs

RalfJung · 2022-06-27T17:40:10Z

Thanks. :)
@bors r+

I'm open tom follow-on PRs to get this to be a supported platform that passes all tests. :D

bors · 2022-06-27T17:40:12Z

📌 Commit 5719897 has been approved by RalfJung

bors · 2022-06-27T17:40:17Z

⌛ Testing commit 5719897 with merge f5593de...

bors · 2022-06-27T18:03:30Z

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing f5593de to master...

b-ncMN force-pushed the freebsd-target-support branch from fa966c5 to 5f1ef76 Compare June 9, 2022 14:09