Resolve preadv/pwritev as weak symbols on Android - #5212
Conversation
|
Thanks! Are you an Android expert? Maybe you can help with my question in rust-lang/rust#159346 (which would make the |
| let result = this.symlink(target, linkpath)?; | ||
| this.write_scalar(result, dest)?; | ||
| } | ||
| "link" => { |
There was a problem hiding this comment.
IIRC this behaves different on different Unixes regarding how it deals with symlinks. On Linux it behaves like fs::hard_link but on other Unixes it may not. So please make this shim only available for "Linux-like" targets (Linux, Android), and add a comment also giving a citation for why this is the right symlink behavior.
That'll also fix the Solaris-related CI failure.
| matches!(name, "gettid") | ||
| // `preadv`/`pwritev` are accessed via `weak!` linkage by std since bionic | ||
| // only gained them in API level 24. | ||
| matches!(name, "gettid" | "preadv" | "pwritev") |
There was a problem hiding this comment.
Please move the "preadv" | "pwritev" cases into the is_dyn_sym function in the file that defines these symbols.
|
Reminder, once the PR becomes ready for a review, use |
rustbot
added
the
S-waiting-on-author
|
Given the discussion in rust-lang/rust#159346, please remove the |
Joel-Wwalker
changed the title
link shim and Android weak preadv/pwritev
|
Done: removed the @rustbot ready |
rustbot
added
S-waiting-on-review
| // `preadv`/`pwritev` are set up as weak symbols in `init_extern_statics` (on Android, | ||
| // where std uses them via `weak!` since bionic only gained them in API level 24), so | ||
| // we allow them here too. | ||
| "preadv" | "pwritev" => true, |
There was a problem hiding this comment.
This should exclude solaris where the functions do not exist
| // std uses `libc::link` on Android, which Miri does not implement; | ||
| // rust-lang/rust#159346 switches std to `linkat`, and then this cfg can go. |
There was a problem hiding this comment.
This comment will conflict with my PR where I am removing the cfg. ;)
So I think it's better to just not touch the hard_link parts here and avoid conflicts.
rustbot
added
S-waiting-on-author
|
Both done: excluded Solaris from the |
|
This looks great, thanks! Please squash the commits. You can squash manually if there are multiple independent commits you want to preserve, or use @rustbot author |
std on Android does vectored I/O at offsets through weak preadv/pwritev symbols with a raw syscall fallback (bionic gained them in API level 24). The weak symbols did not resolve under Miri, so these calls failed. Register them for Android so they land in the existing shims, and un-skip the Android case in the fs tests.
8c2ace1 to
382645c
Compare
|
Squashed. @rustbot ready |
rustbot
added
S-waiting-on-review
3 participants
std on Android does vectored reads and writes at offsets through weak
preadv/pwritevsymbols with a raw syscall fallback (bionic gained them in API level 24). The weak symbols did not resolve under Miri, so these calls failed. Register them for Android so they land in the existing shims, and un-skip the Android case in the fs tests.This is the vectored I/O half of #5080. The
fs::hard_linkhalf is being handled by rust-lang/rust#159346, which switches std tolinkaton Android; Miri already supportslinkat.