Re: [PATCH] Fix inline frame unwinding breakage

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

On 24-04-2020 15:19, Luis Machado wrote:
> 
> 
> On 4/24/20 9:23 AM, Tom de Vries wrote:
>> On 24-04-2020 13:37, Luis Machado wrote:
>>> 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).
>>>
>>
>> Also Intel here (FWIW: Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz).
>>
> 
> Yikes. I have the exact same. There may be system differences affecting
> the tests then (libraries and/or compiler).
> 
> I have this compiler: gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04).
> 
> 
>>> 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.
>>
>> Understood.
>>
>> Can you file a PR for the amd64-tailcall-cxx.exp FAIL that you're seeing
>> before the patch, and attach the exec?
> 
> Sure. But before i do that, i have these failure with the patch reverted:
> 
> FAIL: gdb.arch/amd64-entry-value-inline.exp: p y
> FAIL: gdb.arch/amd64-entry-value-param-dwarf5.exp: call 1: p y
> FAIL: gdb.arch/amd64-entry-value-param-dwarf5.exp: call 1: p b
> FAIL: gdb.arch/amd64-entry-value-param-dwarf5.exp: call 2: p y
> FAIL: gdb.arch/amd64-entry-value-param-dwarf5.exp: call 2: p b
> FAIL: gdb.arch/amd64-entry-value-param-dwarf5.exp: call 3: p y
> FAIL: gdb.arch/amd64-entry-value-param-dwarf5.exp: call 3: p b
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p y
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 1: p b
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 2: p y
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 2: p b
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 3: p y
> FAIL: gdb.arch/amd64-entry-value-param.exp: call 3: p b
> FAIL: gdb.arch/amd64-entry-value-paramref.exp: frame
> FAIL: gdb.arch/amd64-tailcall-cxx.exp: bt
> FAIL: gdb.arch/amd64-tailcall-noret.exp: bt
> FAIL: gdb.arch/amd64-tailcall-self.exp: bt
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: only: entry_equal: stop (stopped
> at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: only: entry_equal:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: only: entry_different: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: only: entry_different:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: only: validity: stop (stopped at
> wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: only: validity:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: preferred: entry_equal: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: preferred: entry_equal:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: preferred: entry_different: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: preferred: entry_different:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: preferred: validity: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: preferred: validity:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: if-needed: validity: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: if-needed: validity:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: both: entry_equal: stop (stopped
> at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: both: entry_equal:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: both: entry_different: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: both: entry_different:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: both: validity: stop (stopped at
> wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: both: validity:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: compact: entry_equal: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: compact: entry_equal:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: compact: entry_different: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: compact: entry_different:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: compact: validity: stop (stopped
> at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: compact: validity:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: default: entry_equal: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: default: entry_equal:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: default: entry_different: stop
> (stopped at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: default: entry_different:
> -stack-list-variables (unexpected output)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: default: validity: stop (stopped
> at wrong place)
> FAIL: gdb.mi/mi2-amd64-entry-value.exp: default: validity:
> -stack-list-variables (unexpected output)
> 
> Also a bunch of failures for gdb.base/gnu-ifunc.exp, but i think this is
> unrelated.
> 
> Which ones do you want me to open bugs against?

I think you're running into
https://sourceware.org/bugzilla/show_bug.cgi?id=24892 .

I can reproduce the same failure by running with target board
unix/-fPIE/-pie.

Thanks,
- Tom