Hi lianbo build error fix on attached patch v4.
Thanks
Guanyou
Guanyou Chen <chenguanyou9...@gmail.com> 于2025年1月16日周四 16:00写道：
> Hi lianbo
>
> only test step.
>
> rm -rf gdb-10.2/
> make target=arm64
>
> Thanks
> Guanyou
>
> lijiang <liji...@redhat.com> 于2025年1月16日周四 14:58写道：
>
>>
>>
>> On Wed, Jan 15, 2025 at 9:29 PM Guanyou Chen <chenguanyou9...@gmail.com>
>> wrote:
>>
>>> Hi lianbo
>>>
>>> attached pacth v3.
>>>
>>
>> Thank you for the update.
>>
>> Have you tested patch v3? I got an error and failed to apply the gdb
>> patch:
>> ...
>> patching file gdb-10.2/gdb/stack.c
>> Hunk #1 succeeded at 2130 (offset 3 lines).
>> patching file gdb-10.2/gdb/frame.c
>> Hunk #1 FAILED at 944.
>> 1 out of 2 hunks FAILED
>> make[6]: Nothing to be done for 'all'.
>> ...
>>
>> Thanks
>> Lianbo
>>
>>
>>> > Can you help double check if a sanity check needs to be added? I saw
>>> the value 'pc|mask' is always checked in crash code as below:
>>> > ...
>>> > LR = regs->regs[30];
>>> > if (is_kernel_text (LR |
>>> ms->CONFIG_ARM64_KERNELPACMASK))
>>> > LR |= ms->CONFIG_ARM64_KERNELPACMASK;
>>> > ...
>>> > if yes, the value pc can be passed as an argument in
>>> crash_get_kernel_pac_mask(), and then deal with this one.
>>>
>>> /* arm64 kernel lr maybe has patuh */
>>> void crash_decode_ptrauth_pc(ulong *pc);
>>> void crash_decode_ptrauth_pc(ulong *pc)
>>> {
>>> #ifdef ARM64
>>> struct machine_specific *ms = machdep->machspec;
>>> if (is_kernel_text(*pc | ms->CONFIG_ARM64_KERNELPACMASK))
>>> *pc |= ms->CONFIG_ARM64_KERNELPACMASK;
>>> #endif /* !ARM64 */
>>> }
>>>
>>> Guanyou Chen <chenguanyou9...@gmail.com> 于2025年1月15日周三 17:55写道：
>>>
>>>> Hi lianbo
>>>>
>>>> attach patch v2.
>>>>
>>>> lijiang <liji...@redhat.com> 于2025年1月15日周三 16:42写道：
>>>>
>>>>> Thank you for the patch, Guanyou.
>>>>>
>>>>> On Tue, Dec 31, 2024 at 7:22 PM <
>>>>> devel-requ...@lists.crash-utility.osci.io> wrote:
>>>>>
>>>>>> Date: Thu, 26 Dec 2024 00:08:50 +0800
>>>>>> From: Guanyou Chen <chenguanyou9...@gmail.com>
>>>>>> Subject: [Crash-utility] [PATCH] arm64: add pac mask to better support
>>>>>> gdb stack unwind
>>>>>> To: Lianbo <liji...@redhat.com>, Tao Liu <l...@redhat.com>,
>>>>>> devel@lists.crash-utility.osci.io
>>>>>> Message-ID:
>>>>>> <
>>>>>> cahs3rmxjg6oxb_zmgnr60kripoxo9tnb_63k+rjjhfk1t7a...@mail.gmail.com>
>>>>>> Content-Type: multipart/mixed; boundary="0000000000009f35d6062a1a727c"
>>>>>>
>>>>>> --0000000000009f35d6062a1a727c
>>>>>> Content-Type: multipart/alternative;
>>>>>> boundary="0000000000009f35d5062a1a727a"
>>>>>>
>>>>>> --0000000000009f35d5062a1a727a
>>>>>> Content-Type: text/plain; charset="UTF-8"
>>>>>>
>>>>>> Hi Lianbo & Tao
>>>>>>
>>>>>> Currently, gdb passthroughs of 'bt', 'frame', 'up', 'down',
>>>>>> 'info, locals' don't work on arm64 machine enabled pauth.
>>>>>> This is due to gdb not knowing the lr register real values
>>>>>> to unwind the stack frames.
>>>>>>
>>>>>> ----------------------------
>>>>>> gdb passthrough (eg. "bt") | |
>>>>>> crash -------------------------> | |
>>>>>> | gdb_interface |
>>>>>> | |
>>>>>> | |
>>>>>> | ---------------------- |
>>>>>> get_kernel_pac_mask | | | |
>>>>>> crash_target<-------------------------+--| gdb | |
>>>>>> --------------------------+->| | |
>>>>>> arm64: CONFIG_ARM64_KERNELPACMASK| | | |
>>>>>> other: ~0UL | | | |
>>>>>> | ---------------------- |
>>>>>> ----------------------------
>>>>>>
>>>>>> With the patch:
>>>>>> crash> gdb bt
>>>>>> #0 __switch_to (prev=prev@entry=0xffffff8001af92c0,
>>>>>> next=next@entry=0xffffff889da7a580)
>>>>>> at /proc/self/cwd/common/arch/arm64/kernel/process.c:569
>>>>>> #1 0xffffffd3602132c0 in context_switch (rq=0xffffff8a7295a080,
>>>>>> prev=0xffffff8001af92c0, next=0xffffff889da7a580, rf=<optimized out>)
>>>>>> at
>>>>>> /proc/self/cwd/common/kernel/sched/core.c:5515
>>>>>> #2 __schedule (sched_mode=<optimized out>, sched_mode@entry
>>>>>> =2147859424)
>>>>>> at /proc/self/cwd/common/kernel/sched/core.c:6843
>>>>>> #3 0xffffffd3602136d8 in schedule () at
>>>>>> /proc/self/cwd/common/kernel/sched/core.c:6917
>>>>>> ...
>>>>>>
>>>>>> Without the patch:
>>>>>> crash> gdb bt
>>>>>> #0 __switch_to (prev=0xffffff8001af92c0,
>>>>>> next=0xffffff889da7a580) at
>>>>>> /proc/self/cwd/common/arch/arm64/kernel/process.c:569
>>>>>> #1 0x9fc5c5d3602132c0 in ?? ()
>>>>>> Backtrace stopped: previous frame identical to this frame (corrupt
>>>>>> stack?)
>>>>>>
>>>>>> Signed-off-by: Guanyou.Chen <chenguan...@xiaomi.com>
>>>>>> ---
>>>>>> gdb-10.2.patch | 24 ++++++++++++++++++++++++
>>>>>> gdb_interface.c | 11 +++++++++++
>>>>>> 2 files changed, 35 insertions(+)
>>>>>>
>>>>>> diff --git a/gdb-10.2.patch b/gdb-10.2.patch
>>>>>> index c867660..4c13a6b 100644
>>>>>> --- a/gdb-10.2.patch
>>>>>> +++ b/gdb-10.2.patch
>>>>>> @@ -16216,3 +16216,27 @@ exit 0
>>>>>> printf_filtered (_("Backtrace stopped: %s\n"),
>>>>>> frame_stop_reason_string (trailing));
>>>>>> }
>>>>>> +--- gdb-10.2/gdb/frame.c.orig
>>>>>> ++++ gdb-10.2/gdb/frame.c
>>>>>> +@@ -944,6 +944,10 @@ frame_find_by_id (struct frame_id id)
>>>>>> + return NULL;
>>>>>> + }
>>>>>> +
>>>>>> ++#ifdef CRASH_MERGE
>>>>>> ++extern "C" unsigned long crash_get_kernel_pac_mask(void);
>>>>>> ++#endif
>>>>>> ++
>>>>>> + static CORE_ADDR
>>>>>> + frame_unwind_pc (frame_info_ptr this_frame)
>>>>>> + {
>>>>>> +@@ -974,6 +978,10 @@ frame_unwind_pc (struct frame_info_ptr
>>>>>> *this_frame)
>>>>>> + try
>>>>>> + {
>>>>>> + pc = gdbarch_unwind_pc (prev_gdbarch, this_frame);
>>>>>> ++#ifdef CRASH_MERGE
>>>>>> ++ CORE_ADDR mask = crash_get_kernel_pac_mask();
>>>>>> ++ pc |= mask;
>>>>>>
>>>>>
>>>>> Can you help double check if a sanity check needs to be added? I saw
>>>>> the value 'pc|mask' is always checked in crash code as below:
>>>>> ...
>>>>> LR = regs->regs[30];
>>>>> if (is_kernel_text (LR |
>>>>> ms->CONFIG_ARM64_KERNELPACMASK))
>>>>> LR |= ms->CONFIG_ARM64_KERNELPACMASK;
>>>>> ...
>>>>> if yes, the value pc can be passed as an argument in
>>>>> crash_get_kernel_pac_mask(), and then deal with this one.
>>>>>
>>>>>
>>>>>> ++#endif
>>>>>> + pc_p = true;
>>>>>> + }
>>>>>> + catch (const gdb_exception_error &ex)
>>>>>> diff --git a/gdb_interface.c b/gdb_interface.c
>>>>>> index 315711e..765dafe 100644
>>>>>> --- a/gdb_interface.c
>>>>>> +++ b/gdb_interface.c
>>>>>> @@ -1083,3 +1083,14 @@ int crash_get_current_task_reg (int regno,
>>>>>> const
>>>>>> char *regname,
>>>>>> return machdep->get_current_task_reg(regno, regname, regsize,
>>>>>> value);
>>>>>> }
>>>>>>
>>>>>> +/* arm64 kernel lr pac mask */
>>>>>> +unsigned long crash_get_kernel_pac_mask(void);
>>>>>> +unsigned long crash_get_kernel_pac_mask(void)
>>>>>> +{
>>>>>> +#ifdef ARM64
>>>>>> + struct machine_specific *ms = machdep->machspec;
>>>>>> + return ms->CONFIG_ARM64_KERNELPACMASK;
>>>>>> +#else
>>>>>> + return ~0UL;
>>>>>>
>>>>>
>>>>> The "~0UL" is 0xFFFFFFFFFFFFFFFF, is this expected? Or do you want it
>>>>> to overflow?
>>>>> ++ pc |= mask;
>>>>>
>>>>> It probably has the same result, but the "return 0UL" should be more
>>>>> readable. Please see kernel code:
>>>>> vmcoreinfo_append_str("NUMBER(KERNELPACMASK)=0x%llx\n",
>>>>>
>>>>> system_supports_address_auth() ?
>>>>>
>>>>> ptrauth_kernel_pac_mask() : 0);
>>>>>
>>>>>
>>>>> BTW: I cannot apply your patch with git command, and ran into some
>>>>> issues, probably it is a coding issue.
>>>>>
>>>>> Thanks
>>>>> Lianbo
>>>>>
>>>>>
>>>>>> +#endif /* !ARM64 */
>>>>>> +}
>>>>>> --
>>>>>> 2.34.1
>>>>>>
>>>>>> Thanks,
>>>>>> Guanyou
>>>>>>
>>>>>
0001-arm64-add-pac-mask-to-better-support-gdb-stack-unwin.patch.v4
Description: Binary data
-- Crash-utility mailing list -- devel@lists.crash-utility.osci.io To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
