11 ms·

Memory Safety in Rust: A Case Study with C

65 points by wcrichton 9y ago

sidlls 9y ago
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 9y ago
  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 9y ago
    > 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 9y ago
      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 9y ago
        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 9y ago
        https://i.imgur.com/fgufyE1.png https://i.imgur.com/fgufyE1.png
        klodolph 9y ago
        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 9y ago
        But clang complains, it's not about just one tool. test.c:8:9: warning: array index 18 is past the end of the array (which contains 16 elements) [-Warray-bounds] y[18] = 3; // Valgrind thinks this is OK. ^ ~~
        klodolph 9y ago
        The snippet is a demonstration of the limitations of Valgrind, specifically. It would be trivial to change the code so it has the same behavior but doesn't trigger the Clang warning.
    - jcranmer 9y ago
      Pointers to stack objects escaping the function (not necessarily returning) is actually surprisingly easy to do. Sure, the example "Foo f; return &f;" is the sort of situation that isn't going to happen if you're at all experienced, but it's not hard to build cases. I recently had to debug a crash which turned out to be a stack object unexpectedly escaping. Effectively, the flow is this: void foo(Foo *v) { Operand blah; /* Turns out that there's a use-list on some operands, and this adds &blah to that list in that case. */ copy_operand(&blah, &v->operands[1]); free_foo(v); } > A combination of valgrind/sanitizers will catch all of these mistakes. They will catch only those mistakes that occur when you run them in tests. Plenty of memory safety CVEs still show up in programs that do aggressive fuzzing under valgrind/sanitizer testing. Or, as one aphorism has it, "testing cannot prove the absence of bugs, only their presence."
    - klodolph 9y ago
      > Maybe when you're still learning how to program. I used to think so, too. From John Carmack (https://twitter.com/ID_AA_Carmack/status/587077680652230656 https://twitter.com/ID_AA_Carmack/status/587077680652230656) > Found two pointer-to-out-of-scope-stack bugs today. I like tight native code, but C/C++ still makes me worry a lot. And then there's this dubious claim, > A combination of valgrind/sanitizers will catch all of these mistakes. Nope! 1) You have to execute the right code paths before Valgrind or any of the sanitizers will catch your use of a pointer to stack. In relatively simple cases, you might not catch the error even if you have 100% code coverage. 2) Not all platforms have Valgrind or sanitizers working on them, in fact, most don't. > The same class of mistakes can also be made in "memory-safe" languages, just replace pointer with index and memory with array. Sure, you could write an x86 interpreter in Java, and I'm sure that somebody has already done this. But these errors have much more severe consequences in C.
    - Jweb_Guru 9y ago
      Don't use string_view then. Seriously, if people actually programmed C and C++ the way people claim they do, it wouldn't be any more efficient than Java most of the time.
  - jstimpfle 9y ago
    > - Pointer to array outliving invalidation of array Another problem that can be largely avoided with a clear architecture. 1. The vast majority of buffers should be allocated at program startup, stored in a global variable, and freed at the end. 2. In almost all other cases, pointer ownership should just not be moved across functions (mostly "initializer functions" which return allocated memory). I think the idea that a function takes a data pointer without any idea how it was created, and then stores the pointer somewhere in its own structures, comes from garbage-collected languages and OOP culture which has an (I think) irrational aversion to globally architected dataflow. The messy object graphs that are created in this culture are just not manageable without a GC or another mechanically enforced discipline. I'm not a fan of neither C++, nor Rust, nor garbage collected languages, for complex tasks. One can get away with less expertly architected programs in these languages. But, IMHO, they don't support, or do even impede, clear architecture, which ultimately leads to complexity. And this does mean hard to maintain and buggy software - it's just more memory-safe.
    - klodolph 9y ago
      Now the only problem is in communicating and enforcing that architecture, hiring people who understand how important this architecture is or training them, adding these architectural checks to your code review process, having discussions about which cases are exceptions, and figuring out what to do with legacy code.
      - jstimpfle 9y ago
        Legacy code is reduced to a fraction with good architecture, though. Well architected programs are a lot shorter and a lot more straightforward. And I don't really think you can make clear rules or exceptions - so no general discussions needed. Just don't admit data flow designs that aren't completely thought through (and really simple). Yes, it takes experienced developers.
        klodolph 9y ago
        You can point out that the ancient Egyptians built the pyramids just fine, but I'm still going to argue in favor of mechanization. In my experience, good architecture does not materialize often enough to make this a viable strategy except for certain specialized teams. Consider writing code in C when you don't completely understand the problem and haven't devised a solution yet, or when you know it's going to get assigned to a junior developer and you don't have spare time for extra oversight. Or consider that you might ship poorly architected code because it's functional and shipping it will earn you money, or you might inherit large tracts of legacy code that are too expensive to refactor. We all love ourselves some good architecture in our programs, but I'd rather have the penalty for bad architecture be "fails to compile" or "assertion error at runtime" rather than "our product has a security vulnerability". Just because the problem can be solved by throwing better, more experienced developers at it doesn't mean that we should. Let's throw the experienced developers at the more exciting problems and let our tools figure out memory safety for us... at least, most of the time. > Legacy code is reduced to a fraction with good architecture, though. I honestly don't understand this claim at all. I've only worked at one company where I didn't have to work with legacy code. The amount of legacy code I've been personally responsible for has varied from ~20 kloc (fairly manageable) to ~1 Mloc. I don't know what kind of good architecture could reduce that 1 Mloc. I ended up just throwing the address sanitizer at it, working on the test suite, and prioritizing refactors based on expected cost and payoff.
fao_ 9y ago
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.
- seba_dos1 9y ago
  > who the hell gets the address of stack memory and returns it? Most of beginners out there, who then cannot figure out the reason for their crashes, especially when it's not explicitly returned as the value of the function ;)
  - Const-me 9y ago
    Neither of these two languages are exceptionally friendly towards beginners. Python, Ruby, Java/C#, Go are IMO much better for them.
    - macintux 9y ago
      But eventually if someone turns to C no amount of Python experience will prepare them for this category of memory problems, so they still qualify as a “beginner” for this discussion.
      - Const-me 9y ago
        > no amount of Python experience will prepare them for this category of memory problems General programming experience helps a lot. Someone coming to C from a higher level language likely knows to read compiler output and warnings, use debugger, read diagnostic output. With modern C toolset, that experience will help solve 99% of such errors. OTOH, someone coming to Rust encounters set of problems no amount of programming experience help to solve. IMO for beginners, Rust’s learning curve is dangerously close to that of pure functional languages.
- 9y ago
reza_n 9y ago
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.
- reza_n 9y ago
  As another poster pointed out, if I run this via valgrind (after fixing the first error): > valgrind ./a.out ==4871== Invalid write of size 4 ==4871== at 0x40073A: vec_push (test.c:40) ==4871== Invalid write of size 4 ==4871== at 0x40073A: vec_push (test.c:40) ==4871== Invalid read of size 4 ==4871== at 0x4007CD: main (test.c:55) ==4871== Invalid read of size 8 ==4871== at 0x400769: vec_free (test.c:46) ==4871== Invalid free() / delete / delete[] / realloc() ==4871== at 0x4C2BDEC: free (...) ==4871== by 0x400770: vec_free (test.c:46)
- amluto 9y ago
  None of which will help detect the integer overflow unless you're quite serious about your testing.
  - reza_n 9y ago
    Very valid point. The code for that: int new_capacity = vec->capacity * 2; assert(new_capacity > vec->capacity);
    - nbsd4lyfe 9y ago
      asserts are for diagnostic-enabled code, don't use them for security checks.
2trill2spill 9y ago
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 9y ago
  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 9y ago
    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 9y ago
      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 9y ago
        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.suppress http://valgrind.org/docs/manual/manual-core.html#manual-core...
alkonaut 9y ago
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 9y ago
  > 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 9y ago
    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.
shabbyrobe 9y ago
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 9y ago
  > 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 9y ago
    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 :))
  - irundebian 9y ago
    In contrast to Rust's language design, -fsanitize and -Weverything and static source code analyzer can't give guarantees over memory safety. Considering that your main activity in your Github profile is concentrated on JavaScript, I wouldn't be so cheeky. Feel free to post your self-written applications (no fuzzers or similar with uninteresting attack surfaces) written in C for open review.
youdontknowtho 9y ago
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 9y ago
  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 9y ago
    > 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 9y ago
      That’s what I mean by “safe Rust”, so we agree.
      - sidlls 9y ago
        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 9y ago
        That's not a useful definition, though, as that means that all code everywhere is unsafe, since the hardware is.
amluto 9y ago
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 9y ago
  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 9y ago
    (Note that they are both well-defined in Rust)
MrBingley 9y ago
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 9y ago
  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 9y ago
    > 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 9y ago
wott 9y ago
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.
- irundebian 9y ago
  Please don't be so arrogant. Post your github profile, and then we can see if you write code without bugs.
- dang 9y ago
  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
viperscape 9y ago
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 9y ago
  > before you free, check for Null. You can free(NULL), no problem. It will not do anything.
  - syockit 9y ago
    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.
- solidsnack9000 9y ago
  Although I completely disagree with you, I really appreciate that you posted.

65 points by wcrichton 9y ago

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 9y ago
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 9y ago
  > 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 9y ago
    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 9y ago
      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 9y ago
        https://i.imgur.com/fgufyE1.png https://i.imgur.com/fgufyE1.png
        klodolph 9y ago
        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 9y ago
        But clang complains, it's not about just one tool. test.c:8:9: warning: array index 18 is past the end of the array (which contains 16 elements) [-Warray-bounds] y[18] = 3; // Valgrind thinks this is OK. ^ ~~
        klodolph 9y ago
        The snippet is a demonstration of the limitations of Valgrind, specifically. It would be trivial to change the code so it has the same behavior but doesn't trigger the Clang warning.
  - jcranmer 9y ago
    Pointers to stack objects escaping the function (not necessarily returning) is actually surprisingly easy to do. Sure, the example "Foo f; return &f;" is the sort of situation that isn't going to happen if you're at all experienced, but it's not hard to build cases. I recently had to debug a crash which turned out to be a stack object unexpectedly escaping. Effectively, the flow is this: void foo(Foo *v) { Operand blah; /* Turns out that there's a use-list on some operands, and this adds &blah to that list in that case. */ copy_operand(&blah, &v->operands[1]); free_foo(v); } > A combination of valgrind/sanitizers will catch all of these mistakes. They will catch only those mistakes that occur when you run them in tests. Plenty of memory safety CVEs still show up in programs that do aggressive fuzzing under valgrind/sanitizer testing. Or, as one aphorism has it, "testing cannot prove the absence of bugs, only their presence."
  - klodolph 9y ago
    > Maybe when you're still learning how to program. I used to think so, too. From John Carmack (https://twitter.com/ID_AA_Carmack/status/587077680652230656 https://twitter.com/ID_AA_Carmack/status/587077680652230656) > Found two pointer-to-out-of-scope-stack bugs today. I like tight native code, but C/C++ still makes me worry a lot. And then there's this dubious claim, > A combination of valgrind/sanitizers will catch all of these mistakes. Nope! 1) You have to execute the right code paths before Valgrind or any of the sanitizers will catch your use of a pointer to stack. In relatively simple cases, you might not catch the error even if you have 100% code coverage. 2) Not all platforms have Valgrind or sanitizers working on them, in fact, most don't. > The same class of mistakes can also be made in "memory-safe" languages, just replace pointer with index and memory with array. Sure, you could write an x86 interpreter in Java, and I'm sure that somebody has already done this. But these errors have much more severe consequences in C.
  - Jweb_Guru 9y ago
    Don't use string_view then. Seriously, if people actually programmed C and C++ the way people claim they do, it wouldn't be any more efficient than Java most of the time.
- jstimpfle 9y ago
  > - Pointer to array outliving invalidation of array Another problem that can be largely avoided with a clear architecture. 1. The vast majority of buffers should be allocated at program startup, stored in a global variable, and freed at the end. 2. In almost all other cases, pointer ownership should just not be moved across functions (mostly "initializer functions" which return allocated memory). I think the idea that a function takes a data pointer without any idea how it was created, and then stores the pointer somewhere in its own structures, comes from garbage-collected languages and OOP culture which has an (I think) irrational aversion to globally architected dataflow. The messy object graphs that are created in this culture are just not manageable without a GC or another mechanically enforced discipline. I'm not a fan of neither C++, nor Rust, nor garbage collected languages, for complex tasks. One can get away with less expertly architected programs in these languages. But, IMHO, they don't support, or do even impede, clear architecture, which ultimately leads to complexity. And this does mean hard to maintain and buggy software - it's just more memory-safe.
  - klodolph 9y ago
    Now the only problem is in communicating and enforcing that architecture, hiring people who understand how important this architecture is or training them, adding these architectural checks to your code review process, having discussions about which cases are exceptions, and figuring out what to do with legacy code.
    - jstimpfle 9y ago
      Legacy code is reduced to a fraction with good architecture, though. Well architected programs are a lot shorter and a lot more straightforward. And I don't really think you can make clear rules or exceptions - so no general discussions needed. Just don't admit data flow designs that aren't completely thought through (and really simple). Yes, it takes experienced developers.
      - klodolph 9y ago
        You can point out that the ancient Egyptians built the pyramids just fine, but I'm still going to argue in favor of mechanization. In my experience, good architecture does not materialize often enough to make this a viable strategy except for certain specialized teams. Consider writing code in C when you don't completely understand the problem and haven't devised a solution yet, or when you know it's going to get assigned to a junior developer and you don't have spare time for extra oversight. Or consider that you might ship poorly architected code because it's functional and shipping it will earn you money, or you might inherit large tracts of legacy code that are too expensive to refactor. We all love ourselves some good architecture in our programs, but I'd rather have the penalty for bad architecture be "fails to compile" or "assertion error at runtime" rather than "our product has a security vulnerability". Just because the problem can be solved by throwing better, more experienced developers at it doesn't mean that we should. Let's throw the experienced developers at the more exciting problems and let our tools figure out memory safety for us... at least, most of the time. > Legacy code is reduced to a fraction with good architecture, though. I honestly don't understand this claim at all. I've only worked at one company where I didn't have to work with legacy code. The amount of legacy code I've been personally responsible for has varied from ~20 kloc (fairly manageable) to ~1 Mloc. I don't know what kind of good architecture could reduce that 1 Mloc. I ended up just throwing the address sanitizer at it, working on the test suite, and prioritizing refactors based on expected cost and payoff.

fao_ 9y ago

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.

seba_dos1 9y ago
> who the hell gets the address of stack memory and returns it? Most of beginners out there, who then cannot figure out the reason for their crashes, especially when it's not explicitly returned as the value of the function ;)
- Const-me 9y ago
  Neither of these two languages are exceptionally friendly towards beginners. Python, Ruby, Java/C#, Go are IMO much better for them.
  - macintux 9y ago
    But eventually if someone turns to C no amount of Python experience will prepare them for this category of memory problems, so they still qualify as a “beginner” for this discussion.
    - Const-me 9y ago
      > no amount of Python experience will prepare them for this category of memory problems General programming experience helps a lot. Someone coming to C from a higher level language likely knows to read compiler output and warnings, use debugger, read diagnostic output. With modern C toolset, that experience will help solve 99% of such errors. OTOH, someone coming to Rust encounters set of problems no amount of programming experience help to solve. IMO for beginners, Rust’s learning curve is dangerously close to that of pure functional languages.
9y ago

reza_n 9y ago

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.

reza_n 9y ago
As another poster pointed out, if I run this via valgrind (after fixing the first error): > valgrind ./a.out ==4871== Invalid write of size 4 ==4871== at 0x40073A: vec_push (test.c:40) ==4871== Invalid write of size 4 ==4871== at 0x40073A: vec_push (test.c:40) ==4871== Invalid read of size 4 ==4871== at 0x4007CD: main (test.c:55) ==4871== Invalid read of size 8 ==4871== at 0x400769: vec_free (test.c:46) ==4871== Invalid free() / delete / delete[] / realloc() ==4871== at 0x4C2BDEC: free (...) ==4871== by 0x400770: vec_free (test.c:46)
amluto 9y ago
None of which will help detect the integer overflow unless you're quite serious about your testing.
- reza_n 9y ago
  Very valid point. The code for that: int new_capacity = vec->capacity * 2; assert(new_capacity > vec->capacity);
  - nbsd4lyfe 9y ago
    asserts are for diagnostic-enabled code, don't use them for security checks.

2trill2spill 9y ago

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 9y ago
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 9y ago
  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 9y ago
    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 9y ago
      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.suppress http://valgrind.org/docs/manual/manual-core.html#manual-core...

alkonaut 9y ago

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 9y ago
> 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 9y ago
  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.

shabbyrobe 9y ago

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 9y ago
> 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 9y ago
  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 :))
- irundebian 9y ago
  In contrast to Rust's language design, -fsanitize and -Weverything and static source code analyzer can't give guarantees over memory safety. Considering that your main activity in your Github profile is concentrated on JavaScript, I wouldn't be so cheeky. Feel free to post your self-written applications (no fuzzers or similar with uninteresting attack surfaces) written in C for open review.

youdontknowtho 9y ago

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 9y ago
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 9y ago
  > 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 9y ago
    That’s what I mean by “safe Rust”, so we agree.
    - sidlls 9y ago
      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 9y ago
        That's not a useful definition, though, as that means that all code everywhere is unsafe, since the hardware is.

amluto 9y ago

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 9y ago
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 9y ago
  (Note that they are both well-defined in Rust)

MrBingley 9y ago

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 9y ago
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 9y ago
  > 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 9y ago

wott 9y ago

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.

irundebian 9y ago
Please don't be so arrogant. Post your github profile, and then we can see if you write code without bugs.
dang 9y ago
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

viperscape 9y ago

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 9y ago
> before you free, check for Null. You can free(NULL), no problem. It will not do anything.
- syockit 9y ago
  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.
solidsnack9000 9y ago
Although I completely disagree with you, I really appreciate that you posted.