[4/4] Variable size for regs mask in collection list
Commit Message
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(-)
Comments
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 Alves <palves@redhat.com> writes:
> Hi,
>
> I quickly skimmed this one and noticed a couple formatting nits,
> mentioned below.
Thanks! I will change these.
@@ -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);
@@ -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;