From 2e0bb9eec2d455840bc4773391b3313a320b3c23 Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Mon, 1 Mar 2021 05:41:10 -0800 Subject: [PATCH] c++: Completeness of typedef structs [PR 99294] When we read in a class definition, we use fixup_type_variants to propagate the now-completed fields of the class's TYPE to other variants. Unfortunately that doesn't propagate all of them, and in this case we had a typedef to an (incomplete) instantiation. That typedef ended up with a VOIDmode, which blew up gimple expansion as the type itself isn't VOID. Without modules, that information is propagated in finalize_type_size when laying out the class. But that doesn't happen with stream-in -- we already know the layout. There is already some overlap between the two functions, now there's a bit more. In fixup_type_variants, I pay attention to the TYPE_NAME to decide whether to override a user's TYPE_ALIGN -- variants of the main-variant typedef just copy the main-variant. Other variants recalculate. Overaligning is still permitted. I also added a TYPE_ALIGN_RAW accessor, and fixed a bug in the alignment streaming I noticed. I did not refactor TYPE_ALIGN beyond using the new accessor. (It could be written as ((1 << align_raw) >> 1), rather than use the conditional.) PR c++/99294 gcc/ * tree.h (TYPE_ALIGN_RAW): New accessor. (TYPE_ALIGN): Use it. gcc/cp/ * class.c (fixup_type_variants): Propagate mode, precision, alignment & emptiness. * module.cc (trees_out::type_node): Use TYPE_ALIGN_RAW. (trees_in::tree_node): Rematerialize alignment here. gcc/testsuite/ * g++.dg/modules/pr99294.h: New. * g++.dg/modules/pr99294_a.C: New. * g++.dg/modules/pr99294_b.C: New. --- gcc/cp/class.c | 46 +++++++++++++++++++------------- gcc/cp/module.cc | 4 +-- gcc/testsuite/g++.dg/modules/pr99294.h | 14 ++++++++++ gcc/testsuite/g++.dg/modules/pr99294_a.C | 18 +++++++++++++ gcc/testsuite/g++.dg/modules/pr99294_b.C | 12 +++++++++ gcc/tree.h | 10 ++++--- 6 files changed, 81 insertions(+), 23 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/pr99294.h create mode 100644 gcc/testsuite/g++.dg/modules/pr99294_a.C create mode 100644 gcc/testsuite/g++.dg/modules/pr99294_b.C diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 40f5fef..ea007e8 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -2005,35 +2005,45 @@ determine_primary_bases (tree t) /* Update the variant types of T. */ void -fixup_type_variants (tree t) +fixup_type_variants (tree type) { - tree variants; - - if (!t) + if (!type) return; - for (variants = TYPE_NEXT_VARIANT (t); - variants; - variants = TYPE_NEXT_VARIANT (variants)) + for (tree variant = TYPE_NEXT_VARIANT (type); + variant; + variant = TYPE_NEXT_VARIANT (variant)) { /* These fields are in the _TYPE part of the node, not in the TYPE_LANG_SPECIFIC component, so they are not shared. */ - TYPE_HAS_USER_CONSTRUCTOR (variants) = TYPE_HAS_USER_CONSTRUCTOR (t); - TYPE_NEEDS_CONSTRUCTING (variants) = TYPE_NEEDS_CONSTRUCTING (t); - TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variants) - = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t); + TYPE_HAS_USER_CONSTRUCTOR (variant) = TYPE_HAS_USER_CONSTRUCTOR (type); + TYPE_NEEDS_CONSTRUCTING (variant) = TYPE_NEEDS_CONSTRUCTING (type); + TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variant) + = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type); - TYPE_POLYMORPHIC_P (variants) = TYPE_POLYMORPHIC_P (t); - CLASSTYPE_FINAL (variants) = CLASSTYPE_FINAL (t); + TYPE_POLYMORPHIC_P (variant) = TYPE_POLYMORPHIC_P (type); + CLASSTYPE_FINAL (variant) = CLASSTYPE_FINAL (type); - TYPE_BINFO (variants) = TYPE_BINFO (t); + TYPE_BINFO (variant) = TYPE_BINFO (type); /* Copy whatever these are holding today. */ - TYPE_VFIELD (variants) = TYPE_VFIELD (t); - TYPE_FIELDS (variants) = TYPE_FIELDS (t); + TYPE_VFIELD (variant) = TYPE_VFIELD (type); + TYPE_FIELDS (variant) = TYPE_FIELDS (type); + + TYPE_SIZE (variant) = TYPE_SIZE (type); + TYPE_SIZE_UNIT (variant) = TYPE_SIZE_UNIT (type); + + if (!TYPE_USER_ALIGN (variant) + || TYPE_NAME (variant) == TYPE_NAME (type) + || TYPE_ALIGN_RAW (variant) < TYPE_ALIGN_RAW (type)) + { + TYPE_ALIGN_RAW (variant) = TYPE_ALIGN_RAW (type); + TYPE_USER_ALIGN (variant) = TYPE_USER_ALIGN (type); + } - TYPE_SIZE (variants) = TYPE_SIZE (t); - TYPE_SIZE_UNIT (variants) = TYPE_SIZE_UNIT (t); + TYPE_PRECISION (variant) = TYPE_PRECISION (type); + TYPE_MODE_RAW (variant) = TYPE_MODE_RAW (type); + TYPE_EMPTY_P (variant) = TYPE_EMPTY_P (type); } } diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 0cb5bd9..369a026 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -8714,7 +8714,7 @@ trees_out::type_node (tree type) else { if (TYPE_USER_ALIGN (type)) - flags = exact_log2 (TYPE_ALIGN (type)); + flags = TYPE_ALIGN_RAW (type); } if (streaming_p ()) @@ -9510,7 +9510,7 @@ trees_in::tree_node (bool is_use) } else { - res = build_aligned_type (res, 1u << flags); + res = build_aligned_type (res, (1u << flags) >> 1); TYPE_USER_ALIGN (res) = true; } diff --git a/gcc/testsuite/g++.dg/modules/pr99294.h b/gcc/testsuite/g++.dg/modules/pr99294.h new file mode 100644 index 0000000..757113c --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99294.h @@ -0,0 +1,14 @@ + +template +class basic_string; + +typedef basic_string string; + +template +class basic_string +{ + public: + string Frob (); + + basic_string (int); +}; diff --git a/gcc/testsuite/g++.dg/modules/pr99294_a.C b/gcc/testsuite/g++.dg/modules/pr99294_a.C new file mode 100644 index 0000000..ac8b9a7 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99294_a.C @@ -0,0 +1,18 @@ +// PR 99294, ICE with -fno-module-lazy on class completeness +// { dg-additional-options -fmodules-ts } + +// The instantiation of the *definition* of basic_string is used in +// importers, *after* they have instantiated a declaration of it *and* +// created type variants. + +module; + +#include "pr99294.h" + +export module foo; +// { dg-module-cmi foo } + +export inline int greeter (string const &bob) +{ + return sizeof (bob); // instantiates string +} diff --git a/gcc/testsuite/g++.dg/modules/pr99294_b.C b/gcc/testsuite/g++.dg/modules/pr99294_b.C new file mode 100644 index 0000000..30d14d1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr99294_b.C @@ -0,0 +1,12 @@ +// PR 99294, ICE with -fno-module-lazy on class completeness +// { dg-additional-options {-fmodules-ts -fno-module-lazy} } + +#include "pr99294.h" +import foo; + +string Quux () +{ + return 1; +} + +// ICED complaining about Quux RETURN_DECL during gimple expand diff --git a/gcc/tree.h b/gcc/tree.h index 4f33868..f00ea2e 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2078,12 +2078,16 @@ extern tree vector_element_bits_tree (const_tree); to this type. */ #define TYPE_ATTRIBUTES(NODE) (TYPE_CHECK (NODE)->type_common.attributes) +/* Raw access to the alignment field. */ +#define TYPE_ALIGN_RAW(NODE) \ + (TYPE_CHECK (NODE)->type_common.align) + /* The alignment necessary for objects of this type. The value is an int, measured in bits and must be a power of two. We support also an "alignment" of zero. */ -#define TYPE_ALIGN(NODE) \ - (TYPE_CHECK (NODE)->type_common.align \ - ? ((unsigned)1) << ((NODE)->type_common.align - 1) : 0) +#define TYPE_ALIGN(NODE) \ + (TYPE_ALIGN_RAW (NODE) \ + ? ((unsigned)1) << (TYPE_ALIGN_RAW(NODE) - 1) : 0) /* Specify that TYPE_ALIGN(NODE) is X. */ #define SET_TYPE_ALIGN(NODE, X) \ -- 2.7.4