Re: [PATCH] Fix inline frame unwinding breakage

[Tom de Vries <tdevries@suse.de>]

On 24-04-2020 12:58, Luis Machado wrote:
> On 4/24/20 7:02 AM, Luis Machado wrote:
>> On 4/24/20 6:17 AM, Tom de Vries wrote:
>>> On 23-04-2020 19:51, Luis Machado via Gdb-patches wrote:
>>>> On 4/22/20 8:22 AM, Luis Machado wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On 4/22/20 6:37 AM, Andrew Burgess wrote:
>>>>>> * Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
>>>>>> [2020-04-14 18:38:36 -0300]:
>>>>>>
>>>>>>> *** re-sending due to the poor choice of characters for the
>>>>>>> backtrace
>>>>>>> annotations. GIT swallowed parts of it.
>>>>>>>
>>>>>>> There has been some breakage for aarch64-linux, arm-linux and
>>>>>>> s390-linux in
>>>>>>> terms of inline frame unwinding. There may be other targets, but
>>>>>>> these are
>>>>>>> the ones i'm aware of.
>>>>>>>
>>>>>>> The following testcases started to show numerous failures and
>>>>>>> trigger internal
>>>>>>> errors in GDB after commit 1009d92fc621bc4d017029b90a5bfab16e17fde5,
>>>>>>> "Find tailcall frames before inline frames".
>>>>>>>
>>>>>>> gdb.opt/inline-break.exp
>>>>>>> gdb.opt/inline-cmds.exp
>>>>>>> gdb.python/py-frame-inline.exp
>>>>>>> gdb.reverse/insn-reverse.exp
>>>>>>>
>>>>>>> The internal errors were of this kind:
>>>>>>>
>>>>>>> binutils-gdb/gdb/frame.c:579: internal-error: frame_id
>>>>>>> get_frame_id(frame_info*): Assertion `fi->level == 0' failed.
>>>>>>
>>>>>> I have also started seeing this assert on RISC-V, and your patch
>>>>>> resolves this issue for me, so I'm keen to see this merged.
>>>>>
>>>>> Great.
>>>>>
>>>>>>
>>>>>> I took a look through and it all looks good to me - is there anything
>>>>>> holding this back from being merged?
>>>>>
>>>>> Not really. I was waiting for an OK before pushing it.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Andrew
>>>>
>>>> I've pushed this now. Tromey and Andrew OK-ed it on IRC.
>>>
>>> This causes at least:
>>> ...
>>> FAIL: gdb.arch/amd64-entry-value.exp: tailcall: bt
>>> FAIL: gdb.arch/amd64-entry-value.exp: tailcall: p i
>>> FAIL: gdb.arch/amd64-entry-value.exp: tailcall: p i@entry
>>> FAIL: gdb.arch/amd64-entry-value.exp: tailcall: p j
>>> FAIL: gdb.arch/amd64-entry-value.exp: tailcall: p j@entry
>>> FAIL: gdb.arch/amd64-entry-value.exp: p $sp0 == $sp
>>> FAIL: gdb.arch/amd64-entry-value.exp: frame 3
>>> FAIL: gdb.arch/amd64-entry-value.exp: down
>>> FAIL: gdb.arch/amd64-entry-value.exp: disassemble
>>> FAIL: gdb.arch/amd64-entry-value.exp: ambiguous: bt
>>> FAIL: gdb.arch/amd64-entry-value.exp: self: bt
>>> FAIL: gdb.arch/amd64-entry-value.exp: self: bt debug entry-values
>>> FAIL: gdb.arch/amd64-tailcall-cxx.exp: bt
>>> FAIL: gdb.arch/amd64-tailcall-noret.exp: bt
>>> FAIL: gdb.arch/amd64-tailcall-self.exp: bt
>>> ...
>>>
>>> Looking at the first FAIL, before this commit we have:
>>> ...
>>> (gdb) PASS: gdb.arch/amd64-entry-value.exp: continue to breakpoint:
>>> tailcall: breakhere
>>> bt^M
>>> #0  d (i=71, i@entry=70, j=73.5, j@entry=72.5) at
>>> gdb.arch/amd64-entry-value.cc:34^M
>>> #1  0x00000000004006af in c (i=i@entry=7, j=j@entry=7.25) at
>>> gdb.arch/amd64-entry-value.cc:47^M
>>> #2  0x00000000004006cd in b (i=i@entry=5, j=j@entry=5.25) at
>>> gdb.arch/amd64-entry-value.cc:59^M
>>> #3  0x0000000000400524 in main () at gdb.arch/amd64-entry-value.cc:229^M
>>> (gdb) PASS: gdb.arch/amd64-entry-value.exp: tailcall: bt
>>> ...
>>> which has now degraded into:
>>> ...
>>> (gdb) PASS: gdb.arch/amd64-entry-value.exp: continue to breakpoint:
>>> tailcall: breakhere
>>> bt^M
>>> #0  d (i=<optimized out>, j=<optimized out>) at
>>> gdb.arch/amd64-entry-value.cc:34^M
>>> #1  0x0000000000400524 in main () at gdb.arch/amd64-entry-value.cc:229^M
>>> (gdb) FAIL: gdb.arch/amd64-entry-value.exp: tailcall: bt
>>> ...
>>>
>>> Thanks,
>>> - Tom
>>>
>>
>> I'll take a look at it.
> 
> Just a quick update... I did a before/after run and the only regression
> seems to be from gdb.arch/amd64-entry-value.exp.
> 
> The other failures are still there even after reverting the inline frame
> unwinding fix.
> 
> I'll check what's up with the regressed test.
> 
> Could you please confirm this when you have some cycles?

Hi,

I cannot confirm this.  All these FAILs fail with the patch, and pass
with the patch reverted.

Looking at amd64-tailcall-cxx.exp, we have normally:
...
(gdb) bt^M
#0  g (x=x@entry=2) at gdb.arch/amd64-tailcall-cxx1.cc:23^M
#1  0x00000000004004e8 in f (x=x@entry=1) at
gdb.arch/amd64-tailcall-cxx2.cc:23^M
#2  0x00000000004003de in main () at gdb.arch/amd64-tailcall-cxx1.cc:31^M
(gdb) PASS: gdb.arch/amd64-tailcall-cxx.exp: bt
...
and with the patch:
...
(gdb) bt^M
#0  g (x=2) at gdb.arch/amd64-tailcall-cxx1.cc:23^M
#1  0x00000000004003de in main () at gdb.arch/amd64-tailcall-cxx1.cc:31^M
(gdb) FAIL: gdb.arch/amd64-tailcall-cxx.exp: bt
...

So, I'd say it looks very similar to the issue in
gdb.arch/amd64-entry-value.exp.

Thanks,
- Tom