[SOLVED] Implementation of free in C++11 using GNU's g++ 4.8.2

sundialsvcs · 05-07-2014, 11:33 AM

Also ... even with all the goodness that C++ provides in an attempt to make the process of handling memory "easier," there are still a few points to keep in mind:

(1) Never "mix metaphors." When in C++ land, do as C++ does. Do not call C-style memory allocation routines directly.

(2) It is always a good idea to make sure that allocated memory blocks contain all-zero, even if you find that you must memset() it yourself.

(3) When you free or otherwise dispose-of a memory block, you should always explicitly set the pointer's value to NULL, in the very next statement!.

In general, two things will burn you in the

... a "stale pointer," and a memory-block which contains unpredictable but garbage content. You need to make a (self-imposed) cardinal rule which says: "if a pointer is not-NULL, it must point to something valid. Otherwise, it must be known-NULL."

It's usually easy to accomplish these three things, and more, with a simple "wrapper" library that enhances calls to the underlying system-library routines. For instance,immediately before deallocating a block of memory, they fill it with the byte-string $DEADBEEF.

(And, it peeks at addresses to thereby detect, and prevent, "double frees.") It also puts "eyecatchers" on both sides of each allocated block, and wipes-out (changes ...) those eyecatchers upon free. So, if storage gets "scribbled," bang ... the problem shows-up immediately.

The overhead of all this is so negligible that I routinely "leave it in." It's tremendously useful for an application to be able to provide these kinds of self-safeguards "in production," and it's basically free-of-cost to be able to do just that.

By imposing just a little bit of additional discipline upon your program's use of dynamic storage, you can greatly improve its reliability and diagnosability. "Unpredictable memory-content is not your friend."

mina86 · 05-07-2014, 12:03 PM

Quote:

Originally Posted by NevemTeve

Clearing the malloc-ed/new-ed memory still might be a good idea, as valgind seems to read your memory-areas, so non-initialized memories might cause false valgrind messages.

This may cause a legitimate warnings to be silenced though, because Valgrind will always assume the memory is initialised (since it has been zeroed).

Besides, new calls constructor of the type anyway.

Quote:

Originally Posted by sundialsvcs

(2) It is always a good idea to make sure that allocated memory blocks contain all-zero, even if you find that you must memset() it yourself.

I must disagree. It is a good idea to initialise the memory to sensible state, but that does not have, and often is not, “all-zero”. Also, for more complex types the term “all-zero” is meaningless. And like I've said, new will call constructor for you. You're statement is just too simplistic to be true in general.

Quote:

Originally Posted by sundialsvcs

(3) When you free or otherwise dispose-of a memory block, you should always explicitly set the pointer's value to NULL, in the very next statement!.

This of course assuming the pointer will hang around for a while. It's usually no point in setting pointer to NULL in a destructor for instance or at the end of a function.

Either way, as it was mentioned earlier, std::unique_ptr is your friend. If used correctly, it makes it possible to never have to call delete oneself, which greatly reduces risks of memory leaks.

Quote:

Originally Posted by sundialsvcs

I often work with debugging-routines that go even farther than this. Immediately before deallocating a block of memory, they fill it with the byte-string $DEADBEEF.

(And, it peeks at addresses to thereby detect, and prevent, "double frees.") It also puts "eyecatchers" on both sides of each allocated block, and wipes-out (changes ...) those eyecatchers upon free. So, if storage gets "scribbled," bang ... the problem shows-up immediately. The overhead of all this is so negligible that I routinely "leave it in."

This however should not be part of the code that calls new/delete. If so desired, new could be overwritten to do all those things, but expecting programmer to do this when dealing with dynamic data is ridiculous.

sundialsvcs · 05-08-2014, 12:57 PM

Quote:

I must disagree. It is a good idea to initialise the memory to sensible state, but that does not have, and often is not, “all-zero”. Also, for more complex types the term “all-zero” is meaningless. And like I've said, new will call constructor for you. You're statement is just too simplistic to be true in general.

And I'll respectfully stand by my statement. Start by setting the whole memory-block to known-zero, then if necessary run whatever constructors you need, all automagically. This guarantees that the content of the entire block is always known, in all cases. I've lost count of the number of times that has saved my

...

Quote:

This of course assuming the pointer will hang around for a while. It's usually no point in setting pointer to NULL in a destructor for instance or at the end of a function.

Obviously a local variable that is going out-of-scope is, well, going out of scope. Yes. But, watch out about those destructors! Destructors are called in a hierarchy of destructors, and it is quite easy for an outer-level destructor to, unbeknownst to you, "trust" that pointer-value. If the pointer is contained in a property of the object, then yes, I would "immediately set it to NULL."

And my long experience has taught me to be very gun-shy about this sort of thing. I can easily spare a few dozen more micro-seconds, but I have literally lost $10,000.00 (USD) in hard (not "funny") money to a double-free problem caused in ... a nested destructor. The [i](Dephi) code looked like this: (in vendor-supplied code)

Code:

p1.free;
p2.free;
p1 := nil;
p2 := nil;

And the fix was to make it look like this:

Code:

p1.free; p1 := nil;
p2.free; p2 := nil;

But it came too late to save the $10,000.00.

(No comments regarding the remainder of your points raised.)

mina86 · 05-11-2014, 09:38 AM

Quote:

Originally Posted by sundialsvcs

And I'll respectfully stand by my statement. Start by setting the whole memory-block to known-zero, then if necessary run whatever constructors you need, all automagically. This guarantees that the content of the entire block is always known, in all cases. I've lost count of the number of times that has saved my

...

If the all-bit-zero state is not a valid state of an object, setting the whole block to zero does not prevent bugs. Yet it prevents various tools (for example Coverity) from determining whether uninitialised memory is accessed.

sundialsvcs · 05-11-2014, 04:18 PM

It is far better to make sure that uninitialized memory does not get accessed, in the first place. If you are disciplined about making certain that (especially) all dynamically-allocated memory blocks have known NULL content, and if you immediately destroy "stale" pointers the instant they become stale, you will virtually eliminate this concern.

Now, I'm not here to get into any sort of dogmatic argument.

I'm simply describing what works extremely well for me. You adopt the discipline that either a pointer is known to point to a valid, allocated, initialized memory-block, or it is NULL.

I also suggest that you allocate a variable called slop at the end of each memory block. It is literally there to make the block about 8 (known-zero) bytes longer than you know that it needs to be. Almost always, when a memory reference is "off," it's "off by one." Unfortunately, that can "scribble" (ahem ...) "on" the memory-allocation areas used by malloc(), leading to unpredictable crashes. A little "slop" is very cheap insurance. It enables you to find these bugs during testing, and to stomp them out.

ntubski · 05-11-2014, 07:01 PM

Quote:

Originally Posted by mina86

If the all-bit-zero state is not a valid state of an object, setting the whole block to zero does not prevent bugs.

It would make the occurrence of bugs more deterministic though.

Quote:

Yet it prevents various tools (for example Coverity) from determining whether uninitialised memory is accessed.

That seems like a good argument to disable this feature while running such a tool.

ejspeiro · 05-12-2014, 11:55 AM

Dear all, this has been an amazing thread... I will call it solved.

Thanks a lot!

mina86 · 05-12-2014, 12:06 PM

Quote:

Originally Posted by ntubski

That seems like a good argument to disable this feature while running such a tool.

What are we talking about here? Are we talking about code like this one:

Code:

ptr = malloc(sizeof *ptr);
memset(ptr, 0, sizeof *ptr);

or are we talking about a specialised implementation of malloc/free (or new/delete) that zeroes the memory?

In the former case, adding code to be able to disable the zeroing when tools like Valgrind or Coverity are used, is a terrible idea since it complicates the code even more:

Code:

ptr = malloc(sizeof *ptr);
#ifndef DEBUG_BUILD
  memset(ptr, 0, sizeof *ptr);
#endif

If we're talking about the latter, then I would much rather a debug implementation of malloc filled memory with 0xdeadbeef and free with 0xfee1dead. That would make bugs much more apparent.

Adding zeroing inside of malloc would on the other hand make the code less portable since programmer would start depending on the fact that allocated memory is zeroed which is not guaranteed by the standard.