[PATCH] emit a trap for buffer overflow in placement new

[Jonathan Wakely]

On 05/12/17 09:48 +0100, Marc Glisse wrote:
>On Mon, 4 Dec 2017, Martin Sebor wrote:
>
>>On 12/04/2017 03:44 PM, Marc Glisse wrote:
>>>On Mon, 4 Dec 2017, Martin Sebor wrote:
>>>
>>>>The -Wplacement-new option warns for buffer overflow in placement
>>>>new expressions with objects of constant sizes, but because it's
>>>>implemented completely in the C++ front end it misses the more
>>>>interesting non-constant sizes.
>>>>
>>>>The attached patch instruments both forms of operator placement
>>>>new to emit a trap when __builtin_object_size() determines that
>>>>the pointer points to an object less than the specified number
>>>>of bytes.  This is done only when _FORTIFY_SOURCE is defined
>>>>to a non-zero value.  This makes it possible to prevent buffer
>>>>overflow in most of the same cases as in built-ins like strcpy,
>>>>though without warnings when the size is nor a C++ constant
>>>>integer expression.
>>>>
>>>>On x86_64-linux it passes testing with no apparent regressions.
>>>>Can anyone think of problems with this solution?  If not, given
>>>>its simplicity, would it be appropriate even at this stage?
>>>
>>>AFAIK, one can call this operator new manually on any pointer, including
>>>one-past-the-end pointers and null pointers. It is only with new
>>>expressions that the limitation comes in (because it runs a constructor
>>>afterwards). Not that people often do that...
>>
>>Hmm, yes, there don't seem to be any requirements on calling
>>the operator by itself.  That's too bad.  I was going to move
>>the placement new checking into the middle-end to improve
>>the detection and avoid some false positives but the operator
>>disappears too early, before the size of the expression that's
>>being stuffed into the destination is known.
>
>What's wrong with generating the same code you wanted to put in 
>operator new from the front-end, next to the call to operator new, 
>when the front-end handles a new expression?
>
>>The only other solution that comes to mind to detect non-constant
>>overflow is to do something like:
>>
>> void* operator new (size_t n, void *p)
>> {
>>   if (__builtin_object_size (p, 0) < n)
>>    __builtin_warn ("buffer overflow in placement new");
>>   return p;
>> }
>>
>>and emit the warning from __builtin_warn during expansion.  That
>>should work and be usable in other contexts as well (e.g., to
>>implement overflow and other error detection in user-defined
>>functions).  It would be kind of like the middle-end form of
>>Clang attribute diagnose_if.
>
>I may have misunderstood, but that seems to have the same issue of 
>applying to all calls to operator new instead of just new expressions.

Yes, but if it's only a warning then at least it doesn't cause
compilation to fail in the (probably very rare) cases where people are
doing something strange and calling it directly with a too-small
pointer.