public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/2] [gdb/build] Return const reference in thread_info_to_thread_handle
Date: Mon, 21 Aug 2023 12:53:56 +0200	[thread overview]
Message-ID: <20230821105356.869-2-tdevries@suse.de> (raw)
In-Reply-To: <20230821105356.869-1-tdevries@suse.de>

In remote_target::thread_info_to_thread_handle we return a copy:
...
gdb::byte_vector
remote_target::thread_info_to_thread_handle (struct thread_info *tp)
{
  remote_thread_info *priv = get_remote_thread_info (tp);
  return priv->thread_handle;
}
...

Fix this by returning a const reference instead:
...
const gdb::optional<gdb::byte_vector> &
remote_target::thread_info_to_thread_handle (struct thread_info *tp)
...

Returning a gdb::optional allows us to return a nullptr, or std::nullopt in
std::optional terms, something that is required by
thread_db_target::thread_info_to_thread_handle.

In gdb we use gdb::optional instead std::optional, because std::optional is
availabe starting c++17 and we support c++11 and c++14, but gdb::nullopt is
currently not available, though a submission is available [1].

So we use a kludge gdb_optional_byte_vector_nullopt.

Tested on x86_64-linux.

This fixes the build when building with -std=c++20.

[1] https://sourceware.org/pipermail/gdb-patches/2020-May/168800.html
---
 gdb/linux-thread-db.c  | 12 +++++++-----
 gdb/remote.c           | 14 +++++++-------
 gdb/target-debug.h     |  8 ++++++++
 gdb/target-delegates.c | 16 ++++++++--------
 gdb/target.c           | 15 +++++++++++++--
 gdb/target.h           |  9 ++++++---
 6 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 7d9fd57da0c..f238b26445b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -107,7 +107,8 @@ class thread_db_target final : public target_ops
   thread_info *thread_handle_to_thread_info (const gdb_byte *thread_handle,
 					     int handle_len,
 					     inferior *inf) override;
-  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *) override;
+  const gdb::optional<gdb::byte_vector> &
+    thread_info_to_thread_handle (struct thread_info *) override;
 };
 
 static std::string libthread_db_search_path = LIBTHREAD_DB_SEARCH_PATH;
@@ -312,6 +313,7 @@ struct thread_db_thread_info : public private_thread_info
   /* Cached thread state.  */
   td_thrhandle_t th {};
   thread_t tid {};
+  gdb::optional<gdb::byte_vector> thread_handle;
 };
 
 static thread_db_thread_info *
@@ -1724,20 +1726,20 @@ thread_db_target::thread_handle_to_thread_info (const gdb_byte *thread_handle,
 
 /* Return the thread handle associated the thread_info pointer TP.  */
 
-gdb::byte_vector
+const gdb::optional<gdb::byte_vector> &
 thread_db_target::thread_info_to_thread_handle (struct thread_info *tp)
 {
   thread_db_thread_info *priv = get_thread_db_thread_info (tp);
 
   if (priv == NULL)
-    return gdb::byte_vector ();
+    return gdb_optional_byte_vector_nullopt ();
 
   int handle_size = sizeof (priv->tid);
   gdb::byte_vector rv (handle_size);
 
   memcpy (rv.data (), &priv->tid, handle_size);
-
-  return rv;
+  priv->thread_handle.emplace (std::move (rv));
+  return priv->thread_handle;
 }
 
 /* Get the address of the thread local variable in load module LM which
diff --git a/gdb/remote.c b/gdb/remote.c
index 7abe08439b5..a3650bc7003 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -784,8 +784,8 @@ class remote_target : public process_stratum_target
 					     int handle_len,
 					     inferior *inf) override;
 
-  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *tp)
-						 override;
+  const gdb::optional<gdb::byte_vector> &
+    thread_info_to_thread_handle (struct thread_info *tp) override;
 
   void stop (ptid_t) override;
 
@@ -1452,7 +1452,7 @@ struct remote_thread_info : public private_thread_info
 
   /* Thread handle, perhaps a pthread_t or thread_t value, stored as a
      sequence of bytes.  */
-  gdb::byte_vector thread_handle;
+  gdb::optional<gdb::byte_vector> thread_handle;
 
   /* Whether the target stopped for a breakpoint/watchpoint.  */
   enum target_stop_reason stop_reason = TARGET_STOPPED_BY_NO_REASON;
@@ -14538,10 +14538,10 @@ remote_target::thread_handle_to_thread_info (const gdb_byte *thread_handle,
 
       if (tp->inf == inf && priv != NULL)
 	{
-	  if (handle_len != priv->thread_handle.size ())
+	  if (handle_len != priv->thread_handle->size ())
 	    error (_("Thread handle size mismatch: %d vs %zu (from remote)"),
-		   handle_len, priv->thread_handle.size ());
-	  if (memcmp (thread_handle, priv->thread_handle.data (),
+		   handle_len, priv->thread_handle->size ());
+	  if (memcmp (thread_handle, priv->thread_handle->data (),
 		      handle_len) == 0)
 	    return tp;
 	}
@@ -14550,7 +14550,7 @@ remote_target::thread_handle_to_thread_info (const gdb_byte *thread_handle,
   return NULL;
 }
 
-gdb::byte_vector
+const gdb::optional<gdb::byte_vector> &
 remote_target::thread_info_to_thread_handle (struct thread_info *tp)
 {
   remote_thread_info *priv = get_remote_thread_info (tp);
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index acb01d47e7c..9afef943fab 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -236,4 +236,12 @@ target_debug_print_gdb_byte_vector_r (gdb::byte_vector &vector)
 {
   target_debug_print_const_gdb_byte_vector_r (vector);
 }
+
+static void
+target_debug_print_const_gdb_optional_gdb_byte_vector_r (const gdb::optional<gdb::byte_vector> &vector)
+{
+  if (!vector.has_value ())
+    return;
+  target_debug_print_const_gdb_byte_vector_r (*vector);
+}
 #endif /* TARGET_DEBUG_H */
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 06f22d57c27..fe293142a23 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -89,7 +89,7 @@ struct dummy_target : public target_ops
   const char *extra_thread_info (thread_info *arg0) override;
   const char *thread_name (thread_info *arg0) override;
   thread_info *thread_handle_to_thread_info (const gdb_byte *arg0, int arg1, inferior *arg2) override;
-  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *arg0) override;
+  const gdb::optional<gdb::byte_vector> & thread_info_to_thread_handle (struct thread_info *arg0) override;
   void stop (ptid_t arg0) override;
   void interrupt () override;
   void pass_ctrlc () override;
@@ -263,7 +263,7 @@ struct debug_target : public target_ops
   const char *extra_thread_info (thread_info *arg0) override;
   const char *thread_name (thread_info *arg0) override;
   thread_info *thread_handle_to_thread_info (const gdb_byte *arg0, int arg1, inferior *arg2) override;
-  gdb::byte_vector thread_info_to_thread_handle (struct thread_info *arg0) override;
+  const gdb::optional<gdb::byte_vector> & thread_info_to_thread_handle (struct thread_info *arg0) override;
   void stop (ptid_t arg0) override;
   void interrupt () override;
   void pass_ctrlc () override;
@@ -1871,28 +1871,28 @@ debug_target::thread_handle_to_thread_info (const gdb_byte *arg0, int arg1, infe
   return result;
 }
 
-gdb::byte_vector
+const gdb::optional<gdb::byte_vector> &
 target_ops::thread_info_to_thread_handle (struct thread_info *arg0)
 {
   return this->beneath ()->thread_info_to_thread_handle (arg0);
 }
 
-gdb::byte_vector
+const gdb::optional<gdb::byte_vector> &
 dummy_target::thread_info_to_thread_handle (struct thread_info *arg0)
 {
-  return gdb::byte_vector ();
+  return gdb_optional_byte_vector_nullopt ();
 }
 
-gdb::byte_vector
+const gdb::optional<gdb::byte_vector> &
 debug_target::thread_info_to_thread_handle (struct thread_info *arg0)
 {
   gdb_printf (gdb_stdlog, "-> %s->thread_info_to_thread_handle (...)\n", this->beneath ()->shortname ());
-  gdb::byte_vector result
+  const gdb::optional<gdb::byte_vector> & result
     = this->beneath ()->thread_info_to_thread_handle (arg0);
   gdb_printf (gdb_stdlog, "<- %s->thread_info_to_thread_handle (", this->beneath ()->shortname ());
   target_debug_print_struct_thread_info_p (arg0);
   gdb_puts (") = ", gdb_stdlog);
-  target_debug_print_gdb_byte_vector (result);
+  target_debug_print_const_gdb_optional_gdb_byte_vector_r (result);
   gdb_puts ("\n", gdb_stdlog);
   return result;
 }
diff --git a/gdb/target.c b/gdb/target.c
index 16f43d072cd..32383ad69b0 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -55,6 +55,17 @@
 #include "valprint.h"
 #include "cli/cli-decode.h"
 
+/* See target.h.  */
+
+const gdb::optional<gdb::byte_vector> &
+gdb_optional_byte_vector_nullopt ()
+{
+  /* For a generic solution, see this submission (
+     https://sourceware.org/pipermail/gdb-patches/2020-May/168800.html ). */
+  static const gdb::optional<gdb::byte_vector> gdb_optional_byte_vector_nullopt = {};
+  return gdb_optional_byte_vector_nullopt;
+}
+
 static void generic_tls_error (void) ATTRIBUTE_NORETURN;
 
 static void default_terminal_info (struct target_ops *, const char *, int);
@@ -2640,12 +2651,12 @@ target_thread_handle_to_thread_info (const gdb_byte *thread_handle,
 
 /* See target.h.  */
 
-gdb::byte_vector
+const gdb::byte_vector &
 target_thread_info_to_thread_handle (struct thread_info *tip)
 {
   target_ops *target = current_inferior ()->top_target ();
 
-  return target->thread_info_to_thread_handle (tip);
+  return *target->thread_info_to_thread_handle (tip);
 }
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index 6ae400e2cc2..82d474881d1 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -679,8 +679,8 @@ struct target_ops
 						       inferior *inf)
       TARGET_DEFAULT_RETURN (NULL);
     /* See target_thread_info_to_thread_handle.  */
-    virtual gdb::byte_vector thread_info_to_thread_handle (struct thread_info *)
-      TARGET_DEFAULT_RETURN (gdb::byte_vector ());
+    virtual const gdb::optional<gdb::byte_vector> &thread_info_to_thread_handle (struct thread_info *)
+      TARGET_DEFAULT_RETURN (gdb_optional_byte_vector_nullopt ());
     virtual void stop (ptid_t)
       TARGET_DEFAULT_IGNORE ();
     virtual void interrupt ()
@@ -1924,7 +1924,7 @@ extern struct thread_info *target_thread_handle_to_thread_info
 /* Given a thread, return the thread handle, a target-specific sequence of
    bytes which serves as a thread identifier within the program being
    debugged.  */
-extern gdb::byte_vector target_thread_info_to_thread_handle
+extern const gdb::byte_vector &target_thread_info_to_thread_handle
   (struct thread_info *);
 
 /* Attempts to find the pathname of the executable file
@@ -2558,4 +2558,7 @@ extern void target_prepare_to_generate_core (void);
 /* See to_done_generating_core.  */
 extern void target_done_generating_core (void);
 
+/* Type specific nullopt, gdb_optional.h has no support for gdb::nullopt.  */
+extern const gdb::optional<gdb::byte_vector> &gdb_optional_byte_vector_nullopt ();
+
 #endif /* !defined (TARGET_H) */
-- 
2.35.3


  reply	other threads:[~2023-08-21 10:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-21 10:53 [PATCH 1/2] [gdb/build] Support reference return type in make-target-delegates.py Tom de Vries
2023-08-21 10:53 ` Tom de Vries [this message]
2023-08-22 14:43   ` [PATCH 2/2] [gdb/build] Return const reference in thread_info_to_thread_handle Pedro Alves
2023-08-22 17:34     ` Tom de Vries
2023-08-23 15:43       ` Pedro Alves
2023-08-24  6:21         ` Tom de Vries
2023-08-24 11:37           ` Pedro Alves
2023-08-23 15:43 ` [PATCH 1/2] [gdb/build] Support reference return type in make-target-delegates.py Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230821105356.869-2-tdevries@suse.de \
    --to=tdevries@suse.de \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).