aboutsummaryrefslogtreecommitdiff
path: root/gdb/regcache.c
diff options
context:
space:
mode:
authorSimon Marchi <simon.marchi@efficios.com>2021-05-26 09:27:54 -0400
committerSimon Marchi <simon.marchi@polymtl.ca>2021-05-26 15:03:00 -0400
commit11bb5c41eb98d8e7d4d75dfcf620f6f627523e77 (patch)
tree67c712afe801e1e8bc19e9cafa00224e16bf89f1 /gdb/regcache.c
parent983d5689cc07466ddc6664e10aebbb2b2fd49515 (diff)
downloadgdb-11bb5c41eb98d8e7d4d75dfcf620f6f627523e77.zip
gdb-11bb5c41eb98d8e7d4d75dfcf620f6f627523e77.tar.gz
gdb-11bb5c41eb98d8e7d4d75dfcf620f6f627523e77.tar.bz2
gdb: don't zero-initialize reg_buffer contents
The reg_buffer constructor zero-initializes (value-initializes, in C++ speak) the gdb_bytes of the m_registers array. This is not necessary, as these bytes are only meaningful if the corresponding register_status is REG_VALID. If the corresponding register_status is REG_VALID, then they will have been overwritten with the actual register data when reading the registers from the system into the reg_buffer. Fix that by removing the empty parenthesis following the new expression, meaning that the bytes will now be default-initialized, meaning they'll be left uninitialized. For reference, this is explained here: https://en.cppreference.com/w/cpp/language/new#Construction These new expressions were added in 835dcf92618e ("Use std::unique_ptr in reg_buffer"). As mentioned in that commit message, the use of value-initialisation was done on purpose to keep existing behavior, but now there is some data that suggest it would be beneficial not to do it, which is why I suggest changing it. This doesn't make a big difference on typical architectures where the register buffer is not that big. However, on ROCm (AMD GPU), the register buffer is about 65000 bytes big, so the reg_buffer constructor shows up in profiling. If you want to make some tests and profile it on a standard system, it's always possible to change: - m_registers.reset (new gdb_byte[m_descr->sizeof_raw_registers] ()); + m_registers.reset (new gdb_byte[65000] ()); and run a program that constantly hits a breakpoint with a false condition. For example, by doing this change and running the following program: static void break_here () {} int main () { for (int i = 0; i < 100000; i++) break_here (); } with the following GDB incantation: /usr/bin/time ./gdb -nx --data-directory=data-directory -q test -ex "b break_here if 0" -ex r -batch I get, for value-intializing: 11.75user 7.68system 0:18.54elapsed 104%CPU (0avgtext+0avgdata 56644maxresident)k And for default-initializing: 6.83user 8.42system 0:14.12elapsed 108%CPU (0avgtext+0avgdata 56512maxresident)k gdb/ChangeLog: * regcache.c (reg_buffer::reg_buffer): Default-initialize m_registers array. Change-Id: I5071a4444dee0530ce1bc58ebe712024ddd2b158
Diffstat (limited to 'gdb/regcache.c')
-rw-r--r--gdb/regcache.c7
1 files changed, 5 insertions, 2 deletions
diff --git a/gdb/regcache.c b/gdb/regcache.c
index d238630..51e9eff 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -184,15 +184,18 @@ reg_buffer::reg_buffer (gdbarch *gdbarch, bool has_pseudo)
gdb_assert (gdbarch != NULL);
m_descr = regcache_descr (gdbarch);
+ /* We don't zero-initialize the M_REGISTERS array, as the bytes it contains
+ aren't meaningful as long as the corresponding register status is not
+ REG_VALID. */
if (has_pseudo)
{
- m_registers.reset (new gdb_byte[m_descr->sizeof_cooked_registers] ());
+ m_registers.reset (new gdb_byte[m_descr->sizeof_cooked_registers]);
m_register_status.reset
(new register_status[m_descr->nr_cooked_registers] ());
}
else
{
- m_registers.reset (new gdb_byte[m_descr->sizeof_raw_registers] ());
+ m_registers.reset (new gdb_byte[m_descr->sizeof_raw_registers]);
m_register_status.reset
(new register_status[gdbarch_num_regs (gdbarch)] ());
}