Bug 91222 - [10 Regression] 507.cactuBSSN_r build fails in warn_types_mismatch at ipa-devirt.c:1006 since r273571
Summary: [10 Regression] 507.cactuBSSN_r build fails in warn_types_mismatch at ipa-dev...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 10.0
: P1 normal
Target Milestone: 10.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
: 91273 (view as bug list)
Depends on:
Blocks: spec
  Show dependency treegraph
 
Reported: 2019-07-22 10:08 UTC by Martin Liška
Modified: 2019-12-02 08:16 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work: 9.1.0
Known to fail: 10.0
Last reconfirmed: 2019-10-01 00:00:00


Attachments
clear TYPE_NAME in free_lang_data for anonymous types (404 bytes, patch)
2019-08-27 14:38 UTC, Jason Merrill
Details | Diff
clear TYPE_NAME in free_lang_data for extern "C" anonymous types (688 bytes, patch)
2019-08-27 18:48 UTC, Jason Merrill
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2019-07-22 10:08:46 UTC
Following reduced test-case fails:

$ cat 1.ii
extern "C" {
struct {
  double AlphaDriver;
  double BetaDriver;
  double EpsDiss;
  double LapseACoeff;
  double LapseAdvectionCoeff;
  double MinimumLapse;
  double ShiftAdvectionCoeff;
  double ShiftBCoeff;
  double ShiftGammaCoeff;
  double SpatialBetaDriverRadius;
  double SpatialShiftGammaCoeffRadius;
  double harmonicF;
  const char *UseSpatialBetaDriver;
  const char *calculate_ADMBase_variables_at;
  const char *my_initial_boundary_condition;
  const char *my_rhs_boundary_condition;
  int ML_BSSN_Advect_calc_every;
  int ML_BSSN_Advect_calc_offset;
  int ML_BSSN_Dissipation_calc_every;
  int ML_BSSN_Dissipation_calc_offset;
  int ML_BSSN_InitGamma_calc_every;
  int ML_BSSN_InitGamma_calc_offset;
  int ML_BSSN_InitRHS_calc_every;
  int ML_BSSN_InitRHS_calc_offset;
  int ML_BSSN_MaxNumArrayEvolvedVars;
  int ML_BSSN_MaxNumEvolvedVars;
  int ML_BSSN_Minkowski_calc_every;
  int ML_BSSN_Minkowski_calc_offset;
  int ML_BSSN_RHSStaticBoundary_calc_every;
  int ML_BSSN_RHSStaticBoundary_calc_offset;
  int ML_BSSN_RHS_calc_every;
  int ML_BSSN_RHS_calc_offset;
  int ML_BSSN_boundary_calc_every;
  int ML_BSSN_boundary_calc_offset;
  int ML_BSSN_constraints_calc_every;
  int ML_BSSN_constraints_calc_offset;
  int ML_BSSN_convertFromADMBaseGamma_calc_every;
  int ML_BSSN_convertFromADMBaseGamma_calc_offset;
  int ML_BSSN_convertFromADMBase_calc_every;
  int ML_BSSN_convertFromADMBase_calc_offset;
  int ML_BSSN_convertToADMBaseDtLapseShiftBoundary_calc_every;
  int ML_BSSN_convertToADMBaseDtLapseShiftBoundary_calc_offset;
  int ML_BSSN_convertToADMBaseDtLapseShift_calc_every;
  int ML_BSSN_convertToADMBaseDtLapseShift_calc_offset;
  int ML_BSSN_convertToADMBaseFakeDtLapseShift_calc_every;
  int ML_BSSN_convertToADMBaseFakeDtLapseShift_calc_offset;
  int ML_BSSN_convertToADMBase_calc_every;
  int ML_BSSN_convertToADMBase_calc_offset;
  int ML_BSSN_enforce_calc_every;
  int ML_BSSN_enforce_calc_offset;
  int ShiftAlphaPower;
  int conformalMethod;
  int fdOrder;
  int harmonicN;
  int harmonicShift;
  int other_timelevels;
  int rhs_timelevels;
  int timelevels;
  int verbose;
} ml_bssnrest_;
}

$ cat 2.ii
extern "C" {
extern struct {
  double AlphaDriver;
  double BetaDriver;
  double EpsDiss;
  double LapseACoeff;
  double LapseAdvectionCoeff;
  double MinimumLapse;
  double ShiftAdvectionCoeff;
  double ShiftBCoeff;
  double ShiftGammaCoeff;
  double SpatialBetaDriverRadius;
  double SpatialShiftGammaCoeffRadius;
  double harmonicF;
  const char *UseSpatialBetaDriver;
  const char *calculate_ADMBase_variables_at;
  const char *my_initial_boundary_condition;
  const char *my_rhs_boundary_condition;
  int ML_BSSN_Advect_calc_every;
  int ML_BSSN_Advect_calc_offset;
  int ML_BSSN_Dissipation_calc_every;
  int ML_BSSN_Dissipation_calc_offset;
  int ML_BSSN_InitGamma_calc_every;
  int ML_BSSN_InitGamma_calc_offset;
  int ML_BSSN_InitRHS_calc_every;
  int ML_BSSN_InitRHS_calc_offset;
  int ML_BSSN_MaxNumArrayEvolvedVars;
  int ML_BSSN_MaxNumEvolvedVars;
  int ML_BSSN_Minkowski_calc_every;
  int ML_BSSN_Minkowski_calc_offset;
  int ML_BSSN_RHSStaticBoundary_calc_every;
  int ML_BSSN_RHSStaticBoundary_calc_offset;
  int ML_BSSN_RHS_calc_every;
  int ML_BSSN_RHS_calc_offset;
  int ML_BSSN_boundary_calc_every;
  int ML_BSSN_boundary_calc_offset;
  int ML_BSSN_constraints_calc_every;
  int ML_BSSN_constraints_calc_offset;
  int ML_BSSN_convertFromADMBaseGamma_calc_every;
  int ML_BSSN_convertFromADMBaseGamma_calc_offset;
  int ML_BSSN_convertFromADMBase_calc_every;
  int ML_BSSN_convertFromADMBase_calc_offset;
  int ML_BSSN_convertToADMBaseDtLapseShiftBoundary_calc_every;
  int ML_BSSN_convertToADMBaseDtLapseShiftBoundary_calc_offset;
  int ML_BSSN_convertToADMBaseDtLapseShift_calc_every;
  int ML_BSSN_convertToADMBaseDtLapseShift_calc_offset;
  int ML_BSSN_convertToADMBaseFakeDtLapseShift_calc_every;
  int ML_BSSN_convertToADMBaseFakeDtLapseShift_calc_offset;
  int ML_BSSN_convertToADMBase_calc_every;
  int ML_BSSN_convertToADMBase_calc_offset;
  int ML_BSSN_enforce_calc_every;
  int ML_BSSN_enforce_calc_offset;
  int ShiftAlphaPower;
  int conformalMethod;
  int fdOrder;
  int harmonicN;
  int harmonicShift;
  int other_timelevels;
  int rhs_timelevels;
  int timelevels;
  int verbose;
} ml_bssnrest_;
int fdOrder(ml_bssnrest_.fdOrder);
}

$ gcc -flto 1.ii 2.ii -shared -fPIC
2.ii:62:3: warning: 'ml_bssnrest_' violates the C++ One Definition Rule [-Wodr]
   62 | } ml_bssnrest_;
      |   ^
lto1: internal compiler error: Segmentation fault
0xc7f71f crash_signal
	/home/marxin/Programming/gcc/gcc/toplev.c:326
0x7f6ba9cc5e4f ???
	/usr/src/debug/glibc-2.29-7.3.x86_64/signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
0xa70bdc tree_check(tree_node*, char const*, int, char const*, tree_code)
	/home/marxin/Programming/gcc/gcc/tree.h:3219
0xa70bdc warn_types_mismatch(tree_node*, tree_node*, unsigned int, unsigned int)
	/home/marxin/Programming/gcc/gcc/ipa-devirt.c:1006
0x7e4a9c lto_symtab_merge_decls_2
	/home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:722
0x7e4a9c lto_symtab_merge_decls_1
	/home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:861
0x7e4a9c lto_symtab_merge_decls()
	/home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:887
0x7f0628 read_cgraph_and_symbols(unsigned int, char const**)
	/home/marxin/Programming/gcc/gcc/lto/lto-common.c:2839
0x7d6e32 lto_main()
	/home/marxin/Programming/gcc/gcc/lto/lto.c:616
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
lto-wrapper: fatal error: gcc returned 1 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
Comment 1 Martin Liška 2019-07-23 14:40:05 UTC
We end with:

(gdb) bt
#0  0x000000000084a2d3 in tree_check (__t=0x0, __f=0x171bb40 "/home/marxin/Programming/gcc/gcc/ipa-devirt.c", __l=1006, __g=0x171c128 "warn_types_mismatch", __c=IDENTIFIER_NODE) at /home/marxin/Programming/gcc/gcc/tree.h:3219
#1  0x0000000000a74c9c in warn_types_mismatch (t1=0x7ffff789edc8, t2=0x7ffff74aa1f8, loc1=245888, loc2=491776) at /home/marxin/Programming/gcc/gcc/ipa-devirt.c:1006
#2  0x00000000007ebaed in lto_symtab_merge_decls_2 (diagnosed_p=<optimized out>, first=<optimized out>) at /home/marxin/Programming/gcc/gcc/tree.h:3335
#3  lto_symtab_merge_decls_1 (first=<optimized out>) at /home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:861
#4  lto_symtab_merge_decls () at /home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:887
#5  0x00000000007f7679 in read_cgraph_and_symbols (nfiles=<optimized out>, fnames=<optimized out>) at /home/marxin/Programming/gcc/gcc/lto/lto-common.c:2839
#6  0x00000000007dde83 in lto_main () at /home/marxin/Programming/gcc/gcc/lto/lto.c:616
#7  0x0000000000c90f20 in compile_file () at /home/marxin/Programming/gcc/gcc/toplev.c:456
#8  0x00000000007b6318 in do_compile () at /home/marxin/Programming/gcc/gcc/toplev.c:2188
#9  toplev::main (this=this@entry=0x7fffffffda4e, argc=<optimized out>, argc@entry=17, argv=<optimized out>, argv@entry=0x7fffffffdb48) at /home/marxin/Programming/gcc/gcc/toplev.c:2323
#10 0x00000000007b9e3f in main (argc=17, argv=0x7fffffffdb48) at /home/marxin/Programming/gcc/gcc/main.c:39

$ (gdb) p debug_tree(t1)
 <record_type 0x7ffff789edc8 cxx-odr-p BLK
    size <integer_cst 0x7ffff789c558 type <integer_type 0x7ffff76b60a8 bitsizetype> constant 2432>
    unit-size <integer_cst 0x7ffff775df18 type <integer_type 0x7ffff76b6000 sizetype> constant 304>
    align:64 warn_if_not_align:0 symtab:0 alias-set 5 canonical-type 0x7ffff789edc8
    fields <field_decl 0x7ffff74a3688 AlphaDriver
        type <real_type 0x7ffff76bd348 double DF
            size <integer_cst 0x7ffff76a1ba0 constant 64>
            unit-size <integer_cst 0x7ffff76a1bb8 constant 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type 0x7ffff76bd348 precision:64
            pointer_to_this <pointer_type 0x7ffff76bd888>>
        nonlocal DF pr91222.ii:3:10 size <integer_cst 0x7ffff76a1ba0 64> unit-size <integer_cst 0x7ffff76a1bb8 8>
        align:64 warn_if_not_align:0 offset_align 128
        offset <integer_cst 0x7ffff76a1bd0 constant 0>
        bit-offset <integer_cst 0x7ffff76a1c18 constant 0> context <record_type 0x7ffff789edc8>
        chain <field_decl 0x7ffff74a8098 BetaDriver type <real_type 0x7ffff76bd348 double>
            nonlocal DF pr91222.ii:4:10 size <integer_cst 0x7ffff76a1ba0 64> unit-size <integer_cst 0x7ffff76a1bb8 8>
            align:64 warn_if_not_align:0 offset_align 128 offset <integer_cst 0x7ffff76a1bd0 0> bit-offset <integer_cst 0x7ffff76a1ba0 64> context <record_type 0x7ffff789edc8> chain <field_decl 0x7ffff74a3720 EpsDiss>>> context <translation_unit_decl 0x7ffff76ab168 pr91222.ii>>
$3 = void
(gdb) p debug_tree(t2)
 <record_type 0x7ffff74aa1f8 cxx-odr-p BLK
    size <integer_cst 0x7ffff789c558 type <integer_type 0x7ffff76b60a8 bitsizetype> constant 2432>
    unit-size <integer_cst 0x7ffff775df18 type <integer_type 0x7ffff76b6000 sizetype> constant 304>
    align:64 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x7ffff74aa1f8
    fields <field_decl 0x7ffff74a9be0 AlphaDriver
        type <real_type 0x7ffff76bd348 double DF
            size <integer_cst 0x7ffff76a1ba0 constant 64>
            unit-size <integer_cst 0x7ffff76a1bb8 constant 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set 2 canonical-type 0x7ffff76bd348 precision:64
            pointer_to_this <pointer_type 0x7ffff76bd888>>
        nonlocal DF pr91222-2.ii:3:10 size <integer_cst 0x7ffff76a1ba0 64> unit-size <integer_cst 0x7ffff76a1bb8 8>
        align:64 warn_if_not_align:0 offset_align 128
        offset <integer_cst 0x7ffff76a1bd0 constant 0>
        bit-offset <integer_cst 0x7ffff76a1c18 constant 0> context <record_type 0x7ffff74aa1f8>
        chain <field_decl 0x7ffff74ab558 BetaDriver type <real_type 0x7ffff76bd348 double>
            nonlocal DF pr91222-2.ii:4:10 size <integer_cst 0x7ffff76a1ba0 64> unit-size <integer_cst 0x7ffff76a1bb8 8>
            align:64 warn_if_not_align:0 offset_align 128 offset <integer_cst 0x7ffff76a1bd0 0> bit-offset <integer_cst 0x7ffff76a1ba0 64> context <record_type 0x7ffff74aa1f8> chain <field_decl 0x7ffff74a9c78 EpsDiss>>> context <translation_unit_decl 0x7ffff76ab1e0 pr91222-2.ii>
    pointer_to_this <pointer_type 0x7ffff74aa2a0>>

$ (gdb) p n1
$5 = (tree) 0x0
(gdb) p n2
$6 = (tree) 0x0
Comment 2 Jan Hubicka 2019-07-29 08:13:14 UTC
This patch fixes the ICE
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c        (revision 273865)
+++ ipa-devirt.c        (working copy)
@@ -1003,7 +1003,7 @@ warn_types_mismatch (tree t1, tree t2, l
        n2 = DECL_NAME (n2);
       /* Most of the time, the type names will match, do not be unnecesarily
          verbose.  */
-      if (IDENTIFIER_POINTER (n1) != IDENTIFIER_POINTER (n2))
+      if (n1 != n2)
         inform (loc_t1,
                "type %qT defined in anonymous namespace cannot match "
                "type %qT across the translation unit boundary",
however the warnings output are:
2.ii:62:3: warning: ‘ml_bssnrest_’ violates the C++ One Definition Rule [-Wodr]
   62 | } ml_bssnrest_;
      |   ^
1.ii:2:8: note: type ‘struct <anon>’ defined in anonymous namespace cannot match across the translation unit boundary
    2 | struct {
      |        ^
2.ii:2:15: note: the incompatible type defined in another translation unit
    2 | extern struct {
      |               ^
1.ii:62:3: note: ‘ml_bssnrest_’ was previously declared here
   62 | } ml_bssnrest_;
      |   ^

Now I wonder why C++ FE makes the struct anonymous namespace when it is declared with extern "C".
I have checked that type_in_anonymous_namespace_p(prevailing_type) returns true because mangled name is <anon> which comes from C++ FE.

I will check in the patch to avoid ICE. Jason, can you please look if it is correct to consider these types anonymous?
Comment 3 Jan Hubicka 2019-07-29 08:19:10 UTC
Author: hubicka
Date: Mon Jul 29 08:18:38 2019
New Revision: 273866

URL: https://gcc.gnu.org/viewcvs?rev=273866&root=gcc&view=rev
Log:

	PR lto/91222
	* ipa-devirt.c (warn_types_mismatch): Compare indentifiers
	than INDENTIFIER_POINTER.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-devirt.c
Comment 4 Jan Hubicka 2019-07-29 08:20:40 UTC

I am changing component to C++. It seems to me that types in question should not be considered anonymous namespace.
Comment 5 Martin Liška 2019-07-31 08:31:03 UTC
(In reply to Jan Hubicka from comment #3)
> Author: hubicka
> Date: Mon Jul 29 08:18:38 2019
> New Revision: 273866
> 
> URL: https://gcc.gnu.org/viewcvs?rev=273866&root=gcc&view=rev
> Log:
> 
> 	PR lto/91222
> 	* ipa-devirt.c (warn_types_mismatch): Compare indentifiers
> 	than INDENTIFIER_POINTER.
> 
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/ipa-devirt.c

Can you Honza please attach a test-case for it to test-suite?
Comment 6 Martin Liška 2019-07-31 08:36:44 UTC
Honza, I can still see cactuBSSN_r failing with:

$ cat 1.i
struct {
} admbaserest_;

$ cat 2.ii
extern "C" {
struct {
} admbaserest_;
}

$ gcc 1.i 2.ii -flto
1.i:2:3: warning: type of ‘admbaserest_’ does not match original declaration [-Wlto-type-mismatch]
    2 | } admbaserest_;
      |   ^
lto1: internal compiler error: in warn_types_mismatch, at ipa-devirt.c:995
0x62691c warn_types_mismatch(tree_node*, tree_node*, unsigned int, unsigned int)
	/home/marxin/Programming/gcc/gcc/ipa-devirt.c:995
0x7e78cc lto_symtab_merge_decls_2
	/home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:722
0x7e78cc lto_symtab_merge_decls_1
	/home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:861
0x7e78cc lto_symtab_merge_decls()
	/home/marxin/Programming/gcc/gcc/lto/lto-symtab.c:887
0x7f3488 read_cgraph_and_symbols(unsigned int, char const**)
	/home/marxin/Programming/gcc/gcc/lto/lto-common.c:2839
0x7d9c72 lto_main()
	/home/marxin/Programming/gcc/gcc/lto/lto.c:616
Comment 7 Jan Hubicka 2019-07-31 09:04:11 UTC
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91222
> 
> --- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
> (In reply to Jan Hubicka from comment #3)
> > Author: hubicka
> > Date: Mon Jul 29 08:18:38 2019
> > New Revision: 273866
> > 
> > URL: https://gcc.gnu.org/viewcvs?rev=273866&root=gcc&view=rev
> > Log:
> > 
> > 	PR lto/91222
> > 	* ipa-devirt.c (warn_types_mismatch): Compare indentifiers
> > 	than INDENTIFIER_POINTER.
> > 
> > Modified:
> >     trunk/gcc/ChangeLog
> >     trunk/gcc/ipa-devirt.c
> 
> Can you Honza please attach a test-case for it to test-suite?

I still think we should not warn in this case, so plan is to commit it
after we discuss the underlying issue why C++ FE declares the types as
anonymous.

Honza
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.
Comment 8 Jason Merrill 2019-08-14 20:52:42 UTC
(In reply to Jan Hubicka from comment #2)
> 2.ii:62:3: warning: ‘ml_bssnrest_’ violates the C++ One Definition Rule
> [-Wodr]
>    62 | } ml_bssnrest_;
>       |   ^
> 1.ii:2:8: note: type ‘struct <anon>’ defined in anonymous namespace cannot
> match across the translation unit boundary
>     2 | struct {
>       |        ^
> 2.ii:2:15: note: the incompatible type defined in another translation unit
>     2 | extern struct {
>       |               ^
> 1.ii:62:3: note: ‘ml_bssnrest_’ was previously declared here
>    62 | } ml_bssnrest_;
>       |   ^
> 
> Now I wonder why C++ FE makes the struct anonymous namespace when it is
> declared with extern "C".
> I have checked that type_in_anonymous_namespace_p(prevailing_type) returns
> true because mangled name is <anon> which comes from C++ FE.
> 
> I will check in the patch to avoid ICE. Jason, can you please look if it is
> correct to consider these types anonymous?

They aren't in the anonymous namespace, but they are themselves anonymous, so they have no linkage.  The standard says,

A type without linkage shall not be used as the type of a variable or function with external linkage unless
— the entity has C language linkage (7.5), or
— the entity is declared within an unnamed namespace (7.3.1), or
— the entity is not odr-used (3.2) or is defined in the same translation unit.

Here 1.ii is OK under the third bullet, and 2.ii under the first bullet.  Within extern "C" I guess we need to do structural comparison for anonymous types rather than rely on the ODR.
Comment 9 Martin Liška 2019-08-27 14:05:23 UTC
(In reply to Jason Merrill from comment #8)
> (In reply to Jan Hubicka from comment #2)
> > 2.ii:62:3: warning: ‘ml_bssnrest_’ violates the C++ One Definition Rule
> > [-Wodr]
> >    62 | } ml_bssnrest_;
> >       |   ^
> > 1.ii:2:8: note: type ‘struct <anon>’ defined in anonymous namespace cannot
> > match across the translation unit boundary
> >     2 | struct {
> >       |        ^
> > 2.ii:2:15: note: the incompatible type defined in another translation unit
> >     2 | extern struct {
> >       |               ^
> > 1.ii:62:3: note: ‘ml_bssnrest_’ was previously declared here
> >    62 | } ml_bssnrest_;
> >       |   ^
> > 
> > Now I wonder why C++ FE makes the struct anonymous namespace when it is
> > declared with extern "C".
> > I have checked that type_in_anonymous_namespace_p(prevailing_type) returns
> > true because mangled name is <anon> which comes from C++ FE.
> > 
> > I will check in the patch to avoid ICE. Jason, can you please look if it is
> > correct to consider these types anonymous?
> 
> They aren't in the anonymous namespace, but they are themselves anonymous,
> so they have no linkage.  The standard says,
> 
> A type without linkage shall not be used as the type of a variable or
> function with external linkage unless
> — the entity has C language linkage (7.5), or
> — the entity is declared within an unnamed namespace (7.3.1), or
> — the entity is not odr-used (3.2) or is defined in the same translation
> unit.
> 
> Here 1.ii is OK under the third bullet, and 2.ii under the first bullet. 
> Within extern "C" I guess we need to do structural comparison for anonymous
> types rather than rely on the ODR.

Honza?
Comment 10 Jan Hubicka 2019-08-27 14:10:41 UTC
> > 
> > They aren't in the anonymous namespace, but they are themselves anonymous,
> > so they have no linkage.  The standard says,
> > 
> > A type without linkage shall not be used as the type of a variable or
> > function with external linkage unless
> > — the entity has C language linkage (7.5), or
> > — the entity is declared within an unnamed namespace (7.3.1), or
> > — the entity is not odr-used (3.2) or is defined in the same translation
> > unit.
> > 
> > Here 1.ii is OK under the third bullet, and 2.ii under the first bullet. 
> > Within extern "C" I guess we need to do structural comparison for anonymous
> > types rather than rely on the ODR.
> 
> Honza?

In order to disable this for ODR merging, we need to arrange the
(already long) TYPE_DECL condition in tree.c:need_assembler_name_p
to be false for them or C++ get_assembler_name langhook to return NULL
in this case.

I wonder how we can arrange that?
Honza
Comment 11 Jason Merrill 2019-08-27 14:38:28 UTC
Created attachment 46765 [details]
clear TYPE_NAME in free_lang_data for anonymous types

Perhaps like this?
Comment 12 Jan Hubicka 2019-08-27 14:42:26 UTC
> Created attachment 46765 [details]
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46765&action=edit
> clear TYPE_NAME in free_lang_data for anonymous types
> 
> Perhaps like this?

It seems that this will disable ODR handling of all anonymous types, not
only those declared as "extern C".  We make use of the info (by mangling
them to <anon>) to handle them during devirtualization and also TBAA. So
perhaps we want to be more specific here?

Honza
Comment 13 Jason Merrill 2019-08-27 18:46:44 UTC
Ah, I was reading the passage a bit wrong: where the extern "C" matters is not on the type, but on the variable (ml_bssnrest_).  Because it's extern "C", declarations in different translation units correspond even though we can't use the same type in both.

But that still doesn't make the types the same, and the use of the variable in 2.ii has undefined behavior because it is accessing the value of the object through the wrong type, so the warning is correct.  We may want to allow it anyway for C compatibility.  Thoughts?
Comment 14 Jason Merrill 2019-08-27 18:48:51 UTC
Created attachment 46769 [details]
clear TYPE_NAME in free_lang_data for extern "C" anonymous types

Here's a more focused variant of my earlier patch.
Comment 15 Jason Merrill 2019-08-27 20:08:39 UTC
(In reply to Jason Merrill from comment #13)
> But that still doesn't make the types the same, and the use of the variable
> in 2.ii has undefined behavior because it is accessing the value of the
> object through the wrong type, so the warning is correct.  We may want to
> allow it anyway for C compatibility.  Thoughts?

This does seem like significant trouble to make something work that isn't actually valid C++, and it isn't hard to fix the code by giving the struct a name.  Is the problematic header part of publicly available source code or only the benchmark?
Comment 16 Martin Liška 2019-08-28 07:25:58 UTC
(In reply to Jason Merrill from comment #15)
> (In reply to Jason Merrill from comment #13)
> > But that still doesn't make the types the same, and the use of the variable
> > in 2.ii has undefined behavior because it is accessing the value of the
> > object through the wrong type, so the warning is correct.  We may want to
> > allow it anyway for C compatibility.  Thoughts?
> 
> This does seem like significant trouble to make something work that isn't
> actually valid C++, and it isn't hard to fix the code by giving the struct a
> name.  Is the problematic header part of publicly available source code or
> only the benchmark?

I've just done rebuild of openSUSE:Factory with current gcc10 master and I see it only in one package: aegisub. So I would consider it very rare.
Comment 17 Steve Ellcey 2019-09-23 17:56:40 UTC
I tested Jason's patch on my Aarch64 box and it fixed the ICE.  Any chance we could check that patch in so that we could build SPEC 2017 with -flto?

I don't know if we want to allow this mismatch or not but we certainly don't want GCC to ICE and this patch does fix that.  I guess it also allows the types to match or it wouldn't have created an executable.  If we don't want to allow the type match we would have to have SPEC modify the test sources to get it to work.
Comment 18 Martin Liška 2019-09-24 14:16:49 UTC
I would like to see this also fixed. But I know Honza has some comments about the patch. Honza?
Comment 19 ktkachov 2019-10-01 09:58:56 UTC
Still seeing this today building cactuBSSN_r with -flto
Comment 20 Jan Hubicka 2019-10-01 10:01:38 UTC
> Still seeing this today building cactuBSSN_r with -flto
Sorry for that - I had some unexpected developments after cauldron.
I am back from vacation now and will fix it ASAP.

Honza
Comment 21 Jan Hubicka 2019-10-01 18:22:02 UTC
Author: hubicka
Date: Tue Oct  1 18:21:31 2019
New Revision: 276420

URL: https://gcc.gnu.org/viewcvs?rev=276420&root=gcc&view=rev
Log:
	PR lto/91222
	* ipa-devirt.c (warn_types_mismatch): Do not ICE when anonymous type
	is matched with non-C++ type
	* g++.dg/lto/odr-6_0.C: New testcase.
	* g++.dg/lto/odr-6_1.c: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/lto/odr-6_0.C
    trunk/gcc/testsuite/g++.dg/lto/odr-6_1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-devirt.c
    trunk/gcc/testsuite/ChangeLog
Comment 22 Jan Hubicka 2019-10-01 18:25:19 UTC
Hi,
I have commited patch that avoids the ICE while producing warning.
However I wonder why C++ FE is silent here
$ cat 2.ii
extern "C" {
struct {
} admbaserest_;
}

We probably could warn about two things
 1) using extern "C" for anonymous types seems pointless since you can not legally bind them anyway
 2) declaring public symbol with anonymous type seems wrong.
We could also make C++ FE to do right thing for LTO and do not declare extern "C" anonymous types as being anonymous to the middle-end, but perhaps we just want the codebases to be fixed anyway and with warning it should be easy to do so?

Honza
Comment 23 Jan Hubicka 2019-10-01 18:26:06 UTC
I think C++ FE person is needed here, so I am unassigning myself for now.
Comment 24 ktkachov 2019-10-02 09:14:11 UTC
Thanks. Unfortunately I still see the ICE building 507.cactuBSSN_r on aarch64 with -flto in the same place:
 995       gcc_assert (TYPE_NAME (t1)
 996                   && TREE_CODE (TYPE_NAME (t1)) == TYPE_DECL);
Comment 25 Jan Hubicka 2019-10-02 09:36:00 UTC
> --- Comment #24 from ktkachov at gcc dot gnu.org ---
> Thanks. Unfortunately I still see the ICE building 507.cactuBSSN_r on aarch64
> with -flto in the same place:
>  995       gcc_assert (TYPE_NAME (t1)
>  996                   && TREE_CODE (TYPE_NAME (t1)) == TYPE_DECL);
Sorry to hear that seems cactusBSSN triggers a lot of interesting
behaviour here.  This conditional should fix it.

Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 276420)
+++ ipa-devirt.c	(working copy)
@@ -986,8 +986,8 @@ warn_types_mismatch (tree t1, tree t2, l
       || (type_with_linkage_p (TYPE_MAIN_VARIANT (t2))
 	  && type_in_anonymous_namespace_p (TYPE_MAIN_VARIANT (t2))))
     {
-      if (type_with_linkage_p (TYPE_MAIN_VARIANT (t1))
-	  && !type_in_anonymous_namespace_p (TYPE_MAIN_VARIANT (t1)))
+      if (!type_with_linkage_p (TYPE_MAIN_VARIANT (t1))
+	  || !type_in_anonymous_namespace_p (TYPE_MAIN_VARIANT (t1)))
 	{
 	  std::swap (t1, t2);
 	  std::swap (loc_t1, loc_t2);
Comment 26 ktkachov 2019-10-02 11:23:53 UTC
(In reply to Jan Hubicka from comment #25)
> > --- Comment #24 from ktkachov at gcc dot gnu.org ---
> > Thanks. Unfortunately I still see the ICE building 507.cactuBSSN_r on aarch64
> > with -flto in the same place:
> >  995       gcc_assert (TYPE_NAME (t1)
> >  996                   && TREE_CODE (TYPE_NAME (t1)) == TYPE_DECL);
> Sorry to hear that seems cactusBSSN triggers a lot of interesting
> behaviour here.  This conditional should fix it.
> 
> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c	(revision 276420)
> +++ ipa-devirt.c	(working copy)
> @@ -986,8 +986,8 @@ warn_types_mismatch (tree t1, tree t2, l
>        || (type_with_linkage_p (TYPE_MAIN_VARIANT (t2))
>  	  && type_in_anonymous_namespace_p (TYPE_MAIN_VARIANT (t2))))
>      {
> -      if (type_with_linkage_p (TYPE_MAIN_VARIANT (t1))
> -	  && !type_in_anonymous_namespace_p (TYPE_MAIN_VARIANT (t1)))
> +      if (!type_with_linkage_p (TYPE_MAIN_VARIANT (t1))
> +	  || !type_in_anonymous_namespace_p (TYPE_MAIN_VARIANT (t1)))
>  	{
>  	  std::swap (t1, t2);
>  	  std::swap (loc_t1, loc_t2);

Thanks! That fixes the benchmark build (and the rest of SPEC builds fine with -flto). It also bootstraps and tests on aarch64-none-linux-gnu fine.
Comment 27 Jan Hubicka 2019-10-02 12:42:07 UTC
Author: hubicka
Date: Wed Oct  2 12:41:36 2019
New Revision: 276454

URL: https://gcc.gnu.org/viewcvs?rev=276454&root=gcc&view=rev
Log:
	PR c++/91222
	* ipa-devirt.c (warn_types_mismatch): Fix conditional on anonymous
	namespace types.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-devirt.c
Comment 28 Jan Hubicka 2019-10-02 12:47:45 UTC
> Thanks! That fixes the benchmark build (and the rest of SPEC builds fine with
> -flto). It also bootstraps and tests on aarch64-none-linux-gnu fine.
Thanks! My testing concluded independently so I went ahead and comitted
the patch.  Funilly enough I have problem to get some reduced testcase,
perhaps Martin will chime in.

The main confusion here was the fact that I did not assume anonymous
namespace type to be ever matched agains non-C++ type, but in the
testcase it happens which triggered two not so well tought out code
paths.

Honza
Comment 29 Jakub Jelinek 2019-11-22 18:40:45 UTC
So can this be closed now as fixed?
Comment 30 Martin Liška 2019-11-27 17:41:05 UTC
*** Bug 91273 has been marked as a duplicate of this bug. ***
Comment 31 Martin Liška 2019-12-02 08:16:32 UTC
It's fixed.