[Crash-utility] Re: [PATCH v4 07/16] Stop stack unwinding at non-kernel address

Hello Lianbo, On 24/06/24 05:32PM, lijiang wrote: True. Though it seems to be okay for it to print the last frame with a non-kernel address, as in this snippet from gdb: for (fi = trailing; fi && count--; fi = get_prev_frame (fi)) ... print_frame_info (fp_opts, fi, 1, LOCATION, 1, 0); Seems that frame #13, fi was not NULL. Seeing Tao's change, it compares the current frame's NIP/PC to see if it should return NULL (which I think is nice and works). Here the 'this_frame' would have been frame 12, (which would have called `'get_prev_frame' to get the frame 13) ``` frame_pc_p = get_frame_pc_if_available (this_frame, &frame_pc); #ifdef CRASH_MERGE if (!is_kvaddr(frame_pc)) { return NULL; } #endif ``` Tao's condition will hit when 'get_prev_frame(this_frame=frame#13)' will be called to get the frame #14, which will return NULL and hence break out of the loop. This is based on what I recall and a quick look at the implementation, please feel free to correct Lianbo/Tao. Thanks, Aditya Gupta > > > Thanks > Lianbo > > > > Cc: Sourabh Jain <sourabhjain(a)linux.ibm.com&gt; > > Cc: Hari Bathini <hbathini(a)linux.ibm.com&gt; > > Cc: Mahesh J Salgaonkar <mahesh(a)linux.ibm.com&gt; > > Cc: Naveen N. Rao <naveen.n.rao(a)linux.vnet.ibm.com&gt; > > Cc: Lianbo Jiang <lijiang(a)redhat.com&gt; > > Cc: HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; > > Cc: Tao Liu <ltao(a)redhat.com&gt; > > Cc: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; > > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > > --- > > defs.h | 1 + > > gdb-10.2.patch | 26 ++++++++++++++++++++++++++ > > gdb_interface.c | 6 ++++++ > > 3 files changed, 33 insertions(+) > > > > diff --git a/defs.h b/defs.h > > index 012ffdc..c0e6a29 100644 > > --- a/defs.h > > +++ b/defs.h > > @@ -7902,6 +7902,7 @@ extern unsigned char *gdb_prettyprint_arrays; > > extern unsigned int *gdb_repeat_count_threshold; > > extern unsigned char *gdb_stop_print_at_null; > > extern unsigned int *gdb_output_radix; > > +int is_kvaddr(ulong); > > > > /* > > * gdb/top.c > > diff --git a/gdb-10.2.patch b/gdb-10.2.patch > > index 0bed96a..3ed40c0 100644 > > --- a/gdb-10.2.patch > > +++ b/gdb-10.2.patch > > @@ -16171,3 +16171,29 @@ exit 0 > > } > > > > /* > > +--- gdb-10.2/gdb/frame.c.orig > > ++++ gdb-10.2/gdb/frame.c > > +@@ -2331,6 +2331,10 @@ inside_entry_func (frame_info *this_frame) > > + This function should not contain target-dependent tests, such as > > + checking whether the program-counter is zero. */ > > + > > ++#ifdef CRASH_MERGE > > ++extern "C" int is_kvaddr(ulong); > > ++#endif > > ++ > > + struct frame_info * > > + get_prev_frame (struct frame_info *this_frame) > > + { > > +@@ -2353,7 +2357,11 @@ get_prev_frame (struct frame_info *this_frame) > > + get_frame_id (this_frame); > > + > > + frame_pc_p = get_frame_pc_if_available (this_frame, &frame_pc); > > +- > > ++#ifdef CRASH_MERGE > > ++ if (!is_kvaddr(frame_pc)) { > > ++ return NULL; > > ++ } > > ++#endif > > + /* tausq/2004-12-07: Dummy frames are skipped because it doesn't make > > much > > + sense to stop unwinding at a dummy frame. One place where a dummy > > + frame may have an address "inside_main_func" is on HPUX. On HPUX, > > the > > diff --git a/gdb_interface.c b/gdb_interface.c > > index b13d5fd..e76ecc6 100644 > > --- a/gdb_interface.c > > +++ b/gdb_interface.c > > @@ -947,6 +947,12 @@ gdb_lookup_module_symbol(ulong addr, ulong *offset) > > } > > } > > > > +int > > +is_kvaddr(ulong addr) > > +{ > > + return IS_KVADDR(addr); > > +} > > + > > /* > > * Used by gdb_interface() to catch gdb-related errors, if desired. > > */ > > -- > > 2.40.1 > >