13

It has been my observation that if free( ptr ) is called where ptr is not a valid pointer to system-allocated memory, an access violation occurs. Let's say that I call free like this:

LPVOID ptr = (LPVOID)0x12345678;
free( ptr );

This will most definitely cause an access violation. Is there a way to test that the memory location pointed to by ptr is valid system-allocated memory?

It seems to me that the the memory management part of the Windows OS kernel must know what memory has been allocated and what memory remains for allocation. Otherwise, how could it know if enough memory remains to satisfy a given request? (rhetorical) That said, it seems reasonable to conclude that there must be a function (or set of functions) that would allow a user to determine if a pointer is valid system-allocated memory. Perhaps Microsoft has not made these functions public. If Microsoft has not provided such an API, I can only presume that it was for an intentional and specific reason. Would providing such a hook into the system prose a significant threat to system security?

Situation Report

Although knowing whether a memory pointer is valid could be useful in many scenarios, this is my particular situation:

I am writing a driver for a new piece of hardware that is to replace an existing piece of hardware that connects to the PC via USB. My mandate is to write the new driver such that calls to the existing API for the current driver will continue to work in the PC applications in which it is used. Thus the only required changes to existing applications is to load the appropriate driver DLL(s) at startup. The problem here is that the existing driver uses a callback to send received serial messages to the application; a pointer to allocated memory containing the message is passed from the driver to the application via the callback. It is then the responsibility of the application to call another driver API to free the memory by passing back the same pointer from the application to the driver. In this scenario the second API has no way to determine if the application has actually passed back a pointer to valid memory.

Jim Fell
  • 13,750
  • 36
  • 127
  • 202
  • 27
    Can't you just write sane code instead? Free everything you allocate, and nothing else. – jalf Jan 04 '11 at 16:09
  • 3
    At one point you must have allocated the memory. So you should know the value already. The need for testing the validity of memory to free tells a long story about the code quality. My suggestion would be to sanitize your memory handling before trying to solve this particular issue. – Kosi2801 Jan 04 '11 at 16:10
  • You can cure one of the possible root of this issue with a safe free : http://en.wikipedia.org/wiki/Dangling_pointer – Matthieu Jan 04 '11 at 16:17
  • You might want to do some digging into the internals of RtlHeap, Microsoft's heap implementation. Note that a single allocation using `HeapAlloc` could very well correspond to several allocations done by the C runtime (which provides `malloc`). It doesn't keep a list of all free memory or any kind of analysis like this -- it has lists of free memory blocks and hands them out as requested, but it by no means maintains a complete picture of memory layout. Plus, usually there is at least one heap per DLL, and the heaps don't know each other (i.e. a valid pointer in heap A is not valid in heap B) – Billy ONeal Jan 04 '11 at 16:21
  • @Jim Fell: I read your edit but I fail to see how it changes the situation. I realize that you want this to be as robust as possible, but it is apparently the application's responsibility to not screw up with respect to memory management (in other words, it is a part of the contract of the interface). It's not much different from the requirement that you have to pass in something valid for `free()` and `delete` or their behavior will be undefined. – In silico Jan 04 '11 at 16:46
  • If the message only has to exist for the duration of the callback, why not call back with a pointer to a stack allocated instance, and modify your free method in the other API to be a no-op? – Nim Jan 04 '11 at 16:48
  • @Jim Fell: See my edit based on your update – John Dibling Jan 04 '11 at 17:02

13 Answers13

22

There's actually some functions called IsBadReadPtr(), IsBadWritePtr(), IsBadStringPtr(), and IsBadCodePtr() that might do the job, but do not use it ever. I mention this only so that you are aware that these options are not to be pursued.

You're much better off making sure you set all your pointers to NULL or 0 when it points to nothing and check against that.

For example:

// Set ptr to zero right after deleting the pointee.
delete ptr; // It's okay to call delete on zero pointers, but it
            // certainly doesn't hurt to check.

Note: This might be a performance issue on some compilers (see the section "Code Size" on this page) so it might actually be worth it to do a self-test against zero first.

ptr = 0;

// Set ptr to zero right after freeing the pointee.
if(ptr != 0)
{
    free(ptr); // According to Matteo Italia (see comments)
               // it's also okay to pass a zero pointer, but
               // again it doesn't hurt.
    ptr = 0;
}

// Initialize to zero right away if this won't take on a value for now.
void* ptr = 0;

Even better is to use some variant of RAII and never have to deal with pointers directly:

class Resource
{
public:
    // You can also use a factory pattern and make this constructor
    // private.
    Resource() : ptr(0)
    {
        ptr = malloc(42); // Or new[] or AcquiteArray() or something
        // Fill ptr buffer with some valid values
    }

    // Allow users to work directly with the resource, if applicable
    void* GetPtr() const { return ptr; }

    ~Resource()
    {
        if(ptr != 0)
        {
            free(ptr); // Or delete[] or ReleaseArray() or something

            // Assignment not actually necessary in this case since
            // the destructor is always the last thing that is called
            // on an object before it dies.
            ptr = 0;            
        }
    }

private:
    void* ptr;
};

Or use the standard containers if applicable (which is really an application of RAII):

std::vector<char> arrayOfChars;
In silico
  • 51,091
  • 10
  • 150
  • 143
  • 2
    IsBadReadPtr has nothing to do with whether the pointer points to a beginning of a valid heap-allocated block. – zeuxcg Jan 04 '11 at 16:13
  • 1
    @In silico: +1 for "DO NOT USE IT EVER". @zeuxcg: Why does the beginning of a heap allocated block matter? Pointers later in the block are still perfectly valid of course. (Might not be for `free` specifically, but the question is asking about the general case) – Billy ONeal Jan 04 '11 at 16:16
  • @zeuxcg: I know that, but again, I mention them only because someone is going to stumble across them and think they can help. They do not. – In silico Jan 04 '11 at 16:16
  • @Billy ONeal Because the question is about free(), and the majority of readable pointers are not safe to pass to free. – zeuxcg Jan 04 '11 at 16:19
  • 1
    @In silico: +1, BTW it's okay to call also `free` on NULL pointers. – Matteo Italia Jan 04 '11 at 16:33
  • 2
    assigning zero to pointer most likely achieves nothing, because nobody would pass non-const l-value references to pointers, rather you got pointer passed by value and you are zeroing a copy, which you quickly discard. – Gene Bushuyev Jan 04 '11 at 17:10
17

Short answer: No.

There is a function in windows that supposedly tells you if a pointer points to real memory (IsBadreadPtr() and it's ilk) but it doesn't work and you should never use it!

The true solution to your problem is to always initialize pointers to NULL, and reset them to NULL once you've deleted them.

EDIT based on your edits:

You're really hinting at a much larger question: How can you be sure your code continues to function properly in the face of client code that screws up?

This really should be a question on its own. There are no simple answers. But it depends on what you mean by "continue to function properly."

There are two theories. One says that even if client code sends you complete crap, you should be able to trudge along, discarding the garbage and processing the good data. A key to accomplishing this is exception handling. If you catch an exception when processing the client's data, roll your state back and try to return as if they had never called you at all.

The other theory is to not even try to continue, and to just fail. Failing can be graceful, and should include some comprehensive logging so that the problem can be identified and hopefully fixed in the lab. Kick up error messages. Tell the user some things to try next time. Generate minidumps, and send them automatically back to the shop. But then, shut down.

I tend to subscribe to the second theory. When client code starts sending crap, the stability of the system is often at risk. They might have corrupted heaps. Needed resources might not be available. Who knows what the problem might be. You might get some good data interspersed with bad, but you dont even know if the good data really is good. So shut down as quickly as you can, to mitigate the risk.

John Dibling
  • 99,718
  • 31
  • 186
  • 324
9

No, you are supposed to know if your pointers point to correctly allocated memory.

sth
  • 222,467
  • 53
  • 283
  • 367
9

To address your specific concern, I don't think you have to worry about checking the pointer. If the application passes your DLL an invalid address, it represents a memory management problem in the application. No matter how you code your driver, you can't fix the real bug.

To help the application developers debug their problem, you could add a magic number to the object you return to the application. When the your library is called to free an object, check for the number, and if it isn't there, print a debug warning and don't free it! I.e.:

#define DATA_MAGIC 0x12345678
struct data {
    int foo;    /* The actual object data. */
    int magic;  /* Magic number for memory debugging. */
};

struct data *api_recv_data() {
    struct data *d = malloc(sizeof(*d));
    d->foo = whatever;
    d->magic = DATA_MAGIC;
    return d;
}

void api_free_data(struct data *d) {
    if (d->magic == DATA_MAGIC) {
        d->magic = 0;
        free(d);
    } else {
        fprintf(stderr, "api_free_data() asked to free invalid data %p\n", d);
    }
}

This is only a debugging technique. This will work the correctly if the application has no memory errors. If the application does have problems, this will probably alert the developer to the mistake. It only works because you're actual problem is much more constrained that your initial question indicates.

Karmastan
  • 5,618
  • 18
  • 24
  • If I understand correctly, this would work because a pointer to a structure and the pointer to the first element in that structure are the same. The application would be oblivious to the other elements of the structure. Very clever! Why would this not work for a release build (without the debug prompts, of course)? – Jim Fell Jan 04 '11 at 17:11
  • @Jim: You could do this two ways: (1) Embed the original data structure as the first element in a new data structure with a magic number (what you're thinking of), or (2) Change the original data structure definition by adding an element. Depending on how the data structure is used, either could work. My code shows (2) being used. It would take a little more work to get (1) to compile without errors. – Karmastan Jan 04 '11 at 17:21
  • @Jim: This will work fine in release builds. Put this technique into the same category as `assert()`. Its only benefit is to find bugs. It does nothing for correct programs except make them slightly slower. Of course, is there such a thing as a correct program? ;) – Karmastan Jan 04 '11 at 17:28
  • Note that accessing `d->magic` will still segfault if `d` is a really invalid pointer like the one in the example in the question. – sth Jan 05 '11 at 01:16
6

No. You are only supposed to have a pointer to memory that you know is valid, usually because you allocated it in the same program. Track your memory allocations properly and then you won't even need this!

Also, you are invoking Undefined Behaviour by attempting to free an invalid pointer, so it may crash or do anything at all.

Also, free is a function of the C++ Standard Library inherited from C, not a WinAPI function.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • 1
    +1 for the last paragraph -- often multiple allocations performed by the CRT are shown as a single allocation to Windows itself. – Billy ONeal Jan 04 '11 at 16:17
5

First of all, in the standard there's nothing that guarantees such a thing (freeing a non-malloced pointer is undefined behavior).

Anyhow, passing by free is just a twisted route to just trying to access that memory; if you wanted to check if the memory pointed by a pointer is readable/writable on Windows, you really should just try and be ready to deal with the SEH exception; this is actually what the IsBadxxxPtr functions do, by translating such exception in their return code.

However, this is an approach that hides subtle bugs, as explained in this Raymond Chen's post; so, long story short, no there's no safe way to determine if a pointer points to something valid, and I think that, if you need to have such a test somewhere, there's some design flaw in that code.

Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
4

I'm not going to echo what every one has already said, just to add to those answers though, this is why smart pointers exist - use them!

Any time you find yourself having to work around crashes due to memory errors - take a step back, a large breath, and fix the underlying problem - it's dangerous to attempt to work around them!

EDIT based on your update:

There are two sane ways that I can think of to do this.

  1. The client application provides a buffer where you put the message, meaning your API does not have worry about managing that memory - this requires changes to your interface and client code.
  2. You change the semantics of the interface, and force the clients of the interface to not worry about memory management (i.e. you call with a pointer to something that is only valid in the context of the callback - if client requires, they make their own copy of the data). This does not change your interface - you can still callback with a pointer, however your clients will need to check that they don't use the buffer outside of that context - potentially if they do, it's probably not what you want and so it could be a good thing that they fix it(?)

Personally I would go for the latter as long as you can be sure that the buffer is not used outside of the callback. If it is, then you'll have to use hackery (such as has been suggested with the magic number - though this is not always guaranteed to work, for example let's say there was some form of buffer overrun from the previous block, and you somehow over-write the magic number with crap - what happens there?)

Nim
  • 33,299
  • 2
  • 62
  • 101
2

Application memory management is up to the application developer to maintain, not the operating system (even in managed languages, the operating system doesn't do that job, a garbage collector does). If you allocate an object on the heap, it is your responsibility to free it properly. If you fail to do so, your application will leak memory. The operating system (in the case of Windows at least) does know how much memory it has let your application have, and will reclaim it when your application closes (or crashes), but there is no documented way (that works) to query a memory address to see if it is an allocated block.

The best suggestion I can give you: learn to manage your memory properly.

Zac Howland
  • 15,777
  • 1
  • 26
  • 42
2

You apparently have determined that you're done with an object that you currently have a pointer to and if that object was malloced you want to free it. This doesn't sound like an unreasonable idea, but the fact that you have a pointer to an object doesn't tell you anything about how that object was allocated (with malloc, with new, with new[], on the stack, as shared memory, as a memory-mapped file, as an APR memory pool, using the Boehm-Demers-Weiser garbage collector, etc.) so there is no way to determine the correct way to deallocate the object (or if deallocation is needed at all; you may have a pointer to an object on the stack). That's the answer to your actual question.

But sometimes it's better to answer the question that should have been asked. And that question is "how can I manage memory in C++ if I can't always tell things like 'how was this object allocated, and how should it be deallocated'?" That's a tricky question, and, while it's not easy, it is possible to manage memory if you follow a few policies. Whenever you hear people complain about properly pairing each malloc with free, each new with delete and each new[] with delete[], etc., you know that they are making their lives harder than necessary by not following a disciplined memory management regime.

I'm going to make a guess that you're passing pointers to a function and when the function is done you want it to clean up the pointers. This policy is generally impossible to get right. Instead I would recommend following a policy that (1) if a function gets a pointer from somebody else, then that "somebody else" is expected to clean up (after all, that "somebody else" knows how the memory was allocated) and (2) if a function allocates an object, then that function's documentation will say what method should be used to deallocate the object. Second, I would highly recommend smart pointers and similar classes.

Stroustrup's advice is:

If I create 10,000 objects and have pointers to them, I need to delete those 10,000 objects, not 9,999, and not 10,001. I don't know how to do that. If I have to handle the 10,000 objects directly, I'm going to screw up. ... So, quite a long time ago I thought, "Well, but I can handle a low number of objects correctly." If I have a hundred objects to deal with, I can be pretty sure I have correctly handled 100 and not 99. If I can get then number down to 10 objects, I start getting happy. I know how to make sure that I have correctly handled 10 and not just 9."

For instance, you want code like this:

#include <cstdlib>
#include <iostream>
#include "boost/shared_ptr.hpp"

namespace {
    // as a side note, there is no reason for this sample function to take int*s
    // instead of ints; except that I need a simple function that uses pointers
    int foo(int* bar, int* baz)
    {
        // note that since bar and baz come from outside the function, somebody
        // else is responsible for cleaning them up
        return *bar + *baz;
    }
}

int main()
{
    boost::shared_ptr<int> quux(new int(2));

    // note, I would not recommend using malloc with shared_ptr in general
    // because the syntax sucks and you have to initialize things yourself
    boost::shared_ptr<int> quuz(reinterpret_cast<int*>(std::malloc(sizeof(int))), std::free);
    *quuz = 3;

    std::cout << foo(quux.get(), quuz.get()) << '\n';
}
Max Lybbert
  • 19,717
  • 4
  • 46
  • 69
  • Why the downvote? Resource management is, in fact, manageable. And the underlying problem is, in fact, undisciplined use of resources. And the fact that pointers can point to memory allocated via things other than `malloc` is, in fact, relevant. – Max Lybbert Jan 04 '11 at 20:53
1

Not without access to the internals of the malloc implementation.

You could perhaps identify some invalid pointers (e.g., ones that don't point anywhere within your process's virtual memory space), but if you take a valid pointer and add 1 to it, it will be invalid for calling free() but will still point within system-allocated memory. (Not to mention the usual problem of calling free on the same pointer more than once).

David Gelhar
  • 27,873
  • 3
  • 67
  • 84
  • Even with access to the internal malloc implementation, there's still no general case way to do this (unless that malloc implementation maintains a list of all allocated pointers in the history of the program) – Billy ONeal Jan 04 '11 at 16:17
1

Aside from the obvious point made by others about this being very bad practice, I see another problem.

Just because a particular address doesn't cause free() to generate an access violation, does not mean it's safe to free that memory. The address could actually be an address within the heap so that no access violation occurs, and freeing it would result in heap corruption. Or it might even be a valid address to free, in which case you've freed some block of memory that might still be in use!

You've really offered no explanation of why such a poor approach should even be considered.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
0

Why would 0x12345678 necessarily be invalid? If your program uses a lot of memory, something could be allocated there. Really, there's only one pointer value you should absolutely rely on being an invalid allocation: NULL.

Karmastan
  • 5,618
  • 18
  • 24
0

C++ does not use 'malloc' but 'new', which usually has a different implementation; therefore 'delete' and 'free' can't be mixed – neither can 'delete' and 'delete[]' (its array-version).

DLLs have their own memory-area and can't be mixed with the memory management system of the non-DLL memory-area.

Every API and language has its own memory management for its own type of memory-objects. I.e.: You do not 'free()' or 'delete' open files, you 'close()' them. The same goes for very other API, even if the type is a pointer to memory instead of a handle.

comonad
  • 5,134
  • 2
  • 33
  • 31