Re: [PATCH] Fix inline frame unwinding breakage

Luis Machado
2020-04-24T13:19:59Z

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?