watchpoint regression debugging with remote protocol (bare metal)

We have noticed a regression in our watchpoint support when debugging
through the remote protocol a program running on a bare metal platform,
when the program uses what we call the Ravenscar Runtime.

This runtime is a subset of the Ada runtime defined by the Ravenscar
Profile.  One of the nice things about this runtime is that it provides
tasking, which is equivalent to the concept of threads in C (it is
actually often mapped to threads, when available). For bare metal
targets, however, there is no OS, and therefore no thread layer.
What we did, then, was add a ravenscar-thread layer, which has insider
knowledge of the runtime to get the list of threads, but also all
necessary info to perform thread switching.

For the record, the commit which caused the regression is:

    commit 799a2abe613be0645b84f5aaa050f2f91e6ae3f7
    Date:   Mon Nov 30 16:05:16 2015 +0000
    Subject: remote: stop reason and watchpoint data address per thread

    Running local-watch-wrong-thread.exp with "maint set target-non-stop
    on" exposes that gdb/remote.c only records whether the target stopped
    for a breakpoint/watchpoint plus the watchpoint data address *for the
    last reported remote event*.  But in non-stop mode, we need to keep
    that info per-thread, as each thread can end up with its own
    last-status pending.

Our testcase is very simple. We have a package defining a global
variable named "Watch"...

    package Pck is
       Watch : Integer := 1974;
    end Pck;

... and a main subprogram which changes its value

    procedure Foo is
    begin
       Pck.Watch := Pck.Watch + 1;
    end Foo;

To reproduce, we built our program as usual, started it in QEMU,
and then connected GDB to QEMU...

    (gdb) target remote :4444
    (gdb) break _ada_foo
    (gdb) cont  <--- this is to make sure the program is started
                     and the variable we want to watch is initialized

... at which point we try to use a watchpoint on our global variable:

    (gdb) watch watch

... but, upon resuming the execution with a "cont", we expected to
get a watchpoint-hit notification, such as...

    (gdb) cont
    Hardware watchpoint 2: watch

    Old value = 1974
    New value = 1975
    0xfff00258 in foo () at /[...]/foo.adb:6
    6       end Foo;

... but unfortunately, we get a SIGTRAP instead:

    (gdb) cont
    Program received signal SIGTRAP, Trace/breakpoint trap.
    foo () at /[...]/foo.adb:6
        6   end Foo;

What happens is that, on the one hand, the change in remote.c
now stores the watchpoint-hit notification info in the thread
that received it; and on the other hand, we have a ravenscar-thread
layer which manages the thread list on top of the remote protocol
layer. The two of them get disconnected, and this eventually results
in GDB not realizing that we hit a watchpoint.  Below is how:

First, once connected and just before inserting our watchpoint,
we have the ravenscar-thread layer which built the list of threads
by extracting some info from inferior memory, giving us the following
two threads:

      (gdb) info threads
      Id   Target Id         Frame
      1    Thread 0 "0Q@" (Ravenscar task) foo () at /[...]/foo.adb:5
    * 2    Thread 0x24618 (Ravenscar task) foo () at /[...]/foo.adb:5

The first thread is the only thread QEMU told GDB about. The second
one is a thread that the ravenscar-thread added. QEMU has now way
to know about those threads, since they are really embedded inside
the program; that's why we have the ravenscar layer, which uses
inside-knowledge to extract the list of threads.

Next, we insert a watchpoint, which applies to all threads. No problem
so far.

Then, we continue; meaning that GDB sends a Z2 packet to QEMU to
get the watchpoint inserted, then a vCont to resume the program's
execution. The program hits the watchpoints, and thererfore QEMU
reports it back:

        Packet received: T05thread:01;watch:000022c4;

Since QEMU knows about one thread and one thread only, it stands
to reason that it would say that the event applies to thread:01,
which is our first thread in the "info threads" listing. That
thread has a ptid of {42000, lwp=1, tid=0}.

This is where Pedro's change kicks in: Seeing this event, and
having determined that the event was reported for thread 01,
and therefore ptid {42000, lwp=1, tid=0}, it saves the watchpoint-hit
event info in the private area of that thread/ptid. Once this is
done, remote.c's event-wait layer returns.

Enter the ravenscar-thread layer of the event-wait, which does
a little dance to delegate the wait to underlying layers with
ptids that those layers know about, and then when the target_beneath's
to_wait is done, tries to figure out which thread is now the active
thread. The code looks like this:

  1.    inferior_ptid = base_ptid;
  2.    beneath->to_wait (beneath, base_ptid, status, 0);
  3.    [...]
  4.        ravenscar_update_inferior_ptid ();
  5.
  6.    return inferior_ptid;

Line 1 is where we reset inferior_ptid to the ptid that
the target_beneath layer knows about, allowing us to then
call its to_wait implementation (line 2). And then, upon
return, we call ravenscar_update_inferior_ptid, which reads
inferior memory to determine which thread is actually active,
setting inferior_ptid accordingly. Then we return that
inferior_ptid (which, again, neither QEMU and therefore nor
the remote.c layer knows about).

Upon return, we eventually arrive to the part where we try
to handle the inferior event: we discover that we got a SIGTRAP
and, as part of its handling, we call watchpoints_triggered,
which calls target_stopped_by_watchpoint, which eventually
remote_stopped_by_watchpoint, where Pedro's change kicks in
again:

    struct thread_info *thread = inferior_thread ();
    return (thread->priv != NULL
            && thread->priv->stop_reason == TARGET_STOPPED_BY_WATCHPOINT);

Because the ravenscar-thread layer changed the inferior_ptid
to the ptid of the active thread, inferior_thread now returns
the private data of that thread. This is not the thread that
QEMU reported the watchpoint-hit on, and thus, the function
returns "no watchpoint hit, mister". Hence GDB not understanding
the SIGTRAP, thus reporting it verbatim.

The way we chose to fix the issue is by making sure that the
ravenscar-thread layer doesn't let the remote layer be called
with inferior_ptid being set to a thread that the remote layer
does not know about.

gdb/ChangeLog:

        * ravenscar-thread.c (ravenscar_stopped_by_sw_breakpoint)
        (ravenscar_stopped_by_hw_breakpoint, ravenscar_stopped_by_watchpoint)
        (ravenscar_stopped_data_address, ravenscar_core_of_thread):
        New functions.
        (init_ravenscar_thread_ops): Set the to_stopped_by_sw_breakpoint,
        to_stopped_by_hw_breakpoint, to_stopped_by_watchpoint,
        to_stopped_data_address and to_core_of_thread fields of
        ravenscar_ops.

0 files changed, 0 insertions, 0 deletions