From d5722aa2fe9e1d76d98865a9ab77a7b9388743c9 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 14 Jun 2017 11:08:52 +0100 Subject: 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. 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 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 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 I.e., a custom allocator that does default construction using default initialization, meaning, no more zero initialization. That's the default_init_allocation class added in this patch. See "Notes" at . #2 - Introduce def_vector I.e., a convenience typedef, because typing the allocator is annoying: using def_vector = std::vector>; #3 - Introduce byte_vector Because gdb_byte vectors will be the common thing, add a convenience "byte_vector" typedef: using byte_vector = def_vector; which is really the same as: std::vector>; 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 uses. One nice thing is that with this allocator, for changes like these: -std::unique_ptr 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 buf (new gdb_byte[size]); or even (C++14): auto buf = std::make_unique (size); // zero-initializes... #5 - Suggest s/std::vector/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 * 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. * 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. * 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/common/byte-vector.h | 62 ++++++++++++++++++++++++++++++++++++++ gdb/common/def-vector.h | 36 ++++++++++++++++++++++ gdb/common/default-init-alloc.h | 67 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+) 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 (limited to 'gdb/common') diff --git a/gdb/common/byte-vector.h b/gdb/common/byte-vector.h new file mode 100644 index 00000000000..c17b14d8b16 --- /dev/null +++ b/gdb/common/byte-vector.h @@ -0,0 +1,62 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef COMMON_BYTE_VECTOR_H +#define COMMON_BYTE_VECTOR_H + +#include "common/def-vector.h" + +namespace gdb { + +/* byte_vector is a gdb_byte std::vector with a custom allocator that + unlike std::vector does not zero-initialize new elements + by default when the vector is created/resized. This is what you + usually want when working with byte buffers, since if you're + creating or growing a buffer you'll most surely want to fill it in + with data, in which case zero-initialization would be a + pessimization. For example: + + gdb::byte_vector buf (some_large_size); + fill_with_data (buf.data (), buf.size ()); + + On the odd case you do need zero initialization, then you can still + call the overloads that specify an explicit value, like: + + gdb::byte_vector buf (some_initial_size, 0); + buf.resize (a_bigger_size, 0); + + (Or use std::vector instead.) + + Note that unlike std::vector, function local + gdb::byte_vector objects constructed with an initial size like: + + gdb::byte_vector buf (some_size); + fill_with_data (buf.data (), buf.size ()); + + usually compile down to the exact same as: + + std::unique_ptr buf (new gdb_byte[some_size]); + fill_with_data (buf.get (), some_size); + + with the former having the advantage of being a bit more readable, + and providing the whole std::vector API, if you end up needing it. +*/ +using byte_vector = gdb::def_vector; + +} /* namespace gdb */ + +#endif /* COMMON_DEF_VECTOR_H */ diff --git a/gdb/common/def-vector.h b/gdb/common/def-vector.h new file mode 100644 index 00000000000..ab9331f94c7 --- /dev/null +++ b/gdb/common/def-vector.h @@ -0,0 +1,36 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef COMMON_DEF_VECTOR_H +#define COMMON_DEF_VECTOR_H + +#include +#include "common/default-init-alloc.h" + +namespace gdb { + +/* A vector that uses an allocator that default constructs using + default-initialization rather than value-initialization. The idea + is to use this when you don't want zero-initialization of elements + of vectors of trivial types. E.g., byte buffers. */ + +template using def_vector + = std::vector>; + +} /* namespace gdb */ + +#endif /* COMMON_DEF_VECTOR_H */ diff --git a/gdb/common/default-init-alloc.h b/gdb/common/default-init-alloc.h new file mode 100644 index 00000000000..4fb852f03de --- /dev/null +++ b/gdb/common/default-init-alloc.h @@ -0,0 +1,67 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef COMMON_DEFAULT_INIT_ALLOC_H +#define COMMON_DEFAULT_INIT_ALLOC_H + +namespace gdb { + +/* An allocator that default constructs using default-initialization + rather than value-initialization. The idea is to use this when you + don't want to default construct elements of containers of trivial + types using zero-initialization. */ + +/* Mostly as implementation convenience, this is implemented as an + adapter that given an allocator A, overrides 'A::construct()'. 'A' + defaults to std::allocator. */ + +template> +class default_init_allocator : public A +{ +public: + /* Pull in A's ctors. */ + using A::A; + + /* Override rebind. */ + template + struct rebind + { + /* A couple helpers just to make it a bit more readable. */ + typedef std::allocator_traits traits_; + typedef typename traits_::template rebind_alloc alloc_; + + /* This is what we're after. */ + typedef default_init_allocator other; + }; + + /* Make the base allocator's construct method(s) visible. */ + using A::construct; + + /* .. and provide an override/overload for the case of default + construction (i.e., no arguments). This is where we construct + with default-init. */ + template + void construct (U *ptr) + noexcept (std::is_nothrow_default_constructible::value) + { + ::new ((void *) ptr) U; /* default-init */ + } +}; + +} /* namespace gdb */ + +#endif /* COMMON_DEFAULT_INIT_ALLOC_H */ -- cgit v1.2.1