Make target_read_alloc & al return vectors

Message ID 1521691401-21512-1-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi March 22, 2018, 4:03 a.m. UTC
  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(-)
  

Comments

Simon Marchi April 7, 2018, 5:20 p.m. UTC | #1
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
  
Pedro Alves April 16, 2018, 8:04 p.m. UTC | #2
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.
  
Andreas Schwab July 7, 2018, 8:54 a.m. UTC | #3
../../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.
  

Patch

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);
 
-
 /* 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"));