gdb: don't zero-initialize reg_buffer contents
authorSimon Marchi <simon.marchi@efficios.com>
Wed, 26 May 2021 13:27:54 +0000 (09:27 -0400)
committerSimon Marchi <simon.marchi@polymtl.ca>
Wed, 26 May 2021 19:03:00 +0000 (15:03 -0400)
commit11bb5c41eb98d8e7d4d75dfcf620f6f627523e77
tree67c712afe801e1e8bc19e9cafa00224e16bf89f1
parent983d5689cc07466ddc6664e10aebbb2b2fd49515
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
gdb/ChangeLog
gdb/regcache.c