More extensive testing of the patch I just committed in r11-7563 to
avoid the false positive -Warray-bounds on accesses to members of
virtual bases has exposed a couple of problems that cause many false
negatives for even basic bugs like:
typedef struct A { int i; } A;
void* g (void)
{
A *p = (A*)malloc (3);
p->i = 0; // missing warning in C++
return p;
}
as well as a number of others, including more complex ones involving
virtual base classes that can, with some additional work, be reliably
detected. Most of them were successfully diagnosed before the fix
for PR 98266. Unfortunately, the tests that exercise this are all
C so the C++ only regressions due to the divergence went unnoticed.
The root cause is twofold: a) the simplistic check for TYPE_BINFO
in the new inbounds_vbase_memaccess_p() function means we exclude
from checking any member accesses whose leading offset is in bounds,
and b) the check for the leading offset misses cases where just
the ending offset of an access is out of bounds.
The attached patch adds code to restrict the original solution
to just the narrow subset of accesses to members of classes with
virtual bases, and also adds a check for the ending offset.
Is the patch okay for trunk?
(The code for has_virtual_base() is adapted from
polymorphic_type_binfo_p() in ipa-utils.h. It seems like a handy
utility that could go in tree.h.)
Martin
PS There is another regression I discovered while working on this.
It's a false positive but unrelated to the r11-7563 fix so I'll
post a patch for separately.
PR middle-end/99502 - missing -Warray-bounds on partial out of bounds
gcc/ChangeLog:
PR middle-end/99502
* gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
Handle void* type. Print the size of even declared objects.
(has_virtual_base_r): New function.
(has_virtual_base): New function.
(inbounds_vbase_memaccess_p): Call has_virtual_base. Also check
the ending offset of the accessed member.
gcc/testsuite/ChangeLog:
PR middle-end/99502
* g++.dg/warn/Warray-bounds-22.C: New test.
* g++.dg/warn/Warray-bounds-23.C: New test.
diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index 54f32051199..34552b7c812 100644
@@ -890,9 +902,46 @@ array_bounds_checker::check_addr_expr (location_t location, tree t)
}
}
+/* Return true if BINFO corresponds to a class with a virtual base class. */
+
+static bool
+has_virtual_base_r (tree binfo)
+{
+ for (unsigned i = 0; i < BINFO_N_BASE_BINFOS (binfo); i++)
+ {
+ tree base = BINFO_BASE_BINFO (binfo, i);
+ if (BINFO_VIRTUAL_P (base) || has_virtual_base_r (base))
+ return true;
+ }
+
+ return false;
+}
+
+/* Return true if TYPE is a class type with virtual functions or with
+ a virtual base class. */
+
+static bool
+has_virtual_base (tree type)
+{
+ tree binfo = TYPE_BINFO (type);
+ if (!binfo)
+ return false;
+ tree btype = BINFO_TYPE (binfo);
+ if (!btype)
+ return false;
+ tree tbtype = TYPE_BINFO (btype);
+ if (!tbtype)
+ return false;
+ if (!BINFO_VTABLE (tbtype))
+ return false;
+
+ return has_virtual_base_r (binfo);
+}
+
/* Return true if T is a reference to a member of a base class that's within
- the bounds of the enclosing complete object. The function "hacks" around
- problems discussed in pr98266 and pr97595. */
+ the bounds of the enclosing complete object. Considers only classes with
+ virtual bases. The function "hacks" around problems discussed in pr98266
+ and pr97595. */
static bool
inbounds_vbase_memaccess_p (tree t)
@@ -904,10 +953,12 @@ inbounds_vbase_memaccess_p (tree t)
if (TREE_CODE (mref) != MEM_REF)
return false;
- /* Consider the access if its type is a derived class. */
+ /* Consider the access only if its type is a derived polymorphic class
+ with a virtual base. */
tree mreftype = TREE_TYPE (mref);
- if (!RECORD_OR_UNION_TYPE_P (mreftype)
- || !TYPE_BINFO (mreftype))
+ if (!RECORD_OR_UNION_TYPE_P (mreftype))
+ return false;
+ if (!has_virtual_base (mreftype))
return false;
/* Compute the size of the referenced object (it could be dynamically
@@ -929,9 +980,19 @@ inbounds_vbase_memaccess_p (tree t)
tree refoff = TREE_OPERAND (mref, 1);
tree fldoff = int_const_binop (PLUS_EXPR, fldpos, refoff);
- /* Return true if the member offset is less than the size of the complete
- object. */
- return tree_int_cst_lt (fldoff, refsize);
+ /* Return false if the member offset is greater or equal to the size
+ of the complete object. */
+ if (!tree_int_cst_lt (fldoff, refsize))
+ return false;
+
+ tree fldsiz = DECL_SIZE_UNIT (fld);
+ if (!fldsiz || TREE_CODE (fldsiz) != INTEGER_CST)
+ return false;
+
+ /* Return true if the offset just past the end of the member is less
+ than or equal to the size of the complete object. */
+ tree fldend = int_const_binop (PLUS_EXPR, fldoff, fldsiz);
+ return tree_int_cst_le (fldend, refsize);
}
/* Callback for walk_tree to check a tree for out of bounds array
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-22.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-22.C
new file mode 100644
index 00000000000..77cdb5ebd43
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-22.C
@@ -0,0 +1,105 @@
+/* PR middle-end/99502 - missing -Warray-bounds on partial out of bounds
+ access in C++
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+typedef __INT8_TYPE__ int8_t;
+typedef __INT16_TYPE__ int16_t;
+typedef __INT32_TYPE__ int32_t;
+
+struct POD32
+{
+ int32_t i32;
+};
+
+int8_t a16[2];
+int8_t apod32[sizeof (POD32)];
+
+void nowarn_pod32_assign ()
+{
+ POD32 *p = (POD32*)apod32;
+ *p = POD32 ();
+}
+
+void nowarn_pod32_assign_member ()
+{
+ POD32 *p = (POD32*)apod32;
+ p->i32 = __LINE__;
+}
+
+
+void warn_pod32_assign ()
+{
+ POD32 *p = (POD32*)a16;
+ *p = POD32 (); // { dg-warning "-Warray-bounds" }
+}
+
+void warn_pod32_assign_member ()
+{
+ POD32 *p = (POD32*)a16;
+ p->i32 = __LINE__; // { dg-warning "-Warray-bounds" }
+}
+
+
+struct BV32 // { dg-warning "-Warray-bounds" "due to pr99525" }
+{
+ int32_t i32;
+
+ virtual ~BV32 ();
+};
+
+int8_t abv32[sizeof (BV32)];
+
+void nowarn_bv32_assign ()
+{
+ BV32 *p = (BV32*)abv32;
+ *p = BV32 ();
+}
+
+void nowarn_bv32_assign_member ()
+{
+ BV32 *p = (BV32*)abv32;
+ p->i32 = __LINE__;
+}
+
+void warn_bv32_assign ()
+{
+ BV32 *p = (BV32*)a16;
+ *p = BV32 (); // { dg-warning "-Warray-bounds" "pr99525" { xfail *-*-* } }
+}
+
+void warn_bv32_assign_member ()
+{
+ BV32 *p = (BV32*)a16;
+ p->i32 = __LINE__; // { dg-warning "-Warray-bounds" }
+}
+
+
+struct DV32: virtual BV32 { };
+
+int8_t adv32[sizeof (DV32)];
+int8_t adv32_m1[sizeof adv32 - 1];
+
+void nowarn_dv32_assign ()
+{
+ DV32 *p = (DV32*)adv32;
+ *p = DV32 ();
+}
+
+void nowarn_dv32_assign_member ()
+{
+ DV32 *p = (DV32*)adv32;
+ p->i32 = __LINE__;
+}
+
+void warn_dv32_assign ()
+{
+ DV32 *p = (DV32*)adv32_m1;
+ *p = DV32 (); // { dg-warning "-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
+
+void warn_dv32_assign_member ()
+{
+ DV32 *p = (DV32*)adv32_m1;
+ p->i32 = __LINE__; // { dg-warning "-Warray-bounds" "pr?????" { xfail *-*-* } }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-23.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-23.C
new file mode 100644
index 00000000000..918554a0b8a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-23.C
@@ -0,0 +1,288 @@
+/* PR middle-end/99502 - missing -Warray-bounds on partial out of bounds
+ access in C++
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+inline void* operator new (__SIZE_TYPE__, void *p) { return p; }
+
+struct B0 { int b0i; };
+struct B1: virtual B0 { int b1i; };
+struct B2: virtual B0 { int b2i; };
+struct D1: B1, B2 { int d1i; };
+struct D2: D1 { int d2i; };
+struct D3: D2 { int d3i; };
+
+void sink (void*);
+
+
+void test_D1 ()
+{
+ {
+ char *p = new char[sizeof (D1)];
+ new (p) D1 ();
+ sink (p);
+ }
+
+ {
+ char *p = new char[sizeof (D1) - 1];
+ new (p) D1 (); // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D1_b0i ()
+{
+ {
+ D1 *p = (D1*)new char[sizeof (D1)];
+ p->b0i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D1 *p = (D1*)new char[3];
+ p->b0i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D1_b1i ()
+{
+ {
+ D1 *p = (D1*)new char[sizeof (D1)];
+ p->b1i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D1 *p = (D1*)new char[3];
+ p->b1i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D1_b2i ()
+{
+ {
+ D1 *p = (D1*)new char[sizeof (D1)];
+ p->b2i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D1 *p = (D1*)new char[3];
+ p->b2i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D1_d1i ()
+{
+ {
+ D1 *p = (D1*)new char[sizeof (D1)];
+ p->d1i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D1 *p = (D1*)new char[3];
+ p->d1i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+
+void test_D2 ()
+{
+ {
+ char *p = new char[sizeof (D2)];
+ new (p) D2 ();
+ sink (p);
+ }
+
+ {
+ char *p = new char[sizeof (D2) - 1];
+ new (p) D2 (); // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D2_b0i ()
+{
+ {
+ D2 *p = (D2*)new char[sizeof (D2)];
+ p->b0i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D2 *p = (D2*)new char[3];
+ p->b0i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D2_b1i ()
+{
+ {
+ D2 *p = (D2*)new char[sizeof (D2)];
+ p->b1i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D2 *p = (D2*)new char[3];
+ p->b1i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D2_b2i ()
+{
+ {
+ D2 *p = (D2*)new char[sizeof (D2)];
+ p->b2i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D2 *p = (D2*)new char[3];
+ p->b2i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D2_d1i ()
+{
+ {
+ D2 *p = (D2*)new char[sizeof (D2)];
+ p->d1i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D2 *p = (D2*)new char[3];
+ p->d1i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D2_d2i ()
+{
+ {
+ D2 *p = (D2*)new char[sizeof (D2)];
+ p->d2i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D2 *p = (D2*)new char[3];
+ p->d2i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+
+void test_D3 ()
+{
+ {
+ char *p = new char[sizeof (D3)];
+ new (p) D3 ();
+ sink (p);
+ }
+
+ {
+ char *p = new char[sizeof (D3) - 1];
+ new (p) D3 (); // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D3_b0i ()
+{
+ {
+ D3 *p = (D3*)new char[sizeof (D3)];
+ p->b0i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D3 *p = (D3*)new char[3];
+ p->b0i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D3_b1i ()
+{
+ {
+ D3 *p = (D3*)new char[sizeof (D3)];
+ p->b1i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D3 *p = (D3*)new char[3];
+ p->b1i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D3_b2i ()
+{
+ {
+ D3 *p = (D3*)new char[sizeof (D3)];
+ p->b2i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D3 *p = (D3*)new char[3];
+ p->b2i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D3_d1i ()
+{
+ {
+ D3 *p = (D3*)new char[sizeof (D3)];
+ p->d1i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D3 *p = (D3*)new char[3];
+ p->d1i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D3_d2i ()
+{
+ {
+ D3 *p = (D3*)new char[sizeof (D3)];
+ p->d2i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D3 *p = (D3*)new char[3];
+ p->d2i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
+
+void test_D3_d3i ()
+{
+ {
+ D3 *p = (D3*)new char[sizeof (D3)];
+ p->d3i = __LINE__;
+ sink (p);
+ }
+
+ {
+ D3 *p = (D3*)new char[3];
+ p->d3i = __LINE__; // { dg-warning "-Warray-bounds" }
+ sink (p);
+ }
+}
