Re: PATCH RFC: Warn about pointer wraparound with -Wstrict-overflow

[Ian Lance Taylor <iant at google dot com>]

"Richard Guenther" <richard.guenther@gmail.com> writes:

> On Mon, Apr 7, 2008 at 9:35 PM, Ian Lance Taylor <iant@google.com> wrote:
>> I'm testing this patch as a response to
>>     http://www.kb.cert.org/vuls/id/162289
>>
>>  This patch treats undefined pointer wraparound optimizations as an
>>  instance of undefined signed overflow optimizations (they are of
>>  course different, but they seem similar to users not educated in
>>  standardese).  You will get a warning with -Wstrict-overflow, and you
>>  can disable the optimization with -fno-strict-overflow.
>>
>>  My plan is to backport this patch to the gcc 4.2 and 4.3 branches.
>
> Please leave at least the 4.2 branch alone.

Why?  The CERT complaint is specifically directed at gcc 4.2.
Skipping 4.2 makes no sense.


>>  Any comments or concerns?
>
> +/* True if pointer types have undefined overflow.  */
> +#define POINTER_TYPE_OVERFLOW_UNDEFINED \
> +  (!flag_wrapv && !flag_trapv && flag_strict_overflow)
>
> don't add flag_trapv here, it doesn't make sense.

If we view pointer wraparound as an overflow, then I think it does.
We should be trapping on the overflow.  Of course, -ftrapv doesn't
work in general.  But that doesn't permit us to elide the potential
overflow here.


> In general I don't think we should do this.  The tests in this stupid
> CERT are bogus and I have never seen such.  Also this test
> will cause many false positives I belive, almost any loop with a
> pointer induction variable should be affected.
> 
> But of course my complaints about -Wstrict-overflow were unheard
> in the past as well ;)

I completely agree with you that the CERT issue is bogus, and that
this test will have plenty of false positives.  But that's really not
the point.  We are defending gcc against an unwarranted attack.  The
way to defend against an unwarranted attack is to add an optional
warning.

Nobody has to use the new warning.  It's coming in at
-Wstrict-overflow=3--using plain -Wstrict-overflow is equivalent to
-Wstrict-overflow=2.

I don't want to have this same fight over and over again.  We can not
ignore our user base.  If you have a counter-proposal, tell us.  Valid
objections to my proposed patch are that it is too hard to maintain or
doesn't do the job.  I'm not interested in hearing objections that
this patch is inappropriate without a counter-proposal beyond ignoring
the issue.

I would never recommend that anybody actually use -Wstrict-overflow,
except maybe once to see what their code base looks like.  That's not
why it exists.

But I will look into making the warning not trigger on compiler
introduced induction variables.

Ian