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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3efdb5d05e8ff620d24805c901836ca2e8ffb196..39f444e1f78ebf289534aee5a445f5b0c189e441 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2021-05-26  Simon Marchi  <simon.marchi@efficios.com>
+
+       * regcache.c (reg_buffer::reg_buffer): Default-initialize
+       m_registers array.
+
 2021-05-26  Tom Tromey  <tom@tromey.com>
 
        * dwarf2/read.c (allocate_type_unit_groups_table)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index d2386308b8226ea4c40ddbbc4ac36d6700a18272..51e9effe6428c476e1af785c4b584155c0ff486f 100644 (file)
--- 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)] ());
     }