* [PATCH][PR84213] DWARF: no location for non-definition DECLs with non-trivial DECL_VALUE_EXPR
@ 2018-02-09 13:33 Pierre-Marie de Rodat
2018-02-09 13:47 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Pierre-Marie de Rodat @ 2018-02-09 13:33 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
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?
gcc/
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)). */
+
+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 trivial
+ 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 (decl))
+ 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);
else
add_location_or_const_value_attribute (die, decl, false);
--
2.1.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][PR84213] DWARF: no location for non-definition DECLs with non-trivial DECL_VALUE_EXPR
2018-02-09 13:33 [PATCH][PR84213] DWARF: no location for non-definition DECLs with non-trivial DECL_VALUE_EXPR Pierre-Marie de Rodat
@ 2018-02-09 13:47 ` Richard Biener
2018-02-09 13:53 ` Pierre-Marie de Rodat
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2018-02-09 13:47 UTC (permalink / raw)
To: Pierre-Marie de Rodat; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 4129 bytes --]
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 ...
> gcc/
> 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?)
Thanks,
Richard.
> +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
> trivial
> + 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
> (decl))
> + 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);
> else
> add_location_or_const_value_attribute (die, decl, false);
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][PR84213] DWARF: no location for non-definition DECLs with non-trivial DECL_VALUE_EXPR
2018-02-09 13:47 ` Richard Biener
@ 2018-02-09 13:53 ` Pierre-Marie de Rodat
2018-02-09 14:05 ` Pierre-Marie de Rodat
0 siblings, 1 reply; 4+ messages in thread
From: Pierre-Marie de Rodat @ 2018-02-09 13:53 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On 02/09/2018 02:47 PM, Richard Biener wrote:
> whitespace fixed here - vertical space missing before the comment
> and the first line of the comment (or cut&paste error with the patch?)
Pasto, or maybe I didnât properly configure Vim on the machine I used.
;-) Iâll make sure these formatting issues are fixed before committing.
Thanks!
--
Pierre-Marie de Rodat
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][PR84213] DWARF: no location for non-definition DECLs with non-trivial DECL_VALUE_EXPR
2018-02-09 13:53 ` Pierre-Marie de Rodat
@ 2018-02-09 14:05 ` Pierre-Marie de Rodat
0 siblings, 0 replies; 4+ messages in thread
From: Pierre-Marie de Rodat @ 2018-02-09 14:05 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On 02/09/2018 02:53 PM, Pierre-Marie de Rodat wrote:
> Pasto, or maybe I didnât properly configure Vim on the machine I used.
> ;-) Iâll make sure these formatting issues are fixed before committing.
> Thanks!
Committed! That was a Thunderbird pasto. I tried to update the PR.
Hoping I did that wellâ¦
--
Pierre-Marie de Rodat
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-09 14:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09 13:33 [PATCH][PR84213] DWARF: no location for non-definition DECLs with non-trivial DECL_VALUE_EXPR Pierre-Marie de Rodat
2018-02-09 13:47 ` Richard Biener
2018-02-09 13:53 ` Pierre-Marie de Rodat
2018-02-09 14:05 ` Pierre-Marie de Rodat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).