On Wed, Feb 22, 2023 at 1:17 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
On 2023/02/21 12:03, Lianbo Jiang wrote:
> For gdb-10.2, the disassembly code may start with "=>", which needs to
> be stripped when calculating the address. Otherwise, parsing the address
> will fail because the current code always assumes that it starts with the
> "0x". For example:
>
> crash> gdb disassemble 0xffffffffa2317add
> Dump of assembler code for function native_queued_spin_lock_slowpath:
> 0xffffffffa2317ab0 <+0>: nopl 0x0(%rax,%rax,1)
> 0xffffffffa2317ab5 <+5>: push %rbp
> 0xffffffffa2317ab6 <+6>: mov %rsp,%rbp
> ...
> 0xffffffffa2317ad3 <+35>: mov %edx,%eax
> 0xffffffffa2317ad5 <+37>: lock cmpxchg %ecx,(%rdi)
> => 0xffffffffa2317ad9 <+41>: cmp %eax,%edx
> 0xffffffffa2317adb <+43>: jne 0xffffffffa2317ac0 <native_queued_spin_lock_slowpath+16>
> 0xffffffffa2317add <+45>: pop %rbp
> 0xffffffffa2317ade <+46>: xchg %ax,%ax
> ...
Probably the following prints the "=> ".
Out of curiosity, what did you do before the gdb disas?
I didn't run any crash commands or gdb commands before the gdb disass, the current command(gdb disassemble 0xffffffffa2317add) is the first one.
The crash-7(gdb-7.6) doesn't have this issue. The "=>" should be the current PC indicator.
The pc_prefix() was added by following commit:
2b28d209243f ("2009-10-21 Paul Pluzhnikov <ppluzhnikov@xxxxxxxxxx>")
modified by this one:
ce406537179a (" * infcmd.c (post_create_inferior): Ignore NOT_AVAILABLE_ERROR errors when reading the `stop_pc'. * printcmd.c (pc_prefix): Use get_frame_pc_if_available instead of get_frame_pc.")
modified by this one:
ce406537179a (" * infcmd.c (post_create_inferior): Ignore NOT_AVAILABLE_ERROR errors when reading the `stop_pc'. * printcmd.c (pc_prefix): Use get_frame_pc_if_available instead of get_frame_pc.")
Thanks.
Lianbo
/* Return a prefix for instruction address:
"=> " for current instruction, else " ". */
const char *
pc_prefix (CORE_ADDR addr)
{
if (has_stack_frames ())
{
struct frame_info *frame;
CORE_ADDR pc;
frame = get_selected_frame (NULL);
if (get_frame_pc_if_available (frame, &pc) && pc == addr)
return "=> ";
}
return " ";
}
Anyway, the patch looks good to me.
Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
Thanks,
Kazu
>
> Without the patch:
> crash> dis 0xffffffffa2317add -r | tail -5
> 0xffffffffa2317ad3 <native_queued_spin_lock_slowpath+35>: mov %edx,%eax
> 0xffffffffa2317ad5 <native_queued_spin_lock_slowpath+37>: lock cmpxchg %ecx,(%rdi)
> 0xffffffffa2317ad5 <native_queued_spin_lock_slowpath+37>: cmp %eax,%edx
> ^^^
> 0xffffffffa2317adb <native_queued_spin_lock_slowpath+43>: jne 0xffffffffa2317ac0 <native_queued_spin_lock_slowpath+16>
> 0xffffffffa2317add <native_queued_spin_lock_slowpath+45>: pop %rbp
>
> With the patch:
> crash> dis 0xffffffffa2317add -r | tail -5
> 0xffffffffa2317ad3 <native_queued_spin_lock_slowpath+35>: mov %edx,%eax
> 0xffffffffa2317ad5 <native_queued_spin_lock_slowpath+37>: lock cmpxchg %ecx,(%rdi)
> 0xffffffffa2317ad9 <native_queued_spin_lock_slowpath+41>: cmp %eax,%edx
> 0xffffffffa2317adb <native_queued_spin_lock_slowpath+43>: jne 0xffffffffa2317ac0 <native_queued_spin_lock_slowpath+16>
> 0xffffffffa2317add <native_queued_spin_lock_slowpath+45>: pop %rbp
>
> Reported-by: Vernon Lovejoy <vlovejoy@xxxxxxxxxx>
> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
> ---
> kernel.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel.c b/kernel.c
> index a42e6ad7d78c..6e98f5f6f6b1 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -2112,6 +2112,10 @@ cmd_dis(void)
> rewind(pc->tmpfile);
>
> while (fgets(buf2, BUFSIZE, pc->tmpfile)) {
> +
> + if (STRNEQ(buf2, "=>"))
> + shift_string_left(buf2, 2);
> +
> strip_beginning_whitespace(buf2);
>
> if (do_load_module_filter)
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
