On Fri, 9 Feb 2018, Pierre-Marie de Rodat wrote:
> This patch restricts the set of cases in which we allow the generation of
> location attributes for variables that are not defined in the current unit.
> For such variables with complex DECL_VALUE_EXPR trees, generating a location
> attribute can end up creating relocations to text symbols in the debug section
> of LTO object files, which is not valid.
> Richard originally suggested to revert r248792 and then enhance
> tree_add_const_value_attribute_for_decl to generate a DW_AT_location attribute
> in the specific case of a DECL_VALUE_EXPR that is an
> INDIRECT_REF(NOP_EXPR(INTEGER_CST)). I ended up with this patch because
> changing a function called "tree_add_const_value_attribute*" to generate
> instead a location attribute whereas there already exists a function called
> "add_location_or_const_value_attribute" felt really wrong. ;-) Especially if
> this would either re-implement part of the latter function in the former one.
> What I did instead was to make dwarf2out_late_global_decl call
> add_location_or_const_value_attribute only when we know it's safe to do so
> (i.e. when it won't generate relocations to text symbols). I have the feeling
> that it is cleaner and from what I could see, it fixes the reported issue with
> no regression.
> Bootstrapped and regtested on x86_64-linux. Ok to commit to trunk?
Ok with ...
> PR lto/84213
> * dwarf2out.c (is_trivial_indirect_ref): New function.
> (dwarf2out_late_global_decl): Do not generate a location attribute for
> variables that have a non-trivial DECL_VALUE_EXPR and that are not
> defined in the current unit.
> gcc/dwarf2out.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 948b3cb..6538596 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -25716,6 +25716,23 @@ dwarf2out_early_global_decl (tree decl)
> symtab->global_info_ready = save;
> +/* Return whether EXPR is an expression with the following pattern:
> + INDIRECT_REF (NOP_EXPR (INTEGER_CST)). */
whitespace fixed here - vertical space missing before the comment
and the first line of the comment (or cut&paste error with the patch?)
> +static bool
> +is_trivial_indirect_ref (tree expr)
> + if (expr == NULL_TREE || TREE_CODE (expr) != INDIRECT_REF)
> + return false;
> + tree nop = TREE_OPERAND (expr, 0);
> + if (nop == NULL_TREE || TREE_CODE (nop) != NOP_EXPR)
> + return false;
> + tree int_cst = TREE_OPERAND (nop, 0);
> + return int_cst != NULL_TREE && TREE_CODE (int_cst) == INTEGER_CST;
> /* Output debug information for global decl DECL. Called from
> toplev.c after compilation proper has finished. */
> @@ -25740,11 +25757,17 @@ dwarf2out_late_global_decl (tree decl)
> if (die)
> /* We get called via the symtab code invoking late_global_decl
> - for symbols that are optimized out. Do not add locations
> - for those, except if they have a DECL_VALUE_EXPR, in which case
> - they are relevant for debuggers. */
> + for symbols that are optimized out.
> + Do not add locations for those, except if they have a
> + DECL_VALUE_EXPR, in which case they are relevant for debuggers.
> + Still don't add a location if the DECL_VALUE_EXPR is not a
> + INDIRECT_REF expression, as this could generate relocations to
> + text symbols in LTO object files, which is invalid. */
> varpool_node *node = varpool_node::get (decl);
> - if ((! node || ! node->definition) && ! DECL_HAS_VALUE_EXPR_P
> + if ((! node || ! node->definition)
> + && ! (DECL_HAS_VALUE_EXPR_P (decl)
> + && is_trivial_indirect_ref (DECL_VALUE_EXPR (decl))))
> tree_add_const_value_attribute_for_decl (die, decl);
> add_location_or_const_value_attribute (die, decl, false);
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)