Skip to content

Commit

Permalink
fix gdb_interface: restore gdb's output streams at end of gdb_interface
Browse files Browse the repository at this point in the history
Currently for most gdb_interface call with a non-null file pointer,
GDB's output stream is replaced with the passed file pointer

'info threads', which is a gdb passthrough, fails to print all thread
after support was added to get registers from crash_target:

    Id   Target Id         Frame
    1    CPU 0             <unavailable> in ?? ()
    2    CPU 1

Also, it was noticed that 'info threads' will print all threads when
'info threads' is run 2nd and subsequent times.

This incomplete output of 'info threads' was due to a subtle bug in
gdb_interface. For any passthrough, it switches the gdb output and
stderr streams to another FILE* passed in `struct gnu_request`.
But it does not restore the original FILE* stream gdb was using.

For each thread, which does not have it's registers yet, gdb goes to
crash_target::fetch_registers to ask for registers. Which in turn might
call 'datatype_info', which causes gdb's output stream to be set to
'/dev/null'

So all output after the call to datatype_info will go to /dev/null, and
hence the data to be printed is lost.

When 'info threads' is run a second time, it sets GDB's output streams
to something which is NOT /dev/null, and since it already has registers
for each thread, it does not go to 'crash_target::fetch_registers' and
hence the output stream is consistent, and we get complete output

So what happened is this:
1. GDB prints thread 1 (CPU 0) correctly since it had it's registers
   cached during initialisation in 'crash_target_init' (at that times
   registers where still unavailable, so it shows unavailable)
2. At CPU 1, to print the frame, it goes to 'crash_target::fetch_registers'
   to get the registers
3. crash_target then calls 'machdep->get_cpu_reg' which finally ends up
   at 'get_netdump_regs' which uses 'datatype_info' to get info of 'elf_prstatus.pr_reg'
4. Inside 'datatype_info', we set the file stream to /dev/null
   (pc->nullfp). Then it passes this request object to gdb_interface
5. gdb_interface replaces GDB's output stream to the passed stream, ie.
   /dev/null
6. Now, ALL FUTURE WRITES EVEN BY GDB, SUCH AS FOR THE 'info threads'
   COMMAND GOES TO THIS STREAM gnu_request->fp, WHICH is /dev/null,
   until someone else sets the non-null stream later
7. Similar behaviour occurs with all other threads
8. And hence we lose all subsequent output by GDB also

When 'info threads' is run a second time:
1. 'gdb_pass_through' passes a default FILE* to 'gdb_interface', which
   is not /dev/null
2. Since the registers are cached for all threads now, it does not go
   back to 'crash_target' for registers, and hence no call is made to
   'datatype_info', hence GDB's output stream is not changing
3. Since the gdb's stream, even though not what it was using originally,
   it's still valid, and not '/dev/null', hence we get the complete output

Fix this by restoring the original output streams, after gdb_interface
has handled the output

Cc: Sourabh Jain <[email protected]>
Cc: Hari Bathini <[email protected]>
Cc: Mahesh J Salgaonkar <[email protected]>
Cc: Naveen N. Rao <[email protected]>
Cc: Lianbo Jiang <[email protected]>
Cc: HAGIO KAZUHITO(萩尾 一仁) <[email protected]>
Cc: Tao Liu <[email protected]>
Signed-off-by: Aditya Gupta <[email protected]>
  • Loading branch information
adi-g15-ibm committed Mar 4, 2024
1 parent e058560 commit 78a8622
Showing 1 changed file with 53 additions and 0 deletions.
53 changes: 53 additions & 0 deletions gdb-10.2.patch
Original file line number Diff line number Diff line change
Expand Up @@ -16087,3 +16087,56 @@ exit 0

/* Print if the thread has not changed, otherwise an event will
be sent. */
--- 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);
}

/*

0 comments on commit 78a8622

Please sign in to comment.