1

I have a simple exercise in C (see code below). The program takes a vector with three components and double each component. The IDE showed me this warning (green squiggle): C6011 dereferencing null pointer v. in this line: v[0] = 12;. I think it's a bug because in the debugger I read the program exited with code 0. What do you think about it?

#include <stdlib.h>
#include <stdint.h> 

void twice_three(uint32_t *x) {
    for (size_t i = 0; i < 3; ++i) {
        x[i] = 2 * *x; 
    }
}

int main(void) {
    uint32_t *v = malloc(3 * sizeof(uint32_t)); 
    v[0] = 12;
    v[1] = 59; 
    v[2] = 83; 
    twice_three(v); 
    free(v); 
    return 0; 
}

NB: I'm using Visual Studio.

wovano
  • 4,543
  • 5
  • 22
  • 49

3 Answers3

2

I believe your IDE is warning you that you didn't make sure that malloc returned something other than NULL. malloc can return NULL when you run out of memory to allocate.

It's debatable whether such a check is needed. In the unlikely event malloc returned NULL, your program would end up getting killed (on modern computers with virtualized memory).[1] So the question is whether you want a clean message or not on exit in the very very rare situation that you run out of memory.

If you do add a check, don't use assert. That's useless. For starters, it only works in dev builds (not production builts) where malloc returning NULL is unlikely, and where it's already super easy to find memory leaks (e.g. by using valgrind). Use a proper check (if (!v) { perror(NULL); exit(1) }).


  1. Since people are trying to debate the issue in the comments despite the rules, it looks like I'll have to go into my claim in more detail.

    A couple of people suggested in the comments that "anything could happen" if you ones doesn't check for NULL, but that's simply not true on modern computers with virtualized memory.

    When the C spec doesn't define the behaviour of something (what is called "undefined behaviour"), it doesn't mean anything can happen; it just means the C language doesn't care what the compiler/machine does in such situations. And a NULL dereference is very well defined on such systems. Catching such situations is a raison d'être of memory virtualization!

    Just like you can rely on other compiler-specific features such as gcc's field packing attributes, one can argue it's fine to rely on memory virtualization to detect a failure by malloc.

ikegami
  • 367,544
  • 15
  • 269
  • 518
  • 2
    You had better in that last case to use `assert(v != NULL);` after the `malloc()` call, as you can end far later with that undefined behaviour (or end correctly, while there's a bug in the program), and no idea of what was the source of the error. Letting the program fail when dealing with malloc is not debatable, as we enter in the field of undefined behaviour, and undefined behaviour means almost anything, including your own data loss, even in modern computers with virtualized memory. – Luis Colorado Nov 11 '21 at 10:37
  • 1
    assert is useless in production code... in development, assert allows you to find the problem as soon as it happens. There should be no reason for `malloc()` to return `NULL` than probably a memory leak, so identifying it soon is crucial. Anyway, your proposal of letting the program run to see what happens next is far worse, as the problem found will probablyh have nothing to do with the source, as the pointer will probably be copied and written at several places when you detect finally an NPE. – Luis Colorado Nov 12 '21 at 07:17
  • 1
    Well I think it was for more than 20 years. I know there are better solutions now... but you cannot assert so strongly that that's completely wrong. I just made a comment to your answer. You assume the questioneer is using an IDE (I use to program in C with vi only) and IMHO, you are not reading (or trying to understand) my point. Anyway, that's enough for me, and give up. My apologies for having a different opinion. I have never said that `assert` was a memory leak detection, but not checking its result could lead you to U.B. (not necessarily to memory leak). – Luis Colorado Nov 17 '21 at 07:27
  • Well, this gave me the idea: _"I believe your IDE is warning you"_ By the way, why do you insist in discussing with me? Have I faulted you to respect or something? You are assuming many things gratuitously. Your first sentence in your answer talks about the use of an IDE, so why do you insist in never have said that? – Luis Colorado Nov 26 '21 at 12:29
  • No, using `assert` wouldn't help. `-fsanitize=address` or a strack trace are a million times better. // It's nonsense to claim that one should use assert because if you run the code on a dev machine, and if your program has a memory leak, and if the leak is so huge that you run out of memory (during dev!), you'll run into behavour that's not defined on other machines. // Your claims about assumptions I supposedly made are proven false. // Your claims that you didn't say assert can be used to detect memory leaks are proven false. // And a final comment that's nothing more than an insult – ikegami Nov 28 '21 at 03:51
  • ...for that it is better to check the return value of malloc() and stop as soon as possible, instead of letting the program run to crash, as from your hint. this is what I suggested (and you will agree on assert making a test and abort, allowing you to do a post-mortem debug), and what made you jump on me to bite my neck veins. Please, leave it, I don't want to discuss. – Luis Colorado Nov 28 '21 at 19:18
  • Re "*and stop as soon as possible*", It already stops as soon as possible without the `assert`. // Re "*and you will agree on assert making a test and abort, allowing you to do a post-mortem debug*", No. As said twice incl in first comment, I said it doesn't help do that at all. `assert` only works in dev, where you already have stack traces and the ability to use `-fsanitize=address`. // Enough with this junk! Comments aren't for discussions, especially not this nonsense! If you're going to check for malloc return NULL, use `if` not `assert`. Limiting the check to dev is the worse of all opts – ikegami Nov 28 '21 at 19:49
  • Why do you state that `assert()` "_only works in dev builds (not production builts)_"? That might be the case for if you define `NDEBUG` in your production builds, but that does not *have* to be the case. There are plenty of companies that use `assert()` in their production code, and the question does not even mention production code to begin with. In almost all code I have seen, `assert()` just works as expected. That doesn't make it the best choice for this specific scenario, but it certainly is possible, and I personally I would prefer this over letting the VMM take care of it (but YMMV). – wovano Nov 28 '21 at 21:31
  • @wovano Yes, you can use a dev/debug build in production. So it *might* work in production. Still not a reason to avoid a solution that always works. – ikegami Nov 28 '21 at 22:43
  • I'm not saying you can use a debug build in production, I'm saying that you can use `assert()` in production code. And I was wondering why your answer asserts (pun intended ;p) that using `assert()` is useless because it only works in dev builds (which is not true in general). I really understand why you wrote this, and there certainly is a truth in it, but I just think that the way you wrote it is incorrect and might give others the wrong idea. Yes, `assert()` can be disabled, but because you have the option to disable it doesn't make it useless. – wovano Nov 28 '21 at 22:49
  • And both solutions do "_always work_". But one solution only "always works" for modern computers with virtualized memory, the other solution only "always works" for any platform with a standard-conforming C compiler. – wovano Nov 28 '21 at 22:51
  • @wovano Re "*I'm saying that you can use assert() in production code.*", Yes, but it's noop unless you use a debug build. If you use `NDEBUG`, it does absolutely nothing – ikegami Nov 28 '21 at 22:53
  • Then don't use `NDEBUG` :-) – wovano Nov 28 '21 at 22:55
  • Yes, like I said, you can use a dev/debug build in production if you want. But that's not a recommendation I'm going to make in general, and it's not something I would expect to be the case in general. – ikegami Nov 28 '21 at 22:55
2

First of all, note that the warning is generated by your compiler (or static analyzer, or linter), not by your debugger, as you initially wrote.

The warning is telling you that your program possibly might dereference a null pointer. The reason for this warning is that you perform a malloc() and then use the result (the pointer) without checking for NULL values. In this specific code example, malloc() will most likely just return the requested block of memory. On any desktop computer or laptop, there's generally no reason why it would fail to allocate 12 bytes. That's why your application just runs fine and exits successfully. However, if this would be part of a larger application and/or run on a memory-limited system such as an embedded system, malloc() could fail and return NULL. Note that malloc() does not only fail if there is not enough memory available, it could also fail if there is no large enough consecutive block of memory available, due to fragmentation.

According to the C standard, dereferencing a NULL pointer is undefined behavior, meaning that anything could happen. On modern computers it would likely get your application killed (which could lead to data loss or corruption, depending on what the application does). On older computers or embedded systems the problem might be undetected and your application would read from or (worse) write to the address NULL (which is most likely 0, but even that isn't guaranteed by the C standard). This could lead to data corruption, crashes or other unexpected behavior at an arbitrary time after this happened.

Note that the compiler/analyzer/linter doesn't know anything about your application or the platform you will be running it on, and it doesn't make any assumptions about it. It just warns you about this possible problem. It's up to you to determine if this specific warning is relevant for your situation and how to deal with it.

Generally speaking, there are three things you can do about it:

  1. If you know for sure that malloc() would never fail (for example, in such a toy example that you would only run on a modern computer with gigabytes of memory) or if you don't care about the results (because the application will be killed by your OS and you don't mind), then there's no need for this warning. Just disable it in your compiler, or ignore the warning message.

  2. If you don't expect malloc() to fail, but do want to be informed when it happens, the quick-and-dirty solution is to add assert(v != NULL); after the malloc. Note that this will also exit your application when it happens, but in a slightly more controlled way, and you'll get an error message stating where the problem occurred. I would recommend this for simple hobby projects, where you do not want to spend much time on error handling and corner cases but just want to have some fun programming :-)

  3. When there is a realistic change that malloc() would fail and you want a well-defined behavior of your application, you should definitely add code to handle that situation (check for NULL values). If this is the case, you would generally have to do more than just add an if-statement. You would have to think about how the application can continue to work or gracefully shutdown without requiring more memory allocations. And on an embedded system, you would also have to think about things such as memory fragmentation.

The easiest fix for the example code in question is add the NULL-check. This would make the warning go away, and (assuming malloc() would not fail) your program would run still the same.

int main(void) {
    uint32_t *v = malloc(3 * sizeof(uint32_t));
    if (v != NULL) {
        v[0] = 12;
        v[1] = 59; 
        v[2] = 83; 
        twice_three(v); 
        free(v); 
    }
    return 0; 
}
wovano
  • 4,543
  • 5
  • 22
  • 49
0
  1. Always check the result of malloc.
  2. Use objects not types in sizeof
int main(void) {
    uint32_t *v = malloc(3 * sizeof(*v)); 
    if(v)
    {
        v[0] = 12;
        v[1] = 59; 
        v[2] = 83; 
        twice_three(v); 
    }
    free(v); 
    return 0; 
}
0___________
  • 60,014
  • 4
  • 34
  • 74