Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gnu_get_libc_[release|version] shims. #2494

Closed
wants to merge 1 commit into from

Conversation

LegNeato
Copy link
Contributor

No description provided.

@LegNeato
Copy link
Contributor Author

Note that even on linux these functions may not exist, as alternative libcs like musl don't implement them. Do we have a way for shims to deal with this case?

@@ -54,9 +54,9 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa"

[[package]]
name = "backtrace"
version = "0.3.65"
version = "0.3.66"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please not updated unrelated packages?

Why does libc need updating, anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this was just cargo update. rust-lang/libc#2862 was only included recently and I thought we'd need it for a passthrough.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No that won't work. libc calls the external fn, so if Miri implements the external fn by calling libc, at best we'll get an infinite loop.

Copy link
Member

@RalfJung RalfJung Aug 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this lock file is only relevant for things Miri needs to call on the host, and we certainly don't want to call the host libc for these functions.

// Miscellaneous
"gnu_get_libc_release" => {
let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let v = this.eval_libc("gnu_get_libc_release")?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, how does this make any sense? This will return a function pointer...

// [The GNU manual notes](https://www.gnu.org/software/gnulib/manual/html_node/Glibc-gnu_002flibc_002dversion_002eh.html):
// > This function is missing on some platforms: macOS 11.1, FreeBSD 13.0, NetBSD 9.0,
// > OpenBSD 6.7, Minix 3.1.8, AIX 5.1, HP-UX 11, IRIX 6.5, Solaris 11.4, Cygwin 2.9, mingw, MSVC 14, Android 9.0.
#[cfg(target_os = "linux")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strictly redundant, since you already have only-target-linux.

@RalfJung
Copy link
Member

Note that even on linux these functions may not exist, as alternative libcs like musl don't implement them. Do we have a way for shims to deal with this case?

Then why should Miri even implement them?

use std::ffi::CStr;
// Release name.
let x = unsafe { libc::gnu_get_libc_release() };
assert!(!x.is_null());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you actually try to print this as a c string, I think you'll notice that the return value is nonsense.

@LegNeato
Copy link
Contributor Author

Going to think about this some more

@LegNeato LegNeato closed this Aug 19, 2022
@saethlin
Copy link
Member

(I was on vacation when you posted this and just remembered to comment now)

FYI, std::process::Command::status calls gnu_get_libc_version and I'm pretty sure that is why it ranks relatively high up on the shim wishlist. So while I agree this looks like an easy shim to introduce, I think that on its own adding it doesn't have a meaningful impact on what programs Miri can run. If you're looking for low-hanging fruit, this isn't it. If you'd like guidance towards some, don't hesitate to ask in the Zulip :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants