Re: [patch] Fix debug info of nested inline functions
Revised patch attached. It still generates the same (fixed) debug info for the reduced testcase. I'll do a full testing cycle if you're happy with it. * dwarf2out.c (function_possibly_abstracted_p): New static function. (gen_subprogram_die): Use it function_possibly_abstracted_p in lieu of cgraph_function_possibly_inlined_p. (gen_inlined_subroutine_die): Return if the origin is to be ignored. (process_scope_var): Do not emit concrete instances of abstracted nested functions from here. (gen_decl_die): Emit the abstract instance if the function is possibly abstracted and not only possibly inlined. (dwarf2out_finish): Find the first non-abstract parent instance and attach concrete instances on the limbo list to it. This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53860 I've reverted the patch, as I can imagine that it will cause other problems. -- Eric Botcazou
Re: [patch] Fix debug info of nested inline functions
On Wed, May 16, 2012 at 2:29 PM, Eric Botcazou ebotca...@adacore.com wrote: Right, and that's why we want your change to split the nested function into abstract and concrete instances. But then it should be fine to attach the abstract instance to the abstract parent normally, I would think. Indeed, this works, but I need to use function_possibly_abstracted_p instead of cgraph_function_possibly_inlined_p in gen_subprogram_die to get DW_AT_inline. Revised patch attached. It still generates the same (fixed) debug info for the reduced testcase. I'll do a full testing cycle if you're happy with it. * dwarf2out.c (function_possibly_abstracted_p): New static function. (gen_subprogram_die): Use it function_possibly_abstracted_p in lieu of cgraph_function_possibly_inlined_p. (gen_inlined_subroutine_die): Return if the origin is to be ignored. (process_scope_var): Do not emit concrete instances of abstracted nested functions from here. (gen_decl_die): Emit the abstract instance if the function is possibly abstracted and not only possibly inlined. (dwarf2out_finish): Find the first non-abstract parent instance and attach concrete instances on the limbo list to it. This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53860 -- H.J.
Re: [patch] Fix debug info of nested inline functions
Does it really matter for concrete instances of inline functions? Nope. Are those the only things with an abstract origin that will end up on limbo? According to the head comment of the block, yes, but I can try and see what happens in real life. If we're always going to attach them to comp_unit_die () anyway, we might as well do that without the loop. We attach them to the first non-abstract parent function with the loop. That's important for Ada because we rely on the static nesting of functions in the debug info to support up-level references in GDB (i.e. access to variables of parent functions from within nested functions). -- Eric Botcazou
Re: [patch] Fix debug info of nested inline functions
On 05/19/2012 04:40 AM, Eric Botcazou wrote: We attach them to the first non-abstract parent function with the loop. That's important for Ada because we rely on the static nesting of functions in the debug info to support up-level references in GDB (i.e. access to variables of parent functions from within nested functions). Aha. The patch is OK. Jason
Re: [patch] Fix debug info of nested inline functions
On 05/16/2012 05:29 PM, Eric Botcazou wrote:
- if (cgraph_function_possibly_inlined_p (decl))
+ if (function_possibly_abstracted_p (decl))
add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_declared_inlined);
else
add_AT_unsigned (subr_die, DW_AT_inline,
DW_INL_declared_not_inlined);
}
else
{
- if (cgraph_function_possibly_inlined_p (decl))
+ if (function_possibly_abstracted_p (decl))
Why do you need this change? As long as we're setting DW_AT_inline, it
shouldn't matter what its value is.
- if (origin origin-die_parent)
- add_child_die (origin-die_parent, die);
+ if (origin)
+ {
+ /* Find the first non-abstract parent instance. */
+ do
+ origin = origin-die_parent;
+ while (origin
+ (origin-die_tag != DW_TAG_subprogram
+|| get_AT (origin, DW_AT_inline)));
+ if (origin)
+ add_child_die (origin, die);
+ else
+ add_child_die (comp_unit_die (), die);
+ }
If we are looking at the DIE for something from a function in non-unit
scope, this will return comp_unit_die() where previously it would have
returned the immediate scope of the function, which might be something
like a namespace/module or type.
Jason
Re: [patch] Fix debug info of nested inline functions
Why do you need this change? As long as we're setting DW_AT_inline, it shouldn't matter what its value is. It's 0 for the nested function without it. If we are looking at the DIE for something from a function in non-unit scope, this will return comp_unit_die() where previously it would have returned the immediate scope of the function, which might be something like a namespace/module or type. Does it really matter for concrete instances of inline functions? -- Eric Botcazou
Re: [patch] Fix debug info of nested inline functions
On 05/18/2012 04:48 PM, Eric Botcazou wrote: Why do you need this change? As long as we're setting DW_AT_inline, it shouldn't matter what its value is. It's 0 for the nested function without it. Ah, I thought that having DW_AT_inline of DW_INL_not_inlined was enough to mark it as an abstract instance, but it seems I was wrong. If we are looking at the DIE for something from a function in non-unit scope, this will return comp_unit_die() where previously it would have returned the immediate scope of the function, which might be something like a namespace/module or type. Does it really matter for concrete instances of inline functions? Nope. Are those the only things with an abstract origin that will end up on limbo? If we're always going to attach them to comp_unit_die () anyway, we might as well do that without the loop. Jason
Re: [patch] Fix debug info of nested inline functions
Right, and that's why we want your change to split the nested function
into abstract and concrete instances. But then it should be fine to
attach the abstract instance to the abstract parent normally, I would
think.
Indeed, this works, but I need to use function_possibly_abstracted_p instead of
cgraph_function_possibly_inlined_p in gen_subprogram_die to get DW_AT_inline.
Revised patch attached. It still generates the same (fixed) debug info for the
reduced testcase. I'll do a full testing cycle if you're happy with it.
* dwarf2out.c (function_possibly_abstracted_p): New static function.
(gen_subprogram_die): Use it function_possibly_abstracted_p in lieu of
cgraph_function_possibly_inlined_p.
(gen_inlined_subroutine_die): Return if the origin is to be ignored.
(process_scope_var): Do not emit concrete instances of abstracted
nested functions from here.
(gen_decl_die): Emit the abstract instance if the function is possibly
abstracted and not only possibly inlined.
(dwarf2out_finish): Find the first non-abstract parent instance and
attach concrete instances on the limbo list to it.
--
Eric Botcazou
Index: dwarf2out.c
===
--- dwarf2out.c (revision 187533)
+++ dwarf2out.c (working copy)
@@ -16586,6 +16586,22 @@ gen_call_site_die (tree decl, dw_die_ref
return die;
}
+/* Return true if an abstract instance of function DECL can be generated in
+ the debug information. */
+
+static bool
+function_possibly_abstracted_p (tree decl)
+{
+ while (decl)
+{
+ if (cgraph_function_possibly_inlined_p (decl))
+ return true;
+ decl = decl_function_context (decl);
+}
+
+ return false;
+}
+
/* Generate a DIE to represent a declared function (either file-scope or
block-local). */
@@ -16738,14 +16754,14 @@ gen_subprogram_die (tree decl, dw_die_re
{
if (DECL_DECLARED_INLINE_P (decl))
{
- if (cgraph_function_possibly_inlined_p (decl))
+ if (function_possibly_abstracted_p (decl))
add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_declared_inlined);
else
add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_declared_not_inlined);
}
else
{
- if (cgraph_function_possibly_inlined_p (decl))
+ if (function_possibly_abstracted_p (decl))
add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_inlined);
else
add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_not_inlined);
@@ -17674,6 +17690,8 @@ gen_inlined_subroutine_die (tree stmt, d
gcc_assert (! BLOCK_ABSTRACT (stmt));
decl = block_ultimate_origin (stmt);
+ if (DECL_IGNORED_P (decl))
+return;
/* Emit info for the abstract instance first, if we haven't yet. We
must emit this even if the block is abstract, otherwise when we
@@ -18615,6 +18633,7 @@ gen_block_die (tree stmt, dw_die_ref con
/* Process variable DECL (or variable with origin ORIGIN) within
block STMT and add it to CONTEXT_DIE. */
+
static void
process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die)
{
@@ -18632,8 +18651,15 @@ process_scope_var (tree stmt, tree decl,
if (die != NULL die-die_parent == NULL)
add_child_die (context_die, die);
else if (TREE_CODE (decl_or_origin) == IMPORTED_DECL)
-dwarf2out_imported_module_or_decl_1 (decl_or_origin, DECL_NAME (decl_or_origin),
+dwarf2out_imported_module_or_decl_1 (decl_or_origin,
+ DECL_NAME (decl_or_origin),
stmt, context_die);
+ /* Do not emit concrete instances of abstracted nested functions within
+ concrete instances of parent functions. */
+ else if (TREE_CODE (decl_or_origin) == FUNCTION_DECL
+ die
+ get_AT (die, DW_AT_inline))
+;
else
gen_decl_die (decl, origin, context_die);
}
@@ -18980,11 +19006,11 @@ gen_decl_die (tree decl, tree origin, dw
? DECL_ORIGIN (origin)
: DECL_ABSTRACT_ORIGIN (decl));
- /* If we're emitting an out-of-line copy of an inline function,
+ /* If we're emitting an out-of-line copy of an abstracted function,
emit info for the abstract instance and set up to refer to it. */
- else if (cgraph_function_possibly_inlined_p (decl)
- ! DECL_ABSTRACT (decl)
- ! class_or_namespace_scope_p (context_die)
+ else if (!DECL_ABSTRACT (decl)
+ function_possibly_abstracted_p (decl)
+ !class_or_namespace_scope_p (context_die)
/* dwarf2out_abstract_function won't emit a die if this is just
a declaration. We must avoid setting DECL_ABSTRACT_ORIGIN in
that case, because that works only if we have a die. */
@@ -21982,8 +22008,19 @@ dwarf2out_finish (const char *filename)
{
dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin);
- if (origin origin-die_parent)
- add_child_die (origin-die_parent, die);
+ if (origin)
+ {
+ /* Find the first non-abstract parent instance. */
+ do
+ origin =
Re: [patch] Fix debug info of nested inline functions
On 05/14/2012 11:54 AM, Eric Botcazou wrote: Hmm, why isn't current_function_decl == decl when we're trying to emit the abstract instance of a nested function? Because it is emitted when the first instance of the parent function is seen, and in this case current_function_decl == parent_decl. Our normal procedure is to generate a declaration when we see a function in its enclosing context, and then fix it up later when we see the definition. Why not handle this similarly? Jason
Re: [patch] Fix debug info of nested inline functions
On 05/14/2012 12:49 PM, Jason Merrill wrote: On 05/14/2012 11:54 AM, Eric Botcazou wrote: Hmm, why isn't current_function_decl == decl when we're trying to emit the abstract instance of a nested function? Because it is emitted when the first instance of the parent function is seen, and in this case current_function_decl == parent_decl. Our normal procedure is to generate a declaration when we see a function in its enclosing context, and then fix it up later when we see the definition. Why not handle this similarly? I suppose the way we handle nested functions, we generate debug info for the nested function before that for the enclosing function, but then we should attach the (abstract) nested function to the enclosing function in process_scope_var. Jason
Re: [patch] Fix debug info of nested inline functions
Our normal procedure is to generate a declaration when we see a function in its enclosing context, and then fix it up later when we see the definition. Why not handle this similarly? Because we want to generate an abstract instance of the nested function within the abstract instance of the parent function. If we wait for the definition of the nested function, and it's out-of-line, we attach the out-of-line instance to the abstract parent, which is the source of the problem. -- Eric Botcazou
Re: [patch] Fix debug info of nested inline functions
On 05/14/2012 04:17 PM, Eric Botcazou wrote: Our normal procedure is to generate a declaration when we see a function in its enclosing context, and then fix it up later when we see the definition. Why not handle this similarly? Because we want to generate an abstract instance of the nested function within the abstract instance of the parent function. If we wait for the definition of the nested function, and it's out-of-line, we attach the out-of-line instance to the abstract parent, which is the source of the problem. Right, and that's why we want your change to split the nested function into abstract and concrete instances. But then it should be fine to attach the abstract instance to the abstract parent normally, I would think. Jason
Re: [patch] Fix debug info of nested inline functions
On 03/02/2012 03:29 PM, Eric Botcazou wrote: I notice that D.7 seems to suggest that if the nested function is not inlinable and shared between all instances of the containing function that we put a normal (non-abstract/concrete) instance of the nested function inside the abstract function for the containing function. But I agree that it's cleaner your way: put an abstract instance there instead so that the abstract instance of the containing function is all abstract. + /* Emit an abstract instance of nested functions within an abstract instance + of their parent. */ + int declaration = ((decl != current_function_decl + !(DECL_INITIAL (decl) != NULL_TREE + DECL_ABSTRACT (decl) + current_function_decl + DECL_ABSTRACT (current_function_decl))) || class_or_namespace_scope_p (context_die)); Hmm, why isn't current_function_decl == decl when we're trying to emit the abstract instance of a nested function? + /* Do not emit concrete instances of abstracted nested functions without + actual instances. */ + else if (TREE_CODE (decl_or_origin) == FUNCTION_DECL + die + get_AT (die, DW_AT_inline)) +; Should without actual instances be something like within concrete instances of containing functions? - if (origin origin-die_parent) + if (origin + origin-die_parent + /* Skip an abtract parent instance. */ + !(origin-die_parent-die_tag == DW_TAG_subprogram + get_AT (origin-die_parent, DW_AT_inline))) add_child_die (origin-die_parent, die); What if the immediate parent is a DW_TAG_lexical_block or some other thing nested inside an abstract subprogram? and you suggested to iterate over DECL_CONTEXT instead of die_parent to find an appropriate parent in order to attach the DIE on the limbo list to. In my earlier comments I seem to have been wrong about the behavior of gen_subprogram_die; now I see that if there is an abstract instance the concrete out-of-line instance is not associated with the decl number. So I guess your earlier limbo handling code was fine apart from the lexical_block issue above. Jason
