Re: [PATCH] Fix inline frame unwinding breakage

[Luis Machado <luis.machado@linaro.org>]

On 4/24/20 8:08 AM, Tom de Vries wrote:
> 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
> 

Ok. I double-checked this and I'm still seeing failures for those that i 
mentioned, even with the patch reverted. It may be the case that these 
tests are not supposed to pass (or the testcase has issues) on non-amd64 
targets (running Intel here).

I'll work with the testcase that does show the issue. Hopefully a fix 
for that will address all the others, but i may need further confirmation.

Thanks,
Luis