[PATCH v3] gdb: Fix false match issue in skip_prologue_using_linetable
Luis Machado
luis.machado@arm.com
Tue May 16 14:19:41 GMT 2023
On 4/24/23 15:15, Tom de Vries wrote:
> On 4/24/23 14:53, Luis Machado wrote:
>> On 4/22/23 09:01, Tom de Vries via Gdb-patches wrote:
>>> On 4/21/23 20:03, Kevin Buettner wrote:
>>>> Hi Tom,
>>>>
>>>> On Tue, 18 Apr 2023 14:15:06 +0200
>>>> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
>>>>
>>>>> On 4/18/23 14:09, Tom de Vries via Gdb-patches wrote:
>>>>>> Co-Authored-By: WANG Rui <r@hev.cc> (fix, tiny change [1])
>>>>>> Co-Authored-By: Tom de Vries <tdevries@suse.de> (test-case)
>>>>>>
>>>>>> [1] https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html
>>>>>
>>>>> I'm not used to deal with these matters, so I'd appreciate some
>>>>> review/approval on this. Is my copyright status assessment correct, and
>>>>> did I write it up correctly?
>>>>
>>>> I refreshed my memory via the link you provided above. Based on what
>>>> is written there, I conclude that Wang Rui's change is not legally
>>>> signficant for copyright purposes.
>>>>
>>>> Also, I've looked over the Rui's patch as well as your test case, and
>>>> it looks good to me. So...
>>>>
>>>> Approved-by: Kevin Buettner <kevinb@redhat.com>
>>>>
>>>
>>> Hi Kevin,
>>>
>>> Thanks for review.
>>>
>>> Committed and also backported to gdb-13-branch, because it was a 12 -> 13 regression.
>>>
>>> Thanks,
>>> - Tom
>>>
>>
>> For some reason aarch64 is grumpy with this test, and it FAIL's the last comparison.
>>
>> Maybe aarch64 is broken in this regard?
>
> Hi Luis,
>
> thanks for reporting this.
>
> I could reproduce it on openSUSE Leap 15.4.
>
> I think there are two independent problems:
> - the aarch64 prologue analyzer walks past the end of the function
> - the test-case assumes that the prologue analyzer will return the first
> insn in foo, rather that some insn in foo.
>
> The WIP patch below addresses both issues, and allows the test-case to pass for me.
>
> [ FWIW, alternatively using some "maint set skip-prologue" value from this RFC ( https://sourceware.org/pipermail/gdb-patches/2022-August/191343.html ) could also suffice to ignore the first problem. ]
>
> Thanks,
> - Tom
>
> ...
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index ec0e51bdaf7..d974595e48f 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -917,12 +917,13 @@ aarch64_analyze_prologue_test (void)
> static CORE_ADDR
> aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> {
> - CORE_ADDR func_addr, limit_pc;
> + CORE_ADDR func_addr, func_end_addr, limit_pc;
>
> /* See if we can determine the end of the prologue via the symbol
> table. If so, then return either PC, or the PC after the
> prologue, whichever is greater. */
> - if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
> + bool func_addr_found = find_pc_partial_function (pc, NULL, &func_addr, &func_end_addr);
> + if (func_addr_found)
> {
> CORE_ADDR post_prologue_pc
> = skip_prologue_using_sal (gdbarch, func_addr);
> @@ -941,7 +942,8 @@ aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> limit_pc = skip_prologue_using_sal (gdbarch, pc);
> if (limit_pc == 0)
> limit_pc = pc + 128; /* Magic. */
> -
> + limit_pc = std::min (limit_pc, func_end_addr - 4);
> +
> /* Try disassembling prologue. */
> return aarch64_analyze_prologue (gdbarch, pc, limit_pc, NULL);
> }
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp
> index 488f85f9674..c506cfd55cc 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp
> @@ -95,15 +95,15 @@ if { $break_addr == "" } {
>
> # Get the "foo_label" address.
>
> -set foo_label_addr ""
> -gdb_test_multiple "print /x &foo_label" "" {
> +set bar_label_addr ""
> +gdb_test_multiple "print /x &bar_label" "" {
> -re -wrap "= ($hex)" {
> - set foo_label_addr $expect_out(1,string)
> + set bar_label_addr $expect_out(1,string)
> pass $gdb_test_name
> }
> }
>
> -if { $foo_label_addr == "" } {
> +if { $bar_label_addr == "" } {
> return
> }
>
> @@ -115,4 +115,4 @@ gdb_test "print &foo_end == &bar_label" " = 1"
> # Check that the breakpoint is set at the expected address. Regression test
> # for PR30369.
>
> -gdb_assert { $break_addr == $foo_label_addr }
> +gdb_assert { $break_addr < $bar_label_addr }
> ...
Sorry, I thought I had replied to this thread. Indeed the above patch addresses this problem with the aarch64 prologue skipper,
and it also fixes things for arm. For arm I suspect we might need the same fix to the prologue skipper that the patch addresses for aarch64.
I can pick it up, refresh and submit if you're happy with it as well.
Regards,
Luis
More information about the Gdb-patches
mailing list