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 <tdevr...@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); > } > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)