Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Catch and flag allocations of 0 bytes. #3180

Open Quuxplusone opened 16 years ago

Quuxplusone commented 16 years ago
Bugzilla Link PR2899
Status NEW
Importance P normal
Reported by Sean McBride (sean@rogue-research.com)
Reported on 2008-10-16 13:21:11 -0700
Last modified on 2011-08-09 17:35:44 -0700
Version unspecified
Hardware Macintosh MacOS X
CC daniel@zuster.org, kremenek@apple.com, llvm-bugs@lists.llvm.org, tom.care@uqconnect.edu.au
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
It is generally accepted that allocating 0 bytes is bad practice.  For example,
see:

https://www.securecoding.cert.org/confluence/display/seccode/MEM04-C.+Do+not+perform+zero+length+allocations

Consider this code:

int main(int argc, char *argv[])
{
    char* foo = malloc(0);
    for (unsigned i = 0; i < PAGE_SIZE * 2; i++)
    {
        foo[i] = 0;
    }
     return 0;
}

With guard malloc on, it only crashes when i is 16.

It would be nice if the static analyzer could flag situations where the size
passed to malloc/new/NewPtr/NewHandle/etc. is provably 0.

The URL above mentions that some tools can do this:
"Compass/ROSE can some violations of this rule. Is particular, it warns when
when the argument to malloc() is a variable that has not been compared against
NULL, or is known at compile time to be 0."
Quuxplusone commented 13 years ago

_Bug 8333 has been marked as a duplicate of this bug._

Quuxplusone commented 13 years ago

cloned to rdar://problem/8673307

Quuxplusone commented 13 years ago

"""It is generally accepted that allocating 0 bytes is bad practice"""

I personally disagree very strongly with this sentiment.

From a mathematical standpoint many algorithms are well defined when allocating 0 bytes. Avoiding that case means introducing unnecessary branches, and obscures the underlying mathematical nature of the code.

Quuxplusone commented 13 years ago

Daniel, on the other hand, the arguments in the cert.org link I gave are pretty compelling, in my opinion:

"When the requested size is zero the behavior of the memory allocation functions malloc(), calloc(), and realloc() is implementation-defined. According to C99, Section 7.20.3 [ISO/IEC 9899:1999]

If the size of the space requested is zero, the behavior is implementation-defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object.

In addition, the amount of storage allocated by a successful call to the allocation function when 0 bytes was requested is unspecified. See unspecified behavior 40 in section J.1 of the standard.

In cases where the memory allocation functions return a non-null pointer, reading from or writing to the allocated memory area results in undefined behavior. Typically, the pointer refers to a zero-length block of memory consisting entirely of control structures. Overwriting these control structures will damage the data structures used by the memory."

Quuxplusone commented 13 years ago

The basic warning is now implemented for malloc() calls:

rdar://problem/8673307 [PR2899] Catch and flag allocations of 0 bytes.

We can assess how much noise this has in practice, and reassess.

Quuxplusone commented 13 years ago
(In reply to comment #5)
> The basic warning is now implemented for malloc() calls:
>
> <rdar://problem/8673307> [PR2899] Catch and flag allocations of 0 bytes.
>
> We can assess how much noise this has in practice, and reassess.

Meant to paste the revision:

http://llvm.org/viewvc/llvm-project?view=rev&revision=119364
Quuxplusone commented 13 years ago

clone to: rdar://problem/8673804

to cover the non-malloc cases.

Quuxplusone commented 13 years ago

FWIW: this new warning was triggered 3 times in my code, no false positives. Awesome!