Re: [PATCH] c++, middle-end, rs6000: Fix C++17 ABI incompatibilities during class layout [PR94707]

Sun, 26 Apr 2020 00:33:43 -0700 

David Edelsohn <dje....@gmail.com> wrote:


On Sat, Apr 25, 2020 at 6:03 AM Jakub Jelinek <ja...@redhat.com> wrote:

Hi!

As reported by Iain and David, powerpc-darwin and powerpc-aix* have C++14
vs. C++17 ABI incompatibilities which are not fixed by mere adding of
cxx17_empty_base_field_p calls.  Unlike the issues that were seen on other
targets where the artificial empty base field affected function argument
passing or returning of values, on these two targets the difference is
during class layout, not afterwards (e.g.
struct empty_base {};
struct S : public empty_base { unsigned long long l[2]; };
will have different __alignof__ (S) between C++14 and C++17 (or possibly
with double instead of unsigned long long too)).

The aim of the calls.c (cxx17_empty_base_field_p) was not to match random
artificial fields with zero size, because they could be created by something else for some other reason, thus the DECL_SIZE is bitsize_zero but TYPE_SIZE 
is non-zero check.  Unfortunately, during the class layout, the empty base
fields have a different type, one which has zero size, so the predicate only 
works after the class is laid out completely.

This patch adds a langhook, which will return true for those cases.  It
shouldn't be problematic with LTO, because lto1 should see only
structures/classes that are already laid out, or if it does some structure
layout, it won't be something that has C++17 empty base artificial fields.
Tested with cross to powerpc-darwin12 on a simple testcase given from Iain, 
but otherwise untested.
Iain/David, could you please test this on your targets?
Ok for trunk?


The patch fixes the AIX failures.


It also fixes them on Darwin.


The rs6000 part is okay.

Thanks for tracking down and fixing all of this fallout.


yes, thanks indeed.
Iain



Thanks, David


2020-04-25  Jakub Jelinek  <ja...@redhat.com>

       PR target/94707
       * langhooks.h (struct lang_hooks_for_decls): Add
       cxx17_empty_base_field_p member.
       * langhooks-def.h (LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P): Define.
       (LANG_HOOKS_DECLS): Use it.
       * calls.c (cxx17_empty_base_field_p): Use
       langhooks.decls.cxx17_empty_base_field_p.
       * config/rs6000/rs6000.c (rs6000_special_round_type_align,
       darwin_rs6000_special_round_type_align): Skip cxx17_empty_base_field_p
       fields.
cp/
       * cp-objcp-common.h (cp_cxx17_empty_base_field_p): Declare.
       (LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P): Redefine.
       * class.c (cp_cxx17_empty_base_field_p): New function.

--- gcc/langhooks.h.jj  2020-03-17 13:50:52.262943386 +0100
+++ gcc/langhooks.h     2020-04-25 11:06:11.921931710 +0200
@@ -226,6 +226,9 @@ struct lang_hooks_for_decls
  /* True if this decl may be called via a sibcall.  */
  bool (*ok_for_sibcall) (const_tree);

+  /* True if this FIELD_DECL is C++17 empty base field.  */
+  bool (*cxx17_empty_base_field_p) (const_tree);
+
  /* Return a tree for the actual data of an array descriptor - or NULL_TREE
     if original tree is not an array descriptor.  If the second argument
     is true, only the TREE_TYPE is returned without generating a new tree.  */
--- gcc/langhooks-def.h.jj      2020-01-12 11:54:36.670409531 +0100
+++ gcc/langhooks-def.h 2020-04-25 11:06:03.300061347 +0200
@@ -241,6 +241,7 @@ extern tree lhd_unit_size_without_reusab
#define LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL lhd_warn_unused_global_decl
#define LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS NULL
#define LANG_HOOKS_DECL_OK_FOR_SIBCALL lhd_decl_ok_for_sibcall
+#define LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P    hook_bool_const_tree_false
#define LANG_HOOKS_OMP_ARRAY_DATA      hook_tree_tree_bool_null
#define LANG_HOOKS_OMP_IS_ALLOCATABLE_OR_PTR hook_bool_const_tree_false
#define LANG_HOOKS_OMP_CHECK_OPTIONAL_ARGUMENT hook_tree_tree_bool_null
@@ -269,6 +270,7 @@ extern tree lhd_unit_size_without_reusab
  LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL, \
  LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS, \
  LANG_HOOKS_DECL_OK_FOR_SIBCALL, \
+  LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P, \
  LANG_HOOKS_OMP_ARRAY_DATA, \
  LANG_HOOKS_OMP_IS_ALLOCATABLE_OR_PTR, \
  LANG_HOOKS_OMP_CHECK_OPTIONAL_ARGUMENT, \
--- gcc/calls.c.jj      2020-04-22 16:44:30.765946555 +0200
+++ gcc/calls.c 2020-04-25 11:14:45.038217088 +0200
@@ -6275,8 +6275,9 @@ cxx17_empty_base_field_p (const_tree fie
         && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))
         && DECL_SIZE (field)
         && integer_zerop (DECL_SIZE (field))
-         && TYPE_SIZE (TREE_TYPE (field))
-         && !integer_zerop (TYPE_SIZE (TREE_TYPE (field))));
+         && ((TYPE_SIZE (TREE_TYPE (field))
+              && !integer_zerop (TYPE_SIZE (TREE_TYPE (field))))
+             || lang_hooks.decls.cxx17_empty_base_field_p (field)));
}

/* Tell the garbage collector about GTY markers in this source file.  */
--- gcc/config/rs6000/rs6000.c.jj 2020-04-22 16:43:14.913087731 +0200 
+++ gcc/config/rs6000/rs6000.c  2020-04-25 11:16:01.536067016 +0200
@@ -7192,7 +7192,9 @@ rs6000_special_round_type_align (tree ty
  tree field = TYPE_FIELDS (type);

  /* Skip all non field decls */
-  while (field != NULL && TREE_CODE (field) != FIELD_DECL)
+  while (field != NULL
+        && (TREE_CODE (field) != FIELD_DECL
+            || cxx17_empty_base_field_p (field)))
    field = DECL_CHAIN (field);

  if (field != NULL && field != type)
@@ -7224,7 +7226,9 @@ darwin_rs6000_special_round_type_align (
  do {
    tree field = TYPE_FIELDS (type);
    /* Skip all non field decls */
-    while (field != NULL && TREE_CODE (field) != FIELD_DECL)
+    while (field != NULL
+          && (TREE_CODE (field) != FIELD_DECL
+              || cxx17_empty_base_field_p (field)))
      field = DECL_CHAIN (field);
    if (! field)
      break;
--- gcc/cp/cp-objcp-common.h.jj 2020-01-12 11:54:36.478412428 +0100
+++ gcc/cp/cp-objcp-common.h    2020-04-25 11:07:27.983788115 +0200
@@ -31,6 +31,7 @@ extern int cp_decl_dwarf_attribute (cons
extern int cp_type_dwarf_attribute (const_tree, int);
extern void cp_common_init_ts (void);
extern tree cp_unit_size_without_reusable_padding (tree);
+extern bool cp_cxx17_empty_base_field_p (const_tree);
extern tree cp_get_global_decls ();
extern tree cp_pushdecl (tree);
extern void cp_register_dumps (gcc::dump_manager *);
@@ -159,6 +160,8 @@ extern tree cxx_simulate_enum_decl (loca
#define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE cp_type_dwarf_attribute
#undef LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING
#define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING cp_unit_size_without_reusable_padding 
+#undef LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P
+#define LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P cp_cxx17_empty_base_field_p

#undef LANG_HOOKS_OMP_PREDETERMINED_SHARING
#define LANG_HOOKS_OMP_PREDETERMINED_SHARING cxx_omp_predetermined_sharing
--- gcc/cp/class.c.jj   2020-04-23 09:46:17.432972832 +0200
+++ gcc/cp/class.c      2020-04-25 11:13:13.060599884 +0200
@@ -6734,6 +6734,23 @@ layout_class_type (tree t, tree *virtual
    sizeof_biggest_empty_class = TYPE_SIZE_UNIT (t);
}

+/* Return true if FIELD is C++17 empty base artificial field.
+   The middle-end cxx17_empty_base_field_p predicate relies on
+   the empty base field having DECL_SIZE of bitsize_zero and
+   TYPE_SIZE of the field not bitsize_zero, but during class layout
+   that is not the case and so this langhook is used to detect those.  */
+
+bool
+cp_cxx17_empty_base_field_p (const_tree field)
+{
+  gcc_assert (TREE_CODE (field) == FIELD_DECL);
+  return (DECL_ARTIFICIAL (field)
+         && TYPE_CONTEXT (TREE_TYPE (field))
+         && CLASS_TYPE_P (TYPE_CONTEXT (TREE_TYPE (field)))
+         && TYPE_LANG_SPECIFIC (TYPE_CONTEXT (TREE_TYPE (field)))
+         && IS_FAKE_BASE_TYPE (TREE_TYPE (field)));
+}
+
/* Determine the "key method" for the class type indicated by TYPE,
   and set CLASSTYPE_KEY_METHOD accordingly.  */


       Jakub

Reply via email to