[patch] Fix PR lto/48148

[patch] Fix PR lto/48148

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]

Hi,

this is the assembler error during an LTO bootstrap:

cc1.ltrans8.s: Assembler messages:
cc1.ltrans8.s:249143: Error: symbol `.Ldebug_info0' is already defined

In the assembly file:

.Letext0:
	.file 93 "/home/eric/svn/gcc/gcc/c-parser.c"
	.section	.debug_info,"",@progbits
.Ldebug_info0:
[...]
	.file 225 "/usr/include/mpc.h"
	.file 226 "/usr/include/mpfr.h"
.Ldebug_info0:


output_comp_unit is passed a completely bogus DIE:

DIE   11: DW_TAG_enumeration_type (0x7ffff2024a50)
  abbrev id: 1 offset: 11 mark: 1
  DW_AT_name: "prec"
  DW_AT_byte_size: 4
  DW_AT_decl_file: "/home/eric/svn/gcc/gcc/c-parser.c" (93)
  DW_AT_decl_line: 5459

created by the call to resolve_addr at line 23613 of dwarf2out.c:

  limbo_die_list = NULL;

  resolve_addr (comp_unit_die ());


#20 0x00000000005941a6 in gen_formal_parameter_die (node=0x7ffff5dc6000,
    origin=0x0, emit_name_p=1 '\001', context_die=0x7ffff4b28780)
    at /home/eric/svn/gcc/gcc/dwarf2out.c:18682
#21 0x0000000000594303 in gen_formal_types_die (
    function_or_method_type=0x7ffff5dcba80, context_die=0x7ffff4b28780)
    at /home/eric/svn/gcc/gcc/dwarf2out.c:18776
#22 0x0000000000592187 in gen_subprogram_die (decl=0x7ffff5dce300,
    context_die=0x7ffff2c13f00) at /home/eric/svn/gcc/gcc/dwarf2out.c:19388
#23 0x000000000059b632 in force_decl_die (decl=0x7ffff5dce300)
    at /home/eric/svn/gcc/gcc/dwarf2out.c:21025
#24 0x000000000059bb25 in resolve_addr (die=0x7ffff3d747d0)
    at /home/eric/svn/gcc/gcc/dwarf2out.c:23115
23115                   force_decl_die (tdecl);
(gdb) p debug_dwarf_die(die)
DIE    0: DW_TAG_GNU_call_site (0x7ffff3d747d0)
  abbrev id: 0 offset: 0 mark: 0
  DW_AT_low_pc: label: *.LVL13657
  DW_AT_abstract_origin: address

	if (die->die_tag == DW_TAG_GNU_call_site
	    && a->dw_attr == DW_AT_abstract_origin)
	  {
	    tree tdecl = SYMBOL_REF_DECL (a->dw_attr_val.v.val_addr);
	    dw_die_ref tdie = lookup_decl_die (tdecl);
	    if (tdie == NULL
		&& DECL_EXTERNAL (tdecl)
		&& DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
	      {
		force_decl_die (tdecl);


The DW_TAG_enumeration_type DIE is for an unrelated type of c-parser.c:

static struct c_expr
c_parser_binary_expression (c_parser *parser, struct c_expr *after)
{
  enum prec {
    PREC_NONE,
    PREC_LOGOR,
    PREC_LOGAND,
    PREC_BITOR,
    PREC_BITXOR,
    PREC_BITAND,
    PREC_EQ,
    PREC_REL,
    PREC_SHIFT,
    PREC_ADD,
    PREC_MULT,
    NUM_PRECS
  };

which happens to be merged with a type at top-level in cpplib.h:

/* C language kind, used when calling cpp_create_reader.  */
enum c_lang {CLK_GNUC89 = 0, CLK_GNUC99, CLK_GNUC1X,
	     CLK_STDC89, CLK_STDC94, CLK_STDC99, CLK_STDC1X,
	     CLK_GNUCXX, CLK_CXX98, CLK_GNUCXX0X, CLK_CXX0X, CLK_ASM};

so the LANG field of:

struct cpp_options
{
  /* Characters between tab stops.  */
  unsigned int tabstop;

  /* The language we're preprocessing.  */
  enum c_lang lang;

gets enum prec as type and things go downhill from there.


I don't think the two enum types should be merged: fields of structures are 
merged only if they have the same identifier, this should apply here as well.
Hence the attached patch.  LTO bootstrapped on x86_64-suse-linux, OK for the 
mainline?


2011-04-18  Eric Botcazou  <ebotcazou@adacore.com>

	PR lto/48148
	* gimple.c (gimple_types_compatible_p_1) <ENUMERAL_TYPE>: Do not
	merge the types if they have different enumeration identifiers.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 454 bytes --]

Index: gimple.c
===================================================================
--- gimple.c	(revision 172617)
+++ gimple.c	(working copy)
@@ -3731,6 +3731,9 @@ gimple_types_compatible_p_1 (tree t1, tr
 
 	    if (tree_int_cst_equal (c1, c2) != 1)
 	      goto different_types;
+
+	    if (mode == GTC_MERGE && TREE_PURPOSE (v1) != TREE_PURPOSE (v2))
+	      goto different_types;
 	  }
 
 	/* If one enumeration has more values than the other, they

Re: [patch] Fix PR lto/48148

[Richard Guenther]

On Mon, Apr 18, 2011 at 11:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is the assembler error during an LTO bootstrap:
>
> cc1.ltrans8.s: Assembler messages:
> cc1.ltrans8.s:249143: Error: symbol `.Ldebug_info0' is already defined
>
> In the assembly file:
>
> .Letext0:
>        .file 93 "/home/eric/svn/gcc/gcc/c-parser.c"
>        .section        .debug_info,"",@progbits
> .Ldebug_info0:
> [...]
>        .file 225 "/usr/include/mpc.h"
>        .file 226 "/usr/include/mpfr.h"
> .Ldebug_info0:
>
>
> output_comp_unit is passed a completely bogus DIE:
>
> DIE   11: DW_TAG_enumeration_type (0x7ffff2024a50)
>  abbrev id: 1 offset: 11 mark: 1
>  DW_AT_name: "prec"
>  DW_AT_byte_size: 4
>  DW_AT_decl_file: "/home/eric/svn/gcc/gcc/c-parser.c" (93)
>  DW_AT_decl_line: 5459
>
> created by the call to resolve_addr at line 23613 of dwarf2out.c:
>
>  limbo_die_list = NULL;
>
>  resolve_addr (comp_unit_die ());
>
>
> #20 0x00000000005941a6 in gen_formal_parameter_die (node=0x7ffff5dc6000,
>    origin=0x0, emit_name_p=1 '\001', context_die=0x7ffff4b28780)
>    at /home/eric/svn/gcc/gcc/dwarf2out.c:18682
> #21 0x0000000000594303 in gen_formal_types_die (
>    function_or_method_type=0x7ffff5dcba80, context_die=0x7ffff4b28780)
>    at /home/eric/svn/gcc/gcc/dwarf2out.c:18776
> #22 0x0000000000592187 in gen_subprogram_die (decl=0x7ffff5dce300,
>    context_die=0x7ffff2c13f00) at /home/eric/svn/gcc/gcc/dwarf2out.c:19388
> #23 0x000000000059b632 in force_decl_die (decl=0x7ffff5dce300)
>    at /home/eric/svn/gcc/gcc/dwarf2out.c:21025
> #24 0x000000000059bb25 in resolve_addr (die=0x7ffff3d747d0)
>    at /home/eric/svn/gcc/gcc/dwarf2out.c:23115
> 23115                   force_decl_die (tdecl);
> (gdb) p debug_dwarf_die(die)
> DIE    0: DW_TAG_GNU_call_site (0x7ffff3d747d0)
>  abbrev id: 0 offset: 0 mark: 0
>  DW_AT_low_pc: label: *.LVL13657
>  DW_AT_abstract_origin: address
>
>        if (die->die_tag == DW_TAG_GNU_call_site
>            && a->dw_attr == DW_AT_abstract_origin)
>          {
>            tree tdecl = SYMBOL_REF_DECL (a->dw_attr_val.v.val_addr);
>            dw_die_ref tdie = lookup_decl_die (tdecl);
>            if (tdie == NULL
>                && DECL_EXTERNAL (tdecl)
>                && DECL_ABSTRACT_ORIGIN (tdecl) == NULL_TREE)
>              {
>                force_decl_die (tdecl);
>
>
> The DW_TAG_enumeration_type DIE is for an unrelated type of c-parser.c:
>
> static struct c_expr
> c_parser_binary_expression (c_parser *parser, struct c_expr *after)
> {
>  enum prec {
>    PREC_NONE,
>    PREC_LOGOR,
>    PREC_LOGAND,
>    PREC_BITOR,
>    PREC_BITXOR,
>    PREC_BITAND,
>    PREC_EQ,
>    PREC_REL,
>    PREC_SHIFT,
>    PREC_ADD,
>    PREC_MULT,
>    NUM_PRECS
>  };
>
> which happens to be merged with a type at top-level in cpplib.h:
>
> /* C language kind, used when calling cpp_create_reader.  */
> enum c_lang {CLK_GNUC89 = 0, CLK_GNUC99, CLK_GNUC1X,
>             CLK_STDC89, CLK_STDC94, CLK_STDC99, CLK_STDC1X,
>             CLK_GNUCXX, CLK_CXX98, CLK_GNUCXX0X, CLK_CXX0X, CLK_ASM};
>
> so the LANG field of:
>
> struct cpp_options
> {
>  /* Characters between tab stops.  */
>  unsigned int tabstop;
>
>  /* The language we're preprocessing.  */
>  enum c_lang lang;
>
> gets enum prec as type and things go downhill from there.
>
>
> I don't think the two enum types should be merged: fields of structures are
> merged only if they have the same identifier, this should apply here as well.
> Hence the attached patch.  LTO bootstrapped on x86_64-suse-linux, OK for the
> mainline?

That has been on my todo for quite a while now ... I didn't think it was
that simple though ;)

Ok for trunk and also ok for the 4.6 branch if you manage to schedule
testing there.

Thanks,
Richard.

>
> 2011-04-18  Eric Botcazou  <ebotcazou@adacore.com>
>
>        PR lto/48148
>        * gimple.c (gimple_types_compatible_p_1) <ENUMERAL_TYPE>: Do not
>        merge the types if they have different enumeration identifiers.
>
>
> --
> Eric Botcazou
>

Re: [patch] Fix PR lto/48148

[Jakub Jelinek]

On Mon, Apr 18, 2011 at 11:45:03PM +0200, Eric Botcazou wrote:
> 2011-04-18  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	PR lto/48148
> 	* gimple.c (gimple_types_compatible_p_1) <ENUMERAL_TYPE>: Do not
> 	merge the types if they have different enumeration identifiers.

While I think it is a step in the right direction, IMHO we can reach the
same error if we merge say enums from:
TU1.c:
void foo (void)
{
  enum A { B, C } e = B;
  bar ((int) e);
}
TU2.c:
enum A { B, C } f;
extern void baz (enum A);
void test (void)
{
  baz (0);
}
If enum A from second TU is merged with enum A from first TU, but DIE
for it isn't needed in the partition containing TU2, then
when trying for call site to add DIE for call to baz it needs to be emitted
and it tries to emit it with the incorrect TYPE_CONTEXT.

IMHO types with incompatible TYPE_CONTEXT should never be compatible.
Where incompatible TYPE_CONTEXT is something that needs some thought,
if both TYPE_CONTEXTs are NULL / TRANSLATION_UNIT_DECL, it is
fine to merge them, if it is a FUNCTION_DECL, it better be the same
FUNCTION_DECL, if it is some type, it better be a gtc compatible type.
TYPE_CONTEXT is IMHO relevant to all types, not just ENUMERAL_TYPE.

Then, if we ever want to make LTO code actually debuggable, probably
types with different TYPE_NAMEs shouldn't be considered to be compatible
either, say enum A { B, C } a; in one TU and enum D { B, C } d; in another TU,
otherwise ptype a or ptype d will be wrong.

	Jakub

Re: [patch] Fix PR lto/48148

[Richard Guenther]

On Tue, Apr 19, 2011 at 12:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Apr 18, 2011 at 11:45:03PM +0200, Eric Botcazou wrote:
>> 2011-04-18  Eric Botcazou  <ebotcazou@adacore.com>
>>
>>       PR lto/48148
>>       * gimple.c (gimple_types_compatible_p_1) <ENUMERAL_TYPE>: Do not
>>       merge the types if they have different enumeration identifiers.
>
> While I think it is a step in the right direction, IMHO we can reach the
> same error if we merge say enums from:
> TU1.c:
> void foo (void)
> {
>  enum A { B, C } e = B;
>  bar ((int) e);
> }
> TU2.c:
> enum A { B, C } f;
> extern void baz (enum A);
> void test (void)
> {
>  baz (0);
> }
> If enum A from second TU is merged with enum A from first TU, but DIE
> for it isn't needed in the partition containing TU2, then
> when trying for call site to add DIE for call to baz it needs to be emitted
> and it tries to emit it with the incorrect TYPE_CONTEXT.

Sure, but that problem applies to all types.

> IMHO types with incompatible TYPE_CONTEXT should never be compatible.
> Where incompatible TYPE_CONTEXT is something that needs some thought,
> if both TYPE_CONTEXTs are NULL / TRANSLATION_UNIT_DECL, it is
> fine to merge them, if it is a FUNCTION_DECL, it better be the same
> FUNCTION_DECL, if it is some type, it better be a gtc compatible type.
> TYPE_CONTEXT is IMHO relevant to all types, not just ENUMERAL_TYPE.
>
> Then, if we ever want to make LTO code actually debuggable, probably
> types with different TYPE_NAMEs shouldn't be considered to be compatible
> either, say enum A { B, C } a; in one TU and enum D { B, C } d; in another TU,
> otherwise ptype a or ptype d will be wrong.

Yeah, now that we merge the TYPE_CANONICAL web separately we
can do with a lot less "real" type merging without any effect on semantics.
We'll just pay with memory usage for better debug info.

Richard.

>        Jakub
>