[Bug tree-optimization/101436] New: Yet another bogus "array subscript is partly outside array bounds"

[Bug tree-optimization/101436] New: Yet another bogus "array subscript is partly outside array bounds"

[cassio.neri at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101436

            Bug ID: 101436
           Summary: Yet another bogus "array subscript is partly outside
                    array bounds"
           Product: gcc
           Version: 11.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: cassio.neri at gmail dot com
  Target Milestone: ---

This bogus warning was reported at least twice recently: #98266 and #101374.
Below is a new case that, it seems, hasn't been addressed yet.

#include <typeinfo>

struct A {
  virtual ~A();
};

template <typename T>
struct B : A {
  T x;
};

struct C1 {
  int i;
  double j;
};

struct C2 {
  int i;
};

void do_something(int);
B<C2> get_BC2();

void h(A& a) {
  if (typeid(a) == typeid(B<C1>)) {
    B<C1>& b = static_cast<B<C1>&>(a);
    int i = b.x.i;
    do_something(i);
  }
}

void foo() {
  B<C2> x = get_BC2();
  h(x);
}

Compiled with '-O3 -Warray-bounds' yields:

<source>: In function 'void foo()':
<source>:27:9: warning: array subscript 'B<C1>[0]' is partly outside array
bounds of 'B<C2> [1]' [-Warray-bounds]
   27 |     int i = b.x.i;
      |         ^
<source>:33:9: note: while referencing 'x'
   33 |   B<C2> x = get_BC2();

FWIW:

1) This is a regression from GCC 10.3.

2) The warning goes away if any of the following changes are made:
  * Remove C1::j.
  * Change type of C1::j to any of int, char, bool, unsigned or float. (Perhaps
any type T such that sizeof(T) <= sizeof(int)).
  * Compile with '-fPIC' (however, if h is marked inline then the warning comes
back).

3) If b is declared as B<C1> (as opposed to B<C1>&), then the warning points to
line 'struct B: A {'.

4) The test case could be simplified further by removing A's virtual destructor
and the typeid check. However, this would make the code to invoke UB and I hope
the code above doesn't.

5) #98266 regards virtual inheritance which does not appear here and a test
cases therein issues no warning when compiled with GCC 11.1.

6) IIUC the warning reported by #101374 happens in GCC's own code and was
caused by some recent change that is not part of GCC 11.1. Indeed a test case
reported therein compiles fine with GCC 11.1 whereas the one above doesn't.

See also:

Test case above: https://godbolt.org/z/n4obaohPs
Test case from  #98266: https://godbolt.org/z/PEjfhs3T6
Test case from #101374: https://godbolt.org/z/Ebb8YszT5

[Bug tree-optimization/101436] Yet another bogus "array subscript is partly outside array bounds"

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101436

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |msebor at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess it would be nice if devirt was able to optimize away the typeid
comparison.
But even when it is not, I'd perhaps understand the warning if b.x.j was
accessed (though still warning on dead code), but b.x.i is present in both
B<C1> and B<C2>.

[Bug tree-optimization/101436] Yet another bogus "array subscript is partly outside array bounds"

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101436

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|https://gcc.gnu.org/bugzill |
                   |a/show_bug.cgi?id=98266,    |
                   |https://gcc.gnu.org/bugzill |
                   |a/show_bug.cgi?id=101374    |
         Resolution|---                         |WONTFIX
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
The reports in pr98266 and pr101374 are unrelated.  This instance of the
warning is by design: it points out an access to an object of one type (B<C2>)
by an lvalue of an unrelated type (B<C1>).  Such accesses are invalid and can
be unsafe regardless of whether the offset of the accessed member is fully
within the bounds of the accessed object (GCC relies on the requirement to
optimize code).

The problem can be reduced to the following C example:

$ cat a.c && gcc -Wall -S -O2 a.c
struct A { int i; };
struct B { struct A a; int j, k; };
struct C { struct A a; int j; };

int is_B (struct A *);   // return nonzero for instances of B

int f (struct C c)
{
  struct A *p = &c.a;
  if (is_B (p))
    {
      struct B *q = (struct B*)p;
      return q->a.i;   // invalid
    }
  return 0;
}
a.c: In function ‘f’:
a.c:13:15: warning: array subscript ‘struct B[0]’ is partly outside array
bounds of ‘struct C[1]’ [-Warray-bounds]
   13 |       return q->a.i;
      |               ^~
a.c:7:17: note: object ‘c’ of size 8
    7 | int f (struct C c)
      |        ~~~~~~~~~^

The cast that safely converts a pointer/reference down an inheritance hierarchy
is dynamic_cast.  Using it avoids the warning.  Using typeid and static_cast is
a convoluted way to do the same thing except that it exposes the potential
problem to the warning code: a) because the typeid equality isn't folded and b)
the static_cast is just a straight conversion whereas dynamic_cast is a
function call whose result in the IL is unrelated to its operand.

[Bug tree-optimization/101436] Yet another bogus "array subscript is partly outside array bounds"

[cassio.neri at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101436

--- Comment #3 from Cassio Neri <cassio.neri at gmail dot com> ---
Because of the typeid check the unsafe static_cast never happens and I think
the compiler should not be warning about a problem that doesn't exist. Besides,
there's no array involved in this code. I appreciate the attempt to emit a good
warning that might improve my code but the message is completely misleading and
make me scratch my head. Here the code is minimal and obvious to figure out
that there's no array. In a large code base I could spend longtime looking for
an array that doesn't exist or I could find an array that has no issue but the
compiler makes me think it has.

Re using a dynamic_cast: I could surely use a dynamic_cast in real code but
this is a compiler test case. IMHO, it should be minimal, straight to the point
at the expense of neglecting other aspects of the language (e.g. better
practices) that could otherwise divert the attention. As I said the virtual
destructor in A and the typeid check were there to avoid obvious UB that would
happen had I unconditionally performed the static_cast. In that UB case
(provided the message were clearer and not misleading talking about arrays) I'd
be very grateful for getting the warning. 

Notice also that I provided a couple of changes that don't make the code any
better w.r.t. an unsafe static_cast (which, again, is never performed). These
changes make the spurious warning to go away (which is good) and this shows
that there's certainly something wrong with the logic that decides to emit the
warning for the code as originally posted.

What about this example which involves no virtual method and where dynamic_cast
cannot help?

struct A {
  int type;
};

struct C1 {
  int i;
  int j;
};

struct C2 {
  int i;
};

template <typename T, int i>
struct B : A {
  B() : A{i} {}
  T x;
};

using BC1 = B<C1, 1>;
using BC2 = B<C2, 2>;

void do_something(int);
BC2 get_BC();

void h(A& a) {
  if (a.type == 1) {
    BC1& b = static_cast<BC1&>(a);
    int i = b.x.i;
    do_something(i);
  }
  else if (a.type == 2) {
    BC2& b = static_cast<BC2&>(a);
    int i = b.x.i;
    do_something(i);
  }
}

void foo() {
  auto x = get_BC();
  h(x);
}

Here again, there are changes that make the code no better w.r.t. a potential
unsafe cast but do make the warning to go away:

1) Change the return type of get_BC to BC1.
2) Remove C1::j.
3) Remove the extra level of indirection given by template class B. (See [2].)

[1] Example above: https://godbolt.org/z/Tha3M6xq3
[2] Example with no template: https://godbolt.org/z/nWsPvTrYr