Hacker News Reader

Comment by ridiculous_fish | Hacker News Reader

The author gives the following function:

    static BYTES: &[u8] = b"hello world";
    fn foo1(index: usize) -> u8 {
        BYTES[index]
    }

and then writes we can't make foo1 commit UB just by giving it a large index ...foo1 will never cause UB without the caller committing UB first.

But strictly speaking this is wrong! If I plug foo1(large_index) into `process::Command::pre_exec()`, then the resulting panic may invoke UB, without the caller having committed UB first.

So you have to decide whether your function is designed to be run within these environments. If it is, and you add an allocation, then that's your fault!

View on HN

That's exactly the case I responded to, but with different operations. Instead of a signal handler, you're talking about calling pre_exec[1], which is marked unsafe.

This is the key bit:

> A safe caller can't be "at fault" for memory corruption or other UB.

The fault is in the programmer writing 'unsafe' to call pre_exec and not upholding the contract dictated by pre_exec. If the caller uses 'unsafe' to invoke code that doesn't uphold what pre_exec demands, then it's the fault of the point at which 'unsafe' is written.

The word "fault" is obviously overloaded, and one can reasonably use it to describe the safe code that allocates or panics as where the "fault" lies. But I feel like it's pretty clear that isn't what is meant in the OP, and is not really what is meant when one talks about using unsafe markers in Rust code to delineate where UB might occur.

If what you said was true, then (outside of Rust bugs and other environment trickery like /proc/self/mem) "safe by default" becomes meaningless. But it clearly isn't. And it does unfortunately depend on what you mean by "fault." In this context, it has to mean somewhere where you utter unsafe. If UB occurs in your program and you can't trace it back to a place where unsafe was uttered incorrectly (again, outside of Rust bugs and environment trickery), then something is seriously wrong.

[1]: https://doc.rust-lang.org/std/os/unix/process/trait.CommandE...

The "fault" lies with any function which fails to uphold its contract. I would argue that if `foo1` were documented as async-signal safe, then it would be at fault for the memory corruption it causes, if called from a signal handler.

> I would argue that if `foo1` were documented as async-signal safe

Agreed there, certainly. But that doesn't have much to do with Rust's safety/soundness stuff, and is more just about adhering to documented contracts. And 'foo1' is not documented as async-signal safe here.

I generally wouldn't rely on any function to be async-signal safe unless it's explicitly documented to be. I'm sure there's probably a heap of std methods where it would be reasonable to assume async-signal safety, particularly ones that don't do I/O, wouldn't reasonably be expected to allocate and are documented to never panic. (Possibly other things, I don't have the requirements for async-signal safety memorized. I just know to be super paranoid and carefully pay attention to it if I'm writing a signal handler. Which is... one of the many reasons I avoid signal handlers if I can help it and instead use something like the `signal-hook` crate.)

Then your code calling `pre_exec` is unsound, it is calling into `foo1` which is not documented to be async-signal-safe. Without any documentation saying otherwise you can't assume a function can be called within weird contexts with non-standard restrictions, and if they update to do something like add an allocation that causes your code to be UB, that's allowed by their documented API.

> But strictly speaking this is wrong!

I agree, and I'm making a correction. Instead of "foo1 will never cause UB without the caller committing UB first", the article now reads "foo1 will never cause UB without the caller writing unsafe first." Thank you!

IMO that correction isn't needed, because it goes without saying in every circumstance anyway. You always need to write 'unsafe' in order to introduce UB. (Modulo Rust bugs and environment trickery.)

But I do agree that your correction is, in fact, correct. :-)

Reply on news.ycombinator.com