[Crash-utility] Re: [Crash-Utility][PATCH v2 06/13] Fix gdb_interface: restore gdb's output streams at end of gdb_interface

Hi Tao, Thanks for sending the series. Maybe you missed CCing people ?

Also this patch's 'author' line is missing, maybe while editing the commit it messed up ?

I will proceed with testing the series, might do it later, currently my mails got messed up, I got some mails in Inbox (those patches with me in Cc), and others in my crash-utility folder (those with Cc: crash-utility).

Thanks, Aditya Gupta On 28/04/24 09:32, Tao Liu wrote: > Currently for most gdb_interface call, in which a non-null file pointer > is passed, GDB's output stream is replaced with the passed file pointer > > Due to this, 'info threads', which is a gdb passthrough, doesn't print any > thread, after support was added to get registers from crash_target: > > crash> info threads > Id Target Id Frame > > This empty output of 'info threads' was due to a subtle bug in > gdb_interface. > > After this gdb passthrough is run, 'datatype_info' is called, with file > pointer set to null_fp (pointing to /dev/null). And after > 'datatype_info' returns, any output by gdb goes to /dev/null, and hence > the output is lost. > > Fix this by restoring the original output streams, after gdb_interface > has handled the output > > After this patch: > > crash> info threads > Id Target Id Frame > * 1 2131 bash 0xc000000000051e40 in crash_fadump (regs=0x0, str=0xc000000002c60510 <buf> "sysrq triggered crash") at arch/powerpc/kernel/fadump.c:735 > > 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; > Signed-off-by: Aditya Gupta <adityag(a)linux.ibm.com&gt; > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > gdb-10.2.patch | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/gdb-10.2.patch b/gdb-10.2.patch > index 3694b13..0bed96a 100644 > --- a/gdb-10.2.patch > +++ b/gdb-10.2.patch > @@ -16118,3 +16118,56 @@ exit 0 > subclass (SYMBOL_NONE) > { > /* We can't use an initializer list for members of a base class, and > +--- gdb-10.2/gdb/ui-file.h.orig > ++++ gdb-10.2/gdb/ui-file.h > +@@ -195,6 +195,7 @@ public: > + > + bool can_emit_style_escape () override; > + > ++ FILE *get_stream(void); > + /* Sets the internal stream to FILE, and saves the FILE's file > + descriptor in M_FD. */ > + void set_stream (FILE *file); > +--- gdb-10.2/gdb/ui-file.c.orig > ++++ gdb-10.2/gdb/ui-file.c > +@@ -161,6 +161,12 @@ stdio_file::~stdio_file () > + fclose (m_file); > + } > + > ++FILE* > ++stdio_file::get_stream(void) > ++{ > ++ return m_file; > ++} > ++ > + void > + stdio_file::set_stream (FILE *file) > + { > +--- gdb-10.2/gdb/symtab.c.orig > ++++ gdb-10.2/gdb/symtab.c > +@@ -6964,8 +6964,12 @@ void > + gdb_command_funnel_1(struct gnu_request *req) > + { > + struct symbol *sym; > ++ FILE *original_stdout_stream = nullptr; > ++ FILE *original_stderr_stream = nullptr; > + > + if (req->command != GNU_VERSION && req->command != GNU_USER_PRINT_OPTION) { > ++ original_stdout_stream = (dynamic_cast< stdio_file * >gdb_stdout)->get_stream(); > ++ original_stderr_stream = (dynamic_cast< stdio_file * >gdb_stderr)->get_stream(); > + (dynamic_cast<stdio_file *>gdb_stdout)->set_stream(req->fp); > + (dynamic_cast<stdio_file *>gdb_stderr)->set_stream(req->fp); > + } > +@@ -7068,6 +7072,12 @@ gdb_command_funnel_1(struct gnu_request *req) > + req->flags |= GNU_COMMAND_FAILED; > + break; > + } > ++ > ++ /* Restore the streams gdb output was using */ > ++ if (original_stdout_stream) > ++ (dynamic_cast<stdio_file *>gdb_stdout)->set_stream(original_stdout_stream); > ++ if (original_stderr_stream) > ++ (dynamic_cast<stdio_file *>gdb_stderr)->set_stream(original_stderr_stream); > + } > + > + /*