-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
iOS: open file on aarch64 breaks permissions #22862
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Is this only for AArch64? I haven't added anything to make Rust on Bitrig AArch64 work yet. |
It's actually depends on how varargs are handled in calling convention of the system on a specific architecture. In case of iOS it bite only on aarch64, but your mileage may vary. |
@vhbit first, thanks to share a potential problem ! Do you have a small test case we could try in order to help to check if we have the same problem ? From your description, it seems a |
In my case - it actually will be any value. Here are tests I've used (I had to call them from C code, use libc;
use std::mem;
#[no_mangle]
pub extern fn rw_file_raw(path: *const libc::c_char) -> libc::c_int {
extern {
fn open(path: *const libc::c_char, flags: libc::c_int, mode: libc::mode_t) -> libc::c_int;
}
unsafe {
let x = [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff];
let fd = open(path,
libc::O_RDWR | libc::O_TRUNC | libc::O_CREAT,
libc::S_IRUSR | libc::S_IWUSR);
if fd < 0 {
return fd;
}
let txt = "HelloWorld";
libc::write(fd, mem::transmute(txt.as_ptr()), txt.len() as libc::size_t);
libc::write(fd, mem::transmute(x.as_ptr()), x.len() as libc::size_t);
libc::close(fd);
}
return 0;
}
#[no_mangle]
pub extern fn rw_file_raw_vararg(path: *const libc::c_char) -> libc::c_int {
extern {
fn open(path: *const libc::c_char, flags: libc::c_int, ...) -> libc::c_int;
}
unsafe {
let x = [0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff];
let fd = open(path,
libc::O_RDWR | libc::O_TRUNC | libc::O_CREAT,
(libc::S_IRUSR | libc::S_IWUSR) as libc::c_uint);
if fd < 0 {
return fd;
}
let txt = "HelloWorld";
libc::write(fd, mem::transmute(txt.as_ptr()), txt.len() as libc::size_t);
libc::write(fd, mem::transmute(x.as_ptr()), x.len() as libc::size_t);
libc::close(fd);
}
return 0;
} |
I might actually prefer to lose the shims entirely and just expose the raw |
if it take the first value from stack, yes it should look like random... I have tested under openbsd, and the two functions produce files with @vhbit thanks again. |
Unfortunately just adding some casts shouldn't work for all platforms - vararg is promoted to What I don't like in exposing raw open function is that there are 4 places now:
Removing shims from What I suggest in this case - leave a shim to be in |
Couldn't all usage sites look like |
Disclaimer: on mobile, can't test now.
I think that's not possible - as I said earlier if vararg is smaller than
c_int (and mode_t sometimes is) it should be promoted to c_int and rustc
asks for that promotion to be stated explicitly (foo as c_int).
All mode constants have type mode_t so compiler already knows that 3rd arg
is mode_t. And it fails to compile on OS X without explicit promotion to
c_int. There might be some kind of internal compiler tricks which might
make 'as mode_t' work, but intuition says that shouldn't help.
That leaves us in situation when on some platforms we don't need any type
changes and on some we need explicit promotion to c_int. That's why
different kind of functions and therefore different kind of calling and
therefore a lot of #[cfg()] on caller's site too.
|
Ah right good point, I missed that part. In that case I think that this is fine by me, but I would request that these functions defined in Rust are also marked with |
@alexcrichton marked as |
@bors: r+ 349b1ca Thanks! |
⌛ Testing commit 349b1ca with merge 74ac507... |
💔 Test failed - auto-linux-64-nopt-t |
According to Apple arm64 calling convention varargs always are passed through stack. Since `open` is actually a vararg function on Darwin's, it means that older declaration caused permissions to be taken from stack, while passed through register => it set file permissions to garbage and it was simply impossible to read/delete files after they were created. They way this commit handles it is to preserve compatibility with existing code - it simply creates a shim unsafe function so all existing callers continue work as nothing happened.
Fixed unused import on Linux. |
⌛ Testing commit 3f4181a with merge f7469df... |
According to Apple's [arm64 calling convention](https://developer.apple.com/library/ios/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html#//apple_ref/doc/uid/TP40013702-SW1) varargs always are passed through stack. Since `open` is actually a vararg function on Darwin, it means that older declaration caused permissions to be taken from stack, while passed through register => it set file permissions to garbage and it was simply impossible to read/delete files after they were created. They way this commit handles it is to preserve compatibility with existing code - it simply creates a shim unsafe function so all existing callers continue work as nothing happened.
💔 Test failed - auto-linux-64-opt |
@bors: retry |
According to Apple's arm64 calling convention varargs always are passed
through stack. Since
open
is actually a vararg function on Darwin,it means that older declaration caused permissions to be taken from
stack, while passed through register => it set file permissions
to garbage and it was simply impossible to read/delete files after they
were created.
They way this commit handles it is to preserve compatibility with
existing code - it simply creates a shim unsafe function so all existing
callers continue work as nothing happened.