linux_android_with_fallback: do not use dlsym on MUSL targets#602

newpavlov · 2025-02-05T11:45:16Z

Fixes #600

tamird · 2025-02-05T12:01:34Z

src/backends/linux_android_with_fallback.rs

+    #[cfg(target_env = "musl")]
+    let raw_ptr = {
+        let fptr: GetRandomFn = libc::getrandom;
+        unsafe { transmute::<GetRandomFn, *mut c_void>(fptr) }


This cast to a pointer and back smells like an indirect call.

There is effectively no overhead to this approach. "Direct" call to libc::getradom compiles down to:

xor edx, edx jmp qword ptr [rip + getrandom@GOTPCREL]

While the "indirect" call compiles to:

mov rax, qword ptr [rip + GETRANDOM_FN] xor edx, edx jmp rax

Yes, the former is a bit friendlier to CPUs, but compared to the syscall cost the difference is negligible. If you care about this difference, then you should use the linux_getrandom opt-in backend.

src/backends/linux_android_with_fallback.rs

tamird · 2025-02-05T16:51:00Z

@newpavlov just force push to remove the two spurious commits - you're going to clutter the git history this way =/

newpavlov · 2025-02-05T16:54:15Z

We always squash commits before merge, so local commit history in PR does not matter much.

tamird · 2025-02-05T16:57:15Z

OK. If this were my project I'd ask you to back out all the unnecessary import churn and random comment changes - but it's not!

tamird

@newpavlov can we please merge this one way or the other to fix the breakage and cut 0.3.2? Thank you.

tamird · 2025-02-10T14:47:29Z

src/backends/linux_android_with_fallback.rs

 const NOT_AVAILABLE: NonNull<c_void> = unsafe { NonNull::new_unchecked(usize::MAX as *mut c_void) };

-static GETRANDOM_FN: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());
+static GETRANDOM_FN: AtomicPtr<c_void> = AtomicPtr::new(core::ptr::null_mut());


could we avoid this noise in the diff? doesn't seem related to the fix being made.

tamird · 2025-02-10T14:48:14Z

src/backends/linux_android_with_fallback.rs

        Some(fptr) => {
-            let getrandom_fn = unsafe { mem::transmute::<NonNull<c_void>, GetRandomFn>(fptr) };
-            let dangling_ptr = ptr::NonNull::dangling().as_ptr();
+            let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) };


the diff on these 2 lines is also not necessary.

tamird · 2025-02-10T14:48:21Z

src/backends/linux_android_with_fallback.rs

 }

-// prevent inlining of the fallback implementation
+// Prevent inlining of the fallback implementation


not necessary

tamird · 2025-02-10T14:48:24Z

src/backends/linux_android_with_fallback.rs

    } else {
        // note: `transmute` is currently the only way to convert a pointer into a function reference
-        let getrandom_fn = unsafe { mem::transmute::<NonNull<c_void>, GetRandomFn>(fptr) };
+        let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) };


not necessary

newpavlov · 2025-02-12T15:28:24Z

I think it's fine to keep minor code tweaks in this PR.

Before releasing v0.3.2 I would like to also merge the linux_raw and efi_rng PRs.

### Added - `efi_rng` opt-in backend [#570] - `linux_raw` opt-in backend [#572] - `.cargo/config.toml` example in the crate-level docs [#591] - `getrandom_test_linux_without_fallback` configuration flag to test that file fallback is not triggered in the `linux_android_with_fallback` backend [#605] - Built-in support for `*-linux-none` targets [#618] - Cygwin support [#626] ### Changed - Update `wasi` dependency to v0.14 [#594] - Add `#[inline]` attribute to the inner functions [#596] - Update WASI and Emscripten links in the crate-level docs [#597] - Do not use `dlsym` on MUSL targets in the `linux_android_with_fallback` backend [#602] - Remove `linux_android.rs` and use `getrandom.rs` instead [#603] - Always use `RtlGenRandom` on Windows targets when compiling with pre-1.78 Rust [#610] - Internal representation of the `Error` type [#614] - Remove `windows-targets` dependency and use [`raw-dylib`] directly [#627] ### Removed - `Error::INTERNAL_START` and `Error::CUSTOM_START` associated constants [#614] [#570]: #570 [#572]: #572 [#591]: #591 [#594]: #594 [#596]: #596 [#597]: #597 [#602]: #602 [#603]: #603 [#605]: #605 [#610]: #610 [#614]: #614 [#618]: #618 [#626]: #626 [#627]: #627 [`raw-dylib`]: https://doc.rust-lang.org/reference/items/external-blocks.html?highlight=link#dylib-versus-raw-dylib

sbwml · 2025-03-20T14:40:57Z

好尴尬 🙃，当我为 mips musl 编译它时，遇到了一些奇奇怪怪的错误：

https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13956208010/job/39067818527#step:5:404

error[E0425]: cannot find value `getrandom` in crate `libc`
  --> /home/runner/work/shadowsocks-rust-mips/shadowsocks-rust-mips/install/registry/src/index.crates.io-1949cf8c6b5b557f/getrandom-0.3.2/src/backends/linux_android_with_fallback.rs:34:39
   |
34 |         let fptr: GetRandomFn = libc::getrandom;
   |                                       ^^^^^^^^^ not found in `libc`

For more information about this error, try `rustc --explain E0425`.

给它定义一个 getrandom 函数它才可以编译 https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13971234091/job/39113320608#step:5:419

newpavlov · 2025-03-20T15:16:27Z

@sbwml
Please use English here, even if it's somewhat broken.

Rust officially uses musl v1.2.3 for the mips-unknown-linux-musl target. Meanwhile, getrandom was added in musl v1.1.20. So it may be a problem with the libc crate, which for some reason does not expose the symbol on this target. It may be worth to open an issue in its repository.

For now you could downgrade getrandom to an older version as a temporary workaround.

tamird · 2025-03-20T15:21:17Z

Indeed, fn getrandom is missing on mips-unknown-linux-musl even though SYS_getrandom is defined: https://github.com/rust-lang/libc/blob/4159080fc4dcec769703d79994ab725a4e6d26b4/src/unix/linux_like/linux/musl/b32/mips/mod.rs#L727

Looks like that was somewhat intentional: rust-lang/libc@457c02d

newpavlov · 2025-03-20T15:34:01Z

Looks like that was somewhat intentional

Either way, it's worth to clarify it with libc developers. The reason for that may not be longer applicable and, at the very least, it should be fine to add it for the MUSL target.

tamird · 2025-03-20T15:39:59Z

The discussion in that PR (rust-lang/libc#1399) suggests that it was failing CI at the time. The musl version was bumped in rust-lang/rust#107129, though. I'll send a patch and see what happens.

tamird · 2025-03-20T15:56:04Z

rust-lang/libc#4346

eareimu · 2025-03-21T02:25:41Z

好尴尬 🙃，当我为 mips musl 编译它时，遇到了一些奇奇怪怪的错误：

https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13956208010/job/39067818527#step:5:404
error[E0425]: cannot find value `getrandom` in crate `libc`
  --> /home/runner/work/shadowsocks-rust-mips/shadowsocks-rust-mips/install/registry/src/index.crates.io-1949cf8c6b5b557f/getrandom-0.3.2/src/backends/linux_android_with_fallback.rs:34:39
   |
34 |         let fptr: GetRandomFn = libc::getrandom;
   |                                       ^^^^^^^^^ not found in `libc`

For more information about this error, try `rustc --explain E0425`.
给它定义一个 getrandom 函数它才可以编译 https://github.com/sbwml/shadowsocks-rust-mips/actions/runs/13971234091/job/39113320608#step:5:419

Same issue, the problem was temporarily solved by specifying the getramdom version in Cargo.toml:

getrandom = "=0.3.1"

Experienced supply poisoning :(

josephlr · 2025-03-22T06:42:54Z

The fix has been merged in rust-lang/libc#4346, once it's released on libc's stable 0.2 branch, the issue should be resolved.

…andom#602)

### Added - `efi_rng` opt-in backend [rust-random#570] - `linux_raw` opt-in backend [rust-random#572] - `.cargo/config.toml` example in the crate-level docs [rust-random#591] - `getrandom_test_linux_without_fallback` configuration flag to test that file fallback is not triggered in the `linux_android_with_fallback` backend [rust-random#605] - Built-in support for `*-linux-none` targets [rust-random#618] - Cygwin support [rust-random#626] ### Changed - Update `wasi` dependency to v0.14 [rust-random#594] - Add `#[inline]` attribute to the inner functions [rust-random#596] - Update WASI and Emscripten links in the crate-level docs [rust-random#597] - Do not use `dlsym` on MUSL targets in the `linux_android_with_fallback` backend [rust-random#602] - Remove `linux_android.rs` and use `getrandom.rs` instead [rust-random#603] - Always use `RtlGenRandom` on Windows targets when compiling with pre-1.78 Rust [rust-random#610] - Internal representation of the `Error` type [rust-random#614] - Remove `windows-targets` dependency and use [`raw-dylib`] directly [rust-random#627] ### Removed - `Error::INTERNAL_START` and `Error::CUSTOM_START` associated constants [rust-random#614] [rust-random#570]: rust-random#570 [rust-random#572]: rust-random#572 [rust-random#591]: rust-random#591 [rust-random#594]: rust-random#594 [rust-random#596]: rust-random#596 [rust-random#597]: rust-random#597 [rust-random#602]: rust-random#602 [rust-random#603]: rust-random#603 [rust-random#605]: rust-random#605 [rust-random#610]: rust-random#610 [rust-random#614]: rust-random#614 [rust-random#618]: rust-random#618 [rust-random#626]: rust-random#626 [rust-random#627]: rust-random#627 [`raw-dylib`]: https://doc.rust-lang.org/reference/items/external-blocks.html?highlight=link#dylib-versus-raw-dylib

linux_android_with_fallback: do not use dlsym on MUSL targets

5c46936

newpavlov mentioned this pull request Feb 5, 2025

linux_android_with_fallback: avoid dlsym #598

Closed

newpavlov requested a review from josephlr February 5, 2025 11:59

tamird reviewed Feb 5, 2025

View reviewed changes

Add #[cold] to use_file_fallback

60f2393

tamird reviewed Feb 5, 2025

View reviewed changes

src/backends/linux_android_with_fallback.rs Outdated Show resolved Hide resolved

Revert addition of #[cold]

3f2d0e5

tamird reviewed Feb 10, 2025

View reviewed changes

newpavlov merged commit b75db5c into master Feb 12, 2025
58 checks passed

newpavlov deleted the fix_musl branch February 12, 2025 15:28

newpavlov mentioned this pull request Mar 7, 2025

Release v0.3.2 #617

Merged

newpavlov mentioned this pull request Mar 21, 2025

error[E0425]: cannot find value getrandom in crate libc on target mipsel-unknown-linux-musl v0.3.2 #636

Closed

takumi-earth pushed a commit to earthlings-dev/getrandom that referenced this pull request Jan 27, 2026

linux_android_with_fallback: do not use dlsym on MUSL targets (rust-r…

4c2e770

…andom#602)

Conversation

newpavlov commented Feb 5, 2025

Uh oh!

tamird Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

newpavlov Feb 5, 2025 • 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!

Uh oh!

tamird commented Feb 5, 2025

Uh oh!

newpavlov commented Feb 5, 2025

Uh oh!

tamird commented Feb 5, 2025

Uh oh!

tamird left a comment

Choose a reason for hiding this comment

Uh oh!

tamird Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

tamird Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

tamird Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

tamird Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

newpavlov commented Feb 12, 2025

Uh oh!

Uh oh!

sbwml commented Mar 20, 2025

Uh oh!

newpavlov commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tamird commented Mar 20, 2025

Uh oh!

newpavlov commented Mar 20, 2025

Uh oh!

tamird commented Mar 20, 2025

Uh oh!

tamird commented Mar 20, 2025

Uh oh!

eareimu commented Mar 21, 2025

Uh oh!

josephlr commented Mar 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

newpavlov Feb 5, 2025 •

edited

Loading

newpavlov commented Mar 20, 2025 •

edited

Loading