On 11/16/20 11:54 PM, Jason Merrill wrote:
On 11/16/20 9:41 PM, Martin Sebor wrote:
The result of DECL_SIZE_UNIT doesn't always reflect the size
of data members of virtual classes. This can lead to objects
of such types appearing smaller than they are to warnings like
-Warray-bounds or -Wstringop-overflow, causing false positives.
To avoid these false positives, the attached replaces the use
of DECL_SIZE_UNIT in component_ref_size in the middle end with
TYPE_SIZE_UNIT.
Unfortunately, that's not enough; the offset between the intermediate
base and the virtual base could be greater than the TYPE_SIZE of the
intermediate base:
extern "C" int printf (const char *, ...);
struct A { char ar1[24]; };
struct B: virtual A { };
struct C { char ar2[42]; };
struct D: B, C { };
int main()
{
D d;
printf ("size %d, offset %d\n", sizeof (B), d.ar1 - (char*)(B*)&d);
}
Here base C is allocated between base B and its virtual base A, so the
offset between them is 50, while the size of B is only 32.
The difference between TYPE_SIZE and DECL_SIZE could be a way to
recognize the case of bases with virtual bases, and then either hunt
down all the virtual bases or just use the bounds of the enclosing
most-derived object.
An advanced analysis of classes with virtual bases is beyond what
I have cycles to tackle at this point (it seems it would also need
to be done in the C++ front end?) It will have to wait until I have
more time or the next stage 1.
So for now, I've changed component_ref_size to fail when DECL_SIZE
isn't equal TYPE_SIZE.
+ /* DECL_SIZE may be less than TYPE_SIZE in C++ when referring
+ to the type of a virtual base class which doesn't reflect
+ the size of the virtual's members (see pr97595). */
The problem isn't with the virtual base class itself (A), but with the
intermediate base class subobject (B), for which DECL_SIZE doesn't
include the size of the virtual base A, because the A base subobject is
allocated separately.
I've also adjusted the comments above the _SIZE macros in tree.h
to more closely reflect what happens there. My main goal isn't
to describe when they don't match with perfect accuracy, just to
point that they may be unequal and (roughly) when.
Martin
PR middle-end/97595 - bogus -Wstringop-overflow due to DECL_SIZE_UNIT underreporting field size
gcc/ChangeLog:
PR middle-end/97595
* tree.c (component_ref_size): Fail when DECL_SIZE != TYPE_SIZE.
* tree.h (DECL_SIZE): Update comment.
gcc/testsuite/ChangeLog:
PR middle-end/97595
* g++.dg/warn/Warray-bounds-14.C: New test.
* g++.dg/warn/Wstringop-overflow-6.C: New test.
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-14.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-14.C
new file mode 100644
index 00000000000..0812f833d74
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-14.C
@@ -0,0 +1,25 @@
+/* PR middle-end/97595 - bogus -Wstringop-overflow due to DECL_SIZE_UNIT
+ underreporting field size
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+struct A { char a[32]; };
+struct B: virtual A { };
+struct C: B { };
+
+struct D
+{
+ B &b;
+ D (B&);
+};
+
+D::D (B &b): b (b) { } // { dg-bogus "-Warray-bounds" }
+
+void f (void*);
+
+void g ()
+{
+ C c;
+ D d (c);
+ f (&d);
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-6.C b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-6.C
new file mode 100644
index 00000000000..8173e601d4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-6.C
@@ -0,0 +1,8 @@
+/* PR middle-end/97595 - bogus -Wstringop-overflow due to DECL_SIZE_UNIT
+ underreporting field size
+ { dg-do compile { target c++11 } }
+ { dg-options "-O2 -Wall -Wsystem-headers" } */
+
+#include <iostream>
+
+template void std::basic_iostream<char>::swap (basic_iostream&);
diff --git a/gcc/tree.c b/gcc/tree.c
index d6ba55319bf..52a145dd018 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -13740,9 +13740,9 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
{
gcc_assert (TREE_CODE (ref) == COMPONENT_REF);
- special_array_member arkbuf;
+ special_array_member sambuf;
if (!sam)
- sam = &arkbuf;
+ sam = &sambuf;
*sam = special_array_member::none;
/* The object/argument referenced by the COMPONENT_REF and its type. */
@@ -13756,7 +13756,13 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
{
tree memtype = TREE_TYPE (member);
if (TREE_CODE (memtype) != ARRAY_TYPE)
- return memsize;
+ /* DECL_SIZE may be less than TYPE_SIZE in C++ when referring
+ to the type of a class with a virtual base which doesn't
+ reflect the size of the virtual's members (see pr97595).
+ If that's the case fail for now and implement something
+ more robust in the future. */
+ return (tree_int_cst_equal (memsize, TYPE_SIZE_UNIT (memtype))
+ ? memsize : NULL_TREE);
bool trailing = array_at_struct_end_p (ref);
bool zero_length = integer_zerop (memsize);
diff --git a/gcc/tree.h b/gcc/tree.h
index 078919bce5d..6bca5a57e82 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1987,8 +1987,10 @@ class auto_suppress_location_wrappers
so they must be checked as well. */
#define TYPE_UID(NODE) (TYPE_CHECK (NODE)->type_common.uid)
-/* Type size in bits as a tree expression. Need not be constant
- and may be null. */
+/* Type size in bits as a tree expression. Need not be constant and may
+ be null. May be greater than DECL_SIZE for a NODE of the same type
+ (in C++, DECL_SIZE of an object of a type with a virtual base class
+ doesn't include the sizes of data members of that class). */
#define TYPE_SIZE(NODE) (TYPE_CHECK (NODE)->type_common.size)
/* Likewise, type size in bytes. */
#define TYPE_SIZE_UNIT(NODE) (TYPE_CHECK (NODE)->type_common.size_unit)
@@ -2521,7 +2523,10 @@ extern tree vector_element_bits_tree (const_tree);
#define DECL_INITIAL(NODE) (DECL_COMMON_CHECK (NODE)->decl_common.initial)
/* Holds the size of the datum, in bits, as a tree expression.
- Need not be constant and may be null. */
+ Need not be constant and may be null. May be less than TYPE_SIZE
+ for a NODE of the same type (in C++, DECL_SIZE of an object of
+ a type with a virtual base class doesn't include the sizes of data
+ members of that class). */
#define DECL_SIZE(NODE) (DECL_COMMON_CHECK (NODE)->decl_common.size)
/* Likewise for the size in bytes. */
#define DECL_SIZE_UNIT(NODE) (DECL_COMMON_CHECK (NODE)->decl_common.size_unit)
