On 12/8/20 11:35 AM, Richard Biener wrote:
>
> + {
> + /* Remove a nonbind marker when the outer scope of the
> + inline function is completely removed. */
> + if (gimple_debug_nonbind_marker_p (stmt)
> + && BLOCK_ABSTRACT_ORIGIN (b))
> + {
> + while (TREE_CODE (b) == BLOCK
> + && !inlined_function_outer_scope_p (b))
> + b = BLOCK_SUPERCONTEXT (b);
>
> So given we never remove a inlined_function_outer_scope_p BLOCK from
> the block tree can we assert that we find such a BLOCK? If we never
> elide those BLOCKs how can it happen that we elide it in the end?
>
> + if (TREE_CODE (b) == BLOCK)
> + {
> + if (TREE_USED (b))
>
For gimple_debug_inline_entry_p inlined_function_outer_scope_p (b)
is always true, and the code always proceeds to gsi_remove the
gimple_debug_inline_entry_p.
But for gimple_debug_begin_stmt_p all paths are actually taken.
> Iff we ever elide such block then this will still never be
> false since we've otherwise removed the BLOCK from the block
I can assure you TREE_USED (b) can be false here.
> tree already when we arrive here. Iff we did that, then the
> above search for a inlined_function_outer_scope_p BLOCK
> might find the "wrong" inline instance.
>
Indeed, that is a good question.
I tried to find out if there are constraints that are
preserved by remove_unused_scope_block_p, that can be
used to prove that the BLOCK_SUPERCONTEXT walking loop
does not fine the "wrong" inline instance.
diff --git a/gcc/tree-ssa-live.c b/gcc/tree-ssa-live.c
index 9ea24a1..997ccee 100644
--- a/gcc/tree-ssa-live.c
+++ b/gcc/tree-ssa-live.c
@@ -525,6 +525,10 @@ remove_unused_scope_block_p (tree scope, bool
in_ctor_dtor_block)
*t = BLOCK_SUBBLOCKS (*t);
while (BLOCK_CHAIN (*t))
{
+ gcc_assert (supercontext == BLOCK_SUPERCONTEXT
(BLOCK_SUPERCONTEXT (*t)));
+ if (!TREE_USED (*t))
+ gcc_assert (!inlined_function_outer_scope_p
(BLOCK_SUPERCONTEXT (*t)));
+ gcc_assert (!TREE_USED (BLOCK_SUPERCONTEXT (*t)));
BLOCK_SUPERCONTEXT (*t) = supercontext;
t = &BLOCK_CHAIN (*t);
}
So I think I can prove that the above assertions hold,
at least when this block is not taken:
else if (!flag_auto_profile && debug_info_level == DINFO_LEVEL_NONE
&& !optinfo_wants_inlining_info_p ())
{
/* Even for -g0 don't prune outer scopes from artificial
functions, otherwise diagnostics using tree_nonartificial_location
will not be emitted properly. */
if (inlined_function_outer_scope_p (scope))
{
tree ao = BLOCK_ORIGIN (scope);
if (ao
&& TREE_CODE (ao) == FUNCTION_DECL
&& DECL_DECLARED_INLINE_P (ao)
&& lookup_attribute ("artificial", DECL_ATTRIBUTES (ao)))
unused = false;
}
}
in that case it should be irrelevant, since we wont have debug_begin_stmt_p
if debug_info_level == DINFO_LEVEL_NONE.
So the above assertions mean, that *if* !TREE_USED (b)
which is the precondition for the while loop, then we know that
BLOCK_SUPERCONTEXT (b) *was* not a inlined_function_outer_scope_p,
and it was replaced by BLOCK_SUPERCONTEXT (BLOCKL_SUPERCONTEXT (b))
so the loop skips one step, but the result is still the same
inline block.
However what concerns me, is that the assertion
if (TREE_USED (*t))
gcc_assert (!inlined_function_outer_scope_p (BLOCK_SUPERCONTEXT (*t))
does not hold. I think that means, that it can happen, that
a USED block is moved out of the inline block. And while I have
no test case for it, that sheds a big question on the correctness
of the debug info when that happens.
But I think that is a different issue if I at all.
> So I think we only eliminate the inline scopes if all
> subblocks are unused but then if there's a used outer
> inline instance your patch will assign that outer inline
> instances BLOCK to debug stmts formerly belonging to the
> elided inline instance, no? (we also mess with
> BLOCK_SUPERCONTEXT during BLOCK removal so I'm not sure
> the walking works as expected)
>
I did not look at before, and just expected it to behave
reasonably, that is just elide *unused* lexical scopes, and
*empty* subroutines, but that it seems to move *used* lexical
scopes to the grandfather scope, while *removing* the now
*emptied* original inline block is odd.
Or maybe I missed something...
> I guess we could, during the remove_unused_scope_block_p
> DFS walk, record and pass down the "current"
> inlined_function_outer_scope_p BLOCK and for BLOCKs
> we elide record that somehow? (since we elide the block
> we could abuse fragment_origin/chain for this)
> Then in the patched loop do
>
I would rather not move *used* blocks out of an inline scope,
whether or not that appears to be unused.
But maybe as a follow up, maybe I find even a test case, where the
debug info is broken, I have debugged a lot optimized code, and never
encountered something like that, but that proves not much.
> if (b && !TREE_USED (b))
> {
> if (BLOCK_ABSTRACT_ORIGIN (b))
> {
> if (!TREE_USED (BLOCK_FRAGMENT_CHAIN (b))
> {
> /* inline instance removed */
> gsi_remove ();
> }
> else
> {
> /* inline instance kept, use it */
> gimple_set_block (stmt, BLOCK_FRAGMENT_CHAIN (b));
> }
> }
> else
> gimple_set_block (stmt, NULL);
> }
>
> ? Not sure what happens if the inline instance ends up split into
> a hot/cold part - then the "punning" of the BLOCK of the stmt to
> the outermost scope could cause gdb to set bogus breakpoints?
>
> Thanks,
> Richard.
>
Yes, it does indeed happen. I have a gdb patch for that already posted
for review:
https://sourceware.org/pipermail/gdb-patches/2020-November/173614.html
It is important to only break at the entry_point_pc, not when crossing
a sub-block boundary of the same subroutine.
Another deficiency that is not completely solvable is line breakpoints
at the exit of a subroutine, the dwarf format specifies the block ranges
just as byte ranges, while the line table can hold different lines
at the same PC, from different scopes, there have different view numbers.
So I would be happy to have the complete call stack, maybe some frames too
much, instead of some frames missing in the middle...
Thanks
Bernd.
