[HN Gopher] Comparative unsafety
       ___________________________________________________________________
        
       Comparative unsafety
        
       Author : lionkor
       Score  : 37 points
       Date   : 2021-04-09 11:36 UTC (11 hours ago)
        
 (HTM) web link (flak.tedunangst.com)
 (TXT) w3m dump (flak.tedunangst.com)
        
       | rectang wrote:
       | In Rust, dereferencing a pointer is unsafe and so must be
       | isolated within an `unsafe` block -- but creating a dangling
       | pointer is not unsafe.
       | 
       | 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...
        
         | stefan_ wrote:
         | In general unsafe should be viral: you can't call unsafe
         | without yourself being unsafe. But it hits the same "snag",
         | then nothing works anymore.
        
           | nynx wrote:
           | This is not true. Rust is based around the idea of
           | encapsulating unsafety within interfaces that are safe to
           | use.
        
           | Jweb_Guru wrote:
           | If `unsafe` were viral it would be useless. Syntactically
           | ill-typed code can still be proven semantically well-typed,
           | and `unsafe` is intended as a marker that the proof of
           | semantic soundness lies outside of Rust's type system. If
           | there is no proof (at least an informal one), you shouldn't
           | be using unsafe, generally speaking.
        
         | jdc wrote:
         | I wonder what a good approach to safe pointer arithmetic would
         | be.
        
       | mqus wrote:
       | I'm somewhat wishing for a `.bind!()` macro or something similar,
       | which will desugar `let y =
       | x.method().bind!().reference_and_do_something()` into `let tmp =
       | x.method(); let y = tmp.reference_and_do_something()` . This
       | should be available for chaining for convenience.
       | 
       | But I see the issues (e.g. which scope should `tmp` be bound
       | to/what is its lifetime?)
        
       | einpoklum wrote:
       | > I wrote some rust code.
       | 
       | 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.
        
         | im3w1l wrote:
         | > 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.
         | 
         | 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?
        
           | ncmncm wrote:
           | No one checks close()'s result because it lies.
           | 
           | What should you write,                 while (close(fd) != 0)
           | {}
           | 
           | ? Or does it risk getting an infinite loop if, e.g., some
           | network connection disappears?
           | 
           | Maybe                 if (fsync(fd) == 0)          while
           | (close(fd) != 0) {}       else abort();
           | 
           | But maybe fsync is interruptible too.
           | 
           | 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.
        
             | cesarb wrote:
             | You cannot retry close(). Quoting from
             | https://man7.org/linux/man-pages/man2/close.2.html#NOTES
             | 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.
             | 
             | That is, once you call close(), the file descriptor is
             | always freed (even if close() returns an error), and can be
             | reused for an open() or similar in another thread. Retrying
             | the close() will fail with EBADF in the best case, and
             | close something another thread has opened in the worst
             | case.
        
               | OskarS wrote:
               | So what is the point of it having a return value at all
               | then?
        
             | rectang wrote:
             | > _No one checks close() 's result because it lies._
             | 
             | 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.
        
               | ncmncm wrote:
               | If you want to know whether your writes are getting out,
               | you will get a much more reliable indication from
               | fsync(). So, instead of exhorting people to check
               | close()'s result, you should exhort them to fsync() first
               | and check that result.
               | 
               | 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.
        
               | rectang wrote:
               | My understanding is fsync() will tell you more than
               | close() only when a flush of the OS cache fails to make
               | it to disk. Is there anything else?
               | 
               | 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.
        
         | thaumasiotes wrote:
         | >> 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 [...]
         | 
         | 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.
        
       | marvel_boy wrote:
       | >Teaching mode requires the stove be preheated to the optimal
       | temperature.
       | 
       | Very poetic. Are you Russian?
        
       | thaumasiotes wrote:
       | > Why am I using my own ffi version of _chown_ instead of the
       | libc crate? The libc crate prototypes _chown_ with unsigned
       | _uid_t_ and _gid_t_ types, and I want to pass -1 because I 'm not
       | interested in changing the group. I'll spare you the long rant
       | about how one should never redeclare system interfaces if you
       | can't take the time to do so properly, because it turns out if
       | you dig into it, _uid_t_ boils down to _uint32_t_ , but even so,
       | the manual for _chown_ says -1 should work and I want it to work.
       | 
       | 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.)
        
         | adwn wrote:
         | Yes, _!0_ would work. Or, to make the intent even more clear:
         | -1_i32 as u32
         | 
         | Definitely better than declaring your own function prototype.
        
         | ChrisSD wrote:
         | In Rust the simplest way would be to use the `u32::MAX`
         | constant. Create a fun alias for it if you want to express
         | intent.
        
           | thaumasiotes wrote:
           | I actually find ~0 more intuitive. u32::MAX makes me think
           | the relevant concept is "a really big number"; ~0 makes me
           | think the relevant concept is "a bunch of bits that are all
           | 1s".
        
             | ChrisSD wrote:
             | Fair enough. But tbh I'm not sure it really matters in this
             | context. It's just a constant that means "ignore this
             | parameter". Whether it's "a really big number" or "a bunch
             | of bits that are all 1s" is incidental.
             | 
             | Btw, you can directly express "all ones" in Rust as:
             | 0b_1111_1111_1111_1111_1111_1111_1111_1111
             | 
             | Though it's somewhat less succinct. ;)
             | 
             | EDIT: Added more ones, thanks adwn!
        
               | adwn wrote:
               | > _0b1111_1111_1111_1111_
               | 
               | That's just 16 bits and will result in the value 65535,
               | not -1.
        
               | ChrisSD wrote:
               | Ha, yes you're right. I'm on my phone and got tired of
               | writing ones. I'll edit when I'm next at my laptop.
        
         | boardwaalk wrote:
         | If your reaction to Rust actually following the prototype of
         | chmod but not accepting conversions signed/unsigned silently is
         | to redefine chmod so you can do it like you'd do in C... that
         | seems a little bullheaded to me. Wrapping a C function is
         | dangerous in a literal and real sense, doing "-1 as u32" is not
         | at all.
        
       | AndrewDucker wrote:
       | This has the most toxic UI I've seen in my life.
       | 
       | Switching away from my browser and back again causes the page to
       | re-render, losing my place as it did so. Just awful!
        
         | GuB-42 wrote:
         | Looking at the source, the progress bar is completely fake, it
         | really does nothing besides making you wait. It even introduces
         | slight delays to make it look more realistic.
         | 
         | 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.
        
           | ChrisSD wrote:
           | It's meant as satire on the needlessly script heavy websites
           | that many people create nowadays. The blog post itself isn't
           | entirely serious either.
        
             | nottorp wrote:
             | Well, unfortunately for me it resulted into an early page
             | close. I thought someone who uses that much javascript for
             | a page that could have been 100% static can't have anything
             | interesting to say.
        
         | nicoburns wrote:
         | I'm pretty sure it's an artificial loading screen too. There's
         | no way it actually takes that long to render. It's quite an
         | interesting artistic statement.
        
           | tyingq wrote:
           | Yep.                 window.setTimeout(makeprogress, delay)
           | 
           | With various repeated hard-coded delays of 50, 250, and
           | 500ms.
        
         | tyingq wrote:
         | Extra maddening, as it works fine with Javascript disabled.
         | Those loading and rendering progress bars are a terrible idea.
        
           | stefan_ wrote:
           | Haha, it's true! Disabling JavaScript leaves a perfectly
           | working article. Is this one big troll?
        
           | caslon wrote:
           | It's a joke. It's not a terrible idea. It's a great idea! A
           | great joke!
        
             | tyingq wrote:
             | Well, aside from the delay in seeing the content, it borks
             | the back button up, etc. Breaking all the pages on your
             | site for a joke is an odd idea to me.
             | 
             | 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.
        
               | [deleted]
        
               | caslon wrote:
               | Yeah, what's wrong about punishing Javascript users? It's
               | funny.
        
         | chriszhang wrote:
         | If I try to search something within the page with Ctrl + F that
         | also causes the whole page to re-render and display "loading
         | ..." and "rendering ..." progress bar.
        
           | [deleted]
        
         | liminal wrote:
         | You can't even search within the page. Ctrl-F hides the content
         | and shows the progress bars.
        
         | ptomato wrote:
         | I'm pretty sure it's explicitly intended solely as punishment
         | for people who have javascript enabled.
        
           | skrebbel wrote:
           | Fwiw I think it's pretty hilarious.
           | 
           | Also I warmly recommend to view-source, the makeprogress()
           | function is a true delight.
        
       | wyldfire wrote:
       | I will concede that it would be nice if I could opt-in to a 'be-
       | really-nitpicky' mode for clippy. I would even do it sometimes.
       | Usually I just want something with a little higher SNR.
        
       | rrss wrote:
       | > If there's an error which is a clear cut bug, I think it should
       | be reported by an error detecting tool, not a linter
       | 
       | Relevant rustc (merged) PR: https://github.com/rust-
       | lang/rust/pull/75671 - "Uplift temporary-cstring-as-ptr lint from
       | clippy into rustc"
        
         | rectang wrote:
         | This seems like a positive development, but there are other
         | `as_ptr` and `as_mut_ptr` functions. The one that I tripped up
         | with was actually from Vec, not CString.
         | 
         | Zooming out, there are innumerable ways to create a dangling
         | pointer. This is really a vexing problem.
        
       | nicoburns wrote:
       | From my perspective as someone who knows Rust but primarily works
       | with high-level languages like JavaScript, some developers who
       | from a background of unsafe languages (particularly C and C++)
       | seem incredibly cavalier around `unsafe` blocks in Rust code.
       | It's like they're desensitised to the unsafety.
       | 
       | 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.
        
         | dundarious wrote:
         | Maybe that's all true, but in the specific example in question,
         | he just does a string conversion and calls into a C library via
         | ffi (for a reason he has justified). I think he has a real
         | point about how the compiler should find this error if Rust is
         | to fully match its marketing, so to speak. Clippy (a tool for
         | style issues as much as anything else) doesn't seem to be an
         | appropriate place for such error checking.
         | 
         | On balance Rust does a great job, but "be even more careful in
         | unsafe Rust than in regular C++" is probably a losing battle.
        
         | [deleted]
        
         | cesarb wrote:
         | > From my perspective as someone who knows Rust but primarily
         | works with high-level languages like JavaScript, some
         | developers who from a background of unsafe languages
         | (particularly C and C++) seem incredibly cavalier around
         | `unsafe` blocks in Rust code. It's like they're desensitised to
         | the unsafety.
         | 
         | 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.
        
       | joosters wrote:
       | Rendering a web page requires an in-page animated progress bar
       | now?
        
         | skrebbel wrote:
         | Only if you have JavaScript enabled.
        
         | horsawlarway wrote:
         | And restarts the whole process every time you change focus away
         | from the window (at least from chrome on win10)
        
       | howeyc wrote:
       | Wait, what? I don't know rust, at all, but why is path freed? I
       | thought rust didn't have GC?
       | 
       | > let path = CString::new(filename.as_str()).unwrap().as_ptr();
       | 
       | Is one of those functions calling free behind the scenes?
        
         | OskarS wrote:
         | I mean, yeah, at the end you only get a raw pointer, the object
         | managing the pointer doesn't exist anymore. I'm not a Rust
         | programmer, but I would imagine it's equivalent to the
         | following C++ code                   auto ptr =
         | std::string(something).c_str();
         | 
         | This creates a std::string, calls c_str() on it to get the
         | pointer. However, since the std::string isn't stored anywhere,
         | its lifetime stops and it is destructed (this is why these are
         | called temporaries), and the pointer is now invalid. The right
         | way of doing it is this:                  std::string str {
         | something };        auto ptr = str.c_str();
         | 
         | Now, str and ptr both live in the same scope, and ptr's
         | lifetime is entirely contained in str's.
        
         | remexre wrote:
         | like C++, rust frees values once they've gone out of scope; the
         | result of
         | CString::new(filename.as_str()).unwrap()
         | 
         | is a temporary which is freed at the end of that line, since
         | the .as_ptr() returns a non-lifetime-encumbered value.
        
         | cesarb wrote:
         | It's freed for the same reason it would be freed in C++.
         | 
         | 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();
         | 
         | That way, the CString would only release its resources at the
         | end of the block.
         | 
         | 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).
        
         | mqus wrote:
         | to be clear, not path is freed, but what path points to,
         | meaning the result of CString::new(...).unwrap()
         | 
         | 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.
        
         | [deleted]
        
       | brundolf wrote:
       | > I used it for a while, but generally found it too tiresome.
       | 
       | 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
        
       ___________________________________________________________________
       (page generated 2021-04-09 23:01 UTC)