Message ID | 20240201171833.3240525-1-ppalka@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++/modules: anon union member of as-base class [PR112580] | expand |
On Thu, 1 Feb 2024, Patrick Palka wrote: > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? Ping. FWIW this fixes the remaining reported xtreme-header FAILs on x86_64-linux with -mx32 (not totally sure why the fails don't happen without -mx32). > > -- >8 -- > > Here when streaming in the fields of the as-base version of > _Formatting_scanner<int> we end up clobbering ANON_AGGR_TYPE_FIELD > of the anonymous union type, since it turns out this type is shared > between the original FIELD_DECL and the as-base FIELD_DECL copy (copied > during layout_class_type). ANON_AGGR_TYPE_FIELD first gets properly set > to the original FIELD_DECL when streaming in the canonical definition of > _Formatting_scanner<int>, and then gets overwritten to the as-base > FIELD_DECL when streaming in the the as-base definition. This leads to > lookup_anon_field giving the wrong answer when resolving the _M_values > use. > > This patch makes us avoid overwriting ANON_AGGR_TYPE_FIELD when streaming > in an as-base class definition, it should already be properly set at that > point. > > PR c++/112580 > > gcc/cp/ChangeLog: > > * module.cc (trees_in::read_class_def): When streaming in > an anonymous union field of an as-base class, don't clobber > ANON_AGGR_TYPE_FIELD. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/anon-3_a.H: New test. > * g++.dg/modules/anon-3_b.C: New test. > --- > gcc/cp/module.cc | 12 ++++++++++-- > gcc/testsuite/g++.dg/modules/anon-3_a.H | 21 +++++++++++++++++++++ > gcc/testsuite/g++.dg/modules/anon-3_b.C | 8 ++++++++ > 3 files changed, 39 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 3c2fef0e3f4..d36ba2eeeb3 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree maybe_template) > > if (TREE_CODE (decl) == FIELD_DECL > && ANON_AGGR_TYPE_P (TREE_TYPE (decl))) > - ANON_AGGR_TYPE_FIELD > - (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl; > + { > + tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl)); > + if (DECL_NAME (defn) == as_base_identifier) > + /* ANON_AGGR_TYPE_FIELD should already point to the > + original FIELD_DECL, don't overwrite it to point > + to the as-base FIELD_DECL copy. */ > + gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type)); > + else > + ANON_AGGR_TYPE_FIELD (anon_type) = decl; > + } > > if (TREE_CODE (decl) == USING_DECL > && TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE) > diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H b/gcc/testsuite/g++.dg/modules/anon-3_a.H > new file mode 100644 > index 00000000000..64a6aab51dd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H > @@ -0,0 +1,21 @@ > +// PR c++/112580 > +// { dg-additional-options "-fmodule-header" } > +// { dg-module-cmi {} } > + > +template <typename _Out> > +struct _Formatting_scanner { > + union { > + int _M_values = 42; > + }; > + virtual int _M_format_arg() { return _M_values; } > +}; > + > +template <typename _Tp> > +auto __do_vformat_to() { > + _Formatting_scanner<int> s; > + return s; > +} > + > +inline auto vformat() { > + return __do_vformat_to<int>(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C b/gcc/testsuite/g++.dg/modules/anon-3_b.C > new file mode 100644 > index 00000000000..d676fe47536 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C > @@ -0,0 +1,8 @@ > +// PR c++/112580 > +// { dg-additional-options "-fmodules-ts" } > + > +import "anon-3_a.H"; > + > +int main() { > + vformat()._M_format_arg(); > +} > -- > 2.43.0.493.gbc7ee2e5e1 > >
On 2/1/24 12:18, Patrick Palka wrote: > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? OK. > -- >8 -- > > Here when streaming in the fields of the as-base version of > _Formatting_scanner<int> we end up clobbering ANON_AGGR_TYPE_FIELD > of the anonymous union type, since it turns out this type is shared > between the original FIELD_DECL and the as-base FIELD_DECL copy (copied > during layout_class_type). ANON_AGGR_TYPE_FIELD first gets properly set > to the original FIELD_DECL when streaming in the canonical definition of > _Formatting_scanner<int>, and then gets overwritten to the as-base > FIELD_DECL when streaming in the the as-base definition. This leads to > lookup_anon_field giving the wrong answer when resolving the _M_values > use. > > This patch makes us avoid overwriting ANON_AGGR_TYPE_FIELD when streaming > in an as-base class definition, it should already be properly set at that > point. > > PR c++/112580 > > gcc/cp/ChangeLog: > > * module.cc (trees_in::read_class_def): When streaming in > an anonymous union field of an as-base class, don't clobber > ANON_AGGR_TYPE_FIELD. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/anon-3_a.H: New test. > * g++.dg/modules/anon-3_b.C: New test. > --- > gcc/cp/module.cc | 12 ++++++++++-- > gcc/testsuite/g++.dg/modules/anon-3_a.H | 21 +++++++++++++++++++++ > gcc/testsuite/g++.dg/modules/anon-3_b.C | 8 ++++++++ > 3 files changed, 39 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/anon-3_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 3c2fef0e3f4..d36ba2eeeb3 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree maybe_template) > > if (TREE_CODE (decl) == FIELD_DECL > && ANON_AGGR_TYPE_P (TREE_TYPE (decl))) > - ANON_AGGR_TYPE_FIELD > - (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl; > + { > + tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl)); > + if (DECL_NAME (defn) == as_base_identifier) > + /* ANON_AGGR_TYPE_FIELD should already point to the > + original FIELD_DECL, don't overwrite it to point > + to the as-base FIELD_DECL copy. */ > + gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type)); > + else > + ANON_AGGR_TYPE_FIELD (anon_type) = decl; > + } > > if (TREE_CODE (decl) == USING_DECL > && TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE) > diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H b/gcc/testsuite/g++.dg/modules/anon-3_a.H > new file mode 100644 > index 00000000000..64a6aab51dd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H > @@ -0,0 +1,21 @@ > +// PR c++/112580 > +// { dg-additional-options "-fmodule-header" } > +// { dg-module-cmi {} } > + > +template <typename _Out> > +struct _Formatting_scanner { > + union { > + int _M_values = 42; > + }; > + virtual int _M_format_arg() { return _M_values; } > +}; > + > +template <typename _Tp> > +auto __do_vformat_to() { > + _Formatting_scanner<int> s; > + return s; > +} > + > +inline auto vformat() { > + return __do_vformat_to<int>(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C b/gcc/testsuite/g++.dg/modules/anon-3_b.C > new file mode 100644 > index 00000000000..d676fe47536 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C > @@ -0,0 +1,8 @@ > +// PR c++/112580 > +// { dg-additional-options "-fmodules-ts" } > + > +import "anon-3_a.H"; > + > +int main() { > + vformat()._M_format_arg(); > +}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 3c2fef0e3f4..d36ba2eeeb3 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -12178,8 +12178,16 @@ trees_in::read_class_def (tree defn, tree maybe_template) if (TREE_CODE (decl) == FIELD_DECL && ANON_AGGR_TYPE_P (TREE_TYPE (decl))) - ANON_AGGR_TYPE_FIELD - (TYPE_MAIN_VARIANT (TREE_TYPE (decl))) = decl; + { + tree anon_type = TYPE_MAIN_VARIANT (TREE_TYPE (decl)); + if (DECL_NAME (defn) == as_base_identifier) + /* ANON_AGGR_TYPE_FIELD should already point to the + original FIELD_DECL, don't overwrite it to point + to the as-base FIELD_DECL copy. */ + gcc_checking_assert (ANON_AGGR_TYPE_FIELD (anon_type)); + else + ANON_AGGR_TYPE_FIELD (anon_type) = decl; + } if (TREE_CODE (decl) == USING_DECL && TREE_CODE (USING_DECL_SCOPE (decl)) == RECORD_TYPE) diff --git a/gcc/testsuite/g++.dg/modules/anon-3_a.H b/gcc/testsuite/g++.dg/modules/anon-3_a.H new file mode 100644 index 00000000000..64a6aab51dd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/anon-3_a.H @@ -0,0 +1,21 @@ +// PR c++/112580 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +template <typename _Out> +struct _Formatting_scanner { + union { + int _M_values = 42; + }; + virtual int _M_format_arg() { return _M_values; } +}; + +template <typename _Tp> +auto __do_vformat_to() { + _Formatting_scanner<int> s; + return s; +} + +inline auto vformat() { + return __do_vformat_to<int>(); +} diff --git a/gcc/testsuite/g++.dg/modules/anon-3_b.C b/gcc/testsuite/g++.dg/modules/anon-3_b.C new file mode 100644 index 00000000000..d676fe47536 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/anon-3_b.C @@ -0,0 +1,8 @@ +// PR c++/112580 +// { dg-additional-options "-fmodules-ts" } + +import "anon-3_a.H"; + +int main() { + vformat()._M_format_arg(); +}