diff mbox

Fix build breakage on GNU/Linux AArch64 and use std::vector on tdesc.reg_defs

Message ID 20170910055056.21186-1-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Sept. 10, 2017, 5:50 a.m. UTC 
  This patch fixes the build breakage that has been happening on AArch64
since September 5th.  The breakage was introduced by the following
commit:

  author	Yao Qi <yao.qi@linaro.org>
	  Tue, 5 Sep 2017 04:54:52 -0400 (09:54 +0100)
  committer	Yao Qi <yao.qi@linaro.org>
	  Tue, 5 Sep 2017 04:54:52 -0400 (09:54 +0100)
  commit	f7000548a2b79d7e5cb924468117ca4245e6b820

  Use VEC for target_desc.reg_defs

The build log for this commit can be seen here:

  <https://gdb-build.sergiodj.net/builders/Ubuntu-AArch64-native-gdbserver-m64/builds/2696/steps/compile%20gdb/logs/stdio>

And the underlying problem is that the code is not calling the new
function "allocate_target_description" to allocate the "struct
target_desc" using "new" instead of XNEW, which end up not properly
initializing the fields of the structure.

I took the opportunity to convert the code to std::vector (instead of
VEC), which makes things even simpler.  This has been regtested on the
BuildBot, without regressions, and fixes the build breakage.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>

	* linux-low.c (handle_extended_wait): Use
	"allocate_target_description" instead of "XNEW".
	* linux-x86-low.c (initialize_low_arch): Likewise.
	* regcache.c (get_thread_regcache): Update code to use "std::vector"
	instead of "VEC" for "target_desc.reg_defs".
	(regcache_cpy): Likewise.
	(registers_to_string): Likewise.
	(registers_from_string): Likewise.
	(find_regno): Likewise.
	(supply_regblock): Likewise.
	(regcache_raw_read_unsigned): Likewise.
	* tdesc.c (init_target_desc): Likewise.
	(tdesc_create_reg): Likewise.
	* tdesc.h: Remove declaration of "tdesc_reg_p".  Include <vector>.
	(struct target_desc) <reg_defs>: Convert to "std::vector".
	(target_desc): Do not initialize "reg_defs".
	(~target_desc): Update code to use "std::vector" instead of "VEC"
	for "target_desc.reg_defs".
	(operator==): Likewise.
---
 gdb/gdbserver/linux-low.c     |  2 +-
 gdb/gdbserver/linux-x86-low.c |  4 ++--
 gdb/gdbserver/regcache.c      | 33 +++++++++++++++++----------------
 gdb/gdbserver/tdesc.c         | 14 ++++++--------
 gdb/gdbserver/tdesc.h         | 26 +++++++++-----------------
 5 files changed, 35 insertions(+), 44 deletions(-)

Comments

Simon Marchi Sept. 10, 2017, 9:02 a.m. UTC | #1 
On 2017-09-10 07:50, Sergio Durigan Junior wrote:
> This patch fixes the build breakage that has been happening on AArch64
> since September 5th.  The breakage was introduced by the following
> commit:
> 
>   author	Yao Qi <yao.qi@linaro.org>
> 	  Tue, 5 Sep 2017 04:54:52 -0400 (09:54 +0100)
>   committer	Yao Qi <yao.qi@linaro.org>
> 	  Tue, 5 Sep 2017 04:54:52 -0400 (09:54 +0100)
>   commit	f7000548a2b79d7e5cb924468117ca4245e6b820
> 
>   Use VEC for target_desc.reg_defs
> 
> The build log for this commit can be seen here:
> 
> 
> <https://gdb-build.sergiodj.net/builders/Ubuntu-AArch64-native-gdbserver-m64/builds/2696/steps/compile%20gdb/logs/stdio>
> 
> And the underlying problem is that the code is not calling the new
> function "allocate_target_description" to allocate the "struct
> target_desc" using "new" instead of XNEW, which end up not properly
> initializing the fields of the structure.
> 
> I took the opportunity to convert the code to std::vector (instead of
> VEC), which makes things even simpler.  This has been regtested on the
> BuildBot, without regressions, and fixes the build breakage.

Hi Sergio,

This patch seems to do two independent changes:

  - use allocate_target_description instead of XNEW
  - replace VEC with std::vector

Can you post separate patches?  You can push the first one directly, I 
would call it obvious (and it's the important bits to fix the build).  
The second one, Yao might want to take a look.

Thanks!

Simon
Sergio Durigan Junior Sept. 10, 2017, 4:12 p.m. UTC | #2 
On Sunday, September 10 2017, Simon Marchi wrote:

> On 2017-09-10 07:50, Sergio Durigan Junior wrote:
>> This patch fixes the build breakage that has been happening on AArch64
>> since September 5th.  The breakage was introduced by the following
>> commit:
>>
>>   author	Yao Qi <yao.qi@linaro.org>
>> 	  Tue, 5 Sep 2017 04:54:52 -0400 (09:54 +0100)
>>   committer	Yao Qi <yao.qi@linaro.org>
>> 	  Tue, 5 Sep 2017 04:54:52 -0400 (09:54 +0100)
>>   commit	f7000548a2b79d7e5cb924468117ca4245e6b820
>>
>>   Use VEC for target_desc.reg_defs
>>
>> The build log for this commit can be seen here:
>>
>>
>> <https://gdb-build.sergiodj.net/builders/Ubuntu-AArch64-native-gdbserver-m64/builds/2696/steps/compile%20gdb/logs/stdio>
>>
>> And the underlying problem is that the code is not calling the new
>> function "allocate_target_description" to allocate the "struct
>> target_desc" using "new" instead of XNEW, which end up not properly
>> initializing the fields of the structure.
>>
>> I took the opportunity to convert the code to std::vector (instead of
>> VEC), which makes things even simpler.  This has been regtested on the
>> BuildBot, without regressions, and fixes the build breakage.
>
> Hi Sergio,
>
> This patch seems to do two independent changes:
>
>  - use allocate_target_description instead of XNEW
>  - replace VEC with std::vector
>
> Can you post separate patches?  You can push the first one directly, I
> would call it obvious (and it's the important bits to fix the build).
> The second one, Yao might want to take a look.

Sure thing, I'll split it and resubmit.  Thanks.
diff mbox

Patch

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6f4b26afb9..a7859cce88 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -570,7 +570,7 @@  handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
 
 	  clone_all_breakpoints (child_thr, event_thr);
 
-	  tdesc = XNEW (struct target_desc);
+	  tdesc = allocate_target_description ();
 	  copy_target_description (tdesc, parent_proc->tdesc);
 	  child_proc->tdesc = tdesc;
 
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index f09871ac36..844a165666 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -2900,7 +2900,7 @@  initialize_low_arch (void)
 {
   /* Initialize the Linux target descriptions.  */
 #ifdef __x86_64__
-  tdesc_amd64_linux_no_xml = XNEW (struct target_desc);
+  tdesc_amd64_linux_no_xml = allocate_target_description ();
   copy_target_description (tdesc_amd64_linux_no_xml,
 			   amd64_linux_read_description (X86_XSTATE_SSE_MASK,
 							 false));
@@ -2911,7 +2911,7 @@  initialize_low_arch (void)
   initialize_low_tdesc ();
 #endif
 
-  tdesc_i386_linux_no_xml = XNEW (struct target_desc);
+  tdesc_i386_linux_no_xml = allocate_target_description ();
   copy_target_description (tdesc_i386_linux_no_xml,
 			   i386_linux_read_description (X86_XSTATE_SSE_MASK));
   tdesc_i386_linux_no_xml->xmltarget = xmltarget_i386_linux_no_xml;
diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
index 43e78a5cf7..75480912fb 100644
--- a/gdb/gdbserver/regcache.c
+++ b/gdb/gdbserver/regcache.c
@@ -54,7 +54,7 @@  get_thread_regcache (struct thread_info *thread, int fetch)
       current_thread = thread;
       /* Invalidate all registers, to prevent stale left-overs.  */
       memset (regcache->register_status, REG_UNAVAILABLE,
-	      VEC_length (tdesc_reg_p, regcache->tdesc->reg_defs));
+	      regcache->tdesc->reg_defs.size ());
       fetch_inferior_registers (regcache, -1);
       current_thread = saved_thread;
       regcache->registers_valid = 1;
@@ -145,9 +145,9 @@  init_register_cache (struct regcache *regcache,
 	= (unsigned char *) xcalloc (1, tdesc->registers_size);
       regcache->registers_owned = 1;
       regcache->register_status
-	= (unsigned char *) xmalloc (VEC_length (tdesc_reg_p, tdesc->reg_defs));
+	= (unsigned char *) xmalloc (tdesc->reg_defs.size ());
       memset ((void *) regcache->register_status, REG_UNAVAILABLE,
-	      VEC_length (tdesc_reg_p, tdesc->reg_defs));
+	      tdesc->reg_defs.size ());
 #else
       gdb_assert_not_reached ("can't allocate memory from the heap");
 #endif
@@ -204,7 +204,7 @@  regcache_cpy (struct regcache *dst, struct regcache *src)
 #ifndef IN_PROCESS_AGENT
   if (dst->register_status != NULL && src->register_status != NULL)
     memcpy (dst->register_status, src->register_status,
-	    VEC_length (tdesc_reg_p, src->tdesc->reg_defs));
+	    src->tdesc->reg_defs.size ());
 #endif
   dst->registers_valid = src->registers_valid;
 }
@@ -217,11 +217,11 @@  registers_to_string (struct regcache *regcache, char *buf)
 {
   unsigned char *registers = regcache->registers;
   const struct target_desc *tdesc = regcache->tdesc;
-  int i;
-  reg *reg;
 
-  for (i = 0; VEC_iterate (tdesc_reg_p, tdesc->reg_defs, i, reg); i++)
+  for (int i = 0; i < tdesc->reg_defs.size (); ++i)
     {
+      struct reg *reg = tdesc->reg_defs[i];
+
       if (regcache->register_status[i] == REG_VALID)
 	{
 	  bin2hex (registers, buf, register_size (tdesc, i));
@@ -257,12 +257,13 @@  registers_from_string (struct regcache *regcache, char *buf)
 int
 find_regno (const struct target_desc *tdesc, const char *name)
 {
-  int i;
-  reg *reg;
+  for (int i = 0; i < tdesc->reg_defs.size (); ++i)
+    {
+      struct reg *reg = tdesc->reg_defs[i];
 
-  for (i = 0; VEC_iterate (tdesc_reg_p, tdesc->reg_defs, i, reg); i++)
-    if (strcmp (name, reg->name) == 0)
-      return i;
+      if (strcmp (name, reg->name) == 0)
+	return i;
+    }
   internal_error (__FILE__, __LINE__, "Unknown register %s requested",
 		  name);
 }
@@ -272,7 +273,7 @@  find_regno (const struct target_desc *tdesc, const char *name)
 struct reg *
 find_register_by_number (const struct target_desc *tdesc, int n)
 {
-  return VEC_index (tdesc_reg_p, tdesc->reg_defs, n);
+  return tdesc->reg_defs[n];
 }
 
 #ifndef IN_PROCESS_AGENT
@@ -388,7 +389,7 @@  supply_regblock (struct regcache *regcache, const void *buf)
       {
 	int i;
 
-	for (i = 0; i < VEC_length (tdesc_reg_p, tdesc->reg_defs); i++)
+	for (i = 0; i < tdesc->reg_defs.size (); i++)
 	  regcache->register_status[i] = REG_VALID;
       }
 #endif
@@ -402,7 +403,7 @@  supply_regblock (struct regcache *regcache, const void *buf)
       {
 	int i;
 
-	for (i = 0; i < VEC_length (tdesc_reg_p, tdesc->reg_defs); i++)
+	for (i = 0; i < tdesc->reg_defs.size (); i++)
 	  regcache->register_status[i] = REG_UNAVAILABLE;
       }
 #endif
@@ -435,7 +436,7 @@  regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
 
   gdb_assert (regcache != NULL);
   gdb_assert (regnum >= 0
-	      && regnum < VEC_length (tdesc_reg_p, regcache->tdesc->reg_defs));
+	      && regnum < regcache->tdesc->reg_defs.size ());
 
   size = register_size (regcache->tdesc, regnum);
 
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 53f36d5564..63d6467d56 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -22,11 +22,9 @@ 
 void
 init_target_desc (struct target_desc *tdesc)
 {
-  int offset, i;
-  struct reg *reg;
+  int offset = 0;
 
-  offset = 0;
-  for (i = 0; VEC_iterate (tdesc_reg_p, tdesc->reg_defs, i, reg); i++)
+  for (reg *reg : tdesc->reg_defs)
     {
       reg->offset = offset;
       offset += reg->size;
@@ -193,23 +191,23 @@  tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 {
   struct target_desc *tdesc = (struct target_desc *) feature;
 
-  while (VEC_length (tdesc_reg_p, tdesc->reg_defs) < regnum)
+  while (tdesc->reg_defs.size () < regnum)
     {
       struct reg *reg = XCNEW (struct reg);
 
       reg->name = "";
       reg->size = 0;
-      VEC_safe_push (tdesc_reg_p, tdesc->reg_defs, reg);
+      tdesc->reg_defs.push_back (reg);
     }
 
   gdb_assert (regnum == 0
-	      || regnum == VEC_length (tdesc_reg_p, tdesc->reg_defs));
+	      || regnum == tdesc->reg_defs.size ());
 
   struct reg *reg = XCNEW (struct reg);
 
   reg->name = name;
   reg->size = bitsize;
-  VEC_safe_push (tdesc_reg_p, tdesc->reg_defs, reg);
+  tdesc->reg_defs.push_back (reg);
 }
 
 /* See arch/tdesc.h.  */
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index 71249e4eda..ec4d6b38dc 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -22,9 +22,7 @@ 
 #include "arch/tdesc.h"
 
 #include "regdef.h"
-
-typedef struct reg *tdesc_reg_p;
-DEF_VEC_P(tdesc_reg_p);
+#include <vector>
 
 struct tdesc_feature
 {};
@@ -36,7 +34,7 @@  struct target_desc : tdesc_feature
 {
   /* A vector of elements of register definitions that
      describe the inferior's register set.  */
-  VEC(tdesc_reg_p) *reg_defs;
+  std::vector<struct reg *> reg_defs;
 
   /* The register cache size, in bytes.  */
   int registers_size;
@@ -66,17 +64,16 @@  struct target_desc : tdesc_feature
 
 public:
   target_desc ()
-    : reg_defs (NULL), registers_size (0)
+    : registers_size (0)
   {}
 
   ~target_desc ()
   {
     int i;
-    struct reg *reg;
 
-    for (i = 0; VEC_iterate (tdesc_reg_p, reg_defs, i, reg); i++)
+    for (reg *reg : reg_defs)
       xfree (reg);
-    VEC_free (tdesc_reg_p, reg_defs);
+    reg_defs.clear ();
 
     xfree ((char *) arch);
     xfree ((char *) osabi);
@@ -90,18 +87,13 @@  public:
 
   bool operator== (const target_desc &other) const
   {
-    if (VEC_length (tdesc_reg_p, reg_defs)
-	!= VEC_length (tdesc_reg_p, other.reg_defs))
+    if (reg_defs.size () != other.reg_defs.size ())
       return false;
 
-    struct reg *reg;
-
-    for (int ix = 0;
-	 VEC_iterate (tdesc_reg_p, reg_defs, ix, reg);
-	 ix++)
+    for (int i = 0; i < reg_defs.size (); ++i)
       {
-	struct reg *reg2
-	  = VEC_index (tdesc_reg_p, other.reg_defs, ix);
+	struct reg *reg = reg_defs[i];
+	struct reg *reg2 = other.reg_defs[i];
 
 	if (reg != reg2 && *reg != *reg2)
 	  return false;