False positive from -Warray-bounds?

[Vincent Lefevre]

On 2011-12-30 09:26:18 +0100, David Brown wrote:
> On 30/12/2011 05:59, Ian Lance Taylor wrote:
> >Vincent Lefevre<vincent+gcc@vinc17.org>  writes:
> >
> >>On 2011-12-29 16:20:48 -0800, Ian Lance Taylor wrote:
> >>>To me this only proves that the compiler is not smart enough to see that
> >>>(s>>  1 == 0) implies that ((s&  0xffff) == 0xffff) can not be true.
> >>>
> >>>Are you suggesting that the compiler should never warn if there is a
> >>>conditional guarding the array access?  Would that in practice be better
> >>>or worse than the current behaviour?
> >>
> >>I think there should be two different options:
> >>   * one that would trigger the warning if the compiler can prove
> >>     that there will always be an out-of-bound access when the
> >>     function is executed (unless the compiler can prove that the
> >>     function will never be executed);

Note: by "unless the compiler can prove that the function will never
be executed", I mean for instance that the function would be declared
as static *and* it is never called or its call is in dead code, like

  if (0)
    foo ();

> >>   * one that would trigger the warning if there may be an out-of-bound
> >>     access.
> >
> >I wonder how often the first one would actually trigger.  And I wonder
> >how much correct code the second one triggers on today.  I personally
> >think it would be reasonable to rewrite the original example to avoid
> >the warning, since the code certainly looks like it can generate an out
> >of bounds access.
> >
> 
> The first one quickly reduces to the halting problem, amongst other issues.

Of course not, unless you want to be perfect, but being perfect has
never been a requirement (just like with other warnings).

> Consider :
> 
> static int a[10];
> extern int foo(int x);
> 
> void bar(void) {
> 	if (foo(1)) a[100]++;
> }
> 
> You can't make a "no false negatives" warning here - it depends
> entirely on foo, what it returns, and /if/ it returns.

This is typically the case where (2) would trigger the warning, but
not (1). I have never said that there should be "no false negatives".
The goal of (1) is to trigger a warning only in the cases where it
is most certain (but without any strong guarantee) that there is a
real problem.

> If code looks dodgy, it should be warned as dodgy - that's what
> warnings are for.

The dodginess is very subjective, and that's not up to gcc to decide,
in particular because comments and function specifications can have
an influence. In the original example, the path from which gcc could
deduce an out-of-bound access was in different function. Perhaps in
the real code, this path was needed in some calls, but could never
occur in the case the array was accessed. I don't see this code as
particularly dodgy, and duplicating the function to handle both kinds
of calls could be worse.

>  If you want to write code that looks dodgy but isn't, as one sometimes
> does, then disable the warning

But disabling all the related warnings could be too much, and hide
real problems. Hence my proposition to have two kinds of warnings.

> - either omit it from the command line, or
> use "#pragma GCC diagnostic" around the code in question.

If the pragma can locally disable the warning, perhaps, but the
pragma itself may trigger warnings with other compilers. Something
that could be activated with the preprocessor would be much better:

#if defined(__GNUC__)
# define CORRECT ...specific GCC extension...
#else
# define CORRECT
#endif

and in the code, something like:

  CORRECT a[foo(1)] = 0;

or

  a[CORRECT(foo(1))] = 0;

> Warnings are there to help people write clearer code and avoid common or
> accidental mistakes - a warning that triggers on /possible/ out-of-bound
> array access fits that purpose fine (IMHO, of course).

The notion of clear code can't be reduced to some basic arbitrary
rules.

> If the compiler could tell without any doubt that the array is being
> accessed illegally, then maybe an error would be better (since the code has
> undefined behaviour at run-time, is the compiler allowed to reject it?).

The compiler cannot tell anything without any doubt, because it
doesn't know whether some function will ever be called (perhaps
except in the case of main() and it descendants when a program
is generated).

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <http://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / Arénaire project (LIP, ENS-Lyon)