| Message ID | 20180817210744.GA15027@delia |
|---|---|
| State | New |
| Headers | show |
| Series | [debug] Fix handling of vlas in lto | expand |
On Fri, 17 Aug 2018, Tom de Vries wrote: > I've rewritten the patch to work generically, not just for DW_AT_upper_bound, > and to reuse the code already there in add_scalar_info. > > OK for trunk? > > Thanks, > - Tom > > [debug] Fix handling of vlas in lto > > Atm, when running vla-1.c with -O0 -flto, we have: > ... > FAIL: gcc.dg/guality/vla-1.c -O0 -flto -fuse-linker-plugin \ > -fno-fat-lto-objects line 17 sizeof (a) == 6 > ... > > The vla a[i + 1] in f1 is gimplified into: > ... > f1 (int i) > { > char a[0:D.1922] [value-expr: *a.0]; > char[0:D.1922] * a.0; > > D.1921 = i + 1; > D.1926 = (sizetype) D.1921; > a.0 = __builtin_alloca_with_align (D.1926, 8); > ... > > The early debug info for the upper bound of the type of vla a that we stream > out is: > ... > DIE 0: DW_TAG_subrange_type (0x7f85029a90f0) > DW_AT_upper_bound: location descriptor: > (0x7f85029a9230) DW_OP_GNU_variable_value die -> 0 (0x7f85029a94b0), 0 > DIE 0: DW_TAG_variable (0x7f85029a94b0) > DW_AT_name: "D.1922" > DW_AT_type: die -> 0 (0x7f85029a3d70) > DW_AT_artificial: 1 > ... > > and in ltrans we have for that same upper bound: > ... > DIE 0: DW_TAG_subrange_type (0x7f5183b57d70) > DW_AT_upper_bound: die -> 0 (0x7f5183b576e0) > DIE 0: DW_TAG_variable (0x7f5183b576e0) > DW_AT_name: "D.4278" > DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 193 (0x7f5183b57730) > ... > where D.4278 has abstract origin D.1922. > > The D.4278 die has no DW_AT_location, so when evaluting "sizeof (a)" in the > debugger, we can't find the information to get the value of D.4278, and the > debugger prints "<optimized out>". > > This patch fixes that by either: > - adding DW_AT_location to the referenced variable die, or > - instead of using a ref for the upper bound, using an exprloc. > > When changing gcc.dg/guality/guality.exp to run the usual flto flavours > "-fno-use-linker-plugin -flto-partition=none" and "-fuse-linker-plugin > -fno-fat-lto-objects" in combination with O0, Og, O1, O2, O3 and Os, this patch > fixes all (20) failures in vla-1.c, leaving only: > ... > No symbol "i" in current context. > UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ > -flto-partition=none line 17 i == 5 > 'a' has unknown type; cast it to its declared type > UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ > -flto-partition=none line 17 sizeof (a) == 6 > ... > > Bootstrapped and reg-tested on x86_64. This looks OK to me. Note that with a gdb with DW_OP_variable_value support we should be able to elide the VLA type in the concrete instance... Not sure how we should go forward there - use a configure test or simply tell people to update gdb? Thanks, Richard. > 2018-08-17 Tom de Vries <tdevries@suse.de> > > * dwarf2out.c (add_scalar_info): Don't add reference to existing die > unless the referenced die describes the added property using > DW_AT_location or DW_AT_const_value. Fall back to exprloc case. > Otherwise, add a DW_AT_location to the referenced die. > > --- > gcc/dwarf2out.c | 32 ++++++++++++++++++++------------ > 1 file changed, 20 insertions(+), 12 deletions(-) > > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index 9ed473088e7..e1dccb42823 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -20598,7 +20598,7 @@ static void > add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > int forms, struct loc_descr_context *context) > { > - dw_die_ref context_die, decl_die; > + dw_die_ref context_die, decl_die = NULL; > dw_loc_list_ref list; > bool strip_conversions = true; > bool placeholder_seen = false; > @@ -20675,7 +20675,7 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > > if (decl != NULL_TREE) > { > - dw_die_ref decl_die = lookup_decl_die (decl); > + decl_die = lookup_decl_die (decl); > > /* ??? Can this happen, or should the variable have been bound > first? Probably it can, since I imagine that we try to create > @@ -20684,8 +20684,12 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > later parameter. */ > if (decl_die != NULL) > { > - add_AT_die_ref (die, attr, decl_die); > - return; > + if (get_AT (decl_die, DW_AT_location) > + || get_AT (decl_die, DW_AT_const_value)) > + { > + add_AT_die_ref (die, attr, decl_die); > + return; > + } > } > } > } > @@ -20729,15 +20733,19 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > || placeholder_seen) > return; > > - if (current_function_decl == 0) > - context_die = comp_unit_die (); > - else > - context_die = lookup_decl_die (current_function_decl); > + if (!decl_die) > + { > + if (current_function_decl == 0) > + context_die = comp_unit_die (); > + else > + context_die = lookup_decl_die (current_function_decl); > + > + decl_die = new_die (DW_TAG_variable, context_die, value); > + add_AT_flag (decl_die, DW_AT_artificial, 1); > + add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, > + context_die); > + } > > - decl_die = new_die (DW_TAG_variable, context_die, value); > - add_AT_flag (decl_die, DW_AT_artificial, 1); > - add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, > - context_die); > add_AT_location_description (decl_die, DW_AT_location, list); > add_AT_die_ref (die, attr, decl_die); > } > >
On 08/20/2018 03:09 PM, Richard Biener wrote: > On Fri, 17 Aug 2018, Tom de Vries wrote: > >> I've rewritten the patch to work generically, not just for DW_AT_upper_bound, >> and to reuse the code already there in add_scalar_info. >> >> OK for trunk? >> >> Thanks, >> - Tom >> >> [debug] Fix handling of vlas in lto >> >> Atm, when running vla-1.c with -O0 -flto, we have: >> ... >> FAIL: gcc.dg/guality/vla-1.c -O0 -flto -fuse-linker-plugin \ >> -fno-fat-lto-objects line 17 sizeof (a) == 6 >> ... >> >> The vla a[i + 1] in f1 is gimplified into: >> ... >> f1 (int i) >> { >> char a[0:D.1922] [value-expr: *a.0]; >> char[0:D.1922] * a.0; >> >> D.1921 = i + 1; >> D.1926 = (sizetype) D.1921; >> a.0 = __builtin_alloca_with_align (D.1926, 8); >> ... >> >> The early debug info for the upper bound of the type of vla a that we stream >> out is: >> ... >> DIE 0: DW_TAG_subrange_type (0x7f85029a90f0) >> DW_AT_upper_bound: location descriptor: >> (0x7f85029a9230) DW_OP_GNU_variable_value die -> 0 (0x7f85029a94b0), 0 >> DIE 0: DW_TAG_variable (0x7f85029a94b0) >> DW_AT_name: "D.1922" >> DW_AT_type: die -> 0 (0x7f85029a3d70) >> DW_AT_artificial: 1 >> ... >> >> and in ltrans we have for that same upper bound: >> ... >> DIE 0: DW_TAG_subrange_type (0x7f5183b57d70) >> DW_AT_upper_bound: die -> 0 (0x7f5183b576e0) >> DIE 0: DW_TAG_variable (0x7f5183b576e0) >> DW_AT_name: "D.4278" >> DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 193 (0x7f5183b57730) >> ... >> where D.4278 has abstract origin D.1922. >> >> The D.4278 die has no DW_AT_location, so when evaluting "sizeof (a)" in the >> debugger, we can't find the information to get the value of D.4278, and the >> debugger prints "<optimized out>". >> >> This patch fixes that by either: >> - adding DW_AT_location to the referenced variable die, or >> - instead of using a ref for the upper bound, using an exprloc. >> >> When changing gcc.dg/guality/guality.exp to run the usual flto flavours >> "-fno-use-linker-plugin -flto-partition=none" and "-fuse-linker-plugin >> -fno-fat-lto-objects" in combination with O0, Og, O1, O2, O3 and Os, this patch >> fixes all (20) failures in vla-1.c, leaving only: >> ... >> No symbol "i" in current context. >> UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ >> -flto-partition=none line 17 i == 5 >> 'a' has unknown type; cast it to its declared type >> UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ >> -flto-partition=none line 17 sizeof (a) == 6 >> ... >> >> Bootstrapped and reg-tested on x86_64. > > This looks OK to me. Committed. > Note that with a gdb with DW_OP_variable_value > support we should be able to elide the VLA type in the concrete > instance... > Using this patch we elide the VLA type in the concrete instance for -O2 -flto: ... diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index dd8f438dfd3..5776185d9c6 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -23712,12 +23712,14 @@ gen_variable_die (tree decl, tree origin, dw_die_ref context_die) && variably_modified_type_p (type, decl_function_context (decl_or_origin))) { +#if 0 if (decl_by_reference_p (decl_or_origin)) add_type_attribute (var_die, TREE_TYPE (type), TYPE_UNQUALIFIED, false, context_die); else add_type_attribute (var_die, type, decl_quals (decl_or_origin), false, context_die); +#endif } goto gen_variable_die_location; ... and using master gdb (which supports DW_OP_variable_value, yay) we get: ... 7 return a[0]; /* { dg-final { gdb-test . "sizeof (a)" "6" } } */ vla-1.gdb:3: Error in sourced command file: value has been optimized out ... When evaluating sizeof (a) in gdb: - we look for the concrete type die of a - since that one is elided, we fall back on the type die of the abstract origin of a - that one has an expr location for the upper bound containing a DW_OP_variable_value referring to a variable in early debug. - that variable has no DW_AT_location, so we end up with "value has been optimized out" AFAIU, that's roughly the issue discussed in PR20426 - "gdb does not interpret DWARF annotating imported units fully" ( https://sourceware.org/bugzilla/show_bug.cgi?id=20426 ),. Is my understanding correct that it's not yet clear how this should be fixed? Thanks, - Tom > Not sure how we should go forward there - use a configure test or > simply tell people to update gdb? > > Thanks, > Richard. > >> 2018-08-17 Tom de Vries <tdevries@suse.de> >> >> * dwarf2out.c (add_scalar_info): Don't add reference to existing die >> unless the referenced die describes the added property using >> DW_AT_location or DW_AT_const_value. Fall back to exprloc case. >> Otherwise, add a DW_AT_location to the referenced die. >> >> --- >> gcc/dwarf2out.c | 32 ++++++++++++++++++++------------ >> 1 file changed, 20 insertions(+), 12 deletions(-) >> >> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c >> index 9ed473088e7..e1dccb42823 100644 >> --- a/gcc/dwarf2out.c >> +++ b/gcc/dwarf2out.c >> @@ -20598,7 +20598,7 @@ static void >> add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, >> int forms, struct loc_descr_context *context) >> { >> - dw_die_ref context_die, decl_die; >> + dw_die_ref context_die, decl_die = NULL; >> dw_loc_list_ref list; >> bool strip_conversions = true; >> bool placeholder_seen = false; >> @@ -20675,7 +20675,7 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, >> >> if (decl != NULL_TREE) >> { >> - dw_die_ref decl_die = lookup_decl_die (decl); >> + decl_die = lookup_decl_die (decl); >> >> /* ??? Can this happen, or should the variable have been bound >> first? Probably it can, since I imagine that we try to create >> @@ -20684,8 +20684,12 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, >> later parameter. */ >> if (decl_die != NULL) >> { >> - add_AT_die_ref (die, attr, decl_die); >> - return; >> + if (get_AT (decl_die, DW_AT_location) >> + || get_AT (decl_die, DW_AT_const_value)) >> + { >> + add_AT_die_ref (die, attr, decl_die); >> + return; >> + } >> } >> } >> } >> @@ -20729,15 +20733,19 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, >> || placeholder_seen) >> return; >> >> - if (current_function_decl == 0) >> - context_die = comp_unit_die (); >> - else >> - context_die = lookup_decl_die (current_function_decl); >> + if (!decl_die) >> + { >> + if (current_function_decl == 0) >> + context_die = comp_unit_die (); >> + else >> + context_die = lookup_decl_die (current_function_decl); >> + >> + decl_die = new_die (DW_TAG_variable, context_die, value); >> + add_AT_flag (decl_die, DW_AT_artificial, 1); >> + add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, >> + context_die); >> + } >> >> - decl_die = new_die (DW_TAG_variable, context_die, value); >> - add_AT_flag (decl_die, DW_AT_artificial, 1); >> - add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, >> - context_die); >> add_AT_location_description (decl_die, DW_AT_location, list); >> add_AT_die_ref (die, attr, decl_die); >> } >> >> >
On Tue, 21 Aug 2018, Tom de Vries wrote: > On 08/20/2018 03:09 PM, Richard Biener wrote: > > On Fri, 17 Aug 2018, Tom de Vries wrote: > > > >> I've rewritten the patch to work generically, not just for DW_AT_upper_bound, > >> and to reuse the code already there in add_scalar_info. > >> > >> OK for trunk? > >> > >> Thanks, > >> - Tom > >> > >> [debug] Fix handling of vlas in lto > >> > >> Atm, when running vla-1.c with -O0 -flto, we have: > >> ... > >> FAIL: gcc.dg/guality/vla-1.c -O0 -flto -fuse-linker-plugin \ > >> -fno-fat-lto-objects line 17 sizeof (a) == 6 > >> ... > >> > >> The vla a[i + 1] in f1 is gimplified into: > >> ... > >> f1 (int i) > >> { > >> char a[0:D.1922] [value-expr: *a.0]; > >> char[0:D.1922] * a.0; > >> > >> D.1921 = i + 1; > >> D.1926 = (sizetype) D.1921; > >> a.0 = __builtin_alloca_with_align (D.1926, 8); > >> ... > >> > >> The early debug info for the upper bound of the type of vla a that we stream > >> out is: > >> ... > >> DIE 0: DW_TAG_subrange_type (0x7f85029a90f0) > >> DW_AT_upper_bound: location descriptor: > >> (0x7f85029a9230) DW_OP_GNU_variable_value die -> 0 (0x7f85029a94b0), 0 > >> DIE 0: DW_TAG_variable (0x7f85029a94b0) > >> DW_AT_name: "D.1922" > >> DW_AT_type: die -> 0 (0x7f85029a3d70) > >> DW_AT_artificial: 1 > >> ... > >> > >> and in ltrans we have for that same upper bound: > >> ... > >> DIE 0: DW_TAG_subrange_type (0x7f5183b57d70) > >> DW_AT_upper_bound: die -> 0 (0x7f5183b576e0) > >> DIE 0: DW_TAG_variable (0x7f5183b576e0) > >> DW_AT_name: "D.4278" > >> DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 193 (0x7f5183b57730) > >> ... > >> where D.4278 has abstract origin D.1922. > >> > >> The D.4278 die has no DW_AT_location, so when evaluting "sizeof (a)" in the > >> debugger, we can't find the information to get the value of D.4278, and the > >> debugger prints "<optimized out>". > >> > >> This patch fixes that by either: > >> - adding DW_AT_location to the referenced variable die, or > >> - instead of using a ref for the upper bound, using an exprloc. > >> > >> When changing gcc.dg/guality/guality.exp to run the usual flto flavours > >> "-fno-use-linker-plugin -flto-partition=none" and "-fuse-linker-plugin > >> -fno-fat-lto-objects" in combination with O0, Og, O1, O2, O3 and Os, this patch > >> fixes all (20) failures in vla-1.c, leaving only: > >> ... > >> No symbol "i" in current context. > >> UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ > >> -flto-partition=none line 17 i == 5 > >> 'a' has unknown type; cast it to its declared type > >> UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ > >> -flto-partition=none line 17 sizeof (a) == 6 > >> ... > >> > >> Bootstrapped and reg-tested on x86_64. > > > > This looks OK to me. > > Committed. > > > Note that with a gdb with DW_OP_variable_value > > support we should be able to elide the VLA type in the concrete > > instance... > > > > Using this patch we elide the VLA type in the concrete instance for -O2 > -flto: > ... > diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > index dd8f438dfd3..5776185d9c6 100644 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -23712,12 +23712,14 @@ gen_variable_die (tree decl, tree origin, > dw_die_ref context_die) > && variably_modified_type_p > (type, decl_function_context (decl_or_origin))) > { > +#if 0 > if (decl_by_reference_p (decl_or_origin)) > add_type_attribute (var_die, TREE_TYPE (type), > TYPE_UNQUALIFIED, false, > context_die); > else > add_type_attribute (var_die, type, > decl_quals (decl_or_origin), > false, context_die); > +#endif > } > > goto gen_variable_die_location; > ... > and using master gdb (which supports DW_OP_variable_value, yay) we get: > ... > 7 return a[0]; /* { dg-final { gdb-test . "sizeof (a)" > "6" } } */ > vla-1.gdb:3: Error in sourced command file: > value has been optimized out > ... > > When evaluating sizeof (a) in gdb: > - we look for the concrete type die of a > - since that one is elided, we fall back on the type die of the abstract > origin of a > - that one has an expr location for the upper bound containing a > DW_OP_variable_value referring to a variable in early debug. > - that variable has no DW_AT_location, so we end up with "value has been > optimized out" > > AFAIU, that's roughly the issue discussed in > PR20426 - "gdb does not interpret DWARF annotating imported units fully" > ( https://sourceware.org/bugzilla/show_bug.cgi?id=20426 ),. > > Is my understanding correct that it's not yet clear how this should be > fixed? ISTR Jakub recently said in a bug or in a mail the debug consumer should lookup DW_OP_variable_value like it would be specified in a user expression and thus it should find the concrete instance which does have a location (and refers to the DIE refered to by DW_OP_variable_value via its DW_AT_abstract_origin). Otherwise DW_OP_variable_value is useless for abstract instances like those created for inlining and we need - as we currently do! - to re-emit type DIEs in each inline instance for annotating them with locations. I suppose for the DWARF proposal an actual example using VLAs and inlining and abstract and inline instances is the best motivating example. Richard. > Thanks, > - Tom > > > Not sure how we should go forward there - use a configure test or > > simply tell people to update gdb? > > > > Thanks, > > Richard. > > > >> 2018-08-17 Tom de Vries <tdevries@suse.de> > >> > >> * dwarf2out.c (add_scalar_info): Don't add reference to existing die > >> unless the referenced die describes the added property using > >> DW_AT_location or DW_AT_const_value. Fall back to exprloc case. > >> Otherwise, add a DW_AT_location to the referenced die. > >> > >> --- > >> gcc/dwarf2out.c | 32 ++++++++++++++++++++------------ > >> 1 file changed, 20 insertions(+), 12 deletions(-) > >> > >> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c > >> index 9ed473088e7..e1dccb42823 100644 > >> --- a/gcc/dwarf2out.c > >> +++ b/gcc/dwarf2out.c > >> @@ -20598,7 +20598,7 @@ static void > >> add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > >> int forms, struct loc_descr_context *context) > >> { > >> - dw_die_ref context_die, decl_die; > >> + dw_die_ref context_die, decl_die = NULL; > >> dw_loc_list_ref list; > >> bool strip_conversions = true; > >> bool placeholder_seen = false; > >> @@ -20675,7 +20675,7 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > >> > >> if (decl != NULL_TREE) > >> { > >> - dw_die_ref decl_die = lookup_decl_die (decl); > >> + decl_die = lookup_decl_die (decl); > >> > >> /* ??? Can this happen, or should the variable have been bound > >> first? Probably it can, since I imagine that we try to create > >> @@ -20684,8 +20684,12 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > >> later parameter. */ > >> if (decl_die != NULL) > >> { > >> - add_AT_die_ref (die, attr, decl_die); > >> - return; > >> + if (get_AT (decl_die, DW_AT_location) > >> + || get_AT (decl_die, DW_AT_const_value)) > >> + { > >> + add_AT_die_ref (die, attr, decl_die); > >> + return; > >> + } > >> } > >> } > >> } > >> @@ -20729,15 +20733,19 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, > >> || placeholder_seen) > >> return; > >> > >> - if (current_function_decl == 0) > >> - context_die = comp_unit_die (); > >> - else > >> - context_die = lookup_decl_die (current_function_decl); > >> + if (!decl_die) > >> + { > >> + if (current_function_decl == 0) > >> + context_die = comp_unit_die (); > >> + else > >> + context_die = lookup_decl_die (current_function_decl); > >> + > >> + decl_die = new_die (DW_TAG_variable, context_die, value); > >> + add_AT_flag (decl_die, DW_AT_artificial, 1); > >> + add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, > >> + context_die); > >> + } > >> > >> - decl_die = new_die (DW_TAG_variable, context_die, value); > >> - add_AT_flag (decl_die, DW_AT_artificial, 1); > >> - add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, > >> - context_die); > >> add_AT_location_description (decl_die, DW_AT_location, list); > >> add_AT_die_ref (die, attr, decl_die); > >> } > >> > >> > > > >
> 2018-08-17 Tom de Vries <tdevries@suse.de> > > * dwarf2out.c (add_scalar_info): Don't add reference to existing die > unless the referenced die describes the added property using > DW_AT_location or DW_AT_const_value. Fall back to exprloc case. > Otherwise, add a DW_AT_location to the referenced die. This breaks Ada though, i.e. any array type whose bound depends on a discriminant is affected: type Array_Type is array (Integer range <>) of Integer; type Record_Type (N : Integer) is record A : Array_Type (1 .. N); end record; .uleb128 0x6 # (DIE (0x66) DW_TAG_array_type) .long .LASF5 # DW_AT_name: "p__record_type__T4s" .long 0x34 # DW_AT_type .long 0x79 # DW_AT_sibling .uleb128 0x7 # (DIE (0x73) DW_TAG_subrange_type) .long 0x2d # DW_AT_type .byte 0 # end of children of DIE 0x66 Testcase attached, compile with -fgnat-encodings=minimal.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 9ed473088e7..e1dccb42823 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -20598,7 +20598,7 @@ static void add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, int forms, struct loc_descr_context *context) { - dw_die_ref context_die, decl_die; + dw_die_ref context_die, decl_die = NULL; dw_loc_list_ref list; bool strip_conversions = true; bool placeholder_seen = false; @@ -20675,7 +20675,7 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, if (decl != NULL_TREE) { - dw_die_ref decl_die = lookup_decl_die (decl); + decl_die = lookup_decl_die (decl); /* ??? Can this happen, or should the variable have been bound first? Probably it can, since I imagine that we try to create @@ -20684,8 +20684,12 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, later parameter. */ if (decl_die != NULL) { - add_AT_die_ref (die, attr, decl_die); - return; + if (get_AT (decl_die, DW_AT_location) + || get_AT (decl_die, DW_AT_const_value)) + { + add_AT_die_ref (die, attr, decl_die); + return; + } } } } @@ -20729,15 +20733,19 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value, || placeholder_seen) return; - if (current_function_decl == 0) - context_die = comp_unit_die (); - else - context_die = lookup_decl_die (current_function_decl); + if (!decl_die) + { + if (current_function_decl == 0) + context_die = comp_unit_die (); + else + context_die = lookup_decl_die (current_function_decl); + + decl_die = new_die (DW_TAG_variable, context_die, value); + add_AT_flag (decl_die, DW_AT_artificial, 1); + add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, + context_die); + } - decl_die = new_die (DW_TAG_variable, context_die, value); - add_AT_flag (decl_die, DW_AT_artificial, 1); - add_type_attribute (decl_die, TREE_TYPE (value), TYPE_QUAL_CONST, false, - context_die); add_AT_location_description (decl_die, DW_AT_location, list); add_AT_die_ref (die, attr, decl_die); }
I've rewritten the patch to work generically, not just for DW_AT_upper_bound, and to reuse the code already there in add_scalar_info. OK for trunk? Thanks, - Tom [debug] Fix handling of vlas in lto Atm, when running vla-1.c with -O0 -flto, we have: ... FAIL: gcc.dg/guality/vla-1.c -O0 -flto -fuse-linker-plugin \ -fno-fat-lto-objects line 17 sizeof (a) == 6 ... The vla a[i + 1] in f1 is gimplified into: ... f1 (int i) { char a[0:D.1922] [value-expr: *a.0]; char[0:D.1922] * a.0; D.1921 = i + 1; D.1926 = (sizetype) D.1921; a.0 = __builtin_alloca_with_align (D.1926, 8); ... The early debug info for the upper bound of the type of vla a that we stream out is: ... DIE 0: DW_TAG_subrange_type (0x7f85029a90f0) DW_AT_upper_bound: location descriptor: (0x7f85029a9230) DW_OP_GNU_variable_value die -> 0 (0x7f85029a94b0), 0 DIE 0: DW_TAG_variable (0x7f85029a94b0) DW_AT_name: "D.1922" DW_AT_type: die -> 0 (0x7f85029a3d70) DW_AT_artificial: 1 ... and in ltrans we have for that same upper bound: ... DIE 0: DW_TAG_subrange_type (0x7f5183b57d70) DW_AT_upper_bound: die -> 0 (0x7f5183b576e0) DIE 0: DW_TAG_variable (0x7f5183b576e0) DW_AT_name: "D.4278" DW_AT_abstract_origin: die -> label: vla_1.c.6719312a + 193 (0x7f5183b57730) ... where D.4278 has abstract origin D.1922. The D.4278 die has no DW_AT_location, so when evaluting "sizeof (a)" in the debugger, we can't find the information to get the value of D.4278, and the debugger prints "<optimized out>". This patch fixes that by either: - adding DW_AT_location to the referenced variable die, or - instead of using a ref for the upper bound, using an exprloc. When changing gcc.dg/guality/guality.exp to run the usual flto flavours "-fno-use-linker-plugin -flto-partition=none" and "-fuse-linker-plugin -fno-fat-lto-objects" in combination with O0, Og, O1, O2, O3 and Os, this patch fixes all (20) failures in vla-1.c, leaving only: ... No symbol "i" in current context. UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ -flto-partition=none line 17 i == 5 'a' has unknown type; cast it to its declared type UNSUPPORTED: gcc.dg/guality/vla-1.c -O3 -flto -fno-use-linker-plugin \ -flto-partition=none line 17 sizeof (a) == 6 ... Bootstrapped and reg-tested on x86_64. 2018-08-17 Tom de Vries <tdevries@suse.de> * dwarf2out.c (add_scalar_info): Don't add reference to existing die unless the referenced die describes the added property using DW_AT_location or DW_AT_const_value. Fall back to exprloc case. Otherwise, add a DW_AT_location to the referenced die. --- gcc/dwarf2out.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)