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

[Marc Glisse]

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.

> Still, I wonder if tightening up the standard to require that
> the pointer point to an object of at least n bytes in size would
> run into problems or be met with objections.

-- 
Marc Glisse