-
Notifications
You must be signed in to change notification settings - Fork 734
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
enable builds on target_env = uclibc by providing a getauxval implementation #1800
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,11 +25,62 @@ fn detect_features() -> u32 { | |
use libc::c_ulong; | ||
|
||
// XXX: The `libc` crate doesn't provide `libc::getauxval` consistently | ||
// across all Android/Linux targets, e.g. musl. | ||
// across all Android/Linux targets, e.g. uclibc. | ||
// If weak linking of extern symbols becomes available, this code could | ||
// choose to dynamically fall back to the procfs version below. | ||
#[cfg(not(target_env = "uclibc"))] | ||
extern "C" { | ||
fn getauxval(type_: c_ulong) -> c_ulong; | ||
} | ||
|
||
#[cfg(not(target_env = "uclibc"))] | ||
fn getauxval_safe(type_: c_ulong) -> c_ulong { | ||
// safety: getauxval seems to have no safety concerns | ||
unsafe { getauxval(type_) } | ||
} | ||
|
||
// Provide a compatible implementation for uclibc based on procfs. | ||
#[cfg(all(target_env = "uclibc"))] | ||
fn getauxval_safe(type_: c_ulong) -> c_ulong { | ||
let fd = unsafe { | ||
// safety: static string has interned NUL terminator | ||
libc::open(b"/proc/self/auxv\0".as_ptr(), libc::O_RDONLY) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could just use libstd here since the target_os is linux and libstd is supported by this target. Thus we could avoid the OTOH, we've traditionally gone to some lengths to avoid File I/O whenever practical in this library. Is there really no way to find auxv in memory for uClibc-ng? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I originally did a libstd implementation, but then I guessed that it would be less preferred than one which didn't add any dependencies. Would you prefer I submit my libstd implementation? glibc uses some tight integration with the kernel's ELF loader to find a spot in memory where the aux vector has been stored away. I could try to duplicate that here, but it felt much larger and more fragile, having looked at the recent addition of getauxval to uclibc here. I get the desire to avoid file IO. Is it any consolation that no actual I/O device will be involved? procfs just talks with the kernel, which just printfs into the user space buffer, so it's very fast. I'm very happy to move this in whatever direction you prefer! |
||
}; | ||
if fd == -1 { | ||
return 0; | ||
} | ||
const AUXV_LEN: usize = core::mem::size_of::<c_ulong>(); | ||
|
||
let val = 'done: loop { | ||
let mut keyval = [0u8; AUXV_LEN * 2]; | ||
let mut read = 0; | ||
let total = core::mem::size_of_val(&keyval); | ||
while read < total { | ||
let block = &mut keyval[read..]; | ||
let cnt = unsafe { | ||
// safety: memory block length is managed by rust. | ||
libc::read(fd, block.as_mut_ptr() as *mut libc::c_void, block.len()) | ||
}; | ||
if cnt <= 0 { | ||
break 'done 0; | ||
} | ||
read += cnt as usize; | ||
} | ||
let key = c_ulong::from_ne_bytes(keyval[0..AUXV_LEN].try_into().unwrap()); | ||
let val = c_ulong::from_ne_bytes(keyval[AUXV_LEN..].try_into().unwrap()); | ||
if key == type_ { | ||
break 'done val; | ||
} | ||
}; | ||
|
||
let _ = unsafe { | ||
// safety: fd was valid above, so it is still valid. | ||
libc::close(fd) | ||
}; | ||
|
||
val | ||
} | ||
|
||
const AT_HWCAP: c_ulong = 16; | ||
|
||
#[cfg(target_arch = "aarch64")] | ||
|
@@ -38,7 +89,7 @@ fn detect_features() -> u32 { | |
#[cfg(target_arch = "arm")] | ||
const HWCAP_NEON: c_ulong = 1 << 12; | ||
|
||
let caps = unsafe { getauxval(AT_HWCAP) }; | ||
let caps = getauxval_safe(AT_HWCAP); | ||
|
||
// We assume NEON is available on AARCH64 because it is a required | ||
// feature. | ||
|
@@ -61,7 +112,7 @@ fn detect_features() -> u32 { | |
#[cfg(target_arch = "arm")] | ||
let caps = { | ||
const AT_HWCAP2: c_ulong = 26; | ||
unsafe { getauxval(AT_HWCAP2) } | ||
getauxval_safe(AT_HWCAP2) | ||
}; | ||
|
||
const HWCAP_AES: c_ulong = 1 << 0 + OFFSET; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If uclibc supports this when dynamically linking, then the conditional logic should include
not(all(target_env = "uclibc", target_feature = "crt-static"))
. Since this is specific to Linux and not Android (why would we support uclibc on Android?), AFAICT, then we'd havenot(all(target_env = "uclibc", target_os = "linux", target_feature = "crt-static"))
, or similar?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant "weak linking" in the sense of defining a symbol which would be non-null when available via the dynamic linker, or null if it were not available at dylib linking time. This is a feature which Rust has considered adding from time to time, mainly for situations these. But Rust doesn't have it (yet) so it's sorta a moot point. I can certainly remove this comment if it's causing confusion.
Here is the possible future Rust "linkage" feature which would support this case: rust-lang/rust#29603
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the comment about weak linking isn't confusing. (Also we do have weak linking with
dlsym
, right?)In your other comment, you said we can't rely on uClibc having getauxval even when dynamically linked, so my comment regarding crt-static is moot, probably. However, the part where we should restrict this to linux (not do it for android) is still relevant.