Hi Aditya,
thank you for working on this.
On 2023/08/01 3:56, Aditya Gupta wrote:
> The Problem:
> ============
>
> Currently crash is unable to show function arguments and local variables, as
> gdb can do. And functionality for moving between frames ('up'/'down') is not
> working in crash.
>
> Crash has 'gdb passthroughs' for things gdb can do, but the gdb passthroughs
> 'bt', 'frame', 'info locals', 'up', 'down' are not working either, due to
> gdb not getting the register values from `crash_target::fetch_registers`,
> which then uses `machdep->get_cpu_reg`, which is not implemented for PPC64
>
> Proposed Solution:
> ==================
>
> Fix the gdb passthroughs by implementing "machdep->get_cpu_reg" for PPC64.
Agree, enabling "gdb bt" and etc is fine to me.
I've also experimented it a bit on x86_64 before:
https://github.com/k-hagio/crash/commits/get_cpu_regs.v1
> This way, "gdb mode in crash" will support this feature for both ELF and
> kdump-compressed vmcore formats, while "gdb" would only have supported ELF
> format
>
> Also, backtrace can be slightly different in gdb and crash (due to gdb
> being able to print inline frames also). so it can cause confusion of 'bt'
> working in crash context, but 'frame'/'up'/'down' working as
> 'gdb passthrough', showing different frames.
> This has been explained in patch #4.
>
> So to prevent confusion mentioned above, implement 'frame', 'up', 'down'
> as commands in default crash mode also, instead of working via gdb
> passthroughs which they do currently.
hmm sorry, I'm not interested in this. I don't think it's so confusing
that we have to prevent with the additional commands and efforts. Crash
users using gdb commands should know what they are doing. Also, the
output of the new "frame" command sounds like almost a portion of the
crash "bt" command, there is no new information.
So I would suggest simply enabling the gdb bt and etc with implementing
machdep->get_cpu_reg().
Thanks,
Kazu
PS. I will be out next week and will be back on 15th August.
>
> So, now in default mode, 'bt','frame','up','down' will be consistent with each
> other.
>
> Implications on Architectures:
> ====================================
>
> No architecture other than PPC64 has been affected, other than in case of
> 'frame', 'up', 'down' commands
>
> 1. frame: As mentioned in patch #2, that frame will not be prohibited, and
> will print:
>
> crash> frame
> #0 <unavailable> in ?? ()
>
> Instead of before prohibited message:
>
> crash> frame
> crash: prohibited gdb command: frame
>
> 2. up/down: These commands will now be run as native crash commands by
> default instead of showing
>
> crash> up
> crash: ambiguous command: up (symbol and gdb command)
> crash> down
> crash: ambiguous command: down (symbol and gdb command)
>
> On PPC64, the default mode ("crash mode") will not have ANY OTHER changes,
> other than the 'frame', 'up', 'down' as mentioned above.
>
> Major change will be in 'gdb mode' on PPC64, that it will print the frames, and
> local variables, instead of failing with errors showing no frame, or showing
> that couldn't get PC
>
> Testing:
> ========
>
> Git tree with this patch series applied: https://github.ibm.com/adityag/crash
> (replace this link with github.com later)
>
> To test 'frame'/'up'/'down' in crash (implemented in patch #3):
>
> crash> bt
> crash> frame
> crash> up
> crash> down
> crash> up 4
>
> To test 'bt'/'frame'/'up'/'down'/'info locals' gdb passthroughs:
>
> crash> set gdb on
> gdb> thread 3 # or any other thread number to change context in gdb
> gdb> bt
> gdb> frame
> gdb> up
> gdb> down
> gdb> info locals
>
> Known Issues:
> =============
>
> 1. In gdb mode, 'info threads' might hang for few seconds, and print only 2
> threads
> 2. In gdb mode, 'bt' might fail to show backtrace in few vmcores collected
> from older kernels. This is a known issue due to register mismatch, and
> its fix has been accepted upstream:
> Commit: https://github.com/linuxppc/linux/commit/b684c09f09e7a6af3794d4233ef785819e72db79
>
> TODO:
> =====
>
> 1. Introduce automatic thread selection in gdb mode, to select the crashing
> thread in gdb, eliminating the need to manually run "thread <id>" after
> switching to gdb mode.
>
>
> Aditya Gupta (5):
> add generic get_dumpfile_regs to read registers
> ppc64: fix gdb passthrough by implementing machdep->get_cpu_reg
> remove 'frame' from prohibited commands list
> implement 'frame', 'up', 'down' inside crash
> make cpu context change transparent to crash/gdb
>
> defs.h | 135 ++++++++++++++++++++++++++
> gdb-10.2.patch | 28 ++++++
> gdb_interface.c | 2 +-
> global_data.c | 3 +
> help.c | 34 +++++++
> kernel.c | 183 +++++++++++++++++++++++++++++++++++
> ppc64.c | 250 +++++++++++++++++++++++++++++++++++++++++++++++-
> task.c | 1 +
> tools.c | 12 ++-
> 9 files changed, 641 insertions(+), 7 deletions(-)
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
