Introduce gdb::byte_vector, add allocator that default-initializes

From: Pedro Alves <palves@redhat.com>

  On 06/14/2017 09:39 AM, Simon Marchi wrote:

> Thanks for the detailed explanation.  It's all very logical, but it's
> also full of small details essential to really understand what's happening.

Yeah...  As custom with "library" code, thankfully uses of the types
can remain oblivious to how the types work under the hood.

I've pushed the patch in now, with the trivial adjustments to both
dwarf2loc.c, after Andreas' series, and to make the new int128_t printing
code use gdb::byte_vector.

From d5722aa2fe9e1d76d98865a9ab77a7b9388743c9 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 14 Jun 2017 11:08:52 +0100
Subject: [PATCH] Introduce gdb::byte_vector, add allocator that
 default-initializes

In some cases we've been replacing heap-allocated gdb_byte buffers
managed with xmalloc/make_cleanup(xfree) with gdb::vector<gdb_byte>.
That usually pessimizes the code a little bit because std::vector
value-initializes elements (which for gdb_byte means
zero-initialization), while if you're creating a temporary buffer,
you're most certaintly going to fill it in with some data.  An
alternative is to use

  unique_ptr<gdb_byte[]> buf (new gdb_byte[size]);

but it looks like that's not very popular.

Recently, a use of obstacks in dwarf2read.c was replaced with
std::vector<gdb_byte> and that as well introduced a pessimization for
always memsetting the buffer when it's garanteed that the zeros will
be overwritten immediately.  (see dwarf2read.c change in this patch to
find it.)

So here's a different take at addressing this issue "by design":

#1 - Introduce default_init_allocator<T>

I.e., a custom allocator that does default construction using default
initialization, meaning, no more zero initialization.  That's the
default_init_allocation<T> class added in this patch.

See "Notes" at
<http://en.cppreference.com/w/cpp/container/vector/resize>.

#2 - Introduce def_vector<T>

I.e., a convenience typedef, because typing the allocator is annoying:

  using def_vector<T> = std::vector<T, gdb::default_init_allocator<T>>;

#3 - Introduce byte_vector

Because gdb_byte vectors will be the common thing, add a convenience
"byte_vector" typedef:

  using byte_vector = def_vector<gdb_byte>;

which is really the same as:

  std::vector<gdb_byte, gdb::default_init_allocator<gdb_byte>>;

The intent then is to make "gdb::byte_vector" be the go-to for dynamic
byte buffers.  So the less friction, the better.

#4 - Adjust current code to use it.

To set the example going forward.  Replace std::vector uses and also
unique_ptr<byte[]> uses.

One nice thing is that with this allocator, for changes like these:

  -std::unique_ptr<byte[]> buf (new gdb_byte[some_size]);
  +gdb::byte_vector buf (some_size);
   fill_with_data (buf.data (), buf.size ());

the generated code is the same as before.  I.e., the compiler
de-structures the vector and gets rid of the unused "reserved vs size"
related fields.

The other nice thing is that it's easier to write
  gdb::byte_vector buf (size);
than
  std::unique_ptr<gdb_byte[]> buf (new gdb_byte[size]);
or even (C++14):
  auto buf = std::make_unique<gdb_byte[]> (size); // zero-initializes...

#5 - Suggest s/std::vector<gdb_byte>/gdb::byte_vector/ going forward.

Note that this commit actually fixes a couple of bugs where the current
code is incorrectly using "std::vector::reserve(new_size)" and then
accessing the vector's internal buffer beyond the vector's size: see
dwarf2loc.c and charset.c.  That's undefined behavior and may trigger
debug mode assertion failures.  With default_init_allocator,
"resize()" behaves like "reserve()" performance wise, in that it
leaves new elements with unspecified values, but, it does that safely
without triggering undefined behavior when you access those values.

gdb/ChangeLog:
2017-06-14  Pedro Alves  <palves@redhat.com>

	* ada-lang.c: Include "common/byte-vector.h".
	(ada_value_primitive_packed_val): Use gdb::byte_vector.
	* charset.c (wchar_iterator::iterate): Resize the vector instead
	of reserving it.
	* common/byte-vector.h: Include "common/def-vector.h".
	(wchar_iterator::m_out): Now a gdb::def_vector<gdb_wchar_t>.
	* cli/cli-dump.c: Include "common/byte-vector.h".
	(dump_memory_to_file, restore_binary_file): Use gdb::byte_vector.
	* common/byte-vector.h: New file.
	* common/def-vector.h: New file.
	* common/default-init-alloc.h: New file.
	* dwarf2loc.c: Include "common/byte-vector.h".
	(rw_pieced_value): Use gdb::byte_vector, and resize the vector
	instead of reserving it.
	* dwarf2read.c: Include "common/byte-vector.h".
	(data_buf::m_vec): Now a gdb::byte_vector.
	* gdb_regex.c: Include "common/def-vector.h".
	(compiled_regex::compiled_regex): Use gdb::def_vector<char>.
	* mi/mi-main.c: Include "common/byte-vector.h".
	(mi_cmd_data_read_memory): Use gdb::byte_vector.
	* printcmd.c: Include "common/byte-vector.h".
	(print_scalar_formatted): Use gdb::byte_vector.
	* valprint.c: Include "common/byte-vector.h".
	(maybe_negate_by_bytes, print_decimal_chars): Use
	gdb::byte_vector.
---
 gdb/ChangeLog                   | 28 +++++++++++++++++
 gdb/ada-lang.c                  | 16 +++++-----
 gdb/charset.c                   |  2 +-
 gdb/charset.h                   |  4 +--
 gdb/cli/cli-dump.c              | 16 +++++-----
 gdb/common/byte-vector.h        | 62 ++++++++++++++++++++++++++++++++++++++
 gdb/common/def-vector.h         | 36 ++++++++++++++++++++++
 gdb/common/default-init-alloc.h | 67 +++++++++++++++++++++++++++++++++++++++++
 gdb/dwarf2loc.c                 |  7 +++--
 gdb/dwarf2read.c                |  3 +-
 gdb/gdb_regex.c                 |  7 +++--
 gdb/mi/mi-main.c                |  5 +--
 gdb/printcmd.c                  |  3 +-
 gdb/valprint.c                  |  5 +--
 14 files changed, 230 insertions(+), 31 deletions(-)
 create mode 100644 gdb/common/byte-vector.h
 create mode 100644 gdb/common/def-vector.h
 create mode 100644 gdb/common/default-init-alloc.h

Introduce gdb::byte_vector, add allocator that default-initializes

Commit Message

Comments

Patch