public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/6] Use gdb::byte_vector in agent_expr
Date: Mon, 19 Jun 2023 15:06:40 -0600	[thread overview]
Message-ID: <20230619-ax-new-v1-2-b26175d997a9@tromey.com> (raw)
In-Reply-To: <20230619-ax-new-v1-0-b26175d997a9@tromey.com>

This changes agent_expr to use gdb::byte_vector rather than manually
reimplementing a vector.
---
 gdb/ax-gdb.c     |  28 +++++++--------
 gdb/ax-general.c | 101 ++++++++++++++++++-------------------------------------
 gdb/ax.h         |   8 +----
 gdb/dwarf2/loc.c |   4 +--
 gdb/remote.c     |  14 ++++----
 gdb/tracepoint.c |  13 +++----
 6 files changed, 63 insertions(+), 105 deletions(-)

diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 927dfc6337e..b0bde62f465 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -818,7 +818,6 @@ static int
 is_nontrivial_conversion (struct type *from, struct type *to)
 {
   agent_expr_up ax (new agent_expr (NULL, 0));
-  int nontrivial;
 
   /* Actually generate the code, and see if anything came out.  At the
      moment, it would be trivial to replicate the code in
@@ -827,8 +826,7 @@ is_nontrivial_conversion (struct type *from, struct type *to)
      way allows this function to be independent of the logic in
      gen_conversion.  */
   gen_conversion (ax.get (), from, to);
-  nontrivial = ax->len > 0;
-  return nontrivial;
+  return !ax->buf.empty ();
 }
 
 
@@ -1719,10 +1717,10 @@ ternop_cond_operation::do_generate_ax (struct expression *exp,
   std::get<1> (m_storage)->generate_ax (exp, ax, &value2);
   gen_usual_unary (ax, &value2);
   end = ax_goto (ax, aop_goto);
-  ax_label (ax, if1, ax->len);
+  ax_label (ax, if1, ax->buf.size ());
   std::get<2> (m_storage)->generate_ax (exp, ax, &value3);
   gen_usual_unary (ax, &value3);
-  ax_label (ax, end, ax->len);
+  ax_label (ax, end, ax->buf.size ());
   /* This is arbitrary - what if B and C are incompatible types? */
   value->type = value2.type;
   value->kind = value2.kind;
@@ -1811,12 +1809,12 @@ unop_sizeof_operation::do_generate_ax (struct expression *exp,
      only way to find an expression's type is to generate code for it.
      So we generate code for the operand, and then throw it away,
      replacing it with code that simply pushes its size.  */
-  int start = ax->len;
+  int start = ax->buf.size ();
 
   std::get<0> (m_storage)->generate_ax (exp, ax, value);
 
   /* Throw away the code we just generated.  */
-  ax->len = start;
+  ax->buf.resize (start);
 
   ax_const_l (ax, value->type->length ());
   value->kind = axs_rvalue;
@@ -2033,18 +2031,18 @@ logical_and_operation::do_generate_ax (struct expression *exp,
   gen_usual_unary (ax, &value1);
   if1 = ax_goto (ax, aop_if_goto);
   go1 = ax_goto (ax, aop_goto);
-  ax_label (ax, if1, ax->len);
+  ax_label (ax, if1, ax->buf.size ());
   std::get<1> (m_storage)->generate_ax (exp, ax, &value2);
   gen_usual_unary (ax, &value2);
   if2 = ax_goto (ax, aop_if_goto);
   go2 = ax_goto (ax, aop_goto);
-  ax_label (ax, if2, ax->len);
+  ax_label (ax, if2, ax->buf.size ());
   ax_const_l (ax, 1);
   end = ax_goto (ax, aop_goto);
-  ax_label (ax, go1, ax->len);
-  ax_label (ax, go2, ax->len);
+  ax_label (ax, go1, ax->buf.size ());
+  ax_label (ax, go2, ax->buf.size ());
   ax_const_l (ax, 0);
-  ax_label (ax, end, ax->len);
+  ax_label (ax, end, ax->buf.size ());
   value->kind = axs_rvalue;
   value->type = builtin_type (ax->gdbarch)->builtin_int;
 }
@@ -2067,10 +2065,10 @@ logical_or_operation::do_generate_ax (struct expression *exp,
   if2 = ax_goto (ax, aop_if_goto);
   ax_const_l (ax, 0);
   end = ax_goto (ax, aop_goto);
-  ax_label (ax, if1, ax->len);
-  ax_label (ax, if2, ax->len);
+  ax_label (ax, if1, ax->buf.size ());
+  ax_label (ax, if2, ax->buf.size ());
   ax_const_l (ax, 1);
-  ax_label (ax, end, ax->len);
+  ax_label (ax, end, ax->buf.size ());
   value->kind = axs_rvalue;
   value->type = builtin_type (ax->gdbarch)->builtin_int;
 }
diff --git a/gdb/ax-general.c b/gdb/ax-general.c
index e43e7732260..d5f4c51e65d 100644
--- a/gdb/ax-general.c
+++ b/gdb/ax-general.c
@@ -28,8 +28,6 @@
 #include "value.h"
 #include "user-regs.h"
 
-static void grow_expr (struct agent_expr *x, int n);
-
 static void append_const (struct agent_expr *x, LONGEST val, int n);
 
 static LONGEST read_const (struct agent_expr *x, int o, int n);
@@ -40,11 +38,6 @@ static void generic_ext (struct agent_expr *x, enum agent_op op, int n);
 
 agent_expr::agent_expr (struct gdbarch *gdbarch, CORE_ADDR scope)
 {
-  this->len = 0;
-  this->size = 1;		/* Change this to a larger value once
-				   reallocation code is tested.  */
-  this->buf = (unsigned char *) xmalloc (this->size);
-
   this->gdbarch = gdbarch;
   this->scope = scope;
 
@@ -58,25 +51,9 @@ agent_expr::agent_expr (struct gdbarch *gdbarch, CORE_ADDR scope)
 
 agent_expr::~agent_expr ()
 {
-  xfree (this->buf);
   xfree (this->reg_mask);
 }
 
-/* Make sure that X has room for at least N more bytes.  This doesn't
-   affect the length, just the allocated size.  */
-static void
-grow_expr (struct agent_expr *x, int n)
-{
-  if (x->len + n > x->size)
-    {
-      x->size *= 2;
-      if (x->size < x->len + n)
-	x->size = x->len + n + 10;
-      x->buf = (unsigned char *) xrealloc (x->buf, x->size);
-    }
-}
-
-
 /* Append the low N bytes of VAL as an N-byte integer to the
    expression X, in big-endian order.  */
 static void
@@ -84,13 +61,11 @@ append_const (struct agent_expr *x, LONGEST val, int n)
 {
   int i;
 
-  grow_expr (x, n);
   for (i = n - 1; i >= 0; i--)
     {
-      x->buf[x->len + i] = val & 0xff;
+      x->buf.push_back (val & 0xff);
       val >>= 8;
     }
-  x->len += n;
 }
 
 
@@ -103,7 +78,7 @@ read_const (struct agent_expr *x, int o, int n)
   LONGEST accum = 0;
 
   /* Make sure we're not reading off the end of the expression.  */
-  if (o + n > x->len)
+  if (o + n > x->buf.size ())
     error (_("GDB bug: ax-general.c (read_const): incomplete constant"));
 
   for (i = 0; i < n; i++)
@@ -117,8 +92,7 @@ read_const (struct agent_expr *x, int o, int n)
 void
 ax_raw_byte (struct agent_expr *x, gdb_byte byte)
 {
-  grow_expr (x, 1);
-  x->buf[x->len++] = byte;
+  x->buf.push_back (byte);
 }
 
 /* Append a simple operator OP to EXPR.  */
@@ -154,9 +128,8 @@ generic_ext (struct agent_expr *x, enum agent_op op, int n)
     error (_("GDB bug: ax-general.c (generic_ext): "
 	     "opcode has inadequate range"));
 
-  grow_expr (x, 2);
-  x->buf[x->len++] = op;
-  x->buf[x->len++] = n;
+  x->buf.push_back (op);
+  x->buf.push_back (n);
 }
 
 
@@ -185,9 +158,8 @@ ax_trace_quick (struct agent_expr *x, int n)
     error (_("GDB bug: ax-general.c (ax_trace_quick): "
 	     "size out of range for trace_quick"));
 
-  grow_expr (x, 2);
-  x->buf[x->len++] = aop_trace_quick;
-  x->buf[x->len++] = n;
+  x->buf.push_back (aop_trace_quick);
+  x->buf.push_back (n);
 }
 
 
@@ -200,12 +172,10 @@ ax_trace_quick (struct agent_expr *x, int n)
 int
 ax_goto (struct agent_expr *x, enum agent_op op)
 {
-  grow_expr (x, 3);
-  x->buf[x->len + 0] = op;
-  x->buf[x->len + 1] = 0xff;
-  x->buf[x->len + 2] = 0xff;
-  x->len += 3;
-  return x->len - 2;
+  x->buf.push_back (op);
+  x->buf.push_back (0xff);
+  x->buf.push_back (0xff);
+  return x->buf.size () - 2;
 }
 
 /* Suppose a given call to ax_goto returns some value PATCH.  When you
@@ -294,11 +264,9 @@ ax_reg (struct agent_expr *x, int reg)
       if (reg < 0 || reg > 0xffff)
 	error (_("GDB bug: ax-general.c (ax_reg): "
 		 "register number out of range"));
-      grow_expr (x, 3);
-      x->buf[x->len] = aop_reg;
-      x->buf[x->len + 1] = (reg >> 8) & 0xff;
-      x->buf[x->len + 2] = (reg) & 0xff;
-      x->len += 3;
+      x->buf.push_back (aop_reg);
+      x->buf.push_back ((reg >> 8) & 0xff);
+      x->buf.push_back ((reg) & 0xff);
     }
 }
 
@@ -312,11 +280,9 @@ ax_tsv (struct agent_expr *x, enum agent_op op, int num)
     internal_error (_("ax-general.c (ax_tsv): variable "
 		      "number is %d, out of range"), num);
 
-  grow_expr (x, 3);
-  x->buf[x->len] = op;
-  x->buf[x->len + 1] = (num >> 8) & 0xff;
-  x->buf[x->len + 2] = (num) & 0xff;
-  x->len += 3;
+  x->buf.push_back (op);
+  x->buf.push_back ((num >> 8) & 0xff);
+  x->buf.push_back ((num) & 0xff);
 }
 
 /* Append a string to the expression.  Note that the string is going
@@ -334,12 +300,11 @@ ax_string (struct agent_expr *x, const char *str, int slen)
     internal_error (_("ax-general.c (ax_string): string "
 		      "length is %d, out of allowed range"), slen);
 
-  grow_expr (x, 2 + slen + 1);
-  x->buf[x->len++] = ((slen + 1) >> 8) & 0xff;
-  x->buf[x->len++] = (slen + 1) & 0xff;
+  x->buf.push_back (((slen + 1) >> 8) & 0xff);
+  x->buf.push_back ((slen + 1) & 0xff);
   for (i = 0; i < slen; ++i)
-    x->buf[x->len++] = str[i];
-  x->buf[x->len++] = '\0';
+    x->buf.push_back (str[i]);
+  x->buf.push_back ('\0');
 }
 \f
 
@@ -375,7 +340,7 @@ ax_print (struct ui_file *f, struct agent_expr *x)
       != aop_last)
     error (_("GDB bug: ax-general.c (ax_print): opcode map out of sync"));
 
-  for (i = 0; i < x->len;)
+  for (i = 0; i < x->buf.size ();)
     {
       enum agent_op op = (enum agent_op) x->buf[i];
 
@@ -386,7 +351,7 @@ ax_print (struct ui_file *f, struct agent_expr *x)
 	  i++;
 	  continue;
 	}
-      if (i + 1 + aop_map[op].op_size > x->len)
+      if (i + 1 + aop_map[op].op_size > x->buf.size ())
 	{
 	  gdb_printf (f, _("%3d  <incomplete opcode %s>\n"),
 		      i, aop_map[op].name);
@@ -471,28 +436,28 @@ ax_reqs (struct agent_expr *ax)
 
   /* Jump target table.  targets[i] is non-zero iff we have found a
      jump to offset i.  */
-  char *targets = (char *) alloca (ax->len * sizeof (targets[0]));
+  char *targets = (char *) alloca (ax->buf.size () * sizeof (targets[0]));
 
   /* Instruction boundary table.  boundary[i] is non-zero iff our scan
      has reached an instruction starting at offset i.  */
-  char *boundary = (char *) alloca (ax->len * sizeof (boundary[0]));
+  char *boundary = (char *) alloca (ax->buf.size () * sizeof (boundary[0]));
 
   /* Stack height record.  If either targets[i] or boundary[i] is
      non-zero, heights[i] is the height the stack should have before
      executing the bytecode at that point.  */
-  int *heights = (int *) alloca (ax->len * sizeof (heights[0]));
+  int *heights = (int *) alloca (ax->buf.size () * sizeof (heights[0]));
 
   /* Pointer to a description of the present op.  */
   struct aop_map *op;
 
-  memset (targets, 0, ax->len * sizeof (targets[0]));
-  memset (boundary, 0, ax->len * sizeof (boundary[0]));
+  memset (targets, 0, ax->buf.size () * sizeof (targets[0]));
+  memset (boundary, 0, ax->buf.size () * sizeof (boundary[0]));
 
   ax->max_height = ax->min_height = height = 0;
   ax->flaw = agent_flaw_none;
   ax->max_data_size = 0;
 
-  for (i = 0; i < ax->len; i += 1 + op->op_size)
+  for (i = 0; i < ax->buf.size (); i += 1 + op->op_size)
     {
       if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
 	{
@@ -508,7 +473,7 @@ ax_reqs (struct agent_expr *ax)
 	  return;
 	}
 
-      if (i + 1 + op->op_size > ax->len)
+      if (i + 1 + op->op_size > ax->buf.size ())
 	{
 	  ax->flaw = agent_flaw_incomplete_instruction;
 	  return;
@@ -543,7 +508,7 @@ ax_reqs (struct agent_expr *ax)
 	  || aop_if_goto == op - aop_map)
 	{
 	  int target = read_const (ax, i + 1, 2);
-	  if (target < 0 || target >= ax->len)
+	  if (target < 0 || target >= ax->buf.size ())
 	    {
 	      ax->flaw = agent_flaw_bad_jump;
 	      return;
@@ -568,7 +533,7 @@ ax_reqs (struct agent_expr *ax)
       /* For unconditional jumps with a successor, check that the
 	 successor is a target, and pick up its stack height.  */
       if (aop_goto == op - aop_map
-	  && i + 3 < ax->len)
+	  && i + 3 < ax->buf.size ())
 	{
 	  if (!targets[i + 3])
 	    {
@@ -589,7 +554,7 @@ ax_reqs (struct agent_expr *ax)
     }
 
   /* Check that all the targets are on boundaries.  */
-  for (i = 0; i < ax->len; i++)
+  for (i = 0; i < ax->buf.size (); i++)
     if (targets[i] && !boundary[i])
       {
 	ax->flaw = agent_flaw_bad_jump;
diff --git a/gdb/ax.h b/gdb/ax.h
index 9c47a8d0b63..608af4c8c68 100644
--- a/gdb/ax.h
+++ b/gdb/ax.h
@@ -86,13 +86,7 @@ struct agent_expr
     ~agent_expr ();
 
     /* The bytes of the expression.  */
-    unsigned char *buf;
-
-    /* The number of bytecode in the expression.  */
-    int len;
-
-    /* Allocated space available currently.  */
-    int size;
+    gdb::byte_vector buf;
 
     /* The target architecture assumed to be in effect.  */
     struct gdbarch *gdbarch;
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index e50e7f1e47e..5b2d58ab44e 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -2399,7 +2399,7 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc,
       int64_t offset;
       int i;
 
-      offsets[op_ptr - base] = expr->len;
+      offsets[op_ptr - base] = expr->buf.size ();
       ++op_ptr;
 
       /* Our basic approach to code generation is to map DWARF
@@ -2734,7 +2734,7 @@ dwarf2_compile_expr_to_ax (struct agent_expr *expr, struct axs_value *loc,
 	  ax_const_l (expr, 0);
 	  ax_simple (expr, aop_swap);
 	  ax_simple (expr, aop_sub);
-	  ax_label (expr, i, expr->len);
+	  ax_label (expr, i, expr->buf.size ());
 	  break;
 
 	case DW_OP_neg:
diff --git a/gdb/remote.c b/gdb/remote.c
index 02ff3a12bdb..5d09fa1758a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10693,9 +10693,9 @@ remote_add_target_side_condition (struct gdbarch *gdbarch,
   /* Send conditions to the target.  */
   for (agent_expr *aexpr : bp_tgt->conditions)
     {
-      xsnprintf (buf, buf_end - buf, "X%x,", aexpr->len);
+      xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
       buf += strlen (buf);
-      for (int i = 0; i < aexpr->len; ++i)
+      for (int i = 0; i < aexpr->buf.size (); ++i)
 	buf = pack_hex_byte (buf, aexpr->buf[i]);
       *buf = '\0';
     }
@@ -10718,9 +10718,9 @@ remote_add_target_side_commands (struct gdbarch *gdbarch,
      cmds parameter.  */
   for (agent_expr *aexpr : bp_tgt->tcommands)
     {
-      sprintf (buf, "X%x,", aexpr->len);
+      sprintf (buf, "X%x,", (int) aexpr->buf.size ());
       buf += strlen (buf);
-      for (int i = 0; i < aexpr->len; ++i)
+      for (int i = 0; i < aexpr->buf.size (); ++i)
 	buf = pack_hex_byte (buf, aexpr->buf[i]);
       *buf = '\0';
     }
@@ -13386,7 +13386,7 @@ remote_target::download_tracepoint (struct bp_location *loc)
 	  size_left = buf.size () - strlen (buf.data ());
 
 	  ret = snprintf (buf.data () + strlen (buf.data ()),
-			  size_left, ":X%x,", aexpr->len);
+			  size_left, ":X%x,", (int) aexpr->buf.size ());
 
 	  if (ret < 0 || ret >= size_left)
 	    error ("%s", err_msg);
@@ -13395,12 +13395,12 @@ remote_target::download_tracepoint (struct bp_location *loc)
 
 	  /* Two bytes to encode each aexpr byte, plus the terminating
 	     null byte.  */
-	  if (aexpr->len * 2 + 1 > size_left)
+	  if (aexpr->buf.size () * 2 + 1 > size_left)
 	    error ("%s", err_msg);
 
 	  pkt = buf.data () + strlen (buf.data ());
 
-	  for (int ndx = 0; ndx < aexpr->len; ++ndx)
+	  for (int ndx = 0; ndx < aexpr->buf.size (); ++ndx)
 	    pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
 	  *pkt = '\0';
 	}
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 0af7404aef1..335df997c05 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -618,7 +618,7 @@ finalize_tracepoint_aexpr (struct agent_expr *aexpr)
 {
   ax_reqs (aexpr);
 
-  if (aexpr->len > MAX_AGENT_EXPR_LEN)
+  if (aexpr->buf.size () > MAX_AGENT_EXPR_LEN)
     error (_("Expression is too complicated."));
 
   report_agent_reqs_errors (aexpr);
@@ -885,7 +885,7 @@ collection_list::add_local_register (struct gdbarch *gdbarch,
 	 corresponding raw registers in the ax mask, but if this isn't
 	 the case add the expression that is generated to the
 	 collection list.  */
-      if (aexpr->len > 0)
+      if (aexpr->buf.size () > 0)
 	add_aexpr (std::move (aexpr));
     }
 }
@@ -1215,18 +1215,19 @@ collection_list::stringify ()
   for (i = 0; i < m_aexprs.size (); i++)
     {
       QUIT;			/* Allow user to bail out with ^C.  */
-      if ((count + 10 + 2 * m_aexprs[i]->len) > MAX_AGENT_EXPR_LEN)
+      if ((count + 10 + 2 * m_aexprs[i]->buf.size ()) > MAX_AGENT_EXPR_LEN)
 	{
 	  str_list.emplace_back (temp_buf.data (), count);
 	  count = 0;
 	  end = temp_buf.data ();
 	}
-      sprintf (end, "X%08X,", m_aexprs[i]->len);
+      sprintf (end, "X%08X,", (int) m_aexprs[i]->buf.size ());
       end += 10;		/* 'X' + 8 hex digits + ',' */
       count += 10;
 
-      end += 2 * bin2hex (m_aexprs[i]->buf, end, m_aexprs[i]->len);
-      count += 2 * m_aexprs[i]->len;
+      end += 2 * bin2hex (m_aexprs[i]->buf.data (), end,
+			  m_aexprs[i]->buf.size ());
+      count += 2 * m_aexprs[i]->buf.size ();
     }
 
   if (count != 0)

-- 
2.39.2


  parent reply	other threads:[~2023-06-19 21:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-19 21:06 [PATCH 0/6] C++-ify and simplify agent expressions Tom Tromey
2023-06-19 21:06 ` [PATCH 1/6] Remove mem2hex Tom Tromey
2023-06-19 21:06 ` Tom Tromey [this message]
2023-06-19 21:06 ` [PATCH 3/6] Use std::vector<bool> for agent_expr::reg_mask Tom Tromey
2023-06-20 15:30   ` John Baldwin
2023-06-20 17:04     ` Tom Tromey
2023-06-19 21:06 ` [PATCH 4/6] Simplify agent_expr constructor Tom Tromey
2023-06-19 21:06 ` [PATCH 5/6] Use bool for agent_expr::tracing Tom Tromey
2023-06-20 15:31   ` John Baldwin
2023-06-20 17:04     ` Tom Tromey
2023-06-19 21:06 ` [PATCH 6/6] Make aop_map 'static' Tom Tromey
2023-06-20 15:39   ` John Baldwin
2023-06-20 17:12     ` Tom Tromey

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20230619-ax-new-v1-2-b26175d997a9@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

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

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