* [PATCH v2 01/12] Call add_active_thread after pushing the ravenscar target
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 02/12] Avoid crash in ravenscar_thread_target::wait Tom Tromey
` (11 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Currently ravenscar-thread.c calls add_active_thread before pushing
the ravenscar target. This yields an initial thread announcement of
"[Thread 0]". Calling add_active_thread after pushing the target
fixes this.
2019-03-26 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target): Don't call
add_active_thread.
(ravenscar_thread_target::add_active_thread): Now public.
(ravenscar_inferior_created): Call add_active_thread after pushing
the target.
---
gdb/ChangeLog | 8 ++++++++
gdb/ravenscar-thread.c | 13 +++++++------
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 72253188c71..7fca51da2a7 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -79,9 +79,6 @@ struct ravenscar_thread_target final : public target_ops
ravenscar_thread_target ()
: m_base_ptid (inferior_ptid)
{
- thread_info *thr = add_active_thread ();
- if (thr != nullptr)
- switch_to_thread (thr);
}
const target_info &info () const override
@@ -124,6 +121,8 @@ struct ravenscar_thread_target final : public target_ops
delete this;
}
+ thread_info *add_active_thread ();
+
private:
/* PTID of the last thread that received an event.
@@ -131,7 +130,6 @@ struct ravenscar_thread_target final : public target_ops
the event, to make it the current task. */
ptid_t m_base_ptid;
- thread_info *add_active_thread ();
ptid_t active_task (int cpu);
bool task_is_currently_active (ptid_t ptid);
bool runtime_initialized ();
@@ -548,8 +546,11 @@ ravenscar_inferior_created (struct target_ops *target, int from_tty)
return;
}
- target_ops_up target_holder (new ravenscar_thread_target ());
- push_target (std::move (target_holder));
+ ravenscar_thread_target *rtarget = new ravenscar_thread_target ();
+ push_target (target_ops_up (rtarget));
+ thread_info *thr = rtarget->add_active_thread ();
+ if (thr != nullptr)
+ switch_to_thread (thr);
}
ptid_t
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 02/12] Avoid crash in ravenscar_thread_target::wait
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
2020-08-05 19:08 ` [PATCH v2 01/12] Call add_active_thread after pushing the ravenscar target Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 03/12] Return event_ptid from ravenscar_thread_target::wait Tom Tromey
` (10 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
An earlier patch caused a Ravenscar regression in
ravenscar_thread_target::wait. In particular, add_active_thread can
return NULL when the runtime is not initialized.
gdb/ChangeLog
2020-08-05 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target::wait): Check
runtime_initialized.
---
gdb/ChangeLog | 5 +++++
gdb/ravenscar-thread.c | 3 ++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 7fca51da2a7..47001de42d3 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -354,7 +354,8 @@ ravenscar_thread_target::wait (ptid_t ptid,
because we might try switching threads (and thus sending packets)
after the remote has disconnected. */
if (status->kind != TARGET_WAITKIND_EXITED
- && status->kind != TARGET_WAITKIND_SIGNALLED)
+ && status->kind != TARGET_WAITKIND_SIGNALLED
+ && runtime_initialized ())
{
m_base_ptid = event_ptid;
this->update_thread_list ();
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 03/12] Return event_ptid from ravenscar_thread_target::wait
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
2020-08-05 19:08 ` [PATCH v2 01/12] Call add_active_thread after pushing the ravenscar target Tom Tromey
2020-08-05 19:08 ` [PATCH v2 02/12] Avoid crash in ravenscar_thread_target::wait Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 04/12] Handle case where Ada task is current but not listed Tom Tromey
` (9 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
ravenscar_thread_target::wait should return the event ptid from the
wrapped "wait" call in the situation where returning the Ravenscar
thread ptid is not appropriate. This probably does not really make a
difference in practice, but it seemed like a reasonable cleanup.
gdb/ChangeLog
2020-08-05 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target::wait): Return
event_ptid.
---
gdb/ChangeLog | 5 +++++
gdb/ravenscar-thread.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 47001de42d3..b89b5f62e5a 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -361,7 +361,7 @@ ravenscar_thread_target::wait (ptid_t ptid,
this->update_thread_list ();
return this->add_active_thread ()->ptid;
}
- return m_base_ptid;
+ return event_ptid;
}
/* Add the thread associated to the given TASK to the thread list
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 04/12] Handle case where Ada task is current but not listed
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (2 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 03/12] Return event_ptid from ravenscar_thread_target::wait Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 05/12] Change names given to Ravenscar threads Tom Tromey
` (8 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Currently, the ravenscar runtime can mark an Ada task as the current
task, before adding it to the list of tasks that can be read by gdb.
In this scenario, gdb can sometimes crash in
ravenscar_get_thread_base_cpu with:
../../src/gdb/ravenscar-thread.c:167: internal-error: int ravenscar_get_thread_base_cpu(ptid_t): Assertion `task_info != NULL' failed.
However, as ravenscar_get_thread_base_cpu is only called to find the
base CPU, we can simply record this when registering the thread, and
look this up later.
2019-03-26 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target) <get_base_cpu,
get_base_thread_from_ravenscar_task>: Now methods.
<m_cpu_map>: New member.
(ravenscar_thread_target::get_thread_base_cpu): Rename from
ravenscar_get_thread_base_cpu. Check m_cpu_map.
(ravenscar_thread_target::task_is_currently_active): Update.
(ravenscar_thread_target::get_base_thread_from_ravenscar_task):
Now a method.
(ravenscar_thread_target::add_active_thread): Put initial thread
into the m_cpu_map.
---
gdb/ChangeLog | 13 +++++++++++++
gdb/ravenscar-thread.c | 40 +++++++++++++++++++++++++++++-----------
2 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index b89b5f62e5a..ee95a1c4bb8 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -30,6 +30,7 @@
#include "top.h"
#include "regcache.h"
#include "objfiles.h"
+#include <unordered_map>
/* This module provides support for "Ravenscar" tasks (Ada) when
debugging on bare-metal targets.
@@ -133,6 +134,14 @@ struct ravenscar_thread_target final : public target_ops
ptid_t active_task (int cpu);
bool task_is_currently_active (ptid_t ptid);
bool runtime_initialized ();
+ int get_thread_base_cpu (ptid_t ptid);
+ ptid_t get_base_thread_from_ravenscar_task (ptid_t ptid);
+
+ /* This maps a TID to the CPU on which it was running. This is
+ needed because sometimes the runtime will report an active task
+ that hasn't yet been put on the list of tasks that is read by
+ ada-tasks.c. */
+ std::unordered_map<long, int> m_cpu_map;
};
/* Return true iff PTID corresponds to a ravenscar task. */
@@ -156,8 +165,8 @@ is_ravenscar_task (ptid_t ptid)
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
+ravenscar_thread_target::get_thread_base_cpu (ptid_t ptid)
{
int base_cpu;
@@ -165,8 +174,15 @@ ravenscar_get_thread_base_cpu (ptid_t 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;
+ if (task_info != NULL)
+ base_cpu = task_info->base_cpu;
+ else
+ {
+ auto iter = m_cpu_map.find (ptid.tid ());
+
+ gdb_assert (iter != m_cpu_map.end ());
+ base_cpu = iter->second;
+ }
}
else
{
@@ -190,8 +206,7 @@ ravenscar_get_thread_base_cpu (ptid_t ptid)
bool
ravenscar_thread_target::task_is_currently_active (ptid_t ptid)
{
- ptid_t active_task_ptid
- = active_task (ravenscar_get_thread_base_cpu (ptid));
+ ptid_t active_task_ptid = active_task (get_thread_base_cpu (ptid));
return ptid == active_task_ptid;
}
@@ -202,15 +217,15 @@ ravenscar_thread_target::task_is_currently_active (ptid_t ptid)
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)
+ptid_t
+ravenscar_thread_target::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);
+ base_cpu = get_thread_base_cpu (ptid);
return ptid_t (ptid.pid (), base_cpu, 0);
}
@@ -227,7 +242,7 @@ ravenscar_thread_target::add_active_thread ()
int base_cpu;
gdb_assert (!is_ravenscar_task (m_base_ptid));
- base_cpu = ravenscar_get_thread_base_cpu (m_base_ptid);
+ base_cpu = get_thread_base_cpu (m_base_ptid);
if (!runtime_initialized ())
return nullptr;
@@ -242,7 +257,10 @@ ravenscar_thread_target::add_active_thread ()
may not always add it to the thread list. Add it here. */
thread_info *active_thr = find_thread_ptid (proc_target, active_ptid);
if (active_thr == nullptr)
- active_thr = add_thread (proc_target, active_ptid);
+ {
+ active_thr = ::add_thread (proc_target, active_ptid);
+ m_cpu_map[active_ptid.tid ()] = base_cpu;
+ }
return active_thr;
}
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 05/12] Change names given to Ravenscar threads
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (3 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 04/12] Handle case where Ada task is current but not listed Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 06/12] Use gdb::function_view in iterate_over_live_ada_tasks Tom Tromey
` (7 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Current a Ravenscar thread is given the same sort of name as a "CPU"
thread; they can only be distinguished by looking at the output of
"info thread".
This patch changes ravenscar-thread.c to distinguish these threads,
like:
(gdb) continue
Continuing.
[New Ravenscar Thread 0x2b910]
2019-03-26 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target) <extra_thread_info>:
Remove.
(ravenscar_thread_target::extra_thread_info): Remove.
(ravenscar_thread_target::pid_to_str): Mention Ravenscar in result;
defer to target beneath for non-Ravenscar threads.
---
gdb/ChangeLog | 8 ++++++++
gdb/ravenscar-thread.c | 13 ++++---------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index ee95a1c4bb8..dbcd4de81f8 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -109,8 +109,6 @@ struct ravenscar_thread_target final : public target_ops
void update_thread_list () override;
- const char *extra_thread_info (struct thread_info *) override;
-
std::string pid_to_str (ptid_t) override;
ptid_t get_ada_task_ptid (long lwp, long thread) override;
@@ -414,12 +412,6 @@ ravenscar_thread_target::active_task (int cpu)
return ptid_t (m_base_ptid.pid (), 0, tid);
}
-const char *
-ravenscar_thread_target::extra_thread_info (thread_info *tp)
-{
- return "Ravenscar task";
-}
-
bool
ravenscar_thread_target::thread_alive (ptid_t ptid)
{
@@ -430,7 +422,10 @@ ravenscar_thread_target::thread_alive (ptid_t ptid)
std::string
ravenscar_thread_target::pid_to_str (ptid_t ptid)
{
- return string_printf ("Thread %#x", (int) ptid.tid ());
+ if (!is_ravenscar_task (ptid))
+ return beneath ()->pid_to_str (ptid);
+
+ return string_printf ("Ravenscar Thread %#x", (int) ptid.tid ());
}
void
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 06/12] Use gdb::function_view in iterate_over_live_ada_tasks
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (4 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 05/12] Change names given to Ravenscar threads Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 07/12] Wrap xfer_partial and enable_btrace for Ravenscar Tom Tromey
` (6 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes iterate_over_live_ada_tasks to accept a
gdb::function_view. This is needed by a subsequent patch.
2019-03-26 Tom Tromey <tromey@adacore.com>
* ada-lang.h (ada_task_list_iterator_ftype): Now a
gdb::function_view.
(iterate_over_live_ada_tasks): Change type of argument.
* ada-tasks.c (iterate_over_live_ada_tasks): Change type
of argument.
---
gdb/ChangeLog | 8 ++++++++
gdb/ada-lang.h | 5 +++--
gdb/ada-tasks.c | 2 +-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 9be597942fd..ae313ce700a 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -375,9 +375,10 @@ extern struct ada_task_info *ada_get_task_info_from_ptid (ptid_t ptid);
extern int ada_get_task_number (thread_info *thread);
-typedef void (ada_task_list_iterator_ftype) (struct ada_task_info *task);
+typedef gdb::function_view<void (struct ada_task_info *task)>
+ ada_task_list_iterator_ftype;
extern void iterate_over_live_ada_tasks
- (ada_task_list_iterator_ftype *iterator);
+ (ada_task_list_iterator_ftype iterator);
extern const char *ada_get_tcb_types_info (void);
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 27b458767a7..d54c8b320a4 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -376,7 +376,7 @@ ada_get_task_info_from_ptid (ptid_t ptid)
terminated yet. */
void
-iterate_over_live_ada_tasks (ada_task_list_iterator_ftype *iterator)
+iterate_over_live_ada_tasks (ada_task_list_iterator_ftype iterator)
{
struct ada_tasks_inferior_data *data;
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 07/12] Wrap xfer_partial and enable_btrace for Ravenscar
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (5 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 06/12] Use gdb::function_view in iterate_over_live_ada_tasks Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 08/12] Update Ravenscar documentation Tom Tromey
` (5 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
A gdb crash showed that the xfer_partial target method was not wrapped
for Ravenscar. This caused remote.c to call
remote::set_general_thread with a Ravenscar "fake" ptid, which showed
up later as an event ptid.
I went through all the target methods and looked to see which ones
could call set_general_thread or set_continue_thread (but not
set_general_process, as I think Ravenscar targets aren't
multi-inferior). This patch wraps the two that I found.
xfer_partial requires special treatment, because it can be called
recursively via get_base_thread_from_ravenscar_task. To avoid a
recursive call, this patch changes update_thread_list to record all
tasks in the m_cpu_map, and changes get_thread_base_cpu to prefer this
map. This avoids some memory reads.
It was unclear to me whether enable_btrace really makes sense for
Ravenscar; but at the same time it seemed harmless to add this patch.
2019-03-26 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (xfer_partial, enable_btrace, add_thread):
New methods.
(ravenscar_thread_target::get_thread_base_cpu): Check m_cpu_map
first.
(ravenscar_thread_target::add_thread): Rename from
ravenscar_add_thread.
(ravenscar_thread_target::update_thread_list): Use a lambda.
(ravenscar_thread_target::xfer_partial): New method.
---
gdb/ChangeLog | 11 ++++++++
gdb/ravenscar-thread.c | 64 +++++++++++++++++++++++++++++++++++-------
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index dbcd4de81f8..a67a5e9a368 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -103,6 +103,13 @@ struct ravenscar_thread_target final : public target_ops
bool stopped_data_address (CORE_ADDR *) override;
+ enum target_xfer_status xfer_partial (enum target_object object,
+ const char *annex,
+ gdb_byte *readbuf,
+ const gdb_byte *writebuf,
+ ULONGEST offset, ULONGEST len,
+ ULONGEST *xfered_len) override;
+
bool thread_alive (ptid_t ptid) override;
int core_of_thread (ptid_t ptid) override;
@@ -113,6 +120,14 @@ struct ravenscar_thread_target final : public target_ops
ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ struct btrace_target_info *enable_btrace (ptid_t ptid,
+ const struct btrace_config *conf)
+ override
+ {
+ ptid = get_base_thread_from_ravenscar_task (ptid);
+ return beneath ()->enable_btrace (ptid, conf);
+ }
+
void mourn_inferior () override;
void close () override
@@ -134,6 +149,7 @@ struct ravenscar_thread_target final : public target_ops
bool runtime_initialized ();
int get_thread_base_cpu (ptid_t ptid);
ptid_t get_base_thread_from_ravenscar_task (ptid_t ptid);
+ void add_thread (struct ada_task_info *task);
/* This maps a TID to the CPU on which it was running. This is
needed because sometimes the runtime will report an active task
@@ -170,16 +186,18 @@ ravenscar_thread_target::get_thread_base_cpu (ptid_t ptid)
if (is_ravenscar_task (ptid))
{
- struct ada_task_info *task_info = ada_get_task_info_from_ptid (ptid);
+ /* Prefer to not read inferior memory if possible, to avoid
+ reentrancy problems with xfer_partial. */
+ auto iter = m_cpu_map.find (ptid.tid ());
- if (task_info != NULL)
- base_cpu = task_info->base_cpu;
+ if (iter != m_cpu_map.end ())
+ base_cpu = iter->second;
else
{
- auto iter = m_cpu_map.find (ptid.tid ());
+ struct ada_task_info *task_info = ada_get_task_info_from_ptid (ptid);
- gdb_assert (iter != m_cpu_map.end ());
- base_cpu = iter->second;
+ gdb_assert (task_info != NULL);
+ base_cpu = task_info->base_cpu;
}
}
else
@@ -383,11 +401,14 @@ ravenscar_thread_target::wait (ptid_t ptid,
/* Add the thread associated to the given TASK to the thread list
(if the thread has already been added, this is a no-op). */
-static void
-ravenscar_add_thread (struct ada_task_info *task)
+void
+ravenscar_thread_target::add_thread (struct ada_task_info *task)
{
if (find_thread_ptid (current_inferior (), task->ptid) == NULL)
- add_thread (current_inferior ()->process_target (), task->ptid);
+ {
+ ::add_thread (current_inferior ()->process_target (), task->ptid);
+ m_cpu_map[task->ptid.tid ()] = task->base_cpu;
+ }
}
void
@@ -398,7 +419,10 @@ ravenscar_thread_target::update_thread_list ()
(m_base_ptid) and the running thread, that may not have been included
to system.tasking.debug's list yet. */
- iterate_over_live_ada_tasks (ravenscar_add_thread);
+ iterate_over_live_ada_tasks ([=] (struct ada_task_info *task)
+ {
+ this->add_thread (task);
+ });
}
ptid_t
@@ -541,6 +565,26 @@ ravenscar_thread_target::core_of_thread (ptid_t ptid)
return beneath ()->core_of_thread (inferior_ptid);
}
+/* Implement the target xfer_partial method. */
+
+enum target_xfer_status
+ravenscar_thread_target::xfer_partial (enum target_object object,
+ const char *annex,
+ gdb_byte *readbuf,
+ const gdb_byte *writebuf,
+ ULONGEST offset, ULONGEST len,
+ ULONGEST *xfered_len)
+{
+ scoped_restore save_ptid = make_scoped_restore (&inferior_ptid);
+ /* Calling get_base_thread_from_ravenscar_task can read memory from
+ the inferior. However, that function is written to prefer our
+ internal map, so it should not result in recursive calls in
+ practice. */
+ inferior_ptid = get_base_thread_from_ravenscar_task (inferior_ptid);
+ return beneath ()->xfer_partial (object, annex, readbuf, writebuf,
+ offset, len, xfered_len);
+}
+
/* Observer on inferior_created: push ravenscar thread stratum if needed. */
static void
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 08/12] Update Ravenscar documentation
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (6 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 07/12] Wrap xfer_partial and enable_btrace for Ravenscar Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 09/12] Fix Ravenscar "process" resume Tom Tromey
` (4 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This documents some recent Ravenscar changes, and further documents
the known limitation where stepping through the runtime initialization
code does not work properly.
2019-03-26 Tom Tromey <tromey@adacore.com>
* gdb.texinfo (Ravenscar Profile): Add examples.
Document runtime initialization limitation.
---
gdb/doc/ChangeLog | 5 +++++
gdb/doc/gdb.texinfo | 28 ++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 09317fa0bd0..77013648b2f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18320,6 +18320,34 @@ using the Ravenscar Profile.
@end table
+@cindex Ravenscar thread
+When Ravenscar task-switching is enabled, Ravenscar tasks are
+announced by @value{GDBN} as if they were threads:
+
+@smallexample
+(gdb) continue
+[New Ravenscar Thread 0x2b8f0]
+@end smallexample
+
+Both Ravenscar tasks and the underlying CPU threads will show up in
+the output of @code{info threads}:
+
+@smallexample
+(gdb) info threads
+ Id Target Id Frame
+ 1 Thread 1 (CPU#0 [running]) simple () at simple.adb:10
+ 2 Thread 2 (CPU#1 [running]) 0x0000000000003d34 in __gnat_initialize_cpu_devices ()
+ 3 Thread 3 (CPU#2 [running]) 0x0000000000003d28 in __gnat_initialize_cpu_devices ()
+ 4 Thread 4 (CPU#3 [halted ]) 0x000000000000c6ec in system.task_primitives.operations.idle ()
+* 5 Ravenscar Thread 0x2b8f0 simple () at simple.adb:10
+ 6 Ravenscar Thread 0x2f150 0x000000000000c6ec in system.task_primitives.operations.idle ()
+@end smallexample
+
+One known limitation of the Ravenscar support in @value{GDBN} is that
+it isn't currently possible to single-step through the runtime
+initialization sequence. If you need to debug this code, you should
+use @code{set ravenscar task-switching off}.
+
@node Ada Settings
@subsubsection Ada Settings
@cindex Ada settings
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 09/12] Fix Ravenscar "process" resume
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (7 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 08/12] Update Ravenscar documentation Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 10/12] Fetch registers from correct thread in ravenscar-thread.c Tom Tromey
` (3 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
A coworker noticed that gdb would send the wrong vCont packet to qemu
when debugging a Ravenscar program:
> (gdb) thread 2
> [Switching to thread 2 (Thread 1.2)]
> #0 0x0000000000001000 in ?? ()
> (gdb) c
[...]
> Sending packet: $vCont;c:p1.1#e2...Ack
Here, we've switched to thread 2, but the packet says to resume thread
1.
This turned out to be a bug in ravenscar_thread_target::resume, which
did not properly handle the case of a "process" resume. In
particular, the resume method would be passed a ptid of (1, 0, 0) --
but then rewrite this to its saved ptid.
This patch fixes the problem by recognizing this case in the resume
method.
2020-07-08 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target::resume): Handle
"is_pid" case.
---
gdb/ChangeLog | 5 +++++
gdb/ravenscar-thread.c | 7 ++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index a67a5e9a368..91f09209bbc 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -363,7 +363,12 @@ ravenscar_thread_target::resume (ptid_t ptid, int step,
/* If we see a wildcard resume, we simply pass that on. Otherwise,
arrange to resume the base ptid. */
inferior_ptid = m_base_ptid;
- if (ptid != minus_one_ptid)
+ if (ptid.is_pid ())
+ {
+ /* We only have one process, so resume all threads of it. */
+ ptid = minus_one_ptid;
+ }
+ else if (ptid != minus_one_ptid)
ptid = m_base_ptid;
beneath ()->resume (ptid, step, siggnal);
}
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 10/12] Fetch registers from correct thread in ravenscar-thread.c
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (8 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 09/12] Fix Ravenscar "process" resume Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 11/12] Set inferior_ptid in ravenscar_thread_target::update_thread_list Tom Tromey
` (2 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Fabien also noticed that gdb would not report a stop correctly when
using Ravenscar. This patch fixes the bug by making a few changes:
* ravenscar_thread_target::wait now updates the inferior ptid before
updating the thread list. This ensures that a new thread is
correctly associated with the underlying CPU.
* The fetch_registers, store_registers, and prepare_to_store methods
now save and restore the regcache's ptid before doing the operation
on the underlying live thread. This ensures that gdb informs the
remote of a thread it knows about, as opposed to using a Ravenscar
thread, which probably will not be recognized.
2020-07-08 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (ravenscar_thread_target::wait): Call
update_inferior_ptid before update_thread_list.
(temporarily_change_regcache_ptid): New class.
(ravenscar_thread_target::fetch_registers)
(ravenscar_thread_target::store_registers)
(ravenscar_thread_target::prepare_to_store): Use base thread when
forwarding operation.
---
gdb/ChangeLog | 10 +++++
gdb/ravenscar-thread.c | 85 +++++++++++++++++++++++++++++++++---------
2 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 91f09209bbc..459e5ea3ed3 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -457,20 +457,55 @@ ravenscar_thread_target::pid_to_str (ptid_t ptid)
return string_printf ("Ravenscar Thread %#x", (int) ptid.tid ());
}
+/* Temporarily set the ptid of a regcache to some other value. When
+ this object is destroyed, the regcache's original ptid is
+ restored. */
+
+class temporarily_change_regcache_ptid
+{
+public:
+
+ temporarily_change_regcache_ptid (struct regcache *regcache, ptid_t new_ptid)
+ : m_regcache (regcache),
+ m_save_ptid (regcache->ptid ())
+ {
+ m_regcache->set_ptid (new_ptid);
+ }
+
+ ~temporarily_change_regcache_ptid ()
+ {
+ m_regcache->set_ptid (m_save_ptid);
+ }
+
+private:
+
+ /* The regcache. */
+ struct regcache *m_regcache;
+ /* The saved ptid. */
+ ptid_t m_save_ptid;
+};
+
void
ravenscar_thread_target::fetch_registers (struct regcache *regcache, int regnum)
{
ptid_t ptid = regcache->ptid ();
- if (runtime_initialized ()
- && is_ravenscar_task (ptid)
- && !task_is_currently_active (ptid))
+ if (runtime_initialized () && is_ravenscar_task (ptid))
{
- struct gdbarch *gdbarch = regcache->arch ();
- struct ravenscar_arch_ops *arch_ops
- = gdbarch_ravenscar_ops (gdbarch);
+ if (task_is_currently_active (ptid))
+ {
+ ptid_t base = get_base_thread_from_ravenscar_task (ptid);
+ temporarily_change_regcache_ptid changer (regcache, base);
+ beneath ()->fetch_registers (regcache, regnum);
+ }
+ else
+ {
+ struct gdbarch *gdbarch = regcache->arch ();
+ struct ravenscar_arch_ops *arch_ops
+ = gdbarch_ravenscar_ops (gdbarch);
- arch_ops->fetch_registers (regcache, regnum);
+ arch_ops->fetch_registers (regcache, regnum);
+ }
}
else
beneath ()->fetch_registers (regcache, regnum);
@@ -482,15 +517,22 @@ ravenscar_thread_target::store_registers (struct regcache *regcache,
{
ptid_t ptid = regcache->ptid ();
- if (runtime_initialized ()
- && is_ravenscar_task (ptid)
- && !task_is_currently_active (ptid))
+ if (runtime_initialized () && is_ravenscar_task (ptid))
{
- struct gdbarch *gdbarch = regcache->arch ();
- struct ravenscar_arch_ops *arch_ops
- = gdbarch_ravenscar_ops (gdbarch);
+ if (task_is_currently_active (ptid))
+ {
+ ptid_t base = get_base_thread_from_ravenscar_task (ptid);
+ temporarily_change_regcache_ptid changer (regcache, base);
+ beneath ()->store_registers (regcache, regnum);
+ }
+ else
+ {
+ struct gdbarch *gdbarch = regcache->arch ();
+ struct ravenscar_arch_ops *arch_ops
+ = gdbarch_ravenscar_ops (gdbarch);
- arch_ops->store_registers (regcache, regnum);
+ arch_ops->store_registers (regcache, regnum);
+ }
}
else
beneath ()->store_registers (regcache, regnum);
@@ -501,11 +543,18 @@ ravenscar_thread_target::prepare_to_store (struct regcache *regcache)
{
ptid_t ptid = regcache->ptid ();
- if (runtime_initialized ()
- && is_ravenscar_task (ptid)
- && !task_is_currently_active (ptid))
+ if (runtime_initialized () && is_ravenscar_task (ptid))
{
- /* Nothing. */
+ if (task_is_currently_active (ptid))
+ {
+ ptid_t base = get_base_thread_from_ravenscar_task (ptid);
+ temporarily_change_regcache_ptid changer (regcache, base);
+ beneath ()->prepare_to_store (regcache);
+ }
+ else
+ {
+ /* Nothing. */
+ }
}
else
beneath ()->prepare_to_store (regcache);
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 11/12] Set inferior_ptid in ravenscar_thread_target::update_thread_list
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (9 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 10/12] Fetch registers from correct thread in ravenscar-thread.c Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-05 19:08 ` [PATCH v2 12/12] Fix remaining Ravenscar regressions Tom Tromey
2020-08-07 15:03 ` [PATCH v2 00/12] Fix " Joel Brobecker
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Commit 2da4b788f ("Don't write to inferior_ptid in
ravenscar-thread.c") caused a Ravenscar regression (which, FWIW, is
understandable because Ravenscar is difficult to test). Namely,
ravenscar_thread_target::update_thread_list calls
iterate_over_live_ada_tasks, which calls ada_build_task_list, which
uses target_has_stack -- which relies on inferior_ptid.
This patch changes update_thread_list to ensure that inferior_ptid is
set before making this call. This avoids various failures on
Ravenscar targets.
2020-07-08 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c (update_thread_list): Set inferior_ptid.
---
gdb/ChangeLog | 4 ++++
gdb/ravenscar-thread.c | 6 ++++++
2 files changed, 10 insertions(+)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 459e5ea3ed3..37df2188cf3 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -419,6 +419,12 @@ ravenscar_thread_target::add_thread (struct ada_task_info *task)
void
ravenscar_thread_target::update_thread_list ()
{
+ /* iterate_over_live_ada_tasks requires that inferior_ptid be set,
+ but this isn't always the case in target methods. So, we ensure
+ it here. */
+ scoped_restore save_ptid = make_scoped_restore (&inferior_ptid,
+ m_base_ptid);
+
/* Do not clear the thread list before adding the Ada task, to keep
the thread that the process stratum has included into it
(m_base_ptid) and the running thread, that may not have been included
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 12/12] Fix remaining Ravenscar regressions
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (10 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 11/12] Set inferior_ptid in ravenscar_thread_target::update_thread_list Tom Tromey
@ 2020-08-05 19:08 ` Tom Tromey
2020-08-07 15:03 ` [PATCH v2 00/12] Fix " Joel Brobecker
12 siblings, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-05 19:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Testing showed a few more Ravenscar regressions arising from upstream.
In particular, gdb now uses the current thread in some places where
inferior_ptid was previously used. This patch fixes the problem by
arranging to save and restore the thread now.
gdb/ChangeLog
2020-08-05 Tom Tromey <tromey@adacore.com>
* ravenscar-thread.c
(ravenscar_thread_target::set_base_thread_from_ravenscar_task):
New method.
(ravenscar_thread_target::wait): Check
runtime_initialized.
(ravenscar_thread_target::prepare_to_store)
(ravenscar_thread_target::stopped_by_sw_breakpoint)
(ravenscar_thread_target::stopped_by_hw_breakpoint)
(ravenscar_thread_target::stopped_by_watchpoint)
(ravenscar_thread_target::stopped_data_address)
(ravenscar_thread_target::core_of_thread): Use
scoped_restore_current_thread and
set_base_thread_from_ravenscar_task.
---
gdb/ChangeLog | 16 ++++++++++++++++
gdb/ravenscar-thread.c | 29 +++++++++++++++++++----------
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 37df2188cf3..387ebcb32e9 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -151,6 +151,15 @@ struct ravenscar_thread_target final : public target_ops
ptid_t get_base_thread_from_ravenscar_task (ptid_t ptid);
void add_thread (struct ada_task_info *task);
+ /* Like switch_to_thread, but uses the base ptid for the thread. */
+ void set_base_thread_from_ravenscar_task (ptid_t ptid)
+ {
+ process_stratum_target *proc_target
+ = as_process_stratum_target (this->beneath ());
+ ptid_t underlying = get_base_thread_from_ravenscar_task (ptid);
+ switch_to_thread (find_thread_ptid (proc_target, underlying));
+ }
+
/* This maps a TID to the CPU on which it was running. This is
needed because sometimes the runtime will report an active task
that hasn't yet been put on the list of tasks that is read by
@@ -571,8 +580,8 @@ ravenscar_thread_target::prepare_to_store (struct regcache *regcache)
bool
ravenscar_thread_target::stopped_by_sw_breakpoint ()
{
- scoped_restore save_ptid = make_scoped_restore (&inferior_ptid);
- inferior_ptid = get_base_thread_from_ravenscar_task (inferior_ptid);
+ scoped_restore_current_thread saver;
+ set_base_thread_from_ravenscar_task (inferior_ptid);
return beneath ()->stopped_by_sw_breakpoint ();
}
@@ -581,8 +590,8 @@ ravenscar_thread_target::stopped_by_sw_breakpoint ()
bool
ravenscar_thread_target::stopped_by_hw_breakpoint ()
{
- scoped_restore save_ptid = make_scoped_restore (&inferior_ptid);
- inferior_ptid = get_base_thread_from_ravenscar_task (inferior_ptid);
+ scoped_restore_current_thread saver;
+ set_base_thread_from_ravenscar_task (inferior_ptid);
return beneath ()->stopped_by_hw_breakpoint ();
}
@@ -591,8 +600,8 @@ ravenscar_thread_target::stopped_by_hw_breakpoint ()
bool
ravenscar_thread_target::stopped_by_watchpoint ()
{
- scoped_restore save_ptid = make_scoped_restore (&inferior_ptid);
- inferior_ptid = get_base_thread_from_ravenscar_task (inferior_ptid);
+ scoped_restore_current_thread saver;
+ set_base_thread_from_ravenscar_task (inferior_ptid);
return beneath ()->stopped_by_watchpoint ();
}
@@ -601,8 +610,8 @@ ravenscar_thread_target::stopped_by_watchpoint ()
bool
ravenscar_thread_target::stopped_data_address (CORE_ADDR *addr_p)
{
- scoped_restore save_ptid = make_scoped_restore (&inferior_ptid);
- inferior_ptid = get_base_thread_from_ravenscar_task (inferior_ptid);
+ scoped_restore_current_thread saver;
+ set_base_thread_from_ravenscar_task (inferior_ptid);
return beneath ()->stopped_data_address (addr_p);
}
@@ -620,8 +629,8 @@ ravenscar_thread_target::mourn_inferior ()
int
ravenscar_thread_target::core_of_thread (ptid_t ptid)
{
- scoped_restore save_ptid = make_scoped_restore (&inferior_ptid);
- inferior_ptid = get_base_thread_from_ravenscar_task (inferior_ptid);
+ scoped_restore_current_thread saver;
+ set_base_thread_from_ravenscar_task (inferior_ptid);
return beneath ()->core_of_thread (inferior_ptid);
}
--
2.26.2
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 00/12] Fix Ravenscar regressions
2020-08-05 19:08 [PATCH v2 00/12] Fix Ravenscar regressions Tom Tromey
` (11 preceding siblings ...)
2020-08-05 19:08 ` [PATCH v2 12/12] Fix remaining Ravenscar regressions Tom Tromey
@ 2020-08-07 15:03 ` Joel Brobecker
2020-08-07 16:26 ` Tom Tromey
2020-08-07 17:11 ` Eli Zaretskii
12 siblings, 2 replies; 16+ messages in thread
From: Joel Brobecker @ 2020-08-07 15:03 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi Tom,
> Some changes in gdb caused regressions in AdaCore's internal Ravenscar
> testing. This is understandable, because a Ravenscar environment is a
> bit of a pain to set up, and so I don't think it's tested in ordinary
> gdb testing.
>
> This series is v2 of some fixes to Ravenscar. A few more fixes have
> been added since the last series, to cope with new regressions. I've
> also dropped the "random thread switch" patch from the series. I hope
> we can discuss that one again sometime, but meanwhile I'd rather get
> these changes in.
I can't approve the documentation bits, but the rest of the patch
series looks good to me.
Thank you!
--
Joel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 00/12] Fix Ravenscar regressions
2020-08-07 15:03 ` [PATCH v2 00/12] Fix " Joel Brobecker
@ 2020-08-07 16:26 ` Tom Tromey
2020-08-07 17:11 ` Eli Zaretskii
1 sibling, 0 replies; 16+ messages in thread
From: Tom Tromey @ 2020-08-07 16:26 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches
>> This series is v2 of some fixes to Ravenscar. A few more fixes have
>> been added since the last series, to cope with new regressions. I've
>> also dropped the "random thread switch" patch from the series. I hope
>> we can discuss that one again sometime, but meanwhile I'd rather get
>> these changes in.
Joel> I can't approve the documentation bits, but the rest of the patch
Joel> series looks good to me.
Thanks. The documentation patch was actually approved here (with one
change, which is included in this update):
https://sourceware.org/pipermail/gdb-patches/2019-March/156463.html
So, I am going to check in this series.
Tom
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 00/12] Fix Ravenscar regressions
2020-08-07 15:03 ` [PATCH v2 00/12] Fix " Joel Brobecker
2020-08-07 16:26 ` Tom Tromey
@ 2020-08-07 17:11 ` Eli Zaretskii
1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2020-08-07 17:11 UTC (permalink / raw)
To: Joel Brobecker; +Cc: tromey, gdb-patches
> Date: Fri, 7 Aug 2020 08:03:12 -0700
> From: Joel Brobecker <brobecker@adacore.com>
>
> I can't approve the documentation bits, but the rest of the patch
> series looks good to me.
The documentation part is also OK.
Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread