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.
This commit is contained in:
parent
05c966f3c9
commit
d5722aa2fe
14 changed files with 230 additions and 31 deletions
|
@ -31,7 +31,7 @@
|
|||
#include "cli/cli-utils.h"
|
||||
#include "gdb_bfd.h"
|
||||
#include "filestuff.h"
|
||||
|
||||
#include "common/byte-vector.h"
|
||||
|
||||
static const char *
|
||||
scan_expression_with_cleanup (const char **cmd, const char *def)
|
||||
|
@ -230,17 +230,17 @@ dump_memory_to_file (const char *cmd, const char *mode, const char *file_format)
|
|||
|
||||
/* FIXME: Should use read_memory_partial() and a magic blocking
|
||||
value. */
|
||||
std::unique_ptr<gdb_byte[]> buf (new gdb_byte[count]);
|
||||
read_memory (lo, buf.get (), count);
|
||||
gdb::byte_vector buf (count);
|
||||
read_memory (lo, buf.data (), count);
|
||||
|
||||
/* Have everything. Open/write the data. */
|
||||
if (file_format == NULL || strcmp (file_format, "binary") == 0)
|
||||
{
|
||||
dump_binary_file (filename, mode, buf.get (), count);
|
||||
dump_binary_file (filename, mode, buf.data (), count);
|
||||
}
|
||||
else
|
||||
{
|
||||
dump_bfd_file (filename, mode, file_format, lo, buf.get (), count);
|
||||
dump_bfd_file (filename, mode, file_format, lo, buf.data (), count);
|
||||
}
|
||||
|
||||
do_cleanups (old_cleanups);
|
||||
|
@ -545,13 +545,13 @@ restore_binary_file (const char *filename, struct callback_data *data)
|
|||
perror_with_name (filename);
|
||||
|
||||
/* Now allocate a buffer and read the file contents. */
|
||||
std::unique_ptr<gdb_byte[]> buf (new gdb_byte[len]);
|
||||
if (fread (buf.get (), 1, len, file) != len)
|
||||
gdb::byte_vector buf (len);
|
||||
if (fread (buf.data (), 1, len, file) != len)
|
||||
perror_with_name (filename);
|
||||
|
||||
/* Now write the buffer into target memory. */
|
||||
len = target_write_memory (data->load_start + data->load_offset,
|
||||
buf.get (), len);
|
||||
buf.data (), len);
|
||||
if (len != 0)
|
||||
warning (_("restore: memory write failed (%s)."), safe_strerror (len));
|
||||
do_cleanups (cleanup);
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue