Appearing to succeed is a valid form of undefined behavior, but it’s still undefined


A customer requested a clarification on the MSDN documentation for the HeapFree function.

The MSDN documentation says that if the lpMem parameter is NULL, then the behavior is undefined. Is this true?

As explicitly stated in MSDN, the behavior is undefined. Observe that the annotation on the lpMem parameter is __in, which means that the parameter must be a non-NULL value provided by the caller. (If NULL were permitted, the annotation would have been __in_opt.)

Undefined behavior means that anything can happen. The program might crash immediately. It might crash five minutes later. It might send email to your boss saying that you screwed up and then read you Vogon poetry. Or maybe not.

MSDN says don't do it, so don't do it.

The customer explained why they were interested in knowing more information about undefined behavior:

We were interested because there is a mismatch between the semantics of a function we are implementing (where NULL is valid and ignored) and the function HeapFree we are using as the implementation. It looks like Windows Vista returns TRUE if you pass NULL.

If there is a mismatch in semantics between the function you are implementing and the function you are calling, it is your responsibility as the programmer to bridge the gap. The customer didn't say what function they were implementing, but I'm guessing it was something like operator delete. Since your function accepts NULL but HeapFree doesn't, it is your responsibility to filter out NULL parameters.

void operator delete(void* ptr) throw ()
{
 if (ptr != NULL)
  HeapFree(CustomHeap, 0, ptr);
}

This concept goes by the fancy name of the Adapter Pattern. The less fancy name is wrapper function.

And the value returned by HeapFree on Windows Vista is irrelevant. Pretending to succeed is a valid form of undefined behavior, because anything qualifies as undefined behavior.

(Of course, you can't assume that returning TRUE will always be the result of triggering undefined behavior. After all, if you could rely on it, then it wouldn't be undefined any more!)

Comments (35)
  1. SimonRev says:

    Uggh, I really wish more people understood this concept properly.  I don't know how often I see code that is undefined but happens to work and how hard it is to convince folks that they cannot rely on that.

    My personal favorite is an intern who once said:  "Look, printf will malloc memory for me!"  Just because this code happened to not crash:

    char *buffer

    sprintf(buffer, "%d", intValue);

  2. Anonymous says:

    I can still remember my first time seeing the meaning of "undefined behavior" from the C programming manual, which impressed me so much.

  3. Anonymous says:

    @SimonRev: I know your pain.

    Incidentally, the world is slowly moving to freeing NULL is a noop. There was a discussion on LKML about changing the code inside kfree from if (unlikely(p == NULL)) return ; to if (likely(p == NULL)) return ; (unlikely and likely are macros that generate compiler hints that generate CPU hints).

    Definitely continue to document do not pass null to this function. It's no longer expected that is the case.

  4. Anonymous says:

    People don't understand contracts in general.

  5. Anonymous says:

    Why does HeapFree not just define its semantics so that passing it a null pointer results in a no-op?  Surely the performance cost "if(lpMem == NULL) return TRUE;" is negligible compared to the cost of actually deallocating the memory back to the heap, as well as the cost of checking if (dwFlags & HEAP_NO_SERIALIZE) followed by optionally locking a critical section.

    In fact, stepping through the disassembly for HeapFree in the VS 2008 runtime on Windows 7, it does in fact check if(lpMem == NULL).  Surely people have already written and shipped software that relies on that fact.  If you take it out for performance, you're just going to have to put it back in in a compatibility shim, so why not just make it part of the contract?  free(), operator delete, and LocalFree() all do (though GlobalFree() is oddly silent on this point).

  6. Anonymous says:

    I'd prefer to say that "[t]his concept goes by the fancy name of Common Sense."

    (On the other hand, I do get driven crazy by people checking for a null pointer before a delete since delete a null pointer is defined behavior.)

  7. Anonymous says:

    I guess the real question is why is this undefined behavior?  If there is special handling for NULL, why not just document it?  If there is not special handling for NULL, why specifically call it out?  Wouldn't it just fail with the same error code as any other invalid pointer?

    [It does fail with the same error code as any other invalid pointer: Passing an invalid pointer invokes undefined behavior! -Raymond]
  8. SimonRev says:

    Now, I do agree the previous posters — free(NULL) is defined behavior, delete NULL is defined behavior as well, so it is non-intuitive that HeapFree would not be.  In particular Adam makes a compelling argument to update the documentation to reflect the actual behavior.

    [Um, API documentation does not describe actual behavior. API documentation describes contractual behavior. -Raymond]
  9. Anonymous says:

    This is the common feature in the Computer sort of culture. There is no undocumented behavior, there is only undocumented feature. The road warrior should go and find all of them!!!!

  10. Anonymous says:

    char *buffer

    sprintf(buffer, "%d", intValue);

    IIRC LLVM will recognize that this is undefined behavior, assume it's dead code, and optimize it to a no-op.

  11. Anonymous says:

    [It does fail with the same error code as any other invalid pointer: Passing an invalid pointer invokes undefined behavior! -Raymond]

    I don't understand how returning a consistent error code is "undefined behavior", but I digress.  My point is that the documentation explicitly calls out NULL as invoking undefined behavior, when in reality ANY invalid pointer invokes undefined behavior.  I understand NULL has a somewhat special status, but from the perspective of this particular contract NULL is not treated any differently from any other invalid pointer.  I would prefer the documentation said something like "Invalid pointers invoke undefined behavior" rather than "NULL invokes undefined behavior", though I suppose by the very nature of taking a pointer it is implied to be a valid pointer.  Maybe I'm just hanging on the semantics too much.

  12. Anonymous says:

    There may be consequences of changing the contract after it has been written. For example, people who detour the heap APIs (e.g., for diagnostic purposes) will implement the old contract, not the new one.

    This implies another sort of contract, a "meta-contract", about how the contract could change.

    Here, @SimonRev is believing the meta-contract says you can always tighten the contract (turning undefined behavior into defined behavior). What you are saying implies the meta-contract says the contract is effectively set into stone (you can never turn undefined behavior into defined behavior, at least for this function). But is it really the case, or is it just catering to people who abuse the API? Is the "meta-contract" written down anywhere? If not, should it be?

    Going too far into that direction would mean, for instance, that you can never add a new flag to a function.

  13. Anonymous says:

    @John: isn't NULL a valid null pointer? And even if it is not considered a valid pointer, you know people will still think of it differently (there are three kinds of pointers: valid, invalid, and null), so it is a good idea to always mention it explicitly.

  14. Anonymous says:

    @Cesar: It is not always known at compile time whether or not NULL is a valid pointer. On some ancient systems, malloc() couldn't use NULL internally because the start of the heap might be NULL. GCC has a -fno-delete-null-pointer-checks for a reason.

  15. Anonymous says:

    @John: I think the reason it makes sense to point out NULL being undefined is because it is a very common mistake. While it is also true that passing invalid pointers is also undefined, it isn't as common of a mistake. The documentation isn't required to document all the undefined behavior of a function (Which I guess would be defined undefined behaviors?), but sometimes defining the most common mistakes can be helpful.

    [Indeed. Because technically the documentation doesn't even need to say what happens when you pass NULL, because NULL is not a valid heap block and is therefore already an invalid parameter. Presumably somebody asked that the behavior of NULL be made explicit (because it is a common point of disagreement among emmory managers), so it was made explicit. On the other hand, memory managers are pretty much in agreement that it is a bad idea to pass a non-NULL pointer that does not represent a valid block, so that was never an issue. -Raymond]
  16. Anonymous says:

    @Joshua: No, NULL is defined by the language standard to be a macro that expands to "an implementation-defined null pointer constant", which is not necessarily represented as all zero bits, and in C++, the integer 0 is required to be converted to a null pointer constant.  On those esoteric systems where a null pointer is not represented by all zero bits, it's the compiler's job to translate between the integer 0 in source code (when being used as a pointer type) and the runtime null pointer representation.

    As for systems where a null pointer is all zero bits and where address 0 can be potentially valid, those quite likely predate the ANSI C standard.  Yes, GCC has an option for compiling code for them, but they're not standard C so all bets are off.

  17. Anonymous says:

    @Adam Rosenfield: Does MS-DOS 6 Predate ANSI C? I don't think so. In large memory model, NULL is a valid pointer to the interrupt vector table. Changing NULL to some non-zero value doesn't help here as all pointers are potentially valid (yes even FFFF:FFFF).

  18. Mott555 says:

    Here's an idea for future versions of Windows. If you have functions with known "undefined" behavior and the caller passes in values that would cause undefined state/behavior, have it generate a random integer and cast it to a pointer of the return type, then return it. People will learn really quickly not to depend on undefined behavior.

    [Who are these "people" you are trying to "teach a lesson"? Certainly not the customer, because the lesson the customer takes away is "Don't upgrade to the next version of Windows. It breaks a lot of apps. -Raymond]
  19. Anonymous says:

    @Joshua: Fair point, but it just shows that MS-DOS 6 does not have an ANSI C-compliant implementation.  C89 section 3.2.2.3 states that "An integral constant expression with the value 0, or such an expression cast to type void * , is called a null pointer constant. […] Such a pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function.".

    [It proves nothing, because the only way to generate such a "valid NULL pointer" is to invoke implementation-defined and/or undefined behavior. (malloc will never return a pointer to the interrupt table, and no C object can be allocated there, and dereferencing a NULL pointer results in undefined behavior.) In other words, in order to observe noncompliance, you must already be noncompliant. -Raymond]
  20. Anonymous says:

    I enjoy Steve Yegge's analogy for undefined (of course it's long):

    steve-yegge.blogspot.com/…/programmers-view-of-universe-part-2.html

  21. Joshua Ganes says:

    This is why I always pass the flags HEAP_NO_VOGON_POETRY, HEAP_NO_EMAIL, and HEAP_CROSS_YOUR_FINGERS when calling such a sensitive function.

  22. Anonymous says:

    @Raymond: Time for nitpicking:

    3.14: "object: region of data storage in the execution environment, the contents of which can represent values"

    So, if there is a 'region of data storage' (Memory), which is part of 'the execution environment' (Machine+Software), there IS most certainly a C object. And it obviously can represents 'values' (Address of interrupt handler)

    6.3.2.3 "An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function."

    Well, I think we have our contradiction between any MS-DOS-Machine an the C standard right here.

    But thats all realy academic and aside from language-lawyers, nobody should have to concern himself about that.

    In modern Systems, there's virtual memory and enough address-space to make address 0 represent the null-pointer and blow up anyone trying to dereference it with a highly satisfying resounding boom.

    @Joshua: Which header is that? ;-)

    [The implementation does not include the interrupt table as part of its execution environment. Therefore 0 will never point to an object. If you manage to dereference 0, then the behavior is undefined. Undefined behavior includes "accessing things that are not part of the execution environment." -Raymond]
  23. SimonRev says:

    [Um, API documentation does not describe actual behavior. API documentation describes contractual behavior. -Raymond]

    We know that Raymond, I don't think anyone here disagrees with that point.  

    Our point is that in this *specific* case the contractual behavior is odd (when compared to similar examples).  Given that the actual behavior mimics the expected behavior, why not extend the contract to cover that common case — it brings the contractual behavior inline with the expected behavior and does require any changes to anyone's code.

    I have no doubt that there are plenty of other examples of this phenomenon where everyone would have been lining up agreeing wholeheartedly with you.

    [There may be consequences of changing the contract after it has been written. For example, people who detour the heap APIs (e.g., for diagnostic purposes) will implement the old contract, not the new one. -Raymond]
  24. Anonymous says:

    Larry Osterman suggests also that passing NULL might format your hard-drive (blogs.msdn.com/…/the-case-of-the-inconsistent-right-shift-results.aspx)

    Assuming you are running with administrative privileges.

  25. Anonymous says:

    Quantum mechanics suggests that passing even a valid pointer as a limited user might result in a reformat your hard drive, though the chance is vanishly small.

  26. Anonymous says:

    @Silly: Hopefully you juts didn't called "Norman Diamond"…

  27. Anonymous says:

    "And the value returned by HeapFree on Windows Vista is irrelevant. Pretending to succeed is a valid form of undefined behavior, because anything qualifies as undefined behavior"

    This sentence alone makes me think that suddenly saying that passing in NULL has a defined behaviour is a bad idea. What if the behaviour is really undefined on XP or Windows 7? It's all very nice that Vista appears to always return success but that doesn't mean all Windowses do that.

  28. cheong00 says:

    On the other hand, I do get driven crazy by people checking for a null pointer before a delete since delete a null pointer is defined behavior.

    Actually, since my job have to concurrently work on different programming language that have diifferent rules on freeing objects, to play safe I always wrap these statements with a check for null.

  29. Anonymous says:

    Even though passing NULL works safely, it's still an error. In debug mode freeing NULL can trigger a debug exception so the debugger can catch it.

    Thus, the behavior is undefined. Because in release mode it's a no-op. But in debug mode, it generates a debugger exception. Maybe in checked mode it does something else. Etc.

  30. Anonymous says:

    If the doc explicitly says "NULL is not allowed" then why wasting time to investigate this? Just write this one-line wrapper function, and move on. It's a very simple wrapper and you don't get any reasonable benefit by using HeapFree directly. So why bother?

  31. Medinoc says:

    While I was swayed by Raymond's argument that it's too late to change the function's contract (I didn't anticipate someone "reading it from the other side"©), one has to admit that it's quite counter-intuitive not to define HeapFree(NULL) as a no-op in the first place.

    HOWEVER, not even the C standard is consistent about this! While free(NULL) is a no-op, fclose(NULL) is an undefined behavior (that causes a crash via Invalid Parameter Handler if tried on Visual Studio's CRT)!

  32. Medinoc says:

    PS: The link about the right shifting makes me glad the C# folks actually defined the behavior.

  33. Anonymous says:

    @Medinoc: You do know that free(NULL) wasn't always defined, right?

  34. Anonymous says:

    @John: "I don't understand how returning a consistent error code is "undefined behavior"…"  

    Are you talking about observed behavior or contractual behavior when you say "consistent"?  In practice, one version of one compiler will probably always do the same thing in the same circumstances, but that doesn't make it defined behavior.

  35. Anonymous says:

    @Original customer from the post: Consistently giving the same error code to me seems about as undefined as consistently crashing (or any other action, really): because the results are undefined, you're not supposed to rely on the outcome. Actually, you're not supposed to be intentionally invoking undefined behaviour because it's going to give Raymond and co. massive support headaches later on. Don't contribute to the compatibility messes that already exist! If you want examples, look through the archives!

Comments are closed.