Comparative unsafety
flak.tedunangst.com
flak.tedunangst.com
What this means is that as soon as you add `unsafe` code which accepts a pointer argument, all the supposedly "safe" code which can affect this pointer argument becomes a potential source of memory errors.
To me, this was unintuitive — I expected the `unsafe` block to require an extra level of scrutiny, not the surrounding code. After making an error similar to the one described in this post, I wondered, "why isn't creating a dangling pointer unsafe?" But after following that train of thought I realized that vast amounts of code would be pulled into `unsafe` blocks as a consequence, so it's not a viable approach — and thus it became apparent why only dereferencing is unsafe.
The lesson seems to be that dereferencing requires extreme care, but I wish potential errors weren't so subtle. The `as_ptr` family of functions has been tripping people up for a long time:
https://users.rust-lang.org/t/cstring-as-ptr-is-incredibly-u...
slice = &slice[1..];
`1234 as *const u8` and `ptr.offset(n)` are also safe, because Rust allows invalid/dangling pointers to exist if they're never dereferenced.
Relevant rustc (merged) PR: https://github.com/rust-lang/rust/pull/75671 - "Uplift temporary-cstring-as-ptr lint from clippy into rustc"
Zooming out, there are innumerable ways to create a dangling pointer. This is really a vexing problem.
Keep in mind that even something like borrowing a RefCell creates a temporary, so once you cast to a pointer and end the lifetime it came from it's very hard for the type system to track back the pointer you got to any particular deallocated temporary in an intelligent way. It pretty much has to be done on a case by case basis, I suspect (but maybe that could be improved--it is definitely the case, from a study someone did recently, that a large percentage of UB in unsafe Rust is due to destructors running early unexpectedly!).
First thing I'd try would be to pass "~0" instead of "-1". Would that work? (Looks like in Rust that would be "!0". I don't know anything about Rust.)
-1_i32 as u32
Btw, you can directly express "all ones" in Rust as:
0b_1111_1111_1111_1111_1111_1111_1111_1111
EDIT: Added more ones, thanks adwn!
That's just 16 bits and will result in the value 65535, not -1.
Eh... as the article points out, the chown documentation specifically says that you are expected to provide negative numbers. The spec is obviously bugged.
This limits the amount of righteousness you can claim over "but we followed the spec (that we knew was wrong)!"
I can't be bothered to try it myself but I'm fairly sure the author could just have written (-1i32) as uid_t and gotten some of their life back.
As someone who is used to working in a language where one absolutely cannot hit memory safety issues or undefined behaviour, you can be damn sure that I'm going to read all the relevant documentation and double/triple the invariants of any unsafe code I write in Rust (and maybe even get it reviewed by someone else).
In some ways unsafe blocks in Rust are more unsafe than the equivalent C or C++ code because you have to uphold Rust's more stringent safety guarantees (e.g. no aliasing of mutable references), but they come with the saving grace that you don't need them very often, so you can afford to really take your time and implement them thoroughly.
On balance Rust does a great job, but "be even more careful in unsafe Rust than in regular C++" is probably a losing battle.
That's true. If you are used to 100% of your code being within the equivalent of a Rust "unsafe" block, having over 90% of your code being verified as safe by the compiler is a luxury. Even if one in ten lines are marked as "unsafe", that's already many times less "unsafe" than what they are used to in their C or C++ code.
Moreover, C and C++ developers are used to all kinds of bizarre memory-saving and cycle-counting code that require "unsafe" in Rust. Things like intrusive double-linked circular linked lists, stashing things in the lower bits of pointers (I mean, it's a pointer to a 32-bit value which will always be 32-bit aligned, the lowest two bits will always be zero, why not use them?), doing XOR on pointers, and plenty of other useful tricks.
As a Rust programmer who's written safe code and some unsafe abstractions, one in ten lines being unsafe means that the remaining 90% of your lines need to be written with the same care as your unsafe code, to avoid feeding invalid arguments into unsafe operations which cause UB. The general approach is to not scatter unsafe code around your entire program, but segregate it into safe abstractions that prevent callers from triggering UB. (Though some APIs make it difficult to achieve, so the alternative is to create unsafe abstractions that expect callers to take care to avoid triggering UB, which requires auditing the callers for security as well.)
On a related note, looking at the comments of a semi-"fluff" blog post commenting on an unsafe-Rust data race (https://news.ycombinator.com/item?id=26713037), I'm the first person across all of the Mozilla bug-tracker, Mozilla blog, and HN comments to point out that Firefox had "data races in safe code" because an unsafe block in a (module-private) "safe" function was unsound, allowing other safe code in the module to cause data races. (Maybe not the first, https://news.ycombinator.com/item?id=26714209 pointed out the 'bad pattern of "sprinkle atomics it until it works"' (though not researching it as deeply as I did) and got downvoted even though they were right in this instance.)
But I see the issues (e.g. which scope should `tmp` be bound to/what is its lifetime?)
> let path = CString::new(filename.as_str()).unwrap().as_ptr();
Is one of those functions calling free behind the scenes?
auto ptr = std::string(something).c_str();
std::string str { something };
auto ptr = str.c_str();
CString::new(filename.as_str()).unwrap()
Like C++, Rust relies heavily on RAII, in which resources are freed once they get out of scope. The "CString::new(filename.as_str()).unwrap()" returns a CString, which owns the memory for the C string. Since it's a temporary (it's not being stored anywhere), its scope ends before the next line, so the resources it owns are freed (by calling its Drop implementation) just after the ".as_ptr()" call.
One solution would be to split it into two lines:
let path = CString::new(filename.as_str()).unwrap();
let path = path.as_ptr();
With references, the borrow checker doesn't let you do it the wrong way; however, the borrow checker only applies to references, not to raw pointers (which is what .as_ptr() returns).
(I assume there’s a good reason why this isn’t the case, but I don’t know Rust well enough to know the answer. I’d be interested to learn, if anyone can explain it for me!)
Edit to clarify: what seems very strange to me is that a function that not only does something potentially unsafe, but returns an unsafe value would be callable outside of an “unsafe” block.
It points to invalid memory as soon as the underlying object is freed. It’s hard to think of a legitimate usage at that point. You’d hope that’s exactly the kind of thing the borrow checker would be able to warn you about.
One situation where you might conceivably want to do something with that dangling pointer is if you were writing an allocator. But in this particular example, we’re not writing an allocator; and if we were, we wouldn’t call .as_ptr() on one of the allocated objects — that would be working at the wrong level of abstraction.
If you really do just need to store the value and not dereference it, shouldn’t it be stored as some integer type? Then to dereference it, you’d first need to cast it to a pointer, and that operation could be marked unsafe.
I could check whether two objects were stored at the same location (like comparing id-s in Python). Or I could write a crappy random number generator based on the pointers that I get from heap allocations. Or I could inspect the memory layout of a struct based on member offsets (pointer::offset_from method - perfectly safe).
Pointers whose pointees are tracked by the compiler are called "references". If you fetch a pointer instead, then you opt-out of this tracking.
> One situation where you might conceivably want to do something with that dangling pointer is if you were writing an allocator. But in this particular example, we’re not writing an allocator; and if we were, we wouldn’t call .as_ptr() on one of the allocated objects — that would be working at the wrong level of abstraction.
I can see someone writing an allocator based on std::vec::Vec, and then ".as_ptr()" is useful.
> If you really do just need to store the value and not dereference it, shouldn’t it be stored as some integer type? Then to dereference it, you’d first need to cast it to a pointer, and that operation could be marked unsafe.
You can't dereference a pointer in Rust per se. You need to convert it to a reference and then use the reference. This conversion is marked unsafe in Rust. There are also some "read" methods defined on pointers - also marked unsafe. But in the example in the article the pointer was dereferenced by C code, so Rust can't do anything about it.
Now imagine how an ".as_ptr()" which prevents taking pointers to temporary objects would need to be implemented. Well, you'd need to be able to write a method in Rust, which couldn't be called on temporaries. So the type-system would need to be able to express something like
fn as_ptr(&'non-temporary self) -> *Self
let ptr;
{
let path = CString::new(filename.as_str()).unwrap();
ptr = path.as_ptr();
}
The solutions here are:
1. Borrow checker for objects with predictable lifetimes.
2. Smart pointers for objects with hard-to-predict lifetimes.
3. For calling C code, being careful, because it's C code and we can't fix C. Also, a linter with checks for common errors like those.
Because it is not bound to a variable, it will get dropped before the next line, even if we create a pointer to it. if you would have borrowed instead of creating a pointer this would have been a compiler error because the borrow would have outlived the original object. But a pointer exists outside of the borrow semantics and lifetimes, that is also why it can only be used inside the unsafe block.
I feel that tying deallocations to destructors is a benefit to safe code, and a hindrance to unsafe code. I feel explicit calls to "destructor" functions that move self (perhaps using defer), plus linear typing to statically ensure you don't forget to call a destructor or call one twice, can provide safety in both situations, but at the expense of verbosity and not interacting well with exceptions. I still believe linear typing (argued against by Gankra in https://gankra.github.io/blah/linear-rust/) will fix multiple deficiencies in Rust:
- It helps write allocation-free code which passes objects around without creating/destroying them (maybe an Alloc effect would help too).
- Making drops explicit makes it harder to screw up when writing unsafe code performing allocations/deallocations. It may or may not help protect against double-frees that occur upon panicking.
- File's Drop implementation ignores the return code of fclose(), since you can't return values from a destructor. So dropping a File directly should be prohibited (except during panic-unwinding, which I feel is a rare scenario where perfect error-checking is less crucial than not risking more panics). Instead you move the file into a close() method/function, and are expected to check the Result<(), error> that comes out.
- From what I've read, trying to perform an asynchronous cancellation operation in the standard Drop method is not possible, since the method lacks access to the executor (unless you hard-code an executor to block on). It would make sense to make explicit dropping illegal (as a lint) and require moving it into a "drop"-like function, but keep Drop around to be called during panic-unwinding. This may result in logically incorrect behavior, but is still memory-safe. And I think it's better than making your async-fn state machines always larger to fit awaiting-a-drop states, and awaiting the async runtime when you panic-unwind.
Yeah, and it bothers me that I can't call `close` and check for errors. All you need is something like this:
pub fn close(self) -> io::Result<()> {
let status = unsafe { libc::close(self.fd) };
mem::forget(self); // prevent drop and invalid double-close
match status {
0 => Ok(()),
_ => Err(io::Error::new(ErrorKind::Other)),
}
}
See discussion at https://github.com/rust-lang/rust/issues/59567
Bidirectional pointers is definitely something it does better than Rust. Constraint references are an interesting idea. Clasps remind me of some Qt objects automatically detaching other objects listening to them, when destroyed (eg. QSortFilterProxyModel's source model, referenced by QSortFilterProxyModel, referenced by QAbstractItemView), but it feels ad-hoc for every individual type, and I don't know if there's underlying principles that all Qt types follow.
The descriptions of the language and others feel a bit crank-ish at times, I got lost a web of hyperlinks to various pages on the site and didn't read all of them, and apparently the language is not ready yet. I might check it out again when it's more complete.
Very poetic. Are you Russian?
This happens in garbage collected languages too. I've been bitten by this in Java (JNI).
Yes, this needs to be caught by the compiler, damn it. And linters shouldn't complain about style either. Style checking should be a separate and very configurable tool.
Worth noting that you can disable individual Clippy warnings instead of the whole thing (per line or for the whole project). I almost always turn off the dead_code warning, especially when I'm iterating
cargo clippy -- -W clippy::pedantic
Here's a list of lints and groups you can turn on/off: https://rust-lang.github.io/rust-clippy/master/index.html
Switching away from my browser and back again causes the page to re-render, losing my place as it did so. Just awful!
window.setTimeout(makeprogress, delay)
Edit: As mentioned by another commenter, seems to be deliberate punishment for having javascript enabled. Also, clicking outside the main window (like in the url bar) does the render thing again. And a search for javascript has some odd results too: https://flak.tedunangst.com/search?q=javascript Last, there's some deliberately awful infinite scrolling too.
Also I warmly recommend to view-source, the makeprogress() function is a true delight.
I like jazz and think rock/pop generally sucks. If I prefaced every blog post with “people who like rock are too dumb to understand the following” I wouldn’t be making a funny joke about music. I’d just be an asshole.
It'd be more like prefacing a podcast about basic music theory with a 5 second pretentious jazz tune.
Why? Just why? The worst part is that the site is actually very light, with none of the ads, analytics and resource hog frameworks that are all too common these days. It probably could load almost instantly if it wasn't for that fake progress bar.
Edit: Judging by the other comments, it looks like it is some kind of political statement against Javascript. I disagree with such practices but well, that's his site, his choice.
Actually, he wrote some Rust and some C code wrapped in Rust.
> I was writing an smtp server... the kind of smtp server you'd write in two days.
(sigh)
> The error would have been detected far sooner had I not been lazy and checked the return value of chown (for a file not found error). But [excused here]
You can't, and mustn't avoid C error checking when writing C code, even if it's wrapped in Rust.
I was a TA in a first-semester course in C programming for several years, and one of the harder things to inculcate is how return value checking is not optional. It's very tempting to assume your standard library function just works, always.
> You can't, and mustn't [...]
It doesn't really sound like he's trying to excuse anything. Quote the intro more fully:
>>> I used unsafe. It was unsafe. After months of contemplating this unfortunate result, I've found someone else to blame.
The situation is worse than that. People check for errors except when they don't. And you are expected to know the difference. Take for instance malloc. It can return NULL but you don't check for it, because that's just the way it's done. Or take close(2).
> If close() is interrupted by a signal that is to be caught, it shall return -1 with errno set to EINTR and the state of fildes is unspecified.
> This permits the behavior that occurs on Linux and many other implementations, where, as with other errors that may be reported by close(), the file descriptor is guaranteed to be closed. However, it also permits another possibility: that the implementation returns an EINTR error and keeps the file descriptor open. (According to its documentation, HP-UX's close() does this.) The caller must then once more use close() to close the file descriptor, to avoid file descriptor leaks. This divergence in implementation behaviors provides a difficult hurdle for portable applications, since on many implementations, close() must not be called again after an EINTR error, and on at least one, close() must be called again.
How did this discrepancy go unnoticed for so long? Because no one checks close return value. Everyone nods and say you should, and then keep writing code without checks, because come one close never fails. If you do check you are the weird edge case. The one people whisper about "why does he have to be such an annoying pedant".
And while I'm ranting, the same goes for undefined behavior. Of course you shouldn't rely on undefined behavior. Of course of course. But maybe, just a little pointer casting is fine?
What should you write,
while (close(fd) != 0) {}
Maybe
if (fsync(fd) == 0)
while (close(fd) != 0) {}
else abort();
Ultimately there is nothing strictly correct to write, and you should just not really count on files being closed before process termination, but call close() as an optimization not to leak too many fds.
Retrying the close() after a failure return is the wrong thing to
do, since this may cause a reused file descriptor from another
thread to be closed. This can occur because the Linux kernel
always releases the file descriptor early in the close operation,
freeing it for reuse; the steps that may return an error, such as
flushing data to the filesystem or device, occur only later in
the close operation.
And since nobody checks the result of close and very few check the result of every single write operation, many "disk full" errors go unnoticed.
A very old programming principle says, "Never check for any failure you are not equipped to act on." It is sometimes used as a reminder to ensure you are always so equipped.
The problem with calling fsync() is that you have to wait for it to finish. There are many scenarios where the extra data integrity guarantees you get from calling fsync() aren't important.
But Cesar is right, you can't re-try it. And fsync() can be very slow on most commonly-used file systems. We are fortunate that now SSDs are fast.
This sounds like a great principle for allowing dangerous bugs to linger in a codebase for years... one can always act upon a failure by aborting.
But they are all in Posix, which is ISO Standard 9945, based on IEEE 1003.1.
Windos has various gestures toward Posix compatibility, which might include _close() and _fsync().
Unix and Posix file system semantics are not among the best achievements of the Unix philosophy or of Posix as a universal system interface, but they have been considered "good enough" by most for a long enough time to make an actually-good replacement increasingly unlikely to take hold.
You should try closing once, and if it fails, either die (e.g. with abort() ), or do something meaningful and relevant to your specific situation.
> Ultimately there is nothing strictly correct to write,
If that is the case for you - then definitely abort().
malloc is worth null-checking, if only to assert/panic/breakpoint/fatal early and cleanly.
Even on linux with overcommit, it'll return null on memory space exhaustion, which can easily happen even in a 64-bit processes if someone feeds your program maliciously crafted data with an oddball size. A crash dump at the point of allocation failure, when the sizes are still on the call stack / in registers, instead of minutes later - when the pointer is actually used - is also a vastly better debugging experience when fuzzing for such bugs.
If malicious data can control offsets into the (null) pointer, an allocation failure can turn into an exploitable buffer overflow (just make the offset large enough), even if the offset is bounds-checked against the "expected" (even larger) allocation size.
But anyway I doubt that people do it even though they should.
A lot of codebases certainly skip such checks.
But it's also fairly common to at least have a wrapper function that mallocs + triggers a fatal error if it returned null - and then to use that wrapper throughout the codebase. Gamedev tends to roll it's own allocators anyways, or need oddball system allocators for some resources, so they typically have a decent place to stick such checks already.
Well, I would take points off for not checking return values, although TBH my basic course did not have file I/O (they would use input and output redirection instead, to keep the programs focused on more fundamental things).
> And while I'm ranting, the same goes for undefined behavior. Of course you shouldn't rely on undefined behavior. Of course of course. But maybe, just a little pointer casting is fine?
That's a trickier subject, because it's not just the hassle of checking return values, it may mean your whole approach to implementing something may go down the drain. Also, it's much less obvious; and compilers don't make an effort to warn you about it; and telling the difference between _undefined_ and _implementation-defined_ behavior is an art which few master (and I'm not even one of those few).
This is entertaining to watch regarding UB:
CppCon 2017: Piotr Padlewski “Undefined Behaviour is awesome!”