[Crash-utility] Re: [PATCH v3 0/5] Improve stack unwind on ppc64

Hi Aditya, On Tue, Dec 5, 2023 at 12:24 PM Aditya Gupta <adityag(a)linux.ibm.com&gt; wrote: > On Tue, Dec 05, 2023 at 10:25:58AM +0800, Tao Liu wrote: >> Hi Aditya, >> >> On Mon, Dec 4, 2023 at 11:47 PM Aditya Gupta <adityag(a)linux.ibm.com&gt; 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. >>> 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 >>> >>> This way other features of 'gdb', such as seeing >>> backtraces/registers/variables/arguments/local variables, moving up and >>> down stack frames, can be used with any ppc64 vmcore, irrespective of >>> being ELF format or kdump-compressed format. >>> >>> < ...snip... > >>> >>> Fixing GDB passthroughs on other architectures >>> ============================================== >>> >>> Much of the work for making gdb passthroughs like 'gdb bt', 'gdb >>> thread', 'gdb info locals' etc. has been done by the patches introducing >>> 'machdep->get_cpu_reg' and this series fixing some issues in that. >>> >>> Other architectures should be able to fix these gdb functionalities by >>> simply implementing 'machdep->get_cpu_reg (cpu, regno, ...)'. >>> >>> The reasoning behind that has been explained with a diagram in commit >>> description of patch #1 >>> >>> I will assist with my findings/observations fixing it on ppc64 whenever needed. >>> >>> < ...snip... > >> Thanks a lot for the PPC64 solution. However do you have any plans for >> supporting stack unwinding on other ARCHs, such as x86_64/86/arm64... >> etc? >> >> I think it is an important feature for crash to support stack frame >> up/down and local variable inspecting. So I'd like to take >> x86_64/64/arm etc works based on your v3 work if they are not on your >> to-do list. Do you have any suggestions? > Thanks for showing the interest, please feel free to implement it for x86_64/arm > also.

> > Some notes: > 1. First thing to try can be to implement 'machdep->get_cpu_reg' for the > architectures to give register values for asked cpu. > 2. Need to check the register numbers for each register, eg. PC (Program > Counter) register is regno=64 in ppc64. If no documentation is there for this > mapping, then can consider just printing register numbers and names in > the loop in 'crash_target::fetch_registers'. > 3. I will recommend applying atleast patch #2, #3, #4, #5. They are architecture > independent and will make the job easier, they take care of synchronising the > crash and gdb contexts (which is required so that gdb can get correct > registers for it's current thread), and also fixes a problematic bug in > gdb_interface which took my days to debug. > Only patch #1 is ppc specific. > 4. Debugging gdb with gdb can be an headache, so I will recommend adding these > to your ~/.gdbinit to ignore some continuous interrupts: > > set history save on > set print pretty on > set pagination off > set confirm off > > # ignore this signal while debugging gdb via gdb > handle SIGTTOU nostop > handle SIGTTOU ignore > handle SIGTTOU noprint > > 5. Putting a breakpoint at 'get_current_frame' can be helpful :) > Thanks a lot for the detailed info! I will keep your suggestions in mind. Thanks, Tao Liu > Thanks, > Aditya Gupta > >> Thanks, >> Tao Liu >> >>> Changelog: >>> ========== >>> >>> V3: >>> + default gdb thread will be the crashing thread, instead of being >>> thread '0' >>> + synchronise crash cpu and gdb thread context >>> + fix bug in gdb_interface, that replaced gdb's output stream, losing >>> output in some cases, such as info threads and extra output in info >>> variables >>> + fix 'info threads' >>> >>> RFC V2: >>> - removed patch implementing 'frame', 'up', 'down' in crash >>> - updated the cover letter by removing the mention of those commands other >>> than the respective gdb passthrough >>> >>> Aditya Gupta (5): >>> ppc64: correct gdb passthroughs by implementing machdep->get_cpu_reg >>> remove 'frame' from prohibited commands list >>> synchronise cpu context changes between crash/gdb >>> fix gdb_interface: restore gdb's output streams at end of >>> gdb_interface >>> fix 'info threads' command >>> >>> crash_target.c | 44 ++++++++++++++++ >>> defs.h | 130 +++++++++++++++++++++++++++++++++++++++++++++++- >>> gdb-10.2.patch | 110 +++++++++++++++++++++++++++++++++++++++- >>> gdb_interface.c | 2 +- >>> kernel.c | 47 +++++++++++++++-- >>> ppc64.c | 95 +++++++++++++++++++++++++++++++++-- >>> task.c | 14 ++++++ >>> tools.c | 2 +- >>> 8 files changed, 434 insertions(+), 10 deletions(-) >>> >>> -- >>> 2.41.0 >>> -- >>> Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io >>> To unsubscribe send an email to devel-leave(a)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