
Memory Safety in Rust: A Case Study with C - wcrichton
http://willcrichton.net/notes/rust-memory-safety/
======
shabbyrobe
A lot of point-missing here about the example C program. Maybe everyone else
in the world is a perfect C coder who never makes a mistake, but I've found
every single one of those mistakes in my own code at one point or another
(though of course, not at quite the same density). The point is that they're
easy to make, and though there is tooling, it's extremely cumbersome and
imperfect.

Worse, over time as a codebase gets more complicated and older - and only if
you're one of the few lucky enough to start with C code that isn't an unholy,
inconsistent no-warnings disaster-mess slop bucket of horror - some of those
warnings have a nasty habit of getting disabled so some middle manager can
expedite some unreasonable commitment out a door to tick a box.

Call me a masochist but I enjoy writing C, particularly with valgrind, *san,
afl, et al at my back. But I felt the point was well made by the author and I
find it hard not to feel like a world where these problems simply didn't exist
at all might be a bit nicer

~~~
2trill2spill
> The point is that they're easy to make, and though there is tooling, it's
> extremely cumbersome and imperfect.

What's hard about adding -fsanitize=address or -Weverything to your Makefile?
Or running a program under Valgrind, or using the clang static analyzer or
coverity? Using AFL can be a little cumbersome but still not that hard.

~~~
seba_dos1
Depending on what you're doing, AddressSanitizer might not be available for
you to use, as there are plenty of ASAN-incompatible constructs that may be
used by libraries your application depends on.

(it's worth looking at other sanitizers anyway, it seems not everyone is aware
of their existence after all; if you can't put C away they're really great
tools for retaining your sanity :))

------
reza_n
Not exactly the best example. If I take this code (test.c) and try to compile
it:

    
    
      > gcc test.c
    

I get an error:

    
    
      test.c:21:3: warning: function returns address of local 
      variable [-Wreturn-local-addr]
         return &vec;
    

After I fix that error, the program segfaults when running. Compile with asan:

    
    
      > gcc test.c -Fsanitize=address -lasan -g
    

Then we can start debugging these problems:

    
    
      ==3261== ERROR: AddressSanitizer: attempting double-free on 0x60040000dfd0:
          ...
          #2 0x4008b5 in vec_free ./test.c:46
    

Not trying to say that this is the best workflow for debugging C, but the
tooling does exist for these kinds of programming errors.

~~~
amluto
None of which will help detect the integer overflow unless you're quite
serious about your testing.

~~~
reza_n
Very valid point. The code for that:

    
    
      int new_capacity = vec->capacity * 2;
      assert(new_capacity > vec->capacity);

~~~
nbsd4lyfe
asserts are for diagnostic-enabled code, don't use them for security checks.

~~~
steveklabnik
This is one area where Rust also differs from C; assert!s are left on in
release mode; you use debug_assert! if you want something only in development.

~~~
nbsd4lyfe
I agree, it's a hella questionable language choice, and not limited to C.

I found out about it through
[https://github.com/rohe/pysaml2/issues/451](https://github.com/rohe/pysaml2/issues/451)

------
sidlls
This is more like "a comparison of terrible C to middling Rust." Rust is
generally a superior language, but this sort of comparing the worst kind of C
to middling (or, worse, well-written) Rust doesn't seem useful to me.

I suppose it could be worse: these sorts of examples could always use
obfuscated C as their source for comparison.

~~~
klodolph
Yeah, sure. Personally, as much as I dislike the way the Rust community
evangelizes the way Rust works (like the Haskell community writing another
monad tutorial), there are some pretty common categories of errors
demonstrated in the C code:

\- Return pointer to stack object (more common than you might think!)

\- Incorrect size passed to malloc()

\- Pointer to array outliving invalidation of array

Some of the remaining C code appears to be garbage intended to distract you
from the errors people actually make, but there is some good stuff in there
and I pull out similar examples when I'm trying to convince people not to
learn C or C++ just to improve the quality of their greenfield projects.

~~~
kbwt
> Return pointer to stack object (more common than you might think!)

Maybe when you're still learning how to program.

A combination of valgrind/sanitizers will catch all of these mistakes. The
same class of mistakes can also be made in "memory-safe" languages, just
replace pointer with index and memory with array.

~~~
danieldk
Valgrind will only catch such indexing errors when you read/write out of
bounds when running Valgrind.

There is a huge difference between out-of-bounds indexing leading to undefined
behavior or an exception/panic.

~~~
klodolph
Valgrind won't catch certain indexing errors even if you exercise them.
Valgrind only checks that the address is valid, not that the address was
derived from a pointer to the object you are accessing.

    
    
        int main() {
            int x[16];
            int y[16];
            int z[16];
            x[0] = 0;
            y[0] = 0;
            z[0] = 0;
            y[18] = 3; // Valgrind thinks this is OK.
            return 0;
        }

~~~
jcelerier
[https://i.imgur.com/fgufyE1.png](https://i.imgur.com/fgufyE1.png)

~~~
klodolph
I think you may have missed the part of the conversation where we were talking
about Valgrind, specifically, and not talking about the address sanitizer.

------
2trill2spill
That C code is bad and doesn't compile on macOS using clang without adding
#include <assert.h> . Just looking at it you can see the bugs. But the main
problem I'm having with this article is comparing a contrived C program with
no tooling to Rust. As the author notes returning a stack address is caught by
clang. Also simply compiling with address sanitizer or running under Valgrind
would have caught the rest of the bugs.

I'm not saying Rust isn't safer than C or not a sweet and useful language it
is those things. What I am saying is comparing Rust to C without all the
tooling that Modern C developers use is kind of disingenuous.

~~~
jstimpfle
I don't even use any tooling except occasionally a debugger (gdb on Linux /
Visual studio on Windows). I have spent more than 500 hours of the last 8
months in C (no complicated data structures), and I'm pretty sure I've spent
less than 2 hours total tracing memory bugs. (not saying there aren't more
hidden).

~~~
2trill2spill
Use as many tools as your platform supports. I've had "stable" C code run just
fine for two years but come crashing to a halt immediately under address
sanitizer. So if code is working just fine now it could still be buggy and not
work on other platforms or when built by other compilers.

~~~
jstimpfle
Yep - might do. Unfortunately, to be honest, the OpenGL implementation I use
printed quite a bit of a mess when run under valgrind, so I haven't tried
again. (Could improve the architecture to bench separately). Update: tried and
valgrind doesn't report problems in my own code under normal operation, except
for me being sloppy at shutdown :-)

~~~
2trill2spill
Check out the docs for suppressing errors in valgrind[1]. Also try the various
compiler sanitizers, as well as the clang static analyzer and if your project
is open source, coverity.

[1]: [http://valgrind.org/docs/manual/manual-core.html#manual-
core...](http://valgrind.org/docs/manual/manual-core.html#manual-
core.suppress)

------
viperscape
Having learned Rust and used it for nearly 2 years, I am now happy using C. I
think most issues in C are easily catchable. Here me out:

1\. If using Int for indexing or any sort of len or count, make sure it's
positive when needed, and within bounds of what's allocated. As in if you plan
on allocating huge data, plan it out and use the right data type.

2\. If you alloc, then free when done. If you free, set to Null; and before
you free, check for Null.

3\. If you realloc, in particular, check that it actually worked and prepare
for basic error handling.

Rust requires all of these steps by default.

Finally, just test some of your code. Rust makes this easy, and encourages it.

I still really like C.

~~~
wott
> _before you free, check for Null._

You can free(NULL), no problem. It will not do anything.

~~~
syockit
It's usually a problem with pointers to structs that have pointer members. In
a typical destruction sequence, you usually free the members before the struct
itself.

------
fao_
Would anyone realistically write this C code though? The second point (the
initial total amount being 0) is something that anyone paying even a cursory
glance at the source code could pick up on, as were many of the others (who
the hell gets the address of stack memory and returns it? This is C 101).

If this was found-in-the-wild C then I wouldn't be bothered with it, but this
is completely contrived.

~~~
danieldk
No experienced C programmer will write this code, but years of buffer
overflow/double free CVEs show that even experienced programmers make such
errors occasionally. And one error can be enough for a system compromise.

Of course, this is not only an argument in favor of in Rust, but any memory-
safe language. Rust just happens to address some of the same problem domains
that C and C++ have traditionally been dominant in.

~~~
viperscape
Why is double free so commonly an issue? Isn't setting to Null after free and
checking for Null before hand a common practice?

~~~
tene
It's usually not calling free() on the same variable within a single function,
although that can sometimes happen. The most common case, I expect, is when
two separate data structures both have pointers to the same object and believe
they own it, then later end up calling free on the same address at different
times.

------
amluto
There's an extra bug in both the C and the rust code. The code assumes that,
if length == capacity and capacity >= 1, then length < capacity *2. This is
false for finite-precision integers. In C, this will manifest as an out of
bound write when the array gets too big. In Rust, it'll panic when array
bounds checking or integer overflow checking catches it.

~~~
tjalfi
A related issue is that overflow on signed integers in C is undefined.
Vec.length, Vec.capacity, and new_capacity should all be changed to size_t to
avoid a compiler optimizing out an overflow check.

Edited to be explicit about programming language.

~~~
steveklabnik
(Note that they are both well-defined in Rust)

------
MrBingley
Another problem with the C code, which is perhaps even more subtle, is the
usage of `int` for the length, capacity, and loop index. On 64-bit platforms
these variables will overflow for very large arrays, which will likely break
the entire implementation. The call to `malloc()` should give a signed-to-
unsigned conversion warning which will hint about this, but unfortunately many
people ignore integer warnings. Incidentally, this could also be caught by
compiling with `-ftrapv` or `-fsanitize=undefined`, but this problem is only
triggered in a rather extreme corner case that is unlikely to be exposed
during testing. The correct integer type to use is `size_t`, which is
guaranteed to be large enough to hold the size of any object in memory.

~~~
jstimpfle
Personally I'm going the other direction. I use int for anything unless
there's a serious need to make an exception. The amount of complexity
introduced by using a zoo of integer sizes and signedness is unmanageable to
me.

There is no point in measuring array sizes in size_t. I don't make bigger
allocations >2G. (At some point this assumption will probably break and I will
have to re-think my approach. Shouldn't we all move to 64-bit integers by
default already?)

~~~
burntsushi
> There is no point to measure array sizes in size_t. I don't make bigger
> allocations >2G.

I think almost every Rust program I've ever published regularly encounters
files greater than 2GB (even 4GB), and if I had used C and its `int` type
everywhere, I'd be in for a very very bad time.

This of course doesn't mean I am allocating >2G on the heap, but I might
memory map the file. Or there might be some other counter (line counter? byte
counter?) where using `int` would just fail.

There are even some alternatives to my tools written in C, and either they or
their dependencies use `int` instead of `size_t`, and that leads to actual
bugs their end users hit in exactly the cases where files are greater than
2GB.

Getting integer sizes right is important, and it's not just in cases where
you're putting >2G on the heap.

~~~
jstimpfle
> Getting integer sizes right is important, and it's not just in cases where
> you're putting >2G on the heap.

I prioritize on getting their _values_ right :-). So far I have not
encountered bugs due to my pretty uniform usage of int. But if they had to
deal with allocations >2G, my programs would just die.

Yup, I make some exceptions as well, for example to measure time in
microseconds, or to measure the size of very large streams. And of course I
try to assert that all downcasts to int are valid, and that my integer
operations don't overflow, etc. (why do CPUs still not support overflow
exceptions?)

I'm pretty sure we could get rid of quite some historical baggage in terms of
integer types. For example, I'm currently working on a network module, and
there is a type socklen_t which is to indicate the size of a socket structure.
I might be missing something, but to me there is no good reason not to use
simply int.

------
alkonaut
Lots of comments seem to say “these aren’t mistakes any experienced C
programmer would make”.

Does that really make the protection against those mistakes less important?

~~~
2trill2spill
> Does that really make the protection against those mistakes less important?

No it doesn't. But from a C programmers point of view it's kinda like can you
show examples of bugs in C that wouldn't immediately be found in code review
or by tools like ASAN, Valgrind, Coverity or even just the C compiler, that
Rust can solve. Those are the examples that would interest the C community.

~~~
pornel
Bugs that sneak through code review are, almost by definition, the hardest to
demonstrate. If a qualified person familiar with the code couldn't see it, it
will take a lot to explain it to an average reader. Hidden bugs are generally
in large and complex codebases, but such codebases are also the worst for
example code in a blog post. So that's why the next best thing is to show it
in obvious case, and leave it for you to extrapolate it to your complex cases.

Also the more complex the problem, the more likely it is incomparable between
C and Rust. The prime example is a controversy whether Heartbleed would have
happened in Rust. On one hand a direct C-to-Rust translation of the whole
system with a custom memory-reusing allocator wouldn't have prevented the bug.
OTOH such approach from Rust perspective is very contrived and unusable in
practice, so one can argue nobody would have structured the code like that in
the first place.

------
youdontknowtho
So rust isn't memory safe? You can still access invalid pointers?

I'm just trying to clarify...it's lifetime safe, but not access safe, or leak
safe?

~~~
steveklabnik
Unsafe code allows programmers to write code the compiler can’t check. Unsafe
code is expected to uphold the safety invariants, but humans can make
mistakes.

Most people mean “safe Rust” when they say “Rust.”

Leaks are memory safe though. They’re hard, but not impossible, to get.

~~~
sidlls
> Most people mean “safe Rust” when they say “Rust.”

I disagree. I think they mean Rust code they see in use that has no explicit
"unsafe" blocks. For example, most people are going to say "let mut v =
Vec::<i32>::new(); v.push(0)" is "safe Rust." It is, in the sense that all the
rules of Rust safety apply to the specific code quoted as-is. It also isn't,
because 'Vec' is littered with unsafe.

~~~
steveklabnik
That’s what I mean by “safe Rust”, so we agree.

~~~
sidlls
We don't, though. The quoted code isn't safe. It has implicit unsafe blocks in
it. You can't claim there are two dialects of Rust if you inlcude the "unsafe"
dialect in your definition of "safe" as long as there isn't an explicit
"unsafe" block present. It's misleading

~~~
steveklabnik
That's not a useful definition, though, as that means that all code everywhere
is unsafe, since the hardware is.

------
wott
The 382th episode in which a rustacean blames C while he has only practised
C++.

1\. The (int PTR) cast of malloc() gives you away immediately. No cast in C.

2\. > _missing free on resize. When the resize occurs, we reassign vec- >data
without freeing the old data pointer, resulting in a memory leak._

Er... C has realloc() for that. Once again, you do delete() + new() only in
C++, not in C.

BTW, you forgot other errors in your "C" program.

~~~
dang
Please don't post snarky dismissals here. Not cool, and you can make your
substantive points without casting them that way.

[https://news.ycombinator.com/newsguidelines.html](https://news.ycombinator.com/newsguidelines.html)

