[4/4] Variable size for regs mask in collection list

Message ID 20180620210855.6385-5-pedromfc@linux.vnet.ibm.com
State New
Headers show
Series
  • Allow larger sizes for tracepoint register masks
Related show

Commit Message

Pedro Franco de Carvalho June 20, 2018, 9:08 p.m.
This patch changes collection_list to allow larger register masks.

The mask is changed from an array to a vector and is initialized with
num_regs + num_pseudoregs. Although no pseudoreg numbers should be set
in the mask, some parts of collection_list still do this.

The mask is also resized as needed when new registers are added to the
mask.

YYYY-MM-DD  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>

	* tracepoint.h (collection_list) <m_regs_mask>: Change type to
	std::vector<unsigned char>.
	* tracepoint.c (collection_list::collection_list): Remove
	m_regs_mask initializer from initializer list. Resize m_regs_mask
	in using the number of registers from the arch.
	(collection_list::add_register): Resize m_regs_mask instead of
	throwing error when regno is too large.
	(collection_list::stringify): Use size () instead of sizeof. Use
	xsnprintf instead of sprintf.
---
 gdb/tracepoint.c | 28 +++++++++++++++++++++-------
 gdb/tracepoint.h |  2 +-
 2 files changed, 22 insertions(+), 8 deletions(-)

-- 
2.13.6

Comments

Pedro Alves June 26, 2018, 4:58 p.m. | #1
Hi,

I quickly skimmed this one and noticed a couple formatting nits,
mentioned below.

On 06/20/2018 10:08 PM, Pedro Franco de Carvalho wrote:

> @@ -1086,9 +1087,21 @@ collection_list::add_static_trace_data ()

>  }

>  

>  collection_list::collection_list ()

> -  : m_regs_mask (),

> -    m_strace_data (false)

> +  : m_strace_data (false)

>  {

> +  /* Guess the number of bytes needed for the register mask. If it's

> +     too low, it will be expanded in add_register. If it's too high,

> +     stringify will ignore the extra bytes.


Double-space after the periods of the first two sentences.

> +

> +     The mask shouldn't include pseudoreg numbers, but

> +     encode_actions_1 currently doesn't handle remote register numbers

> +     and pseudoregs properly for tracepoint actions that don't

> +     generate an AX (e.g. "collect $<pseudoreg>").  */

> +  int num_regs = gdbarch_num_regs (target_gdbarch ())

> +    + gdbarch_num_pseudo_regs (target_gdbarch ());


Wrap in parens so that emacs tab-indents the second line
under gdbarch:

     int num_regs = (gdbarch_num_regs (target_gdbarch ())
                     + gdbarch_num_pseudo_regs (target_gdbarch ()));

Thanks,
Pedro Alves
Pedro Franco de Carvalho June 26, 2018, 6:52 p.m. | #2
Pedro Alves <palves@redhat.com> writes:

> Hi,

>

> I quickly skimmed this one and noticed a couple formatting nits,

> mentioned below.


Thanks! I will change these.

Patch

diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 5af3cfe202..2cffc0251e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -818,9 +818,10 @@  collection_list::add_register (unsigned int regno)
 {
   if (info_verbose)
     printf_filtered ("collect register %d\n", regno);
-  if (regno >= (8 * sizeof (m_regs_mask)))
-    error (_("Internal: register number %d too large for tracepoint"),
-	   regno);
+
+  if (regno / 8 >= m_regs_mask.size ())
+    m_regs_mask.resize ((regno / 8) + 1);
+
   m_regs_mask[regno / 8] |= 1 << (regno % 8);
 }
 
@@ -1086,9 +1087,21 @@  collection_list::add_static_trace_data ()
 }
 
 collection_list::collection_list ()
-  : m_regs_mask (),
-    m_strace_data (false)
+  : m_strace_data (false)
 {
+  /* Guess the number of bytes needed for the register mask. If it's
+     too low, it will be expanded in add_register. If it's too high,
+     stringify will ignore the extra bytes.
+
+     The mask shouldn't include pseudoreg numbers, but
+     encode_actions_1 currently doesn't handle remote register numbers
+     and pseudoregs properly for tracepoint actions that don't
+     generate an AX (e.g. "collect $<pseudoreg>").  */
+  int num_regs = gdbarch_num_regs (target_gdbarch ())
+    + gdbarch_num_pseudo_regs (target_gdbarch ());
+
+  m_regs_mask.resize ((num_regs / 8) + 1);
+
   m_memranges.reserve (128);
   m_aexprs.reserve (128);
 }
@@ -1113,7 +1126,7 @@  collection_list::stringify ()
       str_list.emplace_back (temp_buf, end - temp_buf);
     }
 
-  for (i = sizeof (m_regs_mask) - 1; i > 0; i--)
+  for (i = m_regs_mask.size () - 1; i > 0; i--)
     if (m_regs_mask[i] != 0)    /* Skip leading zeroes in regs_mask.  */
       break;
   if (m_regs_mask[i] != 0)	/* Prepare to send regs_mask to the stub.  */
@@ -1127,7 +1140,8 @@  collection_list::stringify ()
 	  QUIT;			/* Allow user to bail out with ^C.  */
 	  if (info_verbose)
 	    printf_filtered ("%02X", m_regs_mask[i]);
-	  sprintf (end, "%02X", m_regs_mask[i]);
+	  xsnprintf (end, sizeof (temp_buf) - (end - temp_buf),
+		     "%02X", m_regs_mask[i]);
 	  end += 2;
 	}
       str_list.emplace_back (temp_buf);
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 42e413018a..59279a73ef 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -289,7 +289,7 @@  public:
 
 private:
   /* room for up to 256 regs */
-  unsigned char m_regs_mask[32];
+  std::vector<unsigned char> m_regs_mask;
 
   std::vector<memrange> m_memranges;