Re: [PATCH] avoid calling memset et al. with excessively large sizes (PR 79095)

[Jeff Law <law@redhat.com>]

On 01/17/2017 09:12 AM, Martin Sebor wrote:
> On 01/17/2017 08:26 AM, Jeff Law wrote:
>> On 01/16/2017 05:06 PM, Martin Sebor wrote:
>>> The test case submitted in bug 79095 - [7 regression] spurious
>>> stringop-overflow warning shows that GCC optimizes some loops
>>> into calls to memset with size arguments in excess of the object
>>> size limit.  Since such calls will unavoidably lead to a buffer
>>> overflow and memory corruption the attached patch detects them
>>> and replaces them with a trap.  That both prevents the buffer
>>> overflow and eliminates the warning.
>> But doesn't the creation of the bogus memset signal an invalid
>> transformation in the loop optimizer?  ie, if we're going to convert a
>> loop into a memset, then we'd damn well better be sure the loop bounds
>> are reasonable.
>
> I'm not sure that emitting the memset call is necessarily a bug in
> the loop optimizer (which in all likelihood wasn't written with
> the goal of preventing or detecting possible buffer overflows).
> The loop with the excessive bound is in the source code and can
> be reached given the right inputs (calling v.resize(v.size() - 1)
> on an empty vector.  It's a lurking bug in the program that, if
> triggered, will overflow the vector and crash the program (or worse)
> with or without the optimization.
Right, but that doesn't mean that the loop optimizer can turn it into a 
memset.  If the bounds are such that we're going to invoke undefined 
behaviour from memset, then the loop optimizer must leave the loop alone.

>
> What else could the loop optimizer could do in this instance?
> I suppose it could just leave the loop alone and avoid emitting
> the memset call.  That would avoid the warning but mask the
> problem with the overflow.  In my mind, preventing the overflow
> given that we have the opportunity is the right thing to do.
> That is, after all, the goal of the warning.
The right warning in this case is WRT the loop iteration space 
independent of mem*.


>
> As I mentioned privately yesterday, I'm actually pleasantly
> surprised that it's helped identify this opportunity in GCC itself.
> My hope was to eventually go and find the places where GCC emits
> potentially out of bounds calls (based on user inputs) and fix them
> to emit better code on the assumption that they can't be valid or
> replace them with traps if they could happen in a running program.
> It didn't occur to me that the warning itself would help find them.
>
> Martin
>