This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
C++ PATCH: More ABI bugs
- From: Mark Mitchell <mark at codesourcery dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 25 Sep 2002 12:09:54 -0700
- Subject: C++ PATCH: More ABI bugs
- Reply-to: mark at codesourcery dot com
CodeSourcery's nascent ABI testsuite continues to find layout bugs.
This round involves a couple of problems with empty classes. These
are relatively rare, and so the set of people and programs affected is
probably rare as well.
One of these bugs may actually result in the generation of incorrect
code, not only from the point of view of the ABI but from the point of
view of the language. I couldn't easily create an example, but that
doesn't mean one doesn't exist. The basic problem is that we were
using DECL_FIELD_OFFSET to get the offset in bytes of the FIELD_DECL,
which, despite the name and confusing documentation, is not what that
macro does. So, we might think empty bases were in the wrong place,
and thereby allocate two empty bases of the same type at the same
location, which is wrong. A program that failed due to that error
would be rare, but it is possible.
Tested on i686-pc-linux-gnu, applied on the mainline.
--
Mark Mitchell mark@codesourcery.com
CodeSourcery, LLC http://www.codesourcery.com
2002-09-25 Mark Mitchell <mark@codesourcery.com>
* doc/invoke.texi: Add more -Wabi examples.
2002-09-25 Mark Mitchell <mark@codesourcery.com>
* cp/class.c (contains_empty_class_p): New method.
(walk_subobject_offsets): Correct computation of field offset.
(layout_empty_base): Correct placement of emtpy base classes.
(layout_class_type): Warn about ABI changes.
2002-09-25 Mark Mitchell <mark@codesourcery.com>
* gcc/testsuite/g++.dg/abi/empty5.C: New test.
* gcc/testsuite/g++.dg/abi/empty6.C: New test.
* gcc/testsuite/g++.dg/abi/vbase12.C: New test.
Index: cp/class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.468
diff -c -p -r1.468 class.c
*** cp/class.c 23 Sep 2002 09:22:14 -0000 1.468
--- cp/class.c 25 Sep 2002 19:05:52 -0000
*************** static int splay_tree_compare_integer_cs
*** 210,215 ****
--- 210,216 ----
splay_tree_key k2));
static void warn_about_ambiguous_direct_bases PARAMS ((tree));
static bool type_requires_array_cookie PARAMS ((tree));
+ static bool contains_empty_class_p (tree);
/* Macros for dfs walking during vtt construction. See
dfs_ctor_vtable_bases_queue_p, dfs_build_secondary_vptr_vtt_inits
*************** walk_subobject_offsets (type, f, offset,
*** 3544,3554 ****
for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL)
{
r = walk_subobject_offsets (TREE_TYPE (field),
f,
size_binop (PLUS_EXPR,
offset,
! DECL_FIELD_OFFSET (field)),
offsets,
max_offset,
/*vbases_p=*/1);
--- 3545,3563 ----
for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL)
{
+ tree field_offset;
+
+ if (abi_version_at_least (2))
+ field_offset = byte_position (field);
+ else
+ /* In G++ 3.2, DECL_FIELD_OFFSET was used. */
+ field_offset = DECL_FIELD_OFFSET (field);
+
r = walk_subobject_offsets (TREE_TYPE (field),
f,
size_binop (PLUS_EXPR,
offset,
! field_offset),
offsets,
max_offset,
/*vbases_p=*/1);
*************** layout_empty_base (binfo, eoc, offsets,
*** 3720,3729 ****
tree alignment;
tree basetype = BINFO_TYPE (binfo);
bool atend = false;
!
/* This routine should only be used for empty classes. */
my_friendly_assert (is_empty_class (basetype), 20000321);
alignment = ssize_int (CLASSTYPE_ALIGN_UNIT (basetype));
/* This is an empty base class. We first try to put it at offset
zero. */
--- 3729,3745 ----
tree alignment;
tree basetype = BINFO_TYPE (binfo);
bool atend = false;
!
/* This routine should only be used for empty classes. */
my_friendly_assert (is_empty_class (basetype), 20000321);
alignment = ssize_int (CLASSTYPE_ALIGN_UNIT (basetype));
+
+ if (abi_version_at_least (2))
+ BINFO_OFFSET (binfo) = size_zero_node;
+ if (warn_abi && !integer_zerop (BINFO_OFFSET (binfo)))
+ warning ("offset of empty base `%T' may not be ABI-compliant and may"
+ "change in a future version of GCC",
+ BINFO_TYPE (binfo));
/* This is an empty base class. We first try to put it at offset
zero. */
*************** layout_class_type (t, empty_p, vfuns_p,
*** 4915,4920 ****
--- 4931,4947 ----
cp_warning_at ("offset of `%D' is not ABI-compliant and may change in a future version of GCC",
field);
+ /* G++ used to use DECL_FIELD_OFFSET as if it were the byte
+ offset of the field. */
+ if (warn_abi
+ && !tree_int_cst_equal (DECL_FIELD_OFFSET (field),
+ byte_position (field))
+ && contains_empty_class_p (TREE_TYPE (field)))
+ cp_warning_at ("`%D' contains empty classes which may cause base "
+ "classes to be placed at different locations in a "
+ "future version of GCC",
+ field);
+
/* If we needed additional padding after this field, add it
now. */
if (padding)
*************** is_empty_class (type)
*** 6369,6374 ****
--- 6396,6425 ----
return 0;
return integer_zerop (CLASSTYPE_SIZE (type));
+ }
+
+ /* Returns true if TYPE contains an empty class. */
+
+ static bool
+ contains_empty_class_p (tree type)
+ {
+ if (is_empty_class (type))
+ return true;
+ if (CLASS_TYPE_P (type))
+ {
+ tree field;
+ int i;
+
+ for (i = 0; i < CLASSTYPE_N_BASECLASSES (type); ++i)
+ if (contains_empty_class_p (TYPE_BINFO_BASETYPE (type, i)))
+ return true;
+ for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
+ if (contains_empty_class_p (TREE_TYPE (field)))
+ return true;
+ }
+ else if (TREE_CODE (type) == ARRAY_TYPE)
+ return contains_empty_class_p (TREE_TYPE (type));
+ return false;
}
/* Find the enclosing class of the given NODE. NODE can be a *_DECL or
Index: doc/invoke.texi
===================================================================
RCS file: /cvs/gcc/gcc/gcc/doc/invoke.texi,v
retrieving revision 1.188
diff -c -p -r1.188 invoke.texi
*** doc/invoke.texi 23 Sep 2002 15:00:42 -0000 1.188
--- doc/invoke.texi 25 Sep 2002 19:05:52 -0000
*************** union U @{ int i : 4096; @};
*** 1546,1551 ****
--- 1546,1570 ----
Assuming that an @code{int} does not have 4096 bits, G++ will make the
union too small by the number of bits in an @code{int}.
+ @item
+ Empty classes can be placed at incorrect offsets. For example:
+
+ @smallexample
+ struct A @{@};
+
+ struct B @{
+ A a;
+ virtual void f ();
+ @};
+
+ struct C : public B, public A @{@};
+ @end smallexample
+
+ @noindent
+ G++ will place the @code{A} base class of @code{C} at a non-zero offset;
+ it should be placed at offset zero. G++ mistakenly believes that the
+ @code{A} data member of @code{B} is already at offset zero.
+
@end itemize
@item -Wctor-dtor-privacy @r{(C++ only)}
Index: testsuite/g++.dg/abi/empty5.C
===================================================================
RCS file: testsuite/g++.dg/abi/empty5.C
diff -N testsuite/g++.dg/abi/empty5.C
*** /dev/null 1 Jan 1970 00:00:00 -0000
--- testsuite/g++.dg/abi/empty5.C 25 Sep 2002 19:05:52 -0000
***************
*** 0 ****
--- 1,17 ----
+ // { dg-options "-fabi-version=0" }
+
+ struct A {};
+
+ struct B {
+ A a;
+ virtual void f () {}
+ };
+
+ struct C : public B, public A {};
+
+ C c;
+
+ int main () {
+ if ((void*) (A*) &c != &c)
+ return 1;
+ }
Index: testsuite/g++.dg/abi/empty6.C
===================================================================
RCS file: testsuite/g++.dg/abi/empty6.C
diff -N testsuite/g++.dg/abi/empty6.C
*** /dev/null 1 Jan 1970 00:00:00 -0000
--- testsuite/g++.dg/abi/empty6.C 25 Sep 2002 19:05:52 -0000
***************
*** 0 ****
--- 1,8 ----
+ // { dg-options "-Wabi" }
+
+ struct A {};
+
+ struct B {
+ A a; // { dg-warning "empty" }
+ virtual void f () {}
+ };
Index: testsuite/g++.dg/abi/vbase12.C
===================================================================
RCS file: testsuite/g++.dg/abi/vbase12.C
diff -N testsuite/g++.dg/abi/vbase12.C
*** /dev/null 1 Jan 1970 00:00:00 -0000
--- testsuite/g++.dg/abi/vbase12.C 25 Sep 2002 19:05:52 -0000
***************
*** 0 ****
--- 1,14 ----
+ // { dg-do run }
+ // { dg-options "-fabi-version=0" }
+
+ struct A {};
+ struct B { A a; virtual void f () {} };
+ struct C : public B, virtual public A {};
+ struct D : public C, virtual public A {};
+
+ D d;
+
+ int main () {
+ if (((char*)(A*)&d - (char*)&d) != 0)
+ return 1;
+ }