Re: [PATCH] Fix inline frame unwinding breakage

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

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

Looks very likely, since Ubuntu's GCC passes -pie by default. I'll keep 
that in mind.

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