If I’m not supposed to call IsBadXxxPtr, how can I check if a pointer is bad?


Some time ago, I opined that Is­Bad­Xxx­Ptr should really be called Crash­Program­Randomly and you really should just let the program crash if somebody passes you a bad pointer. It is common to put pointer validation code at the start of functions for debugging purposes (as long as you don't make logic decisions based on whether the pointer is valid). But if you can't use Is­Bad­Xxx­Ptr, how can you validate the pointer?

Well, to validate a write pointer, write to it. To validate a read pointer, read from it. If the pointer is invalid, you'll crash, and at a predictable location, before the function has gotten halfway through its processing (making post-mortem debugging more difficult). Here are the functions I used:

// Make sure to disable compiler optimizations in these functions
// so the code won't be removed by the optimizer.
void DebugValidateWritePtr(void *p, size_t cb)
{
 memcpy(p, p, cb);
}

void DebugValidateReadPtr(void *p, size_t cb)
{
 memcmp(p, p, cb);
}

To verify that a buffer can be written to, we write to it by copying it to itself. Similarly, to verify that a buffer can be read, we read from it by comparing it to itself. The result of the operation is not important; we are interested in the side-effect of the memory access itself.

Note that the Debug­Validate­Write­Ptr function is not thread-safe: If another thread modifies the buffer while we are copying it to itself, the write may be lost. But code that does this violates one of the ground rules for programming (specifically the parameter stability requirements). Of course, if your function has specific behavior requirements beyond the ground rules, then that helper function may not work for you. I'm just putting it out there as a courtesy.

Comments (21)
  1. Zarat says:

    From MSDN:

    "If the source and destination overlap, the behavior of memcpy is undefined. Use memmove to handle overlapping regions."

    So the DebugValidateWritePtr should use memmove to work as intended, I guess

  2. Zarat says:

    (Sorry for double posting, I thought of this just right after clicking "Post")

    On the other hand memmove might do a quick check if src and dst are identical and skip the whole operation. While this is valid for the semantics of memmove it isn't what was wanted. So using either memcpy or memmove for this purpose will rely on their implementation.

  3. Kyle S. says:

    How does this avoid the problematic guard page example from the original article?

    [It doesn't. That's why it's called DebugValidateWritePtr and not ValidateWritePtr. See second sentence of this article. And the entire final paragraph. There must be some Internet law for this: No matter how many caveats you put in your article, somebody will ignore them all. -Raymond]
  4. Someone says:

    My C(++) is a bit rusty, but I think you can prevent unwanted interactions with compiler optimizations by using volatile here.

    void DebugValidateWritePtr(void volatile *p, size_t cb)

    {

    memcpy(p, p, cb);

    }

    if that requires casting, wrap this thing in a function that does that casting for you.

    I also think you will have to roll your own memcpy and memcmp if you want to prevent the compiler from special-casing these, as a C++ compiler is allowed to know what standard library calls do. (a typical use is in optimizing "for( int i = 0; i < strlen(s); ++i){}".

  5. Paul M. Parks says:

    @Someone: If this function is only being used in debugging, it's usually the case that optimizations are not enabled (not always, but usually). I don't know about everyone else, but by the time I get around to working with optimized code I have all my debug-only code removed.

  6. sukru says:

    memcpy behavior is undefined, since it relies on the copy direction. for example if you where you shift the following buffer to right by tow items:

    1,2,3,4,5

    and do it from left to right it would become

    1,1,1,1,1,1,1

    on the other hand from right to left you get

    1,2,1,2,3,4,5

    that's what memmove automatically handles.

  7. AC says:

    Why don't the IsBadXxxPtr functions use some kernel functions to check if the memory is valid? (Instead of "try and see if it fails".) The kernel certainly has the ability to check that somehow.

    Apart from that there is no point in using them anyway. If someone passes in valid but "wrong" pointers you might overwrite vital data and then will be crashing anyway somewhere. That's the callee's fault, though.

  8. benjamin says:

    If I'm not supposed to call IsBadXxxPtr, how can I check if a pointer is bad?

    Practice.

  9. Jonathan Pryor says:

    Given the undefined behavior of memcpy here, and the fact that at least some memmove implementations that Google finds don't actually do anything if source == dest, C++ templates might be the better idea:

    template <typename T>

    inline void DebugValidateWritePtr(T* p)

    {*p = *p;}

    This likely isn't ideal either — for non-pointer types it'll invoke T::operator=, which often will check for assignment against 'this' and do nothing — but at least it's a start, doesn't rely on undefined behavior, and will actually perform the write.

    Unfortunately, it's C++.  I suppose we could use a macro for C:

    #define DEBUG_VALIDATE_WRITE_PTR(p) do {*p = *p} while(0)

    though not everyone likes heavy use of macros.

  10. Joshua says:

    @Someone: Your C++ is rusty. Volatile does not inherit through function calls like that *and* the memcpy operation is inherently thread unsafe so volatile won't help.

  11. fred says:

    For anyone concerned about the use of memcpy() in an undefined manner or that it might get optimized away (even though both are probably not concerns in the intended debug scenarios), here's an alternative that should take care of those concerns (I sure hope I didn't bug this somehow…):

    <pre>

    void DebugValidateWritePtr(void* pv, size_t cb)

    {

       char volatile* p = (char*) pv;

       for (;cb != 0; –cb, ++p) {

           char c = *p;

           *p = c;

       }

    }

    </pre>

  12. James says:

    "This idea is fraught with peril, and I fear that my answers to your questions will be interpreted as approval rather than reluctant assistance."

  13. Worf says:

    Is there any reason why we can't use memset rather than memcpy?

    Since it's debug only, setting it to a strange pattern could help debugging odd crashes if someone accesses the buffer while it isn't owned…

    [memset would not be very nice to in/out buffers which are writable but whose contents are still meaningful. -Raymond]
  14. Igor Levicki says:

    So Raymond, are you saying that doing:

    int some_func(void *p)

    {

       if (p == NULL) {

           return E_BADARG;

       }

       // … some code …

       return E_OK;

    }

    Is also a bad idea/practice? Because a lot of people are writing such code, and they are checking the return value from some_func().

    True, the above code won't catch a stray pointer (the one which is not NULL but is invalid anyway), but if you initialize your variables and return proper values on error you should not have such pointers anyway unless your hardware is broken.

  15. programmer says:

    Igor this hardly does any harm but also hardly any good.

    Other opinions may differ, but I does not consider this to be worth the extra if.

  16. avakar says:

    So what exactly is the difference between DebugValidateXXXPtr(p, s) and assert(IsBadXXXPtr(p, s))? Doesn't IsBadXXXPtr touch each byte of the range, just like DebugValidateXXXPtr?

  17. AndyC says:

    @avakar, the difference is that IsBadXXXPtr conceals the original exception, wheras DebugValidateXXXPtr crashes instead allowing you to much more clearly debug the problem.

  18. Leo Davidson says:

    avakar: Read the original article that Raymond links to. It messes up the guard pages and means things like your program potentially crashing next time a thread needs to extend its stack.

  19. Miral says:

    In addition to the problems with memcpy that others have already covered, that usage is not safe if the function is given a write-only pointer.  Granted, those are pretty unusual, but for most "output-only" type functions they shouldn't be illegal, either.

    Igor: explicitly validating parameters within the function is ok if it's part of the function's contract that it needs to be "safe" when given null data.  If it's unspecified, or specified to only accept good data, then crashing on being given a null is usually (but not always) the better thing to do, since it's easier to find a crash than it is to find an error code return value, especially if the app ignores it (as many do).

  20. Paul M. Parks says:

    @Igor: Miral answered, but I'll provide an example. I maintain a library of API functions that accept structures of data, some fields of which contain pointers. If a pointer is NULL, I know not to take any action on it, but if the pointer is non-NULL I have to assume that it is valid, and crash otherwise. NULL is a known state, and actionable. Non-NULL has to indicate a valid pointer (leaving out the ugly case of non-NULL flag values).

    So, no, testing for NULL is not the same as DebugValidateXXXPtr.

  21. Gabe says:

    avakar: IsBadXxxPtr most likely only touches 1 byte per page (4k or 8k), rather than every byte.

Comments are closed.