On Tue, 10 Apr 2018, Jakub Jelinek wrote:
> Hi!
>
> The r257510 change broke -gsplit-dwarf support by introducing a circular
> dependency. Before that revision index_location_lists used to do:
> /* Don't index an entry that has already been indexed
> or won't be output. */
> if (curr->begin_entry != NULL
> || (strcmp (curr->begin, curr->end) == 0 && !curr->force))
> continue;
> r257510 introduced a function which does that
> (strcmp (curr->begin, curr->end) == 0 && !curr->force)
> part extended for LVUs, but also calls size_of_locs. In dwarf4 and earlier
> we really can't emit location expressions >= 64KB in size, so we just
> ignored those during output and the helper function used by the output and
> now this spot uses that too. The problem is that size_of_locs needs
> (in some cases) the split dwarf address table indexes to be computed (the
> indexes are uleb128s and thus need to be sized accurately), but at the point
> index_location_lists is called, those aren't computed and can't easily be,
> because that very function adds new address table entries and the current
> logic requires that once indexes are assigned the hash table is immutable,
> during output we assert we go through the same indexes as were assigned.
> In theory we could introduce another hash table to hold the new table
> entries that didn't have indexes assigned yet, but given that >= 64KB locs
> are extremely rare, I think it is just wasted effort.
>
> This patch just makes sure we create address table entry without computing
> the size_of_locs and so rarely could add an entry nothing will really use;
> it is just an address though, so not a big deal IMHO.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2018-04-10 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/85302
> * dwarf2out.c (skip_loc_list_entry): Don't call size_of_locs if
> SIZEP is NULL.
> (output_loc_list): Pass address of a dummy size variable even in the
> locview handling loop.
> (index_location_lists): Add comment on why skip_loc_list_entry can't
> call size_of_locs.
>
> * g++.dg/debug/dwarf2/pr85302.C: New test.
>
> --- gcc/dwarf2out.c.jj 2018-04-06 19:28:24.000000000 +0200
> +++ gcc/dwarf2out.c 2018-04-10 10:21:21.352384405 +0200
> @@ -10032,18 +10032,22 @@ maybe_gen_llsym (dw_loc_list_ref list)
> gen_llsym (list);
> }
>
> -/* Determine whether or not to skip loc_list entry CURR. If we're not
> +/* Determine whether or not to skip loc_list entry CURR. If SIZEP is
> + NULL, don't consider size of the location expression. If we're not
> to skip it, and SIZEP is non-null, store the size of CURR->expr's
> representation in *SIZEP. */
>
> static bool
> -skip_loc_list_entry (dw_loc_list_ref curr, unsigned long *sizep = 0)
> +skip_loc_list_entry (dw_loc_list_ref curr, unsigned long *sizep = NULL)
> {
> /* Don't output an entry that starts and ends at the same address. */
> if (strcmp (curr->begin, curr->end) == 0
> && curr->vbegin == curr->vend && !curr->force)
> return true;
>
> + if (!sizep)
> + return false;
> +
> unsigned long size = size_of_locs (curr->expr);
>
> /* If the expression is too large, drop it on the floor. We could
> @@ -10053,8 +10057,7 @@ skip_loc_list_entry (dw_loc_list_ref cur
> if (dwarf_version < 5 && size > 0xffff)
> return true;
>
> - if (sizep)
> - *sizep = size;
> + *sizep = size;
>
> return false;
> }
> @@ -10121,7 +10124,9 @@ output_loc_list (dw_loc_list_ref list_he
> for (dw_loc_list_ref curr = list_head; curr != NULL;
> curr = curr->dw_loc_next)
> {
> - if (skip_loc_list_entry (curr))
> + unsigned long size;
> +
> + if (skip_loc_list_entry (curr, &size))
> continue;
>
> vcount++;
> @@ -30887,7 +30892,14 @@ index_location_lists (dw_die_ref die)
> for (curr = list; curr != NULL; curr = curr->dw_loc_next)
> {
> /* Don't index an entry that has already been indexed
> - or won't be output. */
> + or won't be output. Make sure skip_loc_list_entry doesn't
> + call size_of_locs, because that might cause circular dependency,
> + index_location_lists requiring address table indexes to be
> + computed, but adding new indexes through add_addr_table_entry
> + and address table index computation requiring no new additions
> + to the hash table. In the rare case of DWARF[234] >= 64KB
> + location expression, we'll just waste unused address table entry
> + for it. */
> if (curr->begin_entry != NULL
> || skip_loc_list_entry (curr))
> continue;
> --- gcc/testsuite/g++.dg/debug/dwarf2/pr85302.C.jj 2018-04-10
> 10:29:22.994385218 +0200
> +++ gcc/testsuite/g++.dg/debug/dwarf2/pr85302.C 2018-04-10
> 10:33:00.036385099 +0200
> @@ -0,0 +1,14 @@
> +// PR debug/85302
> +// { dg-do compile }
> +// { dg-options "-std=c++11 -gsplit-dwarf -O1" }
> +// { dg-additional-options "-fPIE" { target pie } }
> +
> +struct A { const char *b; A (const char *c) : b(c) {} };
> +struct B { void foo (A); };
> +B e;
> +
> +void
> +bar ()
> +{
> + e.foo ("");
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
