This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][debug] Fix handling of vlas in lto
- From: Richard Biener <rguenther at suse dot de>
- To: Tom de Vries <tdevries at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org, Jakub Jelinek <jakub at redhat dot com>, Cary Coutant <ccoutant at gmail dot com>, Jason Merrill <jason at redhat dot com>
- Date: Mon, 20 Aug 2018 15:09:39 +0200 (CEST)
- Subject: Re: [PATCH][debug] Fix handling of vlas in lto
- References: <20180817100430.GA29582@delia> <20180817210744.GA15027@delia>
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);
> }
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)