[PATCH] Make target_read_alloc & al return vectors

[PATCH] Make target_read_alloc & al return vectors

[Simon Marchi]

This patch started by changing target_read_alloc_1 to return a
byte_vector, to avoid manual memory management (in target_read_alloc_1
and in the callers).  To communicate failures to the callers, it
actually returns a gdb::optional<gdb::byte_vector>.

Adjusting target_read_stralloc was a bit more tricky, since it wants to
return a buffer of char, and not gdb_byte.  Since you can't just cast a
gdb::byte_vector into a gdb::def_vector<char>, I made
target_read_alloc_1 templated, so both versions (that return vectors of
gdb_byte and char) are generated.  Since target_read_stralloc now
returns a gdb::char_vector instead of a gdb::unique_xmalloc_ptr<char>, a
few callers need to be adjusted.

gdb/ChangeLog:

	* common/byte-vector.h (char_vector): New type.
	* target.h (target_read_alloc): Return
	gdb::optional<byte_vector>.
	(target_read_stralloc): Return gdb::optional<char_vector>.
	(target_get_osdata): Return gdb::optional<char_vector>.
	* target.c (target_read_alloc_1): Templatize.  Replacement
	manual memory management with vector.
	(target_read_alloc): Change return type, adjust.
	(target_read_stralloc): Change return type, adjust.
	(target_get_osdata): Change return type, adjust.
	* auxv.c (struct auxv_info) <length>: Remove.
	<data>: Change type to gdb::optional<byte_vector>.
	(auxv_inferior_data_cleanup): Free auxv_info with delete.
	(get_auxv_inferior_data): Allocate auxv_info with new, adjust.
	(target_auxv_search): Adjust.
	(fprint_target_auxv): Adjust.
	* avr-tdep.c (avr_io_reg_read_command): Adjust.
	* linux-tdep.c (linux_spu_make_corefile_notes): Adjust.
	(linux_make_corefile_notes): Adjust.
	* osdata.c (get_osdata): Adjust.
	* remote.c (remote_get_threads_with_qxfer): Adjust.
	(remote_memory_map): Adjust.
	(remote_traceframe_info): Adjust.
	(btrace_read_config): Adjust.
	(remote_read_btrace): Adjust.
	(remote_pid_to_exec_file): Adjust.
	* solib-aix.c (solib_aix_get_library_list): Adjust.
	* solib-dsbt.c (decode_loadmap): Don't free buf.
	(dsbt_get_initial_loadmaps): Adjust.
	* solib-svr4.c (svr4_current_sos_via_xfer_libraries): Adjust.
	* solib-target.c (solib_target_current_sos): Adjust.
	* tracepoint.c (sdata_make_value): Adjust.
	* xml-support.c (xinclude_start_include): Adjust.
	(xml_fetch_content_from_file): Adjust.
	* xml-support.h (xml_fetch_another): Change return type.
	(xml_fetch_content_from_file): Change return type.
	* xml-syscall.c (xml_init_syscalls_info): Adjust.
	* xml-tdesc.c (file_read_description_xml): Adjust.
	(fetch_available_features_from_target): Change return type.
	(target_fetch_description_xml): Adjust.
	(target_read_description_xml): Adjust.
---
 gdb/auxv.c               | 44 ++++++++++---------------
 gdb/avr-tdep.c           | 40 +++++++++++-----------
 gdb/common/byte-vector.h |  1 +
 gdb/linux-tdep.c         | 48 +++++++++++----------------
 gdb/osdata.c             |  6 ++--
 gdb/remote.c             | 32 +++++++++---------
 gdb/solib-aix.c          |  8 ++---
 gdb/solib-dsbt.c         | 19 +++++------
 gdb/solib-svr4.c         |  6 ++--
 gdb/solib-target.c       |  6 ++--
 gdb/target.c             | 86 +++++++++++++++++++-----------------------------
 gdb/target.h             | 37 +++++++++------------
 gdb/tracepoint.c         | 16 ++++-----
 gdb/xml-support.c        | 20 +++++------
 gdb/xml-support.h        | 10 +++---
 gdb/xml-syscall.c        |  6 ++--
 gdb/xml-tdesc.c          | 20 +++++------
 17 files changed, 180 insertions(+), 225 deletions(-)

diff --git a/gdb/auxv.c b/gdb/auxv.c
index 26cc63e..ce7a63b 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -304,8 +304,7 @@ static const struct inferior_data *auxv_inferior_data;
     overhead of transfering data from a remote target to the local host.  */
 struct auxv_info
 {
-  LONGEST length;
-  gdb_byte *data;
+  gdb::optional<gdb::byte_vector> data;
 };
 
 /* Handles the cleanup of the auxv cache for inferior INF.  ARG is ignored.
@@ -323,8 +322,7 @@ auxv_inferior_data_cleanup (struct inferior *inf, void *arg)
   info = (struct auxv_info *) inferior_data (inf, auxv_inferior_data);
   if (info != NULL)
     {
-      xfree (info->data);
-      xfree (info);
+      delete info;
       set_inferior_data (inf, auxv_inferior_data, NULL);
     }
 }
@@ -358,9 +356,8 @@ get_auxv_inferior_data (struct target_ops *ops)
   info = (struct auxv_info *) inferior_data (inf, auxv_inferior_data);
   if (info == NULL)
     {
-      info = XCNEW (struct auxv_info);
-      info->length = target_read_alloc (ops, TARGET_OBJECT_AUXV,
-					NULL, &info->data);
+      info = new auxv_info;
+      info->data = target_read_alloc (ops, TARGET_OBJECT_AUXV, NULL);
       set_inferior_data (inf, auxv_inferior_data, info);
     }
 
@@ -375,20 +372,17 @@ int
 target_auxv_search (struct target_ops *ops, CORE_ADDR match, CORE_ADDR *valp)
 {
   CORE_ADDR type, val;
-  gdb_byte *data;
-  gdb_byte *ptr;
-  struct auxv_info *info;
-
-  info = get_auxv_inferior_data (ops);
+  auxv_info *info = get_auxv_inferior_data (ops);
 
-  data = info->data;
-  ptr = data;
+  if (!info->data)
+    return -1;
 
-  if (info->length <= 0)
-    return info->length;
+  gdb_byte *data = info->data->data ();
+  gdb_byte *ptr = data;
+  size_t len = info->data->size ();
 
   while (1)
-    switch (target_auxv_parse (ops, &ptr, data + info->length, &type, &val))
+    switch (target_auxv_parse (ops, &ptr, data + len, &type, &val))
       {
       case 1:			/* Here's an entry, check it.  */
 	if (type == match)
@@ -528,19 +522,17 @@ fprint_target_auxv (struct ui_file *file, struct target_ops *ops)
 {
   struct gdbarch *gdbarch = target_gdbarch ();
   CORE_ADDR type, val;
-  gdb_byte *data;
-  gdb_byte *ptr;
-  struct auxv_info *info;
   int ents = 0;
+  auxv_info *info = get_auxv_inferior_data (ops);
 
-  info = get_auxv_inferior_data (ops);
+  if (!info->data)
+    return -1;
 
-  data = info->data;
-  ptr = data;
-  if (info->length <= 0)
-    return info->length;
+  gdb_byte *data = info->data->data ();
+  gdb_byte *ptr = data;
+  size_t len = info->data->size ();
 
-  while (target_auxv_parse (ops, &ptr, data + info->length, &type, &val) > 0)
+  while (target_auxv_parse (ops, &ptr, data + len, &type, &val) > 0)
     {
       gdbarch_print_auxv_entry (gdbarch, file, type, val);
       ++ents;
diff --git a/gdb/avr-tdep.c b/gdb/avr-tdep.c
index c44a3aa..2b9be5e 100644
--- a/gdb/avr-tdep.c
+++ b/gdb/avr-tdep.c
@@ -1549,21 +1549,15 @@ avr_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 static void
 avr_io_reg_read_command (const char *args, int from_tty)
 {
-  LONGEST bufsiz = 0;
-  gdb_byte *buf;
-  const char *bufstr;
   char query[400];
-  const char *p;
   unsigned int nreg = 0;
   unsigned int val;
-  int i, j, k, step;
 
   /* Find out how many io registers the target has.  */
-  bufsiz = target_read_alloc (&current_target, TARGET_OBJECT_AVR,
-			      "avr.io_reg", &buf);
-  bufstr = (const char *) buf;
+  gdb::optional<gdb::byte_vector> buf
+    = target_read_alloc (&current_target, TARGET_OBJECT_AVR, "avr.io_reg");
 
-  if (bufsiz <= 0)
+  if (!buf)
     {
       fprintf_unfiltered (gdb_stderr,
 			  _("ERR: info io_registers NOT supported "
@@ -1571,36 +1565,42 @@ avr_io_reg_read_command (const char *args, int from_tty)
       return;
     }
 
+  const char *bufstr = (const char *) buf->data ();
+
   if (sscanf (bufstr, "%x", &nreg) != 1)
     {
       fprintf_unfiltered (gdb_stderr,
 			  _("Error fetching number of io registers\n"));
-      xfree (buf);
       return;
     }
 
-  xfree (buf);
-
   reinitialize_more_filter ();
 
   printf_unfiltered (_("Target has %u io registers:\n\n"), nreg);
 
   /* only fetch up to 8 registers at a time to keep the buffer small */
-  step = 8;
+  int step = 8;
 
-  for (i = 0; i < nreg; i += step)
+  for (int i = 0; i < nreg; i += step)
     {
       /* how many registers this round? */
-      j = step;
+      int j = step;
       if ((i+j) >= nreg)
         j = nreg - i;           /* last block is less than 8 registers */
 
       snprintf (query, sizeof (query) - 1, "avr.io_reg:%x,%x", i, j);
-      bufsiz = target_read_alloc (&current_target, TARGET_OBJECT_AVR,
-				  query, &buf);
+      buf = target_read_alloc (&current_target, TARGET_OBJECT_AVR, query);
+
+      if (!buf)
+        {
+          fprintf_unfiltered (gdb_stderr,
+			      _("ERR: error reading avr.io_reg:%x,%x\n"),
+			      i, j);
+          return;
+        }
 
-      p = (const char *) buf;
-      for (k = i; k < (i + j); k++)
+      const char *p = (const char *) buf->data ();
+      for (int k = i; k < (i + j); k++)
 	{
 	  if (sscanf (p, "%[^,],%x;", query, &val) == 2)
 	    {
@@ -1612,8 +1612,6 @@ avr_io_reg_read_command (const char *args, int from_tty)
 		break;
 	    }
 	}
-
-      xfree (buf);
     }
 }
 
diff --git a/gdb/common/byte-vector.h b/gdb/common/byte-vector.h
index 03c4eb8..22a67b8 100644
--- a/gdb/common/byte-vector.h
+++ b/gdb/common/byte-vector.h
@@ -56,6 +56,7 @@ namespace gdb {
    and providing the whole std::vector API, if you end up needing it.
 */
 using byte_vector = gdb::def_vector<gdb_byte>;
+using char_vector = gdb::def_vector<char>;
 
 } /* namespace gdb */
 
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index b4b87dd..7ea56f0 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1408,47 +1408,41 @@ linux_spu_make_corefile_notes (bfd *obfd, char *note_data, int *note_size)
    };
 
   enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-  gdb_byte *spu_ids;
-  LONGEST i, j, size;
 
   /* Determine list of SPU ids.  */
-  size = target_read_alloc (&current_target, TARGET_OBJECT_SPU,
-			    NULL, &spu_ids);
+  gdb::optional<gdb::byte_vector>
+    spu_ids = target_read_alloc (&current_target, TARGET_OBJECT_SPU, NULL);
+
+  if (!spu_ids)
+    return nullptr;
 
   /* Generate corefile notes for each SPU file.  */
-  for (i = 0; i < size; i += 4)
+  for (size_t i = 0; i < spu_ids->size (); i += 4)
     {
-      int fd = extract_unsigned_integer (spu_ids + i, 4, byte_order);
+      int fd = extract_unsigned_integer (spu_ids->data () + i, 4, byte_order);
 
-      for (j = 0; j < sizeof (spu_files) / sizeof (spu_files[0]); j++)
+      for (size_t j = 0; j < sizeof (spu_files) / sizeof (spu_files[0]); j++)
 	{
 	  char annex[32], note_name[32];
-	  gdb_byte *spu_data;
-	  LONGEST spu_len;
 
 	  xsnprintf (annex, sizeof annex, "%d/%s", fd, spu_files[j]);
-	  spu_len = target_read_alloc (&current_target, TARGET_OBJECT_SPU,
-				       annex, &spu_data);
-	  if (spu_len > 0)
+	  gdb::optional<gdb::byte_vector> spu_data
+	    = target_read_alloc (&current_target, TARGET_OBJECT_SPU, annex);
+
+	  if (spu_data && !spu_data->empty ())
 	    {
 	      xsnprintf (note_name, sizeof note_name, "SPU/%s", annex);
 	      note_data = elfcore_write_note (obfd, note_data, note_size,
 					      note_name, NT_SPU,
-					      spu_data, spu_len);
-	      xfree (spu_data);
+					      spu_data->data (),
+					      spu_data->size ());
 
 	      if (!note_data)
-		{
-		  xfree (spu_ids);
-		  return NULL;
-		}
+		return nullptr;
 	    }
 	}
     }
 
-  if (size > 0)
-    xfree (spu_ids);
-
   return note_data;
 }
 
@@ -1899,8 +1893,6 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
   struct linux_corefile_thread_data thread_args;
   struct elf_internal_linux_prpsinfo prpsinfo;
   char *note_data = NULL;
-  gdb_byte *auxv;
-  int auxv_len;
   struct thread_info *curr_thr, *signalled_thr, *thr;
 
   if (! gdbarch_iterate_over_regset_sections_p (gdbarch))
@@ -1965,13 +1957,13 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
     return NULL;
 
   /* Auxillary vector.  */
-  auxv_len = target_read_alloc (&current_target, TARGET_OBJECT_AUXV,
-				NULL, &auxv);
-  if (auxv_len > 0)
+  gdb::optional<gdb::byte_vector> auxv =
+    target_read_alloc (&current_target, TARGET_OBJECT_AUXV, NULL);
+  if (auxv && !auxv->empty ())
     {
       note_data = elfcore_write_note (obfd, note_data, note_size,
-				      "CORE", NT_AUXV, auxv, auxv_len);
-      xfree (auxv);
+				      "CORE", NT_AUXV, auxv->data (),
+				      auxv->size ());
 
       if (!note_data)
 	return NULL;
diff --git a/gdb/osdata.c b/gdb/osdata.c
index 5c41cb1..50548f7 100644
--- a/gdb/osdata.c
+++ b/gdb/osdata.c
@@ -163,11 +163,11 @@ std::unique_ptr<osdata>
 get_osdata (const char *type)
 {
   std::unique_ptr<osdata> osdata;
-  gdb::unique_xmalloc_ptr<char> xml = target_get_osdata (type);
+  gdb::optional<gdb::char_vector> xml = target_get_osdata (type);
 
   if (xml)
     {
-      if (xml.get ()[0] == '\0')
+      if ((*xml)[0] == '\0')
 	{
 	  if (type)
 	    warning (_("Empty data returned by target.  Wrong osdata type?"));
@@ -175,7 +175,7 @@ get_osdata (const char *type)
 	    warning (_("Empty type list returned by target.  No type data?"));
 	}
       else
-	osdata = osdata_parse (xml.get ());
+	osdata = osdata_parse (xml->data ());
     }
 
   if (osdata == NULL)
diff --git a/gdb/remote.c b/gdb/remote.c
index 501c443..649e3d3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3151,13 +3151,13 @@ remote_get_threads_with_qxfer (struct target_ops *ops,
 #if defined(HAVE_LIBEXPAT)
   if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
     {
-      gdb::unique_xmalloc_ptr<char> xml
+      gdb::optional<gdb::char_vector> xml
 	= target_read_stralloc (ops, TARGET_OBJECT_THREADS, NULL);
 
-      if (xml != NULL && *xml != '\0')
+      if (xml && (*xml)[0] != '\0')
 	{
 	  gdb_xml_parse_quick (_("threads"), "threads.dtd",
-			       threads_elements, xml.get (), context);
+			       threads_elements, xml->data (), context);
 	}
 
       return 1;
@@ -10806,11 +10806,11 @@ static std::vector<mem_region>
 remote_memory_map (struct target_ops *ops)
 {
   std::vector<mem_region> result;
-  gdb::unique_xmalloc_ptr<char> text
+  gdb::optional<gdb::char_vector> text
     = target_read_stralloc (&current_target, TARGET_OBJECT_MEMORY_MAP, NULL);
 
   if (text)
-    result = parse_memory_map (text.get ());
+    result = parse_memory_map (text->data ());
 
   return result;
 }
@@ -12921,11 +12921,11 @@ remote_set_circular_trace_buffer (struct target_ops *self, int val)
 static traceframe_info_up
 remote_traceframe_info (struct target_ops *self)
 {
-  gdb::unique_xmalloc_ptr<char> text
+  gdb::optional<gdb::char_vector> text
     = target_read_stralloc (&current_target, TARGET_OBJECT_TRACEFRAME_INFO,
 			    NULL);
-  if (text != NULL)
-    return parse_traceframe_info (text.get ());
+  if (text)
+    return parse_traceframe_info (text->data ());
 
   return NULL;
 }
@@ -13153,10 +13153,10 @@ btrace_sync_conf (const struct btrace_config *conf)
 static void
 btrace_read_config (struct btrace_config *conf)
 {
-  gdb::unique_xmalloc_ptr<char> xml
+  gdb::optional<gdb::char_vector> xml
     = target_read_stralloc (&current_target, TARGET_OBJECT_BTRACE_CONF, "");
-  if (xml != NULL)
-    parse_xml_btrace_conf (conf, xml.get ());
+  if (xml)
+    parse_xml_btrace_conf (conf, xml->data ());
 }
 
 /* Maybe reopen target btrace.  */
@@ -13353,12 +13353,12 @@ remote_read_btrace (struct target_ops *self,
 		      (unsigned int) type);
     }
 
-  gdb::unique_xmalloc_ptr<char> xml
+  gdb::optional<gdb::char_vector> xml
     = target_read_stralloc (&current_target, TARGET_OBJECT_BTRACE, annex);
-  if (xml == NULL)
+  if (!xml)
     return BTRACE_ERR_UNKNOWN;
 
-  parse_xml_btrace (btrace, xml.get ());
+  parse_xml_btrace (btrace, xml->data ());
 
   return BTRACE_ERR_NONE;
 }
@@ -13392,7 +13392,7 @@ remote_load (struct target_ops *self, const char *name, int from_tty)
 static char *
 remote_pid_to_exec_file (struct target_ops *self, int pid)
 {
-  static gdb::unique_xmalloc_ptr<char> filename;
+  static gdb::optional<gdb::char_vector> filename;
   struct inferior *inf;
   char *annex = NULL;
 
@@ -13415,7 +13415,7 @@ remote_pid_to_exec_file (struct target_ops *self, int pid)
   filename = target_read_stralloc (&current_target,
 				   TARGET_OBJECT_EXEC_FILE, annex);
 
-  return filename.get ();
+  return filename ? filename->data () : nullptr;
 }
 
 /* Implement the to_can_do_single_step target_ops method.  */
diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c
index 5339f11..608a702 100644
--- a/gdb/solib-aix.c
+++ b/gdb/solib-aix.c
@@ -279,10 +279,10 @@ solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
   if (data->library_list != NULL)
     return data->library_list;
 
-  gdb::unique_xmalloc_ptr<char> library_document
+  gdb::optional<gdb::char_vector> library_document
     = target_read_stralloc (&current_target, TARGET_OBJECT_LIBRARIES_AIX,
 			    NULL);
-  if (library_document == NULL && warning_msg != NULL)
+  if (!library_document && warning_msg != NULL)
     {
       warning (_("%s (failed to read TARGET_OBJECT_LIBRARIES_AIX)"),
 	       warning_msg);
@@ -292,9 +292,9 @@ solib_aix_get_library_list (struct inferior *inf, const char *warning_msg)
   if (solib_aix_debug)
     fprintf_unfiltered (gdb_stdlog,
 			"DEBUG: TARGET_OBJECT_LIBRARIES_AIX = \n%s\n",
-			library_document.get ());
+			library_document->data ());
 
-  data->library_list = solib_aix_parse_libraries (library_document.get ());
+  data->library_list = solib_aix_parse_libraries (library_document->data ());
   if (data->library_list == NULL && warning_msg != NULL)
     {
       warning (_("%s (missing XML support?)"), warning_msg);
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index d038ccc..dc40bb9 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -224,10 +224,10 @@ dsbt_print_loadmap (struct int_elf32_dsbt_loadmap *map)
 /* Decode int_elf32_dsbt_loadmap from BUF.  */
 
 static struct int_elf32_dsbt_loadmap *
-decode_loadmap (gdb_byte *buf)
+decode_loadmap (const gdb_byte *buf)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
-  struct ext_elf32_dsbt_loadmap *ext_ldmbuf;
+  const struct ext_elf32_dsbt_loadmap *ext_ldmbuf;
   struct int_elf32_dsbt_loadmap *int_ldmbuf;
 
   int version, seg, nsegs;
@@ -278,7 +278,6 @@ decode_loadmap (gdb_byte *buf)
 				    byte_order);
     }
 
-  xfree (ext_ldmbuf);
   return int_ldmbuf;
 }
 
@@ -292,26 +291,26 @@ static struct dsbt_info *get_dsbt_info (void);
 static void
 dsbt_get_initial_loadmaps (void)
 {
-  gdb_byte *buf;
   struct dsbt_info *info = get_dsbt_info ();
+  gdb::optional<gdb::byte_vector> buf
+    = target_read_alloc (&current_target, TARGET_OBJECT_FDPIC, "exec");
 
-  if (0 >= target_read_alloc (&current_target, TARGET_OBJECT_FDPIC,
-			      "exec", &buf))
+  if (!buf || buf->empty ())
     {
       info->exec_loadmap = NULL;
       error (_("Error reading DSBT exec loadmap"));
     }
-  info->exec_loadmap = decode_loadmap (buf);
+  info->exec_loadmap = decode_loadmap (buf->data ());
   if (solib_dsbt_debug)
     dsbt_print_loadmap (info->exec_loadmap);
 
-  if (0 >= target_read_alloc (&current_target, TARGET_OBJECT_FDPIC,
-			      "interp", &buf))
+  buf = target_read_alloc (&current_target, TARGET_OBJECT_FDPIC, "exec");
+  if (!buf || buf->empty ())
     {
       info->interp_loadmap = NULL;
       error (_("Error reading DSBT interp loadmap"));
     }
-  info->interp_loadmap = decode_loadmap (buf);
+  info->interp_loadmap = decode_loadmap (buf->data ());
   if (solib_dsbt_debug)
     dsbt_print_loadmap (info->interp_loadmap);
 }
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 29e74da..07df197 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1274,13 +1274,13 @@ svr4_current_sos_via_xfer_libraries (struct svr4_library_list *list,
   gdb_assert (annex == NULL || target_augmented_libraries_svr4_read ());
 
   /* Fetch the list of shared libraries.  */
-  gdb::unique_xmalloc_ptr<char> svr4_library_document
+  gdb::optional<gdb::char_vector> svr4_library_document
     = target_read_stralloc (&current_target, TARGET_OBJECT_LIBRARIES_SVR4,
 			    annex);
-  if (svr4_library_document == NULL)
+  if (!svr4_library_document)
     return 0;
 
-  return svr4_parse_libraries (svr4_library_document.get (), list);
+  return svr4_parse_libraries (svr4_library_document->data (), list);
 }
 
 #else
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index 0968e49..7f96e80 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -257,13 +257,13 @@ solib_target_current_sos (void)
   int ix;
 
   /* Fetch the list of shared libraries.  */
-  gdb::unique_xmalloc_ptr<char> library_document
+  gdb::optional<gdb::char_vector> library_document
     = target_read_stralloc (&current_target, TARGET_OBJECT_LIBRARIES, NULL);
-  if (library_document == NULL)
+  if (!library_document)
     return NULL;
 
   /* Parse the list.  */
-  library_list = solib_target_parse_libraries (library_document.get ());
+  library_list = solib_target_parse_libraries (library_document->data ());
 
   if (library_list == NULL)
     return NULL;
diff --git a/gdb/target.c b/gdb/target.c
index 84f5228..4d0fed7 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -48,6 +48,7 @@
 #include <algorithm>
 #include "byte-vector.h"
 #include "terminal.h"
+#include <algorithm>
 
 static void generic_tls_error (void) ATTRIBUTE_NORETURN;
 
@@ -1936,18 +1937,17 @@ target_write (struct target_ops *ops,
 				     NULL, NULL);
 }
 
-/* Read OBJECT/ANNEX using OPS.  Store the result in *BUF_P and return
-   the size of the transferred data.  PADDING additional bytes are
-   available in *BUF_P.  This is a helper function for
-   target_read_alloc; see the declaration of that function for more
-   information.  */
+/* Help for target_read_alloc and target_read_stralloc.  See their comments
+   for details.  */
 
-static LONGEST
+template <typename T>
+gdb::optional<gdb::def_vector<T>>
 target_read_alloc_1 (struct target_ops *ops, enum target_object object,
-		     const char *annex, gdb_byte **buf_p, int padding)
+		     const char *annex)
 {
-  size_t buf_alloc, buf_pos;
-  gdb_byte *buf;
+  gdb::def_vector<T> buf;
+  size_t buf_pos = 0;
+  const int chunk = 4096;
 
   /* This function does not have a length parameter; it reads the
      entire OBJECT).  Also, it doesn't support objects fetched partly
@@ -1958,82 +1958,64 @@ target_read_alloc_1 (struct target_ops *ops, enum target_object object,
 
   /* Start by reading up to 4K at a time.  The target will throttle
      this number down if necessary.  */
-  buf_alloc = 4096;
-  buf = (gdb_byte *) xmalloc (buf_alloc);
-  buf_pos = 0;
   while (1)
     {
       ULONGEST xfered_len;
       enum target_xfer_status status;
 
-      status = target_read_partial (ops, object, annex, &buf[buf_pos],
-				    buf_pos, buf_alloc - buf_pos - padding,
+      buf.resize (buf_pos + chunk);
+
+      status = target_read_partial (ops, object, annex,
+				    (gdb_byte *) &buf[buf_pos],
+				    buf_pos, chunk,
 				    &xfered_len);
 
       if (status == TARGET_XFER_EOF)
 	{
 	  /* Read all there was.  */
-	  if (buf_pos == 0)
-	    xfree (buf);
-	  else
-	    *buf_p = buf;
-	  return buf_pos;
+	  buf.resize (buf_pos);
+	  return buf;
 	}
       else if (status != TARGET_XFER_OK)
 	{
 	  /* An error occurred.  */
-	  xfree (buf);
-	  return TARGET_XFER_E_IO;
+	  return {};
 	}
 
       buf_pos += xfered_len;
 
-      /* If the buffer is filling up, expand it.  */
-      if (buf_alloc < buf_pos * 2)
-	{
-	  buf_alloc *= 2;
-	  buf = (gdb_byte *) xrealloc (buf, buf_alloc);
-	}
-
       QUIT;
     }
 }
 
-/* Read OBJECT/ANNEX using OPS.  Store the result in *BUF_P and return
-   the size of the transferred data.  See the declaration in "target.h"
-   function for more information about the return value.  */
+/* See target.h  */
 
-LONGEST
+gdb::optional<gdb::byte_vector>
 target_read_alloc (struct target_ops *ops, enum target_object object,
-		   const char *annex, gdb_byte **buf_p)
+		   const char *annex)
 {
-  return target_read_alloc_1 (ops, object, annex, buf_p, 0);
+  return target_read_alloc_1<gdb_byte> (ops, object, annex);
 }
 
 /* See target.h.  */
 
-gdb::unique_xmalloc_ptr<char>
+gdb::optional<gdb::char_vector>
 target_read_stralloc (struct target_ops *ops, enum target_object object,
 		      const char *annex)
 {
-  gdb_byte *buffer;
-  char *bufstr;
-  LONGEST i, transferred;
-
-  transferred = target_read_alloc_1 (ops, object, annex, &buffer, 1);
-  bufstr = (char *) buffer;
-
-  if (transferred < 0)
-    return NULL;
+  gdb::optional<gdb::char_vector> buf
+    = target_read_alloc_1<char> (ops, object, annex);
 
-  if (transferred == 0)
-    return gdb::unique_xmalloc_ptr<char> (xstrdup (""));
+  if (!buf)
+    return {};
 
-  bufstr[transferred] = 0;
+  if (buf->back () != '\0')
+    buf->push_back ('\0');
 
   /* Check for embedded NUL bytes; but allow trailing NULs.  */
-  for (i = strlen (bufstr); i < transferred; i++)
-    if (bufstr[i] != 0)
+  for (auto it = std::find (buf->begin (), buf->end (), '\0');
+       it != buf->end (); it++)
+    if (*it != '\0')
       {
 	warning (_("target object %d, annex %s, "
 		   "contained unexpected null characters"),
@@ -2041,7 +2023,7 @@ target_read_stralloc (struct target_ops *ops, enum target_object object,
 	break;
       }
 
-  return gdb::unique_xmalloc_ptr<char> (bufstr);
+  return buf;
 }
 
 /* Memory transfer methods.  */
@@ -2743,7 +2725,7 @@ target_supports_multi_process (void)
 
 /* See target.h.  */
 
-gdb::unique_xmalloc_ptr<char>
+gdb::optional<gdb::char_vector>
 target_get_osdata (const char *type)
 {
   struct target_ops *t;
@@ -2757,7 +2739,7 @@ target_get_osdata (const char *type)
     t = find_default_run_target ("get OS data");
 
   if (!t)
-    return NULL;
+    return {};
 
   return target_read_stralloc (t, TARGET_OBJECT_OSDATA, type);
 }
diff --git a/gdb/target.h b/gdb/target.h
index fd6c30b..c0bd379 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -327,29 +327,26 @@ LONGEST target_write_with_progress (struct target_ops *ops,
 				    void (*progress) (ULONGEST, void *),
 				    void *baton);
 
-/* Wrapper to perform a full read of unknown size.  OBJECT/ANNEX will
-   be read using OPS.  The return value will be -1 if the transfer
-   fails or is not supported; 0 if the object is empty; or the length
-   of the object otherwise.  If a positive value is returned, a
-   sufficiently large buffer will be allocated using xmalloc and
-   returned in *BUF_P containing the contents of the object.
+/* Wrapper to perform a full read of unknown size.  OBJECT/ANNEX will be read
+   using OPS.  The return value will be uninstantiated if the transfer fails or
+   is not supported.
 
    This method should be used for objects sufficiently small to store
    in a single xmalloc'd buffer, when no fixed bound on the object's
    size is known in advance.  Don't try to read TARGET_OBJECT_MEMORY
    through this function.  */
 
-extern LONGEST target_read_alloc (struct target_ops *ops,
-				  enum target_object object,
-				  const char *annex, gdb_byte **buf_p);
+extern gdb::optional<gdb::byte_vector> target_read_alloc
+    (struct target_ops *ops, enum target_object object, const char *annex);
 
-/* Read OBJECT/ANNEX using OPS.  The result is NUL-terminated and
-   returned as a string.  If an error occurs or the transfer is
-   unsupported, NULL is returned.  Empty objects are returned as
-   allocated but empty strings.  A warning is issued if the result
-   contains any embedded NUL bytes.  */
+/* Read OBJECT/ANNEX using OPS.  The result is a NUL-terminated character vector
+   (therefore usable as a NUL-terminated string).  If an error occurs or the
+   transfer is unsupported, the return value will be uninstantiated.  Empty
+   objects are returned as allocated but empty strings.  Therefore, on success,
+   the returned vector is guaranteed to have at least one element.  A warning is
+   issued if the result contains any embedded NUL bytes.  */
 
-extern gdb::unique_xmalloc_ptr<char> target_read_stralloc
+extern gdb::optional<gdb::char_vector> target_read_stralloc
     (struct target_ops *ops, enum target_object object, const char *annex);
 
 /* See target_ops->to_xfer_partial.  */
@@ -2384,15 +2381,11 @@ extern struct target_ops *find_target_beneath (struct target_ops *);
 
 struct target_ops *find_target_at (enum strata stratum);
 
-/* Read OS data object of type TYPE from the target, and return it in
-   XML format.  The result is NUL-terminated and returned as a string.
-   If an error occurs or the transfer is unsupported, NULL is
-   returned.  Empty objects are returned as allocated but empty
-   strings.  */
+/* Read OS data object of type TYPE from the target, and return it in XML
+   format.  The return value follows the same rules as target_read_stralloc.  */
 
-extern gdb::unique_xmalloc_ptr<char> target_get_osdata (const char *type);
+extern gdb::optional<gdb::char_vector> target_get_osdata (const char *type);
 
-\f
 /* Stuff that should be shared among the various remote targets.  */
 
 /* Debugging level.  0 is off, and non-zero values mean to print some debug
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 30fe206..cf97353 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3910,23 +3910,19 @@ static struct value *
 sdata_make_value (struct gdbarch *gdbarch, struct internalvar *var,
 		  void *ignore)
 {
-  LONGEST size;
-  gdb_byte *buf;
-
   /* We need to read the whole object before we know its size.  */
-  size = target_read_alloc (&current_target,
-			    TARGET_OBJECT_STATIC_TRACE_DATA,
-			    NULL, &buf);
-  if (size >= 0)
+  gdb::optional<gdb::byte_vector> buf
+    = target_read_alloc (&current_target, TARGET_OBJECT_STATIC_TRACE_DATA,
+			 NULL);
+  if (buf)
     {
       struct value *v;
       struct type *type;
 
       type = init_vector_type (builtin_type (gdbarch)->builtin_true_char,
-			       size);
+			       buf->size ());
       v = allocate_value (type);
-      memcpy (value_contents_raw (v), buf, size);
-      xfree (buf);
+      memcpy (value_contents_raw (v), buf->data (), buf->size ());
       return v;
     }
   else
diff --git a/gdb/xml-support.c b/gdb/xml-support.c
index 2547882..3775e2c 100644
--- a/gdb/xml-support.c
+++ b/gdb/xml-support.c
@@ -793,13 +793,13 @@ xinclude_start_include (struct gdb_xml_parser *parser,
     gdb_xml_error (parser, _("Maximum XInclude depth (%d) exceeded"),
 		   MAX_XINCLUDE_DEPTH);
 
-  gdb::unique_xmalloc_ptr<char> text = data->fetcher (href,
-						      data->fetcher_baton);
-  if (text == NULL)
+  gdb::optional<gdb::char_vector> text
+    = data->fetcher (href, data->fetcher_baton);
+  if (!text)
     gdb_xml_error (parser, _("Could not load XML document \"%s\""), href);
 
   if (!xml_process_xincludes (data->output, parser->name (),
-			      text.get (), data->fetcher,
+			      text->data (), data->fetcher,
 			      data->fetcher_baton,
 			      data->include_depth + 1))
     gdb_xml_error (parser, _("Parsing \"%s\" failed"), href);
@@ -971,7 +971,7 @@ show_debug_xml (struct ui_file *file, int from_tty,
   fprintf_filtered (file, _("XML debugging is %s.\n"), value);
 }
 
-gdb::unique_xmalloc_ptr<char>
+gdb::optional<gdb::char_vector>
 xml_fetch_content_from_file (const char *filename, void *baton)
 {
   const char *dirname = (const char *) baton;
@@ -990,7 +990,7 @@ xml_fetch_content_from_file (const char *filename, void *baton)
     file = gdb_fopen_cloexec (filename, FOPEN_RT);
 
   if (file == NULL)
-    return NULL;
+    return {};
 
   /* Read in the whole file.  */
 
@@ -1001,16 +1001,16 @@ xml_fetch_content_from_file (const char *filename, void *baton)
   len = ftell (file.get ());
   rewind (file.get ());
 
-  gdb::unique_xmalloc_ptr<char> text ((char *) xmalloc (len + 1));
+  gdb::char_vector text (len + 1);
 
-  if (fread (text.get (), 1, len, file.get ()) != len
+  if (fread (text.data (), 1, len, file.get ()) != len
       || ferror (file.get ()))
     {
       warning (_("Read error from \"%s\""), filename);
-      return NULL;
+      return {};
     }
 
-  text.get ()[len] = '\0';
+  text.back () = '\0';
   return text;
 }
 
diff --git a/gdb/xml-support.h b/gdb/xml-support.h
index 5947623..3abae70 100644
--- a/gdb/xml-support.h
+++ b/gdb/xml-support.h
@@ -24,6 +24,7 @@
 #include "gdb_obstack.h"
 #include "vec.h"
 #include "xml-utils.h"
+#include "common/byte-vector.h"
 
 struct gdb_xml_parser;
 struct gdb_xml_element;
@@ -52,8 +53,8 @@ extern const char *xml_builtin[][2];
 
 /* Callback to fetch a new XML file, based on the provided HREF.  */
 
-typedef gdb::unique_xmalloc_ptr<char> (*xml_fetch_another) (const char *href,
-							    void *baton);
+typedef gdb::optional<gdb::char_vector> (*xml_fetch_another) (const char *href,
+							      void *baton);
 
 /* Append the expansion of TEXT after processing <xi:include> tags in
    RESULT.  FETCHER will be called (with FETCHER_BATON) to retrieve
@@ -231,9 +232,10 @@ ULONGEST gdb_xml_parse_ulongest (struct gdb_xml_parser *parser,
 				 const char *value);
 
 /* Open FILENAME, read all its text into memory, close it, and return
-   the text.  If something goes wrong, return NULL and warn.  */
+   the text.  If something goes wrong, return an uninstantiated optional
+   and warn.  */
 
-extern gdb::unique_xmalloc_ptr<char> xml_fetch_content_from_file
+extern gdb::optional<gdb::char_vector> xml_fetch_content_from_file
     (const char *filename, void *baton);
 
 #endif
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 0ba6d44..bf17642 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -305,12 +305,12 @@ syscall_parse_xml (const char *document, xml_fetch_another fetcher,
 static struct syscalls_info *
 xml_init_syscalls_info (const char *filename)
 {
-  gdb::unique_xmalloc_ptr<char> full_file
+  gdb::optional<gdb::char_vector> full_file
     = xml_fetch_content_from_file (filename, gdb_datadir);
-  if (full_file == NULL)
+  if (!full_file)
     return NULL;
 
-  return syscall_parse_xml (full_file.get (),
+  return syscall_parse_xml (full_file->data (),
 			    xml_fetch_content_from_file,
 			    (void *) ldirname (filename).c_str ());
 }
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 9190d5f..1c3409d 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -668,15 +668,15 @@ tdesc_parse_xml (const char *document, xml_fetch_another fetcher,
 const struct target_desc *
 file_read_description_xml (const char *filename)
 {
-  gdb::unique_xmalloc_ptr<char> tdesc_str
+  gdb::optional<gdb::char_vector> tdesc_str
     = xml_fetch_content_from_file (filename, NULL);
-  if (tdesc_str == NULL)
+  if (!tdesc_str)
     {
       warning (_("Could not open \"%s\""), filename);
       return NULL;
     }
 
-  return tdesc_parse_xml (tdesc_str.get (), xml_fetch_content_from_file,
+  return tdesc_parse_xml (tdesc_str->data (), xml_fetch_content_from_file,
 			  (void *) ldirname (filename).c_str ());
 }
 
@@ -687,7 +687,7 @@ file_read_description_xml (const char *filename)
    is "target.xml".  Other calls may be performed for the DTD or
    for <xi:include>.  */
 
-static gdb::unique_xmalloc_ptr<char>
+static gdb::optional<gdb::char_vector>
 fetch_available_features_from_target (const char *name, void *baton_)
 {
   struct target_ops *ops = (struct target_ops *) baton_;
@@ -706,12 +706,12 @@ fetch_available_features_from_target (const char *name, void *baton_)
 const struct target_desc *
 target_read_description_xml (struct target_ops *ops)
 {
-  gdb::unique_xmalloc_ptr<char> tdesc_str
+  gdb::optional<gdb::char_vector> tdesc_str
     = fetch_available_features_from_target ("target.xml", ops);
-  if (tdesc_str == NULL)
+  if (!tdesc_str)
     return NULL;
 
-  return tdesc_parse_xml (tdesc_str.get (),
+  return tdesc_parse_xml (tdesc_str->data (),
 			  fetch_available_features_from_target,
 			  ops);
 }
@@ -735,15 +735,15 @@ target_fetch_description_xml (struct target_ops *ops)
 
   return {};
 #else
-  gdb::unique_xmalloc_ptr<char>
+  gdb::optional<gdb::char_vector>
     tdesc_str = fetch_available_features_from_target ("target.xml", ops);
-  if (tdesc_str == NULL)
+  if (!tdesc_str)
     return {};
 
   std::string output;
   if (!xml_process_xincludes (output,
 			      _("target description"),
-			      tdesc_str.get (),
+			      tdesc_str->data (),
 			      fetch_available_features_from_target, ops, 0))
     {
       warning (_("Could not load XML target description; ignoring"));
-- 
2.7.4

Re: [PATCH] Make target_read_alloc & al return vectors

[Simon Marchi]

On 2018-03-22 00:03, Simon Marchi wrote:
> This patch started by changing target_read_alloc_1 to return a
> byte_vector, to avoid manual memory management (in target_read_alloc_1
> and in the callers).  To communicate failures to the callers, it
> actually returns a gdb::optional<gdb::byte_vector>.
> 
> Adjusting target_read_stralloc was a bit more tricky, since it wants to
> return a buffer of char, and not gdb_byte.  Since you can't just cast a
> gdb::byte_vector into a gdb::def_vector<char>, I made
> target_read_alloc_1 templated, so both versions (that return vectors of
> gdb_byte and char) are generated.  Since target_read_stralloc now
> returns a gdb::char_vector instead of a gdb::unique_xmalloc_ptr<char>, 
> a
> few callers need to be adjusted.

I pushed this patch.

Simon

Re: [PATCH] Make target_read_alloc & al return vectors

[Pedro Alves]

Hi Simon,

I noticed this recent regression:

 PASS: gdb.threads/gcore-stale-thread.exp: successfully compiled posix threads test case
 PASS: gdb.threads/gcore-stale-thread.exp: set non-stop on
 PASS: gdb.threads/gcore-stale-thread.exp: continue to breakpoint: break-here
-PASS: gdb.threads/gcore-stale-thread.exp: save a corefile
+UNSUPPORTED: gdb.threads/gcore-stale-thread.exp: save a corefile
 PASS: gdb.threads/gcore-stale-thread.exp: exited thread is current due to non-stop

and bisection points at:
 9018be22e02 ("Make target_read_alloc & al return vectors")

diff of gdb.log shows:

 gcore gdb/testsuite/outputs/gdb.threads/gcore-stale-thread/gcore-stale-thread.core
-Saved corefile gdb/testsuite/outputs/gdb.threads/gcore-stale-thread/gcore-stale-thread.core
-(gdb) PASS: gdb.threads/gcore-stale-thread.exp: save a corefile
+Target does not support core file generation.
+(gdb) UNSUPPORTED: gdb.threads/gcore-stale-thread.exp: save a corefile

Thanks,
Pedro Alves

On 03/22/2018 04:03 AM, Simon Marchi wrote:
> This patch started by changing target_read_alloc_1 to return a
> byte_vector, to avoid manual memory management (in target_read_alloc_1
> and in the callers).  To communicate failures to the callers, it
> actually returns a gdb::optional<gdb::byte_vector>.
> 
> Adjusting target_read_stralloc was a bit more tricky, since it wants to
> return a buffer of char, and not gdb_byte.  Since you can't just cast a
> gdb::byte_vector into a gdb::def_vector<char>, I made
> target_read_alloc_1 templated, so both versions (that return vectors of
> gdb_byte and char) are generated.  Since target_read_stralloc now
> returns a gdb::char_vector instead of a gdb::unique_xmalloc_ptr<char>, a
> few callers need to be adjusted.

[pushed] linux_spu_make_corefile_notes: return note_data instead of nullptr (was: [PATCH] Make target_read_alloc & al return vectors)

[Simon Marchi]

On 2018-04-16 04:04 PM, Pedro Alves wrote:
> Hi Simon,
> 
> I noticed this recent regression:
> 
>  PASS: gdb.threads/gcore-stale-thread.exp: successfully compiled posix threads test case
>  PASS: gdb.threads/gcore-stale-thread.exp: set non-stop on
>  PASS: gdb.threads/gcore-stale-thread.exp: continue to breakpoint: break-here
> -PASS: gdb.threads/gcore-stale-thread.exp: save a corefile
> +UNSUPPORTED: gdb.threads/gcore-stale-thread.exp: save a corefile
>  PASS: gdb.threads/gcore-stale-thread.exp: exited thread is current due to non-stop
> 
> and bisection points at:
>  9018be22e02 ("Make target_read_alloc & al return vectors")
> 
> diff of gdb.log shows:
> 
>  gcore gdb/testsuite/outputs/gdb.threads/gcore-stale-thread/gcore-stale-thread.core
> -Saved corefile gdb/testsuite/outputs/gdb.threads/gcore-stale-thread/gcore-stale-thread.core
> -(gdb) PASS: gdb.threads/gcore-stale-thread.exp: save a corefile
> +Target does not support core file generation.
> +(gdb) UNSUPPORTED: gdb.threads/gcore-stale-thread.exp: save a corefile

Sorry about that, here's what I pushed to fix it:

From 07d28c777757148d13e429c4463da52cbb50d297 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 16 Apr 2018 16:47:06 -0400
Subject: [PATCH] linux_spu_make_corefile_notes: return note_data instead of
 nullptr

Since commit

  9018be2 ("Make target_read_alloc & al return vectors")

the test gdb.threads/gcore-stale-thread.exp test results in UNSUPPORTED:

  UNSUPPORTED: gdb.threads/gcore-stale-thread.exp: save a corefile

The problem is that the linux_spu_make_corefile_notes started returning
nullptr when reading TARGET_OBJECT_SPU fails.  The previous (and proper)
behaviour is to return the note_data received as a parameter, so that
other functions may continue to append to this buffer.

With this patch, the test goes back to PASS.

gdb/ChangeLog:

	* linux-tdep.c (linux_spu_make_corefile_notes): Return note_data
	instead of nullptr.
---
 gdb/ChangeLog    | 5 +++++
 gdb/linux-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ad7b993..3993491 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-04-16  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* linux-tdep.c (linux_spu_make_corefile_notes): Return note_data
+	instead of nullptr.
+
 2018-04-16  Pedro Alves  <palves@redhat.com>

 	* MAINTAINERS (sh): Remove.
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 552a2a4..24bfd74 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1426,7 +1426,7 @@ linux_spu_make_corefile_notes (bfd *obfd, char *note_data, int *note_size)
     spu_ids = target_read_alloc (&current_target, TARGET_OBJECT_SPU, NULL);

   if (!spu_ids)
-    return nullptr;
+    return note_data;

   /* Generate corefile notes for each SPU file.  */
   for (size_t i = 0; i < spu_ids->size (); i += 4)
-- 
2.7.4

Re: [PATCH] Make target_read_alloc & al return vectors

[Andreas Schwab]

../../gdb/ia64-tdep.c: In function ‘LONGEST getunwind_table(gdb_byte**)’:
../../gdb/ia64-tdep.c:2664:16: error: too many arguments to function ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > target_read_alloc(target_ops*, target_object, const char*)’
     NULL, buf_p);
                ^
In file included from ../../gdb/inferior.h:41:0,
                 from ../../gdb/ia64-tdep.c:21:
../../gdb/target.h:341:40: note: declared here
 extern gdb::optional<gdb::byte_vector> target_read_alloc
                                        ^
../../gdb/ia64-tdep.c:2663:5: error: cannot convert ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > >’ to ‘LONGEST {aka long int}’ in assignment
   x = target_read_alloc (current_top_target (), TARGET_OBJECT_UNWIND_TABLE,
     ^

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Make target_read_alloc & al return vectors

[Simon Marchi]

On 2018-07-07 04:54 AM, Andreas Schwab wrote:
> ../../gdb/ia64-tdep.c: In function ‘LONGEST getunwind_table(gdb_byte**)’:
> ../../gdb/ia64-tdep.c:2664:16: error: too many arguments to function ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > target_read_alloc(target_ops*, target_object, const char*)’
>      NULL, buf_p);
>                 ^
> In file included from ../../gdb/inferior.h:41:0,
>                  from ../../gdb/ia64-tdep.c:21:
> ../../gdb/target.h:341:40: note: declared here
>  extern gdb::optional<gdb::byte_vector> target_read_alloc
>                                         ^
> ../../gdb/ia64-tdep.c:2663:5: error: cannot convert ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > >’ to ‘LONGEST {aka long int}’ in assignment
>    x = target_read_alloc (current_top_target (), TARGET_OBJECT_UNWIND_TABLE,
>      ^
> 
> Andreas.
> 

Hi Andreas,

I have been unable to build libunwind for ia64 before (using the 1.2
branch), that's why I missed it.  Now I tried to build the 1.1 and it
works fine.

I fixed up the call to target_read_alloc to the best of my knowledge,
with as little changes to the code as possible.  However, I can't test
the result more than build it.  Can you please review the patch below?

Just to confirm, do you also see errors later in the build, like:

In file included from /home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:39:
/home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:114:1: error: ISO C++ forbids converting a string constant to ‘char*’ [-Werror=write-strings]
 static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
 ^~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/common/preprocessor.h:28:25: note: in definition of macro ‘STRINGIFY_1’
 #define STRINGIFY_1(x) #x
                         ^
?

----

From efd663c4d54b64f394d51faabc74f249d0bf32a7 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sat, 7 Jul 2018 10:55:14 -0400
Subject: [PATCH] Fix compilation error in ia64-tdep.c with libunwind-ia64

Commit

  9018be22e022 ("Make target_read_alloc & al return vectors")

failed to update the code in ia64-tdep.c, for HAVE_LIBUNWIND_IA64_H.
This patch fixes that.

gdb/ChangeLog:

	* ia64-tdep.c (ktab_buf): New global.
	(getunwind_table): Return a gdb::optional<gdb::byte_vector>.
	(get_kernel_table): Adjust.
---
 gdb/ia64-tdep.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 0df62e26ab39..2fab60c1d01e 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -69,6 +69,7 @@ struct ia64_table_entry
   };

 static struct ia64_table_entry *ktab = NULL;
+static gdb::optional<gdb::byte_vector> ktab_buf;

 #endif

@@ -2647,11 +2648,9 @@ ia64_access_mem (unw_addr_space_t as,
 }

 /* Call low-level function to access the kernel unwind table.  */
-static LONGEST
-getunwind_table (gdb_byte **buf_p)
+static gdb::optional<gdb::byte_vector>
+getunwind_table ()
 {
-  LONGEST x;
-
   /* FIXME drow/2005-09-10: This code used to call
      ia64_linux_xfer_unwind_table directly to fetch the unwind table
      for the currently running ia64-linux kernel.  That data should
@@ -2660,10 +2659,8 @@ getunwind_table (gdb_byte **buf_p)
      we should find a way to override the corefile layer's
      xfer_partial method.  */

-  x = target_read_alloc (current_top_target (), TARGET_OBJECT_UNWIND_TABLE,
-			 NULL, buf_p);
-
-  return x;
+  return target_read_alloc (current_top_target (), TARGET_OBJECT_UNWIND_TABLE,
+			    NULL);
 }

 /* Get the kernel unwind table.  */				
@@ -2674,15 +2671,12 @@ get_kernel_table (unw_word_t ip, unw_dyn_info_t *di)

   if (!ktab)
     {
-      gdb_byte *ktab_buf;
-      LONGEST size;
-
-      size = getunwind_table (&ktab_buf);
-      if (size <= 0)
+      ktab_buf = getunwind_table ();
+      if (!ktab_buf)
 	return -UNW_ENOINFO;

-      ktab = (struct ia64_table_entry *) ktab_buf;
-      ktab_size = size;
+      ktab = (struct ia64_table_entry *) ktab_buf->data ();
+      ktab_size = ktab_buf->size ();

       for (etab = ktab; etab->start_offset; ++etab)
         etab->info_offset += KERNEL_START;
-- 
2.18.0

Re: [PATCH] Make target_read_alloc & al return vectors

[Simon Marchi]

On 2018-07-07 11:10 AM, Simon Marchi wrote:
> In file included from /home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:39:
> /home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:114:1: error: ISO C++ forbids converting a string constant to ‘char*’ [-Werror=write-strings]
>  static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
>  ^~~~~~~~~~~~~~
> /home/simark/src/binutils-gdb/gdb/common/preprocessor.h:28:25: note: in definition of macro ‘STRINGIFY_1’
>  #define STRINGIFY_1(x) #x
>                          ^

Actually, it was quite obvious, so I just pushed this patch.

Simon


From e83f4d97504515f8c5f290eeb25eac87cca6892f Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sat, 7 Jul 2018 11:13:14 -0400
Subject: [PATCH] ia64-tdep.c: Fix -Wwrite-strings errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Simply add const where necessary to get rid of errors like:

/home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:114:1: error: ISO C++ forbids converting a string constant to ‘char*’ [-Werror=write-strings]
 static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
 ^~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/common/preprocessor.h:28:25: note: in definition of macro ‘STRINGIFY_1’
 #define STRINGIFY_1(x) #x
                         ^
/home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:114:29: note: in expansion of macro ‘STRINGIFY’
 static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
                             ^~~~~~~~~

gdb/ChangeLog:

	* ia64-tdep.c (get_reg_name, get_fpreg_name, get_saveloc_name,
	is_signal_frame_name, step_name, init_remote_name,
	create_addr_space_name, destroy_addr_space_name,
	search_unwind_table_name, find_dyn_list_name): Constify.
---
 gdb/ChangeLog             |  7 +++++++
 gdb/ia64-libunwind-tdep.c | 22 ++++++++++++----------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 17146e810a37..076f571e2a8b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-07-07  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* ia64-tdep.c (get_reg_name, get_fpreg_name, get_saveloc_name,
+	is_signal_frame_name, step_name, init_remote_name,
+	create_addr_space_name, destroy_addr_space_name,
+	search_unwind_table_name, find_dyn_list_name): Constify.
+
 2018-07-05  Simon Marchi  <simon.marchi@polymtl.ca>

 	* darwin-nat.c (darwin_pthread_kill): New function.
diff --git a/gdb/ia64-libunwind-tdep.c b/gdb/ia64-libunwind-tdep.c
index 99782b2d7016..b7eef4ac644a 100644
--- a/gdb/ia64-libunwind-tdep.c
+++ b/gdb/ia64-libunwind-tdep.c
@@ -111,17 +111,19 @@ struct libunwind_frame_cache
 #define LIBUNWIND_SO_7 "libunwind-" STRINGIFY(UNW_TARGET) ".so.7"
 #endif

-static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
-static char *get_fpreg_name = STRINGIFY(UNW_OBJ(get_fpreg));
-static char *get_saveloc_name = STRINGIFY(UNW_OBJ(get_save_loc));
-static char *is_signal_frame_name = STRINGIFY(UNW_OBJ(is_signal_frame));
-static char *step_name = STRINGIFY(UNW_OBJ(step));
-static char *init_remote_name = STRINGIFY(UNW_OBJ(init_remote));
-static char *create_addr_space_name = STRINGIFY(UNW_OBJ(create_addr_space));
-static char *destroy_addr_space_name = STRINGIFY(UNW_OBJ(destroy_addr_space));
-static char *search_unwind_table_name
+static const char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
+static const char *get_fpreg_name = STRINGIFY(UNW_OBJ(get_fpreg));
+static const char *get_saveloc_name = STRINGIFY(UNW_OBJ(get_save_loc));
+static const char *is_signal_frame_name = STRINGIFY(UNW_OBJ(is_signal_frame));
+static const char *step_name = STRINGIFY(UNW_OBJ(step));
+static const char *init_remote_name = STRINGIFY(UNW_OBJ(init_remote));
+static const char *create_addr_space_name
+  = STRINGIFY(UNW_OBJ(create_addr_space));
+static const char *destroy_addr_space_name
+  = STRINGIFY(UNW_OBJ(destroy_addr_space));
+static const char *search_unwind_table_name
   = STRINGIFY(UNW_OBJ(search_unwind_table));
-static char *find_dyn_list_name = STRINGIFY(UNW_OBJ(find_dyn_list));
+static const char *find_dyn_list_name = STRINGIFY(UNW_OBJ(find_dyn_list));

 static struct libunwind_descr *
 libunwind_descr (struct gdbarch *gdbarch)
-- 
2.18.0

Re: [PATCH] Make target_read_alloc & al return vectors

[Andreas Schwab]

This is the next error:

../../gdb/ia64-linux-nat.c:73:8: error: conflicting return type specified for ‘virtual bool ia64_linux_nat_target::have_steppable_watchpoint()’
   bool have_steppable_watchpoint () { return 1; }
        ^
In file included from ../../gdb/inferior.h:41:0,
                 from ../../gdb/ia64-linux-nat.c:22:
../../gdb/target.h:554:17: error:   overriding ‘virtual int target_ops::have_steppable_watchpoint()’
     virtual int have_steppable_watchpoint ()
                 ^

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Make target_read_alloc & al return vectors

[Simon Marchi]

On 2018-07-07 13:23, Andreas Schwab wrote:
> This is the next error:
> 
> ../../gdb/ia64-linux-nat.c:73:8: error: conflicting return type
> specified for ‘virtual bool
> ia64_linux_nat_target::have_steppable_watchpoint()’
>    bool have_steppable_watchpoint () { return 1; }
>         ^
> In file included from ../../gdb/inferior.h:41:0,
>                  from ../../gdb/ia64-linux-nat.c:22:
> ../../gdb/target.h:554:17: error:   overriding ‘virtual int
> target_ops::have_steppable_watchpoint()’
>      virtual int have_steppable_watchpoint ()
>                  ^

The fix for this is probably obvious (change int for bool).  Can you 
take care of it?  I'm not on my gdb-development-capable computer, and 
probably don't have an ia64 cross-compiler handy.  Does the patch for 
target_read_alloc look good to you?

Simon

Re: [PATCH] Make target_read_alloc & al return vectors

[Andreas Schwab]

On Jul 07 2018, Simon Marchi <simark@simark.ca> wrote:

> On 2018-07-07 13:23, Andreas Schwab wrote:
>> This is the next error:
>>
>> ../../gdb/ia64-linux-nat.c:73:8: error: conflicting return type
>> specified for ‘virtual bool
>> ia64_linux_nat_target::have_steppable_watchpoint()’
>>    bool have_steppable_watchpoint () { return 1; }
>>         ^
>> In file included from ../../gdb/inferior.h:41:0,
>>                  from ../../gdb/ia64-linux-nat.c:22:
>> ../../gdb/target.h:554:17: error:   overriding ‘virtual int
>> target_ops::have_steppable_watchpoint()’
>>      virtual int have_steppable_watchpoint ()
>>                  ^
>
> The fix for this is probably obvious (change int for bool).  Can you take
> care of it?  I'm not on my gdb-development-capable computer, and probably
> don't have an ia64 cross-compiler handy.  Does the patch for
> target_read_alloc look good to you?

This is the next error:

  CXXLD  gdb
ia64-linux-nat.o:(.data.rel.ro._ZTV21ia64_linux_nat_target[_ZTV21ia64_linux_nat_target]+0x170): undefined reference to `ia64_linux_nat_target::can_use_hw_breakpoint(bptype, int, int)'
ia64-linux-nat.o:(.data.rel.ro._ZTV21ia64_linux_nat_target[_ZTV21ia64_linux_nat_target]+0x650): undefined reference to `ia64_linux_nat_target::read_description()'
ia64-linux-nat.o:(.data.rel.ro._ZTV21ia64_linux_nat_target[_ZTV21ia64_linux_nat_target]+0xba0): undefined reference to `ia64_linux_nat_target::low_new_thread(lwp_info*)'
collect2: error: ld returned 1 exit status

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Make target_read_alloc & al return vectors

[Andreas Schwab]

On Jul 07 2018, Simon Marchi <simark@simark.ca> wrote:

> 	* ia64-tdep.c (get_reg_name, get_fpreg_name, get_saveloc_name,

s/tdep/libunwind-tdep/

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Make target_read_alloc & al return vectors

[Simon Marchi]

On 2018-07-07 11:10 AM, Simon Marchi wrote:
> On 2018-07-07 04:54 AM, Andreas Schwab wrote:
>> ../../gdb/ia64-tdep.c: In function ‘LONGEST getunwind_table(gdb_byte**)’:
>> ../../gdb/ia64-tdep.c:2664:16: error: too many arguments to function ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > target_read_alloc(target_ops*, target_object, const char*)’
>>      NULL, buf_p);
>>                 ^
>> In file included from ../../gdb/inferior.h:41:0,
>>                  from ../../gdb/ia64-tdep.c:21:
>> ../../gdb/target.h:341:40: note: declared here
>>  extern gdb::optional<gdb::byte_vector> target_read_alloc
>>                                         ^
>> ../../gdb/ia64-tdep.c:2663:5: error: cannot convert ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > >’ to ‘LONGEST {aka long int}’ in assignment
>>    x = target_read_alloc (current_top_target (), TARGET_OBJECT_UNWIND_TABLE,
>>      ^
>>
>> Andreas.
>>
> 
> Hi Andreas,
> 
> I have been unable to build libunwind for ia64 before (using the 1.2
> branch), that's why I missed it.  Now I tried to build the 1.1 and it
> works fine.
> 
> I fixed up the call to target_read_alloc to the best of my knowledge,
> with as little changes to the code as possible.  However, I can't test
> the result more than build it.  Can you please review the patch below?
> 
> Just to confirm, do you also see errors later in the build, like:
> 
> In file included from /home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:39:
> /home/simark/src/binutils-gdb/gdb/ia64-libunwind-tdep.c:114:1: error: ISO C++ forbids converting a string constant to ‘char*’ [-Werror=write-strings]
>  static char *get_reg_name = STRINGIFY(UNW_OBJ(get_reg));
>  ^~~~~~~~~~~~~~
> /home/simark/src/binutils-gdb/gdb/common/preprocessor.h:28:25: note: in definition of macro ‘STRINGIFY_1’
>  #define STRINGIFY_1(x) #x
>                          ^
> ?
> 
> ----
> 
> From efd663c4d54b64f394d51faabc74f249d0bf32a7 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Sat, 7 Jul 2018 10:55:14 -0400
> Subject: [PATCH] Fix compilation error in ia64-tdep.c with libunwind-ia64
> 
> Commit
> 
>   9018be22e022 ("Make target_read_alloc & al return vectors")
> 
> failed to update the code in ia64-tdep.c, for HAVE_LIBUNWIND_IA64_H.
> This patch fixes that.

I pushed this patch.

Simon