[Ada/commit] various ravenscar-thread enhancements

[Ada/commit] various ravenscar-thread enhancements

[Joel Brobecker]

Hello,

I am about to push the following patches, which contribute a number
of enhancements we have made in in the AdaCore version of this module
during the past couple of years.

   [Ada 1/6] watchpoint regression debugging with remote protocol
   [Ada 2/6] Provide the "Base CPU" in output of "info task" (if set
   [Ada 3/6] Add multiple-CPU support in ravenscar-thread.c
   [Ada 4/6] problem debugging ravenscar programs if runtime is
   [Ada 5/6] (Ada) crash connecting to TSIM simulator
   [Ada 6/6] ravenscar: update inferior ptid with event ptid

I re-tested those patches on ppc-elf, with AdaCore's gdb testsuite.

Thank you,
-- 
Joel

[Ada 2/6] Provide the "Base CPU" in output of "info task" (if set by runtime).

[Joel Brobecker]

At the user level, this patch enhances the debugger to print the ID
of the base CPU a task is running on:

        (gdb) info task 3
        Ada Task: 0x13268
        Name: raven1
        Thread: 0x13280
        LWP: 0
 !!!->  Base CPU: 1
        No parent
        Base Priority: 127
        State: Runnable

This new field is only printed when the base CPU is nonzero or, in
other words, if the base CPU info is being provided by the runtime.
For instance, on native systems, where threads/processes can "jump"
from CPU to CPU, the info is not available, and the output of the
command above then remains unchanged.

At the internal level, the real purpose of this change is to prepare
the way for ravenscar-thread to start handling SMP systems. For that,
we'll need to know which CPU each task is running on...  More info
on that in the commit that actually adds support for it.

gdb/ChangeLog:

        * ada-lang.h (struct ada_task_info) <base_cpu>: New field.
        * ada-lang.c (struct atcb_fieldno) <base_cpu>: New field.
        (get_tcb_types_info): Set fieldnos.base_cpu.
        (read_atcb): Set task_info->base_cpu.
        (info_task): Print "Base CPU" info if set by runtime.
---
 gdb/ada-lang.h  |  6 ++++++
 gdb/ada-tasks.c | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index d15fbbdd42..c11fb48c1b 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -141,6 +141,12 @@ struct ada_task_info
   /* If the task is accepting a rendezvous with another task, this field
      contains the ID of the calling task.  Zero otherwise.  */
   CORE_ADDR caller_task;
+
+  /* The CPU on which the task is running.  This is dependent on
+     the runtime actually providing that info, which is not always
+     the case.  Normally, we should be able to count on it on
+     bare-metal targets.  */
+  int base_cpu;
 };
 
 /* Assuming V points to an array of S objects,  make sure that it contains at
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 1d5542d5d6..e29b563d6c 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -123,6 +123,7 @@ struct atcb_fieldnos
   int activation_link;
   int call;
   int ll;
+  int base_cpu;
 
   /* Fields in Task_Primitives.Private_Data.  */
   int ll_thread;
@@ -535,6 +536,7 @@ get_tcb_types_info (void)
                                                   "activation_link", 1);
   fieldnos.call = ada_get_field_index (common_type, "call", 1);
   fieldnos.ll = ada_get_field_index (common_type, "ll", 0);
+  fieldnos.base_cpu = ada_get_field_index (common_type, "base_cpu", 0);
   fieldnos.ll_thread = ada_get_field_index (ll_type, "thread", 0);
   fieldnos.ll_lwp = ada_get_field_index (ll_type, "lwp", 1);
   fieldnos.call_self = ada_get_field_index (call_type, "self", 0);
@@ -748,6 +750,10 @@ read_atcb (CORE_ADDR task_id, struct ada_task_info *task_info)
         }
     }
 
+  task_info->base_cpu
+    = value_as_long (value_field (common_value,
+				  pspace_data->atcb_fieldno.base_cpu));
+
   /* And finally, compute the task ptid.  Note that there are situations
      where this cannot be determined:
        - The task is no longer alive - the ptid is irrelevant;
@@ -1179,6 +1185,10 @@ info_task (struct ui_out *uiout, const char *taskno_str, struct inferior *inf)
   printf_filtered (_("Thread: %#lx\n"), ptid_get_tid (task_info->ptid));
   printf_filtered (_("LWP: %#lx\n"), ptid_get_lwp (task_info->ptid));
 
+  /* If set, print the base CPU.  */
+  if (task_info->base_cpu != 0)
+    printf_filtered (_("Base CPU: %d\n"), task_info->base_cpu);
+
   /* Print who is the parent (if any).  */
   if (task_info->parent != 0)
     parent_taskno = get_task_number_from_id (task_info->parent, inf);
-- 
2.11.0

[Ada 4/6] problem debugging ravenscar programs if runtime is stripped

[Joel Brobecker]

Trying to debug a program using a stripped version of the ravenscar
runtime, we can get the following error:

    (gdb) cont
    Continuing.
    Cannot find Ada_Task_Control_Block type. Aborting

This is because the ravenscar-thread layer makes the assumption that
the runtime is built the way we expect it, meaning that the Ada tasking
units we rely on for Ada tasking debugging, are built with debugging
information, and that this debug information has not been stripped from
the runtime.

When this assumption is not true, resuming such a program can trigger
the error above, which then leads GDB a little confused. For instance,
we can see things like:

     (gdb) bt
     Target is executing.

This patch fixes the issue by disabling the ravenscar thread layer
if we detect that the runtime is missing some of the debugging info
we need in order to support Ada task debugging. This is the best
we can do, as the ravenscar-thread layer actually depends on the
ada-tasks layer to implement thread debugging.

gdb/ChangeLog:

        * ada-lang.h (ada_get_tcb_types_info): Add declaration.
        * ada-tasks.c (ada_get_tcb_types_info): Renames get_tcb_types_info.
        Make non-static.  Change return type to char *.  Adjust code
        accordingly.  Rewrite the function's documentation.
        (read_atcb): Adjust call to get_tcb_types_info accordingly.
        * ravenscar-thread.c (ravenscar_inferior_created): Check that
        we have enough debugging information in the runtime to support
        Ada task debugging before we enable the ravenscar-thread layer.
---
 gdb/ada-lang.h         |  2 ++
 gdb/ada-tasks.c        | 39 ++++++++++++++++++++++-----------------
 gdb/ravenscar-thread.c |  8 ++++++++
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 36a5820ee3..0530e9aacd 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -409,6 +409,8 @@ typedef void (ada_task_list_iterator_ftype) (struct ada_task_info *task);
 extern void iterate_over_live_ada_tasks
   (ada_task_list_iterator_ftype *iterator);
 
+extern const char *ada_get_tcb_types_info (void);
+
 extern int ada_build_task_list (void);
 
 extern void print_ada_task_info (struct ui_out *uiout,
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 16cee35d86..eac6c5be91 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -466,18 +466,17 @@ read_fat_string_value (char *dest, struct value *val, int max_len)
   dest[len] = '\0';
 }
 
-/* Get from the debugging information the type description of all types
-   related to the Ada Task Control Block that will be needed in order to
-   read the list of known tasks in the Ada runtime.  Also return the
-   associated ATCB_FIELDNOS.
-
-   Error handling:  Any data missing from the debugging info will cause
-   an error to be raised, and none of the return values to be set.
-   Users of this function can depend on the fact that all or none of the
-   return values will be set.  */
-
-static void
-get_tcb_types_info (void)
+/* Get, from the debugging information, the type description of all types
+   related to the Ada Task Control Block that are needed in order to
+   read the list of known tasks in the Ada runtime.  If all of the info
+   needed to do so is found, then save that info in the module's per-
+   program-space data, and return NULL.  Otherwise, if any information
+   cannot be found, leave the per-program-space data untouched, and
+   return an error message explaining what was missing (that error
+   message does NOT need to be deallocated).  */
+
+const char *
+ada_get_tcb_types_info (void)
 {
   struct type *type;
   struct type *common_type;
@@ -518,7 +517,7 @@ get_tcb_types_info (void)
 					    NULL).symbol;
 
       if (atcb_sym == NULL || atcb_sym->type == NULL)
-        error (_("Cannot find Ada_Task_Control_Block type. Aborting"));
+        return _("Cannot find Ada_Task_Control_Block type");
 
       type = atcb_sym->type;
     }
@@ -531,11 +530,11 @@ get_tcb_types_info (void)
     }
 
   if (common_atcb_sym == NULL || common_atcb_sym->type == NULL)
-    error (_("Cannot find Common_ATCB type. Aborting"));
+    return _("Cannot find Common_ATCB type");
   if (private_data_sym == NULL || private_data_sym->type == NULL)
-    error (_("Cannot find Private_Data type. Aborting"));
+    return _("Cannot find Private_Data type");
   if (entry_call_record_sym == NULL || entry_call_record_sym->type == NULL)
-    error (_("Cannot find Entry_Call_Record type. Aborting"));
+    return _("Cannot find Entry_Call_Record type");
 
   /* Get the type for Ada_Task_Control_Block.Common.  */
   common_type = common_atcb_sym->type;
@@ -583,6 +582,7 @@ get_tcb_types_info (void)
   pspace_data->atcb_ll_type = ll_type;
   pspace_data->atcb_call_type = call_type;
   pspace_data->atcb_fieldno = fieldnos;
+  return NULL;
 }
 
 /* Build the PTID of the task from its COMMON_VALUE, which is the "Common"
@@ -630,7 +630,12 @@ read_atcb (CORE_ADDR task_id, struct ada_task_info *task_info)
     = get_ada_tasks_pspace_data (current_program_space);
 
   if (!pspace_data->initialized_p)
-    get_tcb_types_info ();
+    {
+      const char *err_msg = ada_get_tcb_types_info ();
+
+      if (err_msg != NULL)
+	error (_("%s. Aborting"), err_msg);
+    }
 
   tcb_value = value_from_contents_and_address (pspace_data->atcb_type,
 					       NULL, task_id);
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 83dfd9d4ad..e7922277ce 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -519,12 +519,20 @@ ravenscar_core_of_thread (struct target_ops *ops, ptid_t ptid)
 static void
 ravenscar_inferior_created (struct target_ops *target, int from_tty)
 {
+  const char *err_msg;
 
   if (!ravenscar_task_support
       || gdbarch_ravenscar_ops (target_gdbarch ()) == NULL
       || !has_ravenscar_runtime ())
     return;
 
+  err_msg = ada_get_tcb_types_info ();
+  if (err_msg != NULL)
+    {
+      warning (_("%s. Task/thread support disabled.\n"), err_msg);
+      return;
+    }
+
   ravenscar_update_inferior_ptid ();
   push_target (&ravenscar_ops);
 }
-- 
2.11.0

[Ada 5/6] (Ada) crash connecting to TSIM simulator

[Joel Brobecker]

Connecting to a TSIM simulator over the remote protocol causes GDB
to crash with the following failed assertion:

    (gdb) tar remote :1234
    Remote debugging using :1234
    /[...]/gdb/ravenscar-thread.c:182: internal-error: ravenscar_update_inferior_ptid: Assertion `!is_ravenscar_task (inferior_ptid)' failed.
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n) y

What happens is the following. Upon connection to the target, GDB
sends a 'qfThreadInfo' query, which is the query asking the target
for the ID of the first thread, and TSIM replies 'm0':

    Sending packet: $qfThreadInfo#bb...Ack
    Packet received: m0

As a result of this, GDB takes the '0' as the TID, and because of it,
constructs a ptid whose value is {42000, 0, 0}. This trips our
!is_ravenscar_task check, because all it does to identify threads
corresponding to ravenscar tasks is that their lwp is null, because
that's how we construct their ptid.

But this is unfortunatly not sufficient when debugging with TSIM,
because the thread ID that TSIM returns causes the creation of
a ptid whose lwp is zero, which matches the current identification
scheme and yet is clearly not a ravenscar task.

The fix is to also make sure that the ptid's tid field is nonzero.

gdb/ChangeLog:

        * ravenscar-thread.c (is_ravenscar_task): Also verify that
        the ptid's TID is nonzero.
---
 gdb/ravenscar-thread.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index e7922277ce..00593b84db 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -101,8 +101,14 @@ static void ravenscar_inferior_created (struct target_ops *target,
 static int
 is_ravenscar_task (ptid_t ptid)
 {
-  /* By construction, ravenscar tasks have their LWP set to zero.  */
-  return ptid_get_lwp (ptid) == 0;
+  /* By construction, ravenscar tasks have their LWP set to zero.
+     Also make sure that the TID is nonzero, as some remotes, when
+     asked for the list of threads, will return the first thread
+     as having its TID set to zero.  For instance, TSIM version
+     2.0.48 for LEON3 sends 'm0' as a reply to the 'qfThreadInfo'
+     query, which the remote protocol layer then treats as a thread
+     whose TID is 0.  This is obviously not a ravenscar task.  */
+  return ptid_get_lwp (ptid) == 0 && ptid_get_tid (ptid) != 0;
 }
 
 /* Given PTID, which can be either a ravenscar task or a CPU thread,
-- 
2.11.0

[Ada 1/6] watchpoint regression debugging with remote protocol (bare metal)

[Joel Brobecker]

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.
---
 gdb/ravenscar-thread.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 850b84d2b5..7edfa29263 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -326,6 +326,66 @@ ravenscar_prepare_to_store (struct target_ops *self,
     }
 }
 
+/* Implement the to_stopped_by_sw_breakpoint target_ops "method".  */
+
+static int
+ravenscar_stopped_by_sw_breakpoint (struct target_ops *ops)
+{
+  ptid_t saved_ptid = inferior_ptid;
+  struct target_ops *beneath = find_target_beneath (ops);
+  int result;
+
+  inferior_ptid = base_ptid;
+  result = beneath->to_stopped_by_sw_breakpoint (beneath);
+  inferior_ptid = saved_ptid;
+  return result;
+}
+
+/* Implement the to_stopped_by_hw_breakpoint target_ops "method".  */
+
+static int
+ravenscar_stopped_by_hw_breakpoint (struct target_ops *ops)
+{
+  ptid_t saved_ptid = inferior_ptid;
+  struct target_ops *beneath = find_target_beneath (ops);
+  int result;
+
+  inferior_ptid = base_ptid;
+  result = beneath->to_stopped_by_hw_breakpoint (beneath);
+  inferior_ptid = saved_ptid;
+  return result;
+}
+
+/* Implement the to_stopped_by_watchpoint target_ops "method".  */
+
+static int
+ravenscar_stopped_by_watchpoint (struct target_ops *ops)
+{
+  ptid_t saved_ptid = inferior_ptid;
+  struct target_ops *beneath = find_target_beneath (ops);
+  int result;
+
+  inferior_ptid = base_ptid;
+  result = beneath->to_stopped_by_watchpoint (beneath);
+  inferior_ptid = saved_ptid;
+  return result;
+}
+
+/* Implement the to_stopped_data_address target_ops "method".  */
+
+static int
+ravenscar_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
+{
+  ptid_t saved_ptid = inferior_ptid;
+  struct target_ops *beneath = find_target_beneath (ops);
+  int result;
+
+  inferior_ptid = base_ptid;
+  result = beneath->to_stopped_data_address (beneath, addr_p);
+  inferior_ptid = saved_ptid;
+  return result;
+}
+
 static void
 ravenscar_mourn_inferior (struct target_ops *ops)
 {
@@ -336,6 +396,21 @@ ravenscar_mourn_inferior (struct target_ops *ops)
   unpush_target (&ravenscar_ops);
 }
 
+/* Implement the to_core_of_thread target_ops "method".  */
+
+static int
+ravenscar_core_of_thread (struct target_ops *ops, ptid_t ptid)
+{
+  ptid_t saved_ptid = inferior_ptid;
+  struct target_ops *beneath = find_target_beneath (ops);
+  int result;
+
+  inferior_ptid = base_ptid;
+  result = beneath->to_core_of_thread (beneath, inferior_ptid);
+  inferior_ptid = saved_ptid;
+  return result;
+}
+
 /* Observer on inferior_created: push ravenscar thread stratum if needed.  */
 
 static void
@@ -369,6 +444,12 @@ init_ravenscar_thread_ops (void)
   ravenscar_ops.to_fetch_registers = ravenscar_fetch_registers;
   ravenscar_ops.to_store_registers = ravenscar_store_registers;
   ravenscar_ops.to_prepare_to_store = ravenscar_prepare_to_store;
+  ravenscar_ops.to_stopped_by_sw_breakpoint
+    = ravenscar_stopped_by_sw_breakpoint;
+  ravenscar_ops.to_stopped_by_hw_breakpoint
+    = ravenscar_stopped_by_hw_breakpoint;
+  ravenscar_ops.to_stopped_by_watchpoint = ravenscar_stopped_by_watchpoint;
+  ravenscar_ops.to_stopped_data_address = ravenscar_stopped_data_address;
   ravenscar_ops.to_thread_alive = ravenscar_thread_alive;
   ravenscar_ops.to_update_thread_list = ravenscar_update_thread_list;
   ravenscar_ops.to_pid_to_str = ravenscar_pid_to_str;
@@ -381,6 +462,7 @@ init_ravenscar_thread_ops (void)
   ravenscar_ops.to_has_registers = default_child_has_registers;
   ravenscar_ops.to_has_execution = default_child_has_execution;
   ravenscar_ops.to_stratum = thread_stratum;
+  ravenscar_ops.to_core_of_thread = ravenscar_core_of_thread;
   ravenscar_ops.to_magic = OPS_MAGIC;
 }
 
-- 
2.11.0

[Ada 6/6] ravenscar: update inferior ptid with event ptid

[Joel Brobecker]

From: Jerome Guitton <guitton@adacore.com>

When debugging a program using a ravenscar runtime, the thread
layer sometimes gets confused, and even missing some threads.
This was traced to an assumption that ravenscar_wait was making,
which is that calling the "to_wait" target_ops method would
set the inferior_ptid, so that we could then use that assumption
to update our thread_list and current ptid. However, this has not
been the case for quite a while now. This patch fixes the problem
by assigning inferior_ptid the ptid returned by "to_wait".

gdb/ChangeLog:

	* ravenscar-thread.c (ravenscar_wait): Update inferior ptid
	with event ptid from the lower layer before doing the
	ravenscar-specific update.
---
 gdb/ravenscar-thread.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 00593b84db..30745f7caa 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -297,9 +297,10 @@ ravenscar_wait (struct target_ops *ops, ptid_t ptid,
                 int options)
 {
   struct target_ops *beneath = find_target_beneath (ops);
+  ptid_t event_ptid;
 
   inferior_ptid = base_ptid;
-  beneath->to_wait (beneath, base_ptid, status, 0);
+  event_ptid = beneath->to_wait (beneath, base_ptid, status, 0);
   /* Find any new threads that might have been created, and update
      inferior_ptid to the active thread.
 
@@ -310,6 +311,7 @@ ravenscar_wait (struct target_ops *ops, ptid_t ptid,
   if (status->kind != TARGET_WAITKIND_EXITED
       && status->kind != TARGET_WAITKIND_SIGNALLED)
     {
+      inferior_ptid = event_ptid;
       ravenscar_update_thread_list (ops);
       ravenscar_update_inferior_ptid ();
     }
-- 
2.11.0

[Ada 3/6] Add multiple-CPU support in ravenscar-thread.c

[Joel Brobecker]

This patch reworks the ravenscar-thread layer to remove the
assumption that the target only has 1 CPU. In particular,
when connected to a QEMU target over the remote protocol,
QEMU reports each CPU as one thread. This patch adapts
the ravenscar-thread layer to this, and adds a large comment
explaining the general design of this unit.

gdb/ChangeLog:

        * ada-lang.h (ada_get_task_info_from_ptid): Add declaration.
        * ada-tasks.c (ada_get_task_info_from_ptid): New function.
        * ravenscar-thread.c: Add into comment.
        (base_magic_null_ptid): Delete.
        (base_ptid): Change documentation.
        (ravenscar_active_task): Renames ravenscar_running_thread.
        All callers updated throughout.
        (is_ravenscar_task, ravenscar_get_thread_base_cpu): New function.
        (ravenscar_task_is_currently_active): Likewise.
        (get_base_thread_from_ravenscar_task): Ditto.
        (ravenscar_update_inferior_ptid): Adjust to handle multiple CPUs.
        (ravenscar_runtime_initialized): Likewise.
        (get_running_thread_id): Add new parameter "cpu".  Adjust
        implementation to handle this new parameter.
        (ravenscar_fetch_registers): Small adjustment to use
        is_ravenscar_task and ravenscar_task_is_currently_active in
        order to decide whether to use the target beneath or this
        module's arch_ops.
        (ravenscar_store_registers, ravenscar_prepare_to_store): Likewise.
        (ravenscar_stopped_by_sw_breakpoint): Use
        get_base_thread_from_ravenscar_task to get the underlying
        thread, rather than using base_ptid.
        (ravenscar_stopped_by_hw_breakpoint, ravenscar_stopped_by_watchpoint)
        (ravenscar_stopped_data_address, ravenscar_core_of_thread):
        Likewise.
        (ravenscar_inferior_created): Do not set base_magic_null_ptid.
---
 gdb/ada-lang.h         |   2 +
 gdb/ada-tasks.c        |  24 +++++++
 gdb/ravenscar-thread.c | 170 +++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 162 insertions(+), 34 deletions(-)

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index c11fb48c1b..36a5820ee3 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -401,6 +401,8 @@ extern std::vector<ada_exc_info> ada_exceptions_list (const char *regexp);
 
 extern int valid_task_id (int);
 
+extern struct ada_task_info *ada_get_task_info_from_ptid (ptid_t ptid);
+
 extern int ada_get_task_number (ptid_t);
 
 typedef void (ada_task_list_iterator_ftype) (struct ada_task_info *task);
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index e29b563d6c..16cee35d86 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -353,6 +353,30 @@ ada_task_is_alive (struct ada_task_info *task_info)
   return (task_info->state != Terminated);
 }
 
+/* Search through the list of known tasks for the one whose ptid is
+   PTID, and return it.  Return NULL if the task was not found.  */
+
+struct ada_task_info *
+ada_get_task_info_from_ptid (ptid_t ptid)
+{
+  int i, nb_tasks;
+  struct ada_task_info *task;
+  struct ada_tasks_inferior_data *data;
+
+  ada_build_task_list ();
+  data = get_ada_tasks_inferior_data (current_inferior ());
+  nb_tasks = VEC_length (ada_task_info_s, data->task_list);
+
+  for (i = 0; i < nb_tasks; i++)
+    {
+      task = VEC_index (ada_task_info_s, data->task_list, i);
+      if (ptid_equal (task->ptid, ptid))
+	return task;
+    }
+
+  return NULL;
+}
+
 /* Call the ITERATOR function once for each Ada task that hasn't been
    terminated yet.  */
 
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 7edfa29263..83dfd9d4ad 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -31,17 +31,41 @@
 #include "regcache.h"
 #include "objfiles.h"
 
+/* This module provides support for "Ravenscar" tasks (Ada) when
+   debugging on bare-metal targets.
+
+   The typical situation is when debugging a bare-metal target over
+   the remote protocol. In that situation, the system does not know
+   about high-level comcepts such as threads, only about some code
+   running on one or more CPUs. And since the remote protocol does not
+   provide any handling for CPUs, the de facto standard for handling
+   them is to have one thread per CPU, where the thread's ptid has
+   its lwp field set to the CPU number (eg: 1 for the first CPU,
+   2 for the second one, etc).  This module will make that assumption.
+
+   This module then creates and maintains the list of threads based
+   on the list of Ada tasks, with one thread per Ada tasks. The convention
+   is that threads corresponding to the CPUs (see assumption above)
+   have a ptid_t of the form (PID, LWP, 0), which threads corresponding
+   to our Ada tasks have a ptid_t of the form (PID, 0, TID) where TID
+   is the Ada task's ID as extracted from Ada runtime information.
+
+   Switching to a given Ada tasks (or its underlying thread) is performed
+   by fetching the registers of that tasks from the memory area where
+   the registers were saved.  For any of the other operations, the
+   operation is performed by first finding the CPU on which the task
+   is running, switching to its corresponding ptid, and then performing
+   the operation on that ptid using the target beneath us.  */
+
 /* If non-null, ravenscar task support is enabled.  */
 static int ravenscar_task_support = 1;
 
 /* This module's target-specific operations.  */
 static struct target_ops ravenscar_ops;
 
-/* Some base target uses a special value for the null PID (exempli gratia
-   remote).  */
-static ptid_t base_magic_null_ptid;
-
-/* Ptid of the inferior as seen by the process stratum.  */
+/* PTID of the last thread that received an event.
+   This can be useful to determine the associated task that received
+   the event, to make it the current task.  */
 static ptid_t base_ptid;
 
 static const char running_thread_name[] = "__gnat_running_thread_table";
@@ -53,7 +77,7 @@ static const char ravenscar_runtime_initializer[] =
   "system__bb__threads__initialize";
 
 static void ravenscar_update_thread_list (struct target_ops *ops);
-static ptid_t ravenscar_running_thread (void);
+static ptid_t ravenscar_active_task (int cpu);
 static const char *ravenscar_extra_thread_info (struct target_ops *self,
 						struct thread_info *tp);
 static int ravenscar_thread_alive (struct target_ops *ops, ptid_t ptid);
@@ -72,22 +96,100 @@ static int ravenscar_runtime_initialized (void);
 static void ravenscar_inferior_created (struct target_ops *target,
 					int from_tty);
 
+/* Return nonzero iff PTID corresponds to a ravenscar task.  */
+
+static int
+is_ravenscar_task (ptid_t ptid)
+{
+  /* By construction, ravenscar tasks have their LWP set to zero.  */
+  return ptid_get_lwp (ptid) == 0;
+}
+
+/* Given PTID, which can be either a ravenscar task or a CPU thread,
+   return which CPU that ptid is running on.
+
+   This assume that PTID is a valid ptid_t.  Otherwise, a gdb_assert
+   will be triggered.  */
+
+static int
+ravenscar_get_thread_base_cpu (ptid_t ptid)
+{
+  int base_cpu;
+
+  if (is_ravenscar_task (ptid))
+    {
+      struct ada_task_info *task_info = ada_get_task_info_from_ptid (ptid);
+
+      gdb_assert (task_info != NULL);
+      base_cpu = task_info->base_cpu;
+    }
+  else
+    {
+      /* We assume that the LWP of the PTID is equal to the CPU number.  */
+      base_cpu = ptid_get_lwp (ptid);
+    }
+
+  return base_cpu;
+}
+
+/* Given a ravenscar task (identified by its ptid_t PTID), return nonzero
+   if this task is the currently active task on the cpu that task is
+   running on.
+
+   In other words, this function determine which CPU this task is
+   currently running on, and then return nonzero if the CPU in question
+   is executing the code for that task.  If that's the case, then
+   that task's registers are in the CPU bank.  Otherwise, the task
+   is currently suspended, and its registers have been saved in memory.  */
+
+static int
+ravenscar_task_is_currently_active (ptid_t ptid)
+{
+  ptid_t active_task_ptid
+    = ravenscar_active_task (ravenscar_get_thread_base_cpu (ptid));
+
+  return ptid_equal (ptid, active_task_ptid);
+}
+
+/* Return the CPU thread (as a ptid_t) on which the given ravenscar
+   task is running.
+
+   This is the thread that corresponds to the CPU on which the task
+   is running.  */
+
+static ptid_t
+get_base_thread_from_ravenscar_task (ptid_t ptid)
+{
+  int base_cpu;
+
+  if (!is_ravenscar_task (ptid))
+    return ptid;
+
+  base_cpu = ravenscar_get_thread_base_cpu (ptid);
+  return ptid_build (ptid_get_pid (ptid), base_cpu, 0);
+}
+
 /* Fetch the ravenscar running thread from target memory and
    update inferior_ptid accordingly.  */
 
 static void
 ravenscar_update_inferior_ptid (void)
 {
+  int base_cpu;
+
   base_ptid = inferior_ptid;
 
+  gdb_assert (!is_ravenscar_task (inferior_ptid));
+  base_cpu = ravenscar_get_thread_base_cpu (base_ptid);
+
   /* If the runtime has not been initialized yet, the inferior_ptid is
      the only ptid that there is.  */
   if (!ravenscar_runtime_initialized ())
     return;
 
-  /* Make sure we set base_ptid before calling ravenscar_running_thread
+  /* Make sure we set base_ptid before calling ravenscar_active_task
      as the latter relies on it.  */
-  inferior_ptid = ravenscar_running_thread ();
+  inferior_ptid = ravenscar_active_task (base_cpu);
   gdb_assert (!ptid_equal (inferior_ptid, null_ptid));
 
   /* The running thread may not have been added to
@@ -144,14 +246,14 @@ has_ravenscar_runtime (void)
 static int
 ravenscar_runtime_initialized (void)
 {
-  return (!(ptid_equal (ravenscar_running_thread (), null_ptid)));
+  return (!(ptid_equal (ravenscar_active_task (1), null_ptid)));
 }
 
 /* Return the ID of the thread that is currently running.
    Return 0 if the ID could not be determined.  */
 
 static CORE_ADDR
-get_running_thread_id (void)
+get_running_thread_id (int cpu)
 {
   struct bound_minimal_symbol object_msym = get_running_thread_msymbol ();
   int object_size;
@@ -164,8 +266,9 @@ get_running_thread_id (void)
   if (!object_msym.minsym)
     return 0;
 
-  object_addr = BMSYMBOL_VALUE_ADDRESS (object_msym);
   object_size = TYPE_LENGTH (builtin_type_void_data_ptr);
+  object_addr = (BMSYMBOL_VALUE_ADDRESS (object_msym)
+		 + (cpu - 1) * object_size);
   buf_size = object_size;
   buf = (gdb_byte *) alloca (buf_size);
   read_memory (object_addr, buf, buf_size);
@@ -231,9 +334,9 @@ ravenscar_update_thread_list (struct target_ops *ops)
 }
 
 static ptid_t
-ravenscar_running_thread (void)
+ravenscar_active_task (int cpu)
 {
-  CORE_ADDR tid = get_running_thread_id ();
+  CORE_ADDR tid = get_running_thread_id (cpu);
 
   if (tid == 0)
     return null_ptid;
@@ -270,11 +373,9 @@ ravenscar_fetch_registers (struct target_ops *ops,
   struct target_ops *beneath = find_target_beneath (ops);
   ptid_t ptid = regcache_get_ptid (regcache);
 
-  if (!ravenscar_runtime_initialized ()
-      || ptid_equal (ptid, base_magic_null_ptid)
-      || ptid_equal (ptid, ravenscar_running_thread ()))
-    beneath->to_fetch_registers (beneath, regcache, regnum);
-  else
+  if (ravenscar_runtime_initialized ()
+      && is_ravenscar_task (ptid)
+      && !ravenscar_task_is_currently_active (ptid))
     {
       struct gdbarch *gdbarch = regcache->arch ();
       struct ravenscar_arch_ops *arch_ops
@@ -282,6 +383,8 @@ ravenscar_fetch_registers (struct target_ops *ops,
 
       arch_ops->to_fetch_registers (regcache, regnum);
     }
+  else
+    beneath->to_fetch_registers (beneath, regcache, regnum);
 }
 
 static void
@@ -291,11 +394,9 @@ ravenscar_store_registers (struct target_ops *ops,
   struct target_ops *beneath = find_target_beneath (ops);
   ptid_t ptid = regcache_get_ptid (regcache);
 
-  if (!ravenscar_runtime_initialized ()
-      || ptid_equal (ptid, base_magic_null_ptid)
-      || ptid_equal (ptid, ravenscar_running_thread ()))
-    beneath->to_store_registers (beneath, regcache, regnum);
-  else
+  if (ravenscar_runtime_initialized ()
+      && is_ravenscar_task (ptid)
+      && !ravenscar_task_is_currently_active (ptid))
     {
       struct gdbarch *gdbarch = regcache->arch ();
       struct ravenscar_arch_ops *arch_ops
@@ -303,6 +404,8 @@ ravenscar_store_registers (struct target_ops *ops,
 
       arch_ops->to_store_registers (regcache, regnum);
     }
+  else
+    beneath->to_store_registers (beneath, regcache, regnum);
 }
 
 static void
@@ -312,11 +415,9 @@ ravenscar_prepare_to_store (struct target_ops *self,
   struct target_ops *beneath = find_target_beneath (self);
   ptid_t ptid = regcache_get_ptid (regcache);
 
-  if (!ravenscar_runtime_initialized ()
-      || ptid_equal (ptid, base_magic_null_ptid)
-      || ptid_equal (ptid, ravenscar_running_thread ()))
-    beneath->to_prepare_to_store (beneath, regcache);
-  else
+  if (ravenscar_runtime_initialized ()
+      && is_ravenscar_task (ptid)
+      && !ravenscar_task_is_currently_active (ptid))
     {
       struct gdbarch *gdbarch = regcache->arch ();
       struct ravenscar_arch_ops *arch_ops
@@ -324,6 +425,8 @@ ravenscar_prepare_to_store (struct target_ops *self,
 
       arch_ops->to_prepare_to_store (regcache);
     }
+  else
+    beneath->to_prepare_to_store (beneath, regcache);
 }
 
 /* Implement the to_stopped_by_sw_breakpoint target_ops "method".  */
@@ -335,7 +438,7 @@ ravenscar_stopped_by_sw_breakpoint (struct target_ops *ops)
   struct target_ops *beneath = find_target_beneath (ops);
   int result;
 
-  inferior_ptid = base_ptid;
+  inferior_ptid = get_base_thread_from_ravenscar_task (saved_ptid);
   result = beneath->to_stopped_by_sw_breakpoint (beneath);
   inferior_ptid = saved_ptid;
   return result;
@@ -350,7 +453,7 @@ ravenscar_stopped_by_hw_breakpoint (struct target_ops *ops)
   struct target_ops *beneath = find_target_beneath (ops);
   int result;
 
-  inferior_ptid = base_ptid;
+  inferior_ptid = get_base_thread_from_ravenscar_task (saved_ptid);
   result = beneath->to_stopped_by_hw_breakpoint (beneath);
   inferior_ptid = saved_ptid;
   return result;
@@ -365,7 +468,7 @@ ravenscar_stopped_by_watchpoint (struct target_ops *ops)
   struct target_ops *beneath = find_target_beneath (ops);
   int result;
 
-  inferior_ptid = base_ptid;
+  inferior_ptid = get_base_thread_from_ravenscar_task (saved_ptid);
   result = beneath->to_stopped_by_watchpoint (beneath);
   inferior_ptid = saved_ptid;
   return result;
@@ -380,7 +483,7 @@ ravenscar_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   struct target_ops *beneath = find_target_beneath (ops);
   int result;
 
-  inferior_ptid = base_ptid;
+  inferior_ptid = get_base_thread_from_ravenscar_task (saved_ptid);
   result = beneath->to_stopped_data_address (beneath, addr_p);
   inferior_ptid = saved_ptid;
   return result;
@@ -405,7 +508,7 @@ ravenscar_core_of_thread (struct target_ops *ops, ptid_t ptid)
   struct target_ops *beneath = find_target_beneath (ops);
   int result;
 
-  inferior_ptid = base_ptid;
+  inferior_ptid = get_base_thread_from_ravenscar_task (saved_ptid);
   result = beneath->to_core_of_thread (beneath, inferior_ptid);
   inferior_ptid = saved_ptid;
   return result;
@@ -422,7 +525,6 @@ ravenscar_inferior_created (struct target_ops *target, int from_tty)
       || !has_ravenscar_runtime ())
     return;
 
-  base_magic_null_ptid = inferior_ptid;
   ravenscar_update_inferior_ptid ();
   push_target (&ravenscar_ops);
 }
-- 
2.11.0

Re: [Ada 1/6] watchpoint regression debugging with remote protocol (bare metal)

[Pedro Alves]

On 11/21/2017 10:29 PM, Joel Brobecker wrote:
> 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.

...

> 
> 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
> 

...

> 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.

FWIW, this looks like the right solution to me.

Thanks,
Pedro Alves

Re: [Ada 1/6] watchpoint regression debugging with remote protocol (bare metal)

[Joel Brobecker]

> FWIW, this looks like the right solution to me.

Thanks for confirming, Pedro!... and for taking the time to read
this rather long problem description! ;-)

-- 
Joel