gdb: fix for 'set suppress-cli-notifications on' missed case

I noticed this behaviour:

  (gdb) info threads
    Id   Target Id                               Frame
    1    Thread 0xf7dbc700 (LWP 3161872) "thr" 0xf7eb2888 in clone () from /lib/libc.so.6
  * 2    Thread 0xf7dbbb40 (LWP 3161884) "thr" breakpt () at thr.c:19
  (gdb) set suppress-cli-notifications on
  (gdb) thread 1
  (gdb) thread 1
  [Switching to thread 1 (Thread 0xf7dbc700 (LWP 3161872))]
  #0  0xf7eb2888 in clone () from /lib/libc.so.6
  (gdb)

I think that the second 'thread 1' should not produce any output just
like the 'inferior' command, continuing in the same GDB session:

  (gdb) inferior 1
  (gdb)

Without suppress-cli-notifications we would see an inferior, thread,
and frame being printed, but with suppress-cli-notifications set to
on, we get no output.

The difference in behaviours is that in inferior_command (inferior.c),
we always call notify_user_selected_context_changed, even in the case
where the inferior doesn't actually change.

In thread_command (thread.c), we have some code that catches the
thread not changed case, and calls print_selected_thread_frame.  The
notify_user_selected_context_changed function is only called if the
thread actually changes.

I did consider simply extending thread_command to check the global
cli_suppress_notification.user_selected_context state and skipping the
call to print_selected_thread_frame if suppression is on.

However, I realised that calling print_selected_thread_frame actually
introduces a bug.

When the 'thread' command is used to select the currently selected
thread, GDB still calls 'thread_selected'.  And 'thread_select' always
selects frame #0 within that thread, consider this session:

  (gdb) info threads
    Id   Target Id                              Frame
    1    Thread 0xf7dbc700 (LWP 723986) "thr" 0xf7eb2888 in clone () from /lib/libc.so.6
  * 2    Thread 0xf7dbbb40 (LWP 723990) "thr" breakpt () at thr.c:19
  (gdb) bt
  #0  breakpt () at thr.c:19
  #1  0x080491fd in thread_worker (arg=0xffff9514) at thr.c:31
  #2  0xf7f7667e in start_thread () from /lib/libpthread.so.0
  #3  0xf7eb289a in clone () from /lib/libc.so.6
  (gdb) frame 3
  #3  0xf7eb289a in clone () from /lib/libc.so.6
  (gdb) thread 2
  [Switching to thread 2 (Thread 0xf7dbbb40 (LWP 723990))]
  #0  breakpt () at thr.c:19
  19	  while (stop)
  (gdb) frame
  #0  breakpt () at thr.c:19
  19	  while (stop)
  (gdb)

Notice that the frame resets back to frame #0.

By only calling print_selected_thread_frame, and not calling
notify_user_selected_context_changed, this means that GDB will fail to
emit an MI async notification.  It is this async notification which
tells MI consumers that the frame has been reset to #0.

And so, I think that the correct solution is, like with the 'inferior'
command, to always call notify_user_selected_context_changed.

This does mean that in some cases unnecessary MI notifications can be
emitted, however, an MI consumer should be able to handle these.  We
could try to avoid these, but we would need to extend thread_command
to check that neither the thread OR frame has changed after the call
to thread_select, and right now, I'm not sure it's worth adding the
extra complexity.

I've rewritten the gdb.base/cli-suppress-notification.exp test to
cover more cases, especially the reselecting the same thread case.
And I've updated the gdb.mi/user-selected-context-sync.exp test to
allow for the additional MI notifications that are emitted, and to
check the frame reset case.

While working on this change, I did wonder about calls to
notify_user_selected_context_changed for frame related commands.  In
places we do elide calls to notify_user_selected_context_changed if
the frame hasn't changed.  I wondered if there were more bugs here?

I don't think there are though.  While changing the inferior will also
change the selected thread, and the selected frame.  And changing the
thread will also change the selected frame.  Changing the frame is the
"inner most" context related thing that can be changed.  There are no
side effect changes that also need to be notified, so for these cases,
I think we are fine.

Also in infrun.c I fixed a code style issue relating to
notify_user_selected_context_changed.  It's not a functional change
required by this commit, but it's related to this patch, so I'm
including it here.

Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Tested-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Approved-By: Tom Tromey <tom@tromey.com>

0 files changed, 0 insertions, 0 deletions