[pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer.

Pedro Alves
2017-06-12T16:08:18Z

On 05/26/2017 07:25 PM, Jan Kratochvil wrote:
> gdb/ChangeLog
> 2017-05-26 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Code cleanup: C++ify .gdb_index producer.
> * common/common-defs.h <c++11> (std::make_unique): New.
> * dwarf2read.c: Remove include common/gdb_unlinker.h. Add includes
> unordered_set and unordered_map.
> (MAYBE_SWAP): Cast it to offset_type.
> (struct strtab_entry, hash_strtab_entry, eq_strtab_entry)
> (create_strtab, add_string): Remove.
> (file_write, file_write, DataBuf): New.
> (struct symtab_index_entry): Use std::vector for cu_indices.
> (struct mapped_symtab): Use std::vector for data.
> (hash_symtab_entry, eq_symtab_entry, delete_symtab_entry)
> (create_symbol_hash_table, create_mapped_symtab, cleanup_mapped_symtab):
> Remove.
> (find_slot): Change return type. Update it to the new data structures.
> (hash_expand, add_index_entry): Update it to the new data structures.
> (offset_type_compare): Remove.
> (uniquify_cu_indices): Update it to the new data structures.
> (CstrView, CstrViewHasher, VectorHasher): New.
> (add_indices_to_cpool): Remove.
> (write_hash_table): Update it to the new data structures.
> (struct psymtab_cu_index_map, hash_psymtab_cu_index)
> (eq_psymtab_cu_index): Remove.
> (struct addrmap_index_data): Change addr_obstack pointer to DataBuf
> reference and std::unordered_map for cu_index_htab.
> (add_address_entry, add_address_entry_worker, write_address_map)
> (write_psymbols): Update it to the new data structures.
> (write_obstack): Remove.
> (struct signatured_type_index_data): Change types_list to a DataBuf
> reference and psyms_seen to a std::unordered_set reference.
> (write_one_signatured_type, recursively_write_psymbols)
> (write_psymtabs_to_index): Update it to the new data structures.

Thanks. This looks generally good.

It had a number of GDB/GCC code convention issues though.

- Should use /**/ for comments.
- CamelCase should only used in template parameters.
- Function/ctors should be placed before data members.
- Private data members should be prefixed with "m_".
- Some function intro comments still referred to
obstacks, while the functions now take a DataBuf.

Also, the big TRY block would be better done with a RAII type.
Tromey posted a patch to add a unique_ptr with a deleter that calls
fclose, and makes that code use it, but that's not merged yet.
Meanwhile we can add a little RAII class locally that does the
same thing, still avoiding the churn.

The std::vector<gdb_byte> in DataBuf introduces a subtle
pessimization compared to obstacks -- std::vector value-initializes its
elements, which for gdb_byte means zero initialization. But
DataBuf _always_ overwrites those zeros anyway... I've suggested
adding a custom allocator that default-initializes instead before,
and this new case made me go do it. I'll post it in its own thread.

>
> +// Provide C++14 std::make_unique<> for C++11 compilation mode.
> +// A copy from: gcc/libstdc++-v3/include/bits/unique_ptr.h
> +#if __cplusplus <= 201103L
> +namespace std {
> + template<typename _Tp>
> + struct _MakeUniq
> + { typedef unique_ptr<_Tp> __single_object; };
> + template<typename _Tp, typename... _Args>
> + inline typename _MakeUniq<_Tp>::__single_object
> + make_unique(_Args&&... __args)
> + { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
> +}
> +#endif

We've been putting replacements for > C++11 bits in the gdb namespace,
while taking care not to abuse the implementation namespace.

The _MakeUniq type above is used in the original implementation
as SFINAE helper to pick the right version of make_unique between
single object and array version. But since this didn't import the
array MakeUniq<_Tp[]> specializations, that type seems kind of pointless
to whoever reads the imported code as is.

I think that if we want to start using make_unique, it'd be better to
introduce it as a separate patch, add it under the gdb namespace, and
adjust the multiple places in the codebase that do either:

std::unique_ptr<type> ptr (new type());
ptr.reset (new type ());

to use gdb::make_unique at the same time.
I'll post a patch for that later.

Since the series only uses make_unique in a single place, and it's
not actually the main problematic use case that make_unique
protects against, we can just simply drop that std::make_unique
bit and use ptr.reset(new type()) there for now.
Actually, I'm going to remove that heap allocation anyway in
a following patch.

Note that this patch as is actually causes a ~10% performance
drop in .gdb_index generation that can be significant when
running gdb index on big binaries or in a a batch of binaries.

Running "save gdb-index" on a "-g3 -O2" build of gdb, on x86-64
we can see it:

$ ./gdb -data-directory=data-directory --batch -q ./gdb.cxx-ified -ex "save gdb-index ."

Before:

real 0m0.738s
user 0m0.700s
sys 0m0.042s

After:

real 0m0.810s
user 0m0.767s
sys 0m0.045s

There's some variation between the runs, but the drop is
consistent. The above are representative of the average
I get.

With this script that loops 100 times generating the
same index, we can better observe the effects of just
saving the index:

$ cat save-index.cmd
set $i = 0
while $i < 100
save gdb-index .
set $i = $i + 1
end
$ time ./gdb -data-directory=data-directory -nx --batch -q -x save-index.cmd ./gdb.cxx-ified

Before C++ification (average of 5 runs):

~5.7s real time

After (average of 5 runs):

~7.0s real time

At first I suspected that most of the regression could be
solely explained by the fact that libiberty's hash is just more
efficient than the std::unordered_map/std::unordered_set,
because the latter have unfortunately (widely known) bad cache
locality due to use of separate chaining, and that there would
not be much we could do if we use these types.

But running perf on gdb I saw some hot spots that could be fixed
without having to change the data structures implementations used.

(For performance critical code, it may still be a good idea to reuse
for example gcc's hash table/map, which are C++fied versions of
libiberty's open addressing hash tables; I expect they'll have no
trouble beating unordered_map/unordered_set for use cases like
e.g., sets of pointers.)

Anyway, in interest of forward progress:

- I fixed up the style issues mentioned above, and pushed the
result in, as below. I'll post the diff as a reply to this
email.

I'd much appreciate it if you'd adjust the rest of the series
similarly.

- I'll also push in a series of follow up patches that fixes the
performance regression and makes index generation a bit faster
than before this patch, actually; and also cleans up a few things
further while at it.

Tested with --target_board=dwarf4-gdb-index.

Thanks,
Pedro Alves

From bc8f2430e08cc2a520db49a42686e0529be4a3bc Mon Sep 17 00:00:00 2001
From: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Mon, 12 Jun 2017 16:29:53 +0100
Subject: [PATCH] Code cleanup: C++ify .gdb_index producer

gdb/ChangeLog
2017-06-12 Jan Kratochvil <jan.kratochvil@redhat.com>

Code cleanup: C++ify .gdb_index producer.
* dwarf2read.c: Include <unordered_set> and <unordered_map>.
(MAYBE_SWAP) [WORDS_BIGENDIAN]: Cast to offset_type.
(struct strtab_entry, hash_strtab_entry, eq_strtab_entry)
(create_strtab, add_string): Remove.
(file_write, data_buf): New.
(struct symtab_index_entry): Use std::vector for cu_indices.
(struct mapped_symtab): Use std::vector for data.
(hash_symtab_entry, eq_symtab_entry, delete_symtab_entry)
(create_symbol_hash_table, create_mapped_symtab, cleanup_mapped_symtab):
Remove.
(find_slot): Change return type. Update it to the new data structures.
(hash_expand, add_index_entry): Update it to the new data structures.
(offset_type_compare): Remove.
(uniquify_cu_indices): Update it to the new data structures.
(c_str_view, c_str_view_hasher, vector_hasher): New.
(add_indices_to_cpool): Remove.
(write_hash_table): Update it to the new data structures.
(struct psymtab_cu_index_map, hash_psymtab_cu_index)
(eq_psymtab_cu_index): Remove.
(psym_index_map): New typedef.
(struct addrmap_index_data): Change addr_obstack pointer to data_buf
reference and std::unordered_map for cu_index_htab.
(add_address_entry, add_address_entry_worker, write_address_map)
(write_psymbols): Update it to the new data structures.
(write_obstack): Remove.
(struct signatured_type_index_data): Change types_list to a data_buf
reference and psyms_seen to a std::unordered_set reference.
(write_one_signatured_type, recursively_write_psymbols)
(write_psymtabs_to_index): Update it to the new data structures.
---
gdb/ChangeLog | 33 +++
gdb/dwarf2read.c | 709 +++++++++++++++++++++----------------------------------
2 files changed, 308 insertions(+), 434 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 38a40e1..037fa0c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,36 @@
+2017-06-12 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ Code cleanup: C++ify .gdb_index producer.
+ * dwarf2read.c: Include <unordered_set> and <unordered_map>.
+ (MAYBE_SWAP) [WORDS_BIGENDIAN]: Cast to offset_type.
+ (struct strtab_entry, hash_strtab_entry, eq_strtab_entry)
+ (create_strtab, add_string): Remove.
+ (file_write, data_buf): New.
+ (struct symtab_index_entry): Use std::vector for cu_indices.
+ (struct mapped_symtab): Use std::vector for data.
+ (hash_symtab_entry, eq_symtab_entry, delete_symtab_entry)
+ (create_symbol_hash_table, create_mapped_symtab, cleanup_mapped_symtab):
+ Remove.
+ (find_slot): Change return type. Update it to the new data structures.
+ (hash_expand, add_index_entry): Update it to the new data structures.
+ (offset_type_compare): Remove.
+ (uniquify_cu_indices): Update it to the new data structures.
+ (c_str_view, c_str_view_hasher, vector_hasher): New.
+ (add_indices_to_cpool): Remove.
+ (write_hash_table): Update it to the new data structures.
+ (struct psymtab_cu_index_map, hash_psymtab_cu_index)
+ (eq_psymtab_cu_index): Remove.
+ (psym_index_map): New typedef.
+ (struct addrmap_index_data): Change addr_obstack pointer to data_buf
+ reference and std::unordered_map for cu_index_htab.
+ (add_address_entry, add_address_entry_worker, write_address_map)
+ (write_psymbols): Update it to the new data structures.
+ (write_obstack): Remove.
+ (struct signatured_type_index_data): Change types_list to a data_buf
+ reference and psyms_seen to a std::unordered_set reference.
+ (write_one_signatured_type, recursively_write_psymbols)
+ (write_psymtabs_to_index): Update it to the new data structures.
+
2017-06-11 Simon Marchi <simon.marchi@ericsson.com>

* NEWS (Changes since GDB 8.0): Announce {set,show} debug
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b58d0fc..ef21092 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -77,6 +77,8 @@
#include <fcntl.h>
#include <sys/types.h>
#include <algorithm>
+#include <unordered_set>
+#include <unordered_map>

typedef struct symbol *symbolp;
DEF_VEC_P (symbolp);
@@ -2146,7 +2148,7 @@ byte_swap (offset_type value)
#define MAYBE_SWAP(V) byte_swap (V)

#else
-#define MAYBE_SWAP(V) (V)
+#define MAYBE_SWAP(V) static_cast<offset_type> (V)
#endif /* WORDS_BIGENDIAN */

/* Read the given attribute value as an address, taking the attribute's
@@ -23193,69 +23195,69 @@ dwarf2_per_objfile_free (struct objfile *objfile, void *d)
\f
/* The "save gdb-index" command. */

-/* The contents of the hash table we create when building the string
- table. */
-struct strtab_entry
-{
- offset_type offset;
- const char *str;
-};
-
-/* Hash function for a strtab_entry.
-
- Function is used only during write_hash_table so no index format backward
- compatibility is needed. */
+/* Write SIZE bytes from the buffer pointed to by DATA to FILE, with
+ error checking. */

-static hashval_t
-hash_strtab_entry (const void *e)
+static void
+file_write (FILE *file, const void *data, size_t size)
{
- const struct strtab_entry *entry = (const struct strtab_entry *) e;
- return mapped_index_string_hash (INT_MAX, entry->str);
+ if (fwrite (data, 1, size, file) != size)
+ error (_("couldn't data write to file"));
}

-/* Equality function for a strtab_entry. */
+/* Write the contents of VEC to FILE, with error checking. */

-static int
-eq_strtab_entry (const void *a, const void *b)
+template<class Elem>
+static void
+file_write (FILE *file, const std::vector<Elem> &vec)
{
- const struct strtab_entry *ea = (const struct strtab_entry *) a;
- const struct strtab_entry *eb = (const struct strtab_entry *) b;
- return !strcmp (ea->str, eb->str);
+ file_write (file, vec.data (), vec.size() * sizeof (vec[0]));
}

-/* Create a strtab_entry hash table. */
-
-static htab_t
-create_strtab (void)
+/* In-memory buffer to prepare data to be written later to a file. */
+class data_buf
{
- return htab_create_alloc (100, hash_strtab_entry, eq_strtab_entry,
- xfree, xcalloc, xfree);
-}
+public:
+ /* Add SIZE bytes at the end of the buffer. Returns a pointer to
+ the start of the new block. */
+ gdb_byte *append_space (size_t size)
+ {
+ m_vec.resize (m_vec.size () + size);
+ return &*m_vec.end () - size;
+ }

-/* Add a string to the constant pool. Return the string's offset in
- host order. */
+ /* Copy DATA to the end of the buffer. */
+ template<typename T>
+ void append_data (const T &data)
+ {
+ std::copy (reinterpret_cast<const gdb_byte *> (&data),
+ reinterpret_cast<const gdb_byte *> (&data + 1),
+ append_space (sizeof (data)));
+ }

-static offset_type
-add_string (htab_t table, struct obstack *cpool, const char *str)
-{
- void **slot;
- struct strtab_entry entry;
- struct strtab_entry *result;
+ /* Copy CSTR (a null-terminated string) to the end of the buffer.
+ The terminating null is appended too. */
+ void append_cstr0 (const char *cstr)
+ {
+ const size_t size = strlen (cstr) + 1;
+ std::copy (cstr, cstr + size, append_space (size));
+ }

- entry.str = str;
- slot = htab_find_slot (table, &entry, INSERT);
- if (*slot)
- result = (struct strtab_entry *) *slot;
- else
- {
- result = XNEW (struct strtab_entry);
- result->offset = obstack_object_size (cpool);
- result->str = str;
- obstack_grow_str0 (cpool, str);
- *slot = result;
- }
- return result->offset;
-}
+ /* Return the size of the buffer. */
+ size_t size () const
+ {
+ return m_vec.size ();
+ }
+
+ /* Write the buffer to FILE. */
+ void file_write (FILE *file) const
+ {
+ ::file_write (file, m_vec);
+ }
+
+private:
+ std::vector<gdb_byte> m_vec;
+};

/* An entry in the symbol table. */
struct symtab_index_entry
@@ -23266,107 +23268,40 @@ struct symtab_index_entry
offset_type index_offset;
/* A sorted vector of the indices of all the CUs that hold an object
of this name. */
- VEC (offset_type) *cu_indices;
+ std::vector<offset_type> cu_indices;
};

/* The symbol table. This is a power-of-2-sized hash table. */
struct mapped_symtab
{
- offset_type n_elements;
- offset_type size;
- struct symtab_index_entry **data;
-};
-
-/* Hash function for a symtab_index_entry. */
-
-static hashval_t
-hash_symtab_entry (const void *e)
-{
- const struct symtab_index_entry *entry
- = (const struct symtab_index_entry *) e;
- return iterative_hash (VEC_address (offset_type, entry->cu_indices),
- sizeof (offset_type) * VEC_length (offset_type,
- entry->cu_indices),
- 0);
-}
-
-/* Equality function for a symtab_index_entry. */
-
-static int
-eq_symtab_entry (const void *a, const void *b)
-{
- const struct symtab_index_entry *ea = (const struct symtab_index_entry *) a;
- const struct symtab_index_entry *eb = (const struct symtab_index_entry *) b;
- int len = VEC_length (offset_type, ea->cu_indices);
- if (len != VEC_length (offset_type, eb->cu_indices))
- return 0;
- return !memcmp (VEC_address (offset_type, ea->cu_indices),
- VEC_address (offset_type, eb->cu_indices),
- sizeof (offset_type) * len);
-}
-
-/* Destroy a symtab_index_entry. */
-
-static void
-delete_symtab_entry (void *p)
-{
- struct symtab_index_entry *entry = (struct symtab_index_entry *) p;
- VEC_free (offset_type, entry->cu_indices);
- xfree (entry);
-}
-
-/* Create a hash table holding symtab_index_entry objects. */
-
-static htab_t
-create_symbol_hash_table (void)
-{
- return htab_create_alloc (100, hash_symtab_entry, eq_symtab_entry,
- delete_symtab_entry, xcalloc, xfree);
-}
-
-/* Create a new mapped symtab object. */
-
-static struct mapped_symtab *
-create_mapped_symtab (void)
-{
- struct mapped_symtab *symtab = XNEW (struct mapped_symtab);
- symtab->n_elements = 0;
- symtab->size = 1024;
- symtab->data = XCNEWVEC (struct symtab_index_entry *, symtab->size);
- return symtab;
-}
-
-/* Destroy a mapped_symtab. */
+ mapped_symtab ()
+ {
+ data.resize (1024);
+ }

-static void
-cleanup_mapped_symtab (void *p)
-{
- struct mapped_symtab *symtab = (struct mapped_symtab *) p;
- /* The contents of the array are freed when the other hash table is
- destroyed. */
- xfree (symtab->data);
- xfree (symtab);
-}
+ offset_type n_elements = 0;
+ std::vector<std::unique_ptr<symtab_index_entry>> data;
+};

-/* Find a slot in SYMTAB for the symbol NAME. Returns a pointer to
+/* Find a slot in SYMTAB for the symbol NAME. Returns a reference to
the slot.

Function is used only during write_hash_table so no index format backward
compatibility is needed. */

-static struct symtab_index_entry **
+static std::unique_ptr<symtab_index_entry> &
find_slot (struct mapped_symtab *symtab, const char *name)
{
offset_type index, step, hash = mapped_index_string_hash (INT_MAX, name);

- index = hash & (symtab->size - 1);
- step = ((hash * 17) & (symtab->size - 1)) | 1;
+ index = hash & (symtab->data.size () - 1);
+ step = ((hash * 17) & (symtab->data.size () - 1)) | 1;

for (;;)
{
if (!symtab->data[index] || !strcmp (name, symtab->data[index]->name))
- return &symtab->data[index];
- index = (index + step) & (symtab->size - 1);
+ return symtab->data[index];
+ index = (index + step) & (symtab->data.size () - 1);
}
}

@@ -23375,24 +23310,17 @@ find_slot (struct mapped_symtab *symtab, const char *name)
static void
hash_expand (struct mapped_symtab *symtab)
{
- offset_type old_size = symtab->size;
- offset_type i;
- struct symtab_index_entry **old_entries = symtab->data;
-
- symtab->size *= 2;
- symtab->data = XCNEWVEC (struct symtab_index_entry *, symtab->size);
+ auto old_entries = std::move (symtab->data);

- for (i = 0; i < old_size; ++i)
- {
- if (old_entries[i])
- {
- struct symtab_index_entry **slot = find_slot (symtab,
- old_entries[i]->name);
- *slot = old_entries[i];
- }
- }
+ symtab->data.clear ();
+ symtab->data.resize (old_entries.size () * 2);

- xfree (old_entries);
+ for (auto &it : old_entries)
+ if (it != NULL)
+ {
+ auto &ref = find_slot (symtab, it->name);
+ ref = std::move (it);
+ }
}

/* Add an entry to SYMTAB. NAME is the name of the symbol.
@@ -23404,20 +23332,18 @@ add_index_entry (struct mapped_symtab *symtab, const char *name,
int is_static, gdb_index_symbol_kind kind,
offset_type cu_index)
{
- struct symtab_index_entry **slot;
offset_type cu_index_and_attrs;

++symtab->n_elements;
- if (4 * symtab->n_elements / 3 >= symtab->size)
+ if (4 * symtab->n_elements / 3 >= symtab->data.size ())
hash_expand (symtab);

- slot = find_slot (symtab, name);
- if (!*slot)
+ std::unique_ptr<symtab_index_entry> &slot = find_slot (symtab, name);
+ if (slot == NULL)
{
- *slot = XNEW (struct symtab_index_entry);
- (*slot)->name = name;
+ slot.reset (new symtab_index_entry ());
+ slot->name = name;
/* index_offset is set later. */
- (*slot)->cu_indices = NULL;
}

cu_index_and_attrs = 0;
@@ -23432,18 +23358,7 @@ add_index_entry (struct mapped_symtab *symtab, const char *name,
the last entry pushed), but a symbol could have multiple kinds in one CU.
To keep things simple we don't worry about the duplication here and
sort and uniqufy the list after we've processed all symbols. */
- VEC_safe_push (offset_type, (*slot)->cu_indices, cu_index_and_attrs);
-}
-
-/* qsort helper routine for uniquify_cu_indices. */
-
-static int
-offset_type_compare (const void *ap, const void *bp)
-{
- offset_type a = *(offset_type *) ap;
- offset_type b = *(offset_type *) bp;
-
- return (a > b) - (b > a);
+ slot->cu_indices.push_back (cu_index_and_attrs);
}

/* Sort and remove duplicates of all symbols' cu_indices lists. */
@@ -23451,112 +23366,118 @@ offset_type_compare (const void *ap, const void *bp)
static void
uniquify_cu_indices (struct mapped_symtab *symtab)
{
- int i;
-
- for (i = 0; i < symtab->size; ++i)
+ for (const auto &entry : symtab->data)
{
- struct symtab_index_entry *entry = symtab->data[i];
-
- if (entry
- && entry->cu_indices != NULL)
+ if (entry && !entry->cu_indices.empty ())
{
unsigned int next_to_insert, next_to_check;
offset_type last_value;

- qsort (VEC_address (offset_type, entry->cu_indices),
- VEC_length (offset_type, entry->cu_indices),
- sizeof (offset_type), offset_type_compare);
+ std::sort (entry->cu_indices.begin (), entry->cu_indices.end ());

- last_value = VEC_index (offset_type, entry->cu_indices, 0);
+ last_value = entry->cu_indices[0];
next_to_insert = 1;
for (next_to_check = 1;
- next_to_check < VEC_length (offset_type, entry->cu_indices);
+ next_to_check < entry->cu_indices.size ();
++next_to_check)
- {
- if (VEC_index (offset_type, entry->cu_indices, next_to_check)
- != last_value)
- {
- last_value = VEC_index (offset_type, entry->cu_indices,
- next_to_check);
- VEC_replace (offset_type, entry->cu_indices, next_to_insert,
- last_value);
- ++next_to_insert;
- }
- }
- VEC_truncate (offset_type, entry->cu_indices, next_to_insert);
+ if (entry->cu_indices[next_to_check] != last_value)
+ {
+ last_value = entry->cu_indices[next_to_check];
+ entry->cu_indices[next_to_insert] = last_value;
+ ++next_to_insert;
+ }
+ entry->cu_indices.resize (next_to_insert);
}
}
}

-/* Add a vector of indices to the constant pool. */
-
-static offset_type
-add_indices_to_cpool (htab_t symbol_hash_table, struct obstack *cpool,
- struct symtab_index_entry *entry)
+/* A form of 'const char *' suitable for container keys. Only the
+ pointer is stored. The strings themselves are compared, not the
+ pointers. */
+class c_str_view
{
- void **slot;
-
- slot = htab_find_slot (symbol_hash_table, entry, INSERT);
- if (!*slot)
- {
- offset_type len = VEC_length (offset_type, entry->cu_indices);
- offset_type val = MAYBE_SWAP (len);
- offset_type iter;
- int i;
+public:
+ c_str_view (const char *cstr)
+ : m_cstr (cstr)
+ {}

- *slot = entry;
- entry->index_offset = obstack_object_size (cpool);
+ bool operator== (const c_str_view &other) const
+ {
+ return strcmp (m_cstr, other.m_cstr) == 0;
+ }

- obstack_grow (cpool, &val, sizeof (val));
- for (i = 0;
- VEC_iterate (offset_type, entry->cu_indices, i, iter);
- ++i)
- {
- val = MAYBE_SWAP (iter);
- obstack_grow (cpool, &val, sizeof (val));
- }
- }
- else
- {
- struct symtab_index_entry *old_entry
- = (struct symtab_index_entry *) *slot;
- entry->index_offset = old_entry->index_offset;
- entry = old_entry;
- }
- return entry->index_offset;
-}
+private:
+ friend class c_str_view_hasher;
+ const char *const m_cstr;
+};

-/* Write the mapped hash table SYMTAB to the obstack OUTPUT, with
- constant pool entries going into the obstack CPOOL. */
+/* A std::unordered_map::hasher for c_str_view that uses the right
+ hash function for strings in a mapped index. */
+class c_str_view_hasher
+{
+public:
+ size_t operator () (const c_str_view &x) const
+ {
+ return mapped_index_string_hash (INT_MAX, x.m_cstr);
+ }
+};

-static void
-write_hash_table (struct mapped_symtab *symtab,
- struct obstack *output, struct obstack *cpool)
+/* A std::unordered_map::hasher for std::vector<>. */
+template<typename T>
+class vector_hasher
{
- offset_type i;
- htab_t symbol_hash_table;
- htab_t str_table;
+public:
+ size_t operator () (const std::vector<T> &key) const
+ {
+ return iterative_hash (key.data (),
+ sizeof (key.front ()) * key.size (), 0);
+ }
+};

- symbol_hash_table = create_symbol_hash_table ();
- str_table = create_strtab ();
+/* Write the mapped hash table SYMTAB to the data buffer OUTPUT, with
+ constant pool entries going into the data buffer CPOOL. */

- /* We add all the index vectors to the constant pool first, to
- ensure alignment is ok. */
- for (i = 0; i < symtab->size; ++i)
- {
- if (symtab->data[i])
- add_indices_to_cpool (symbol_hash_table, cpool, symtab->data[i]);
- }
+static void
+write_hash_table (mapped_symtab *symtab, data_buf &output, data_buf &cpool)
+{
+ {
+ /* Elements are sorted vectors of the indices of all the CUs that
+ hold an object of this name. */
+ std::unordered_map<std::vector<offset_type>, offset_type,
+ vector_hasher<offset_type>>
+ symbol_hash_table;
+
+ /* We add all the index vectors to the constant pool first, to
+ ensure alignment is ok. */
+ for (const std::unique_ptr<symtab_index_entry> &it : symtab->data)
+ {
+ if (it == NULL)
+ continue;
+ gdb_assert (it->index_offset == 0);
+ const auto insertpair
+ = symbol_hash_table.emplace (it->cu_indices, cpool.size ());
+ it->index_offset = insertpair.first->second;
+ if (!insertpair.second)
+ continue;
+ cpool.append_data (MAYBE_SWAP (it->cu_indices.size ()));
+ for (const auto iter : it->cu_indices)
+ cpool.append_data (MAYBE_SWAP (iter));
+ }
+ }

/* Now write out the hash table. */
- for (i = 0; i < symtab->size; ++i)
+ std::unordered_map<c_str_view, offset_type, c_str_view_hasher> str_table;
+ for (const auto &it : symtab->data)
{
offset_type str_off, vec_off;

- if (symtab->data[i])
+ if (it != NULL)
{
- str_off = add_string (str_table, cpool, symtab->data[i]->name);
- vec_off = symtab->data[i]->index_offset;
+ const auto insertpair = str_table.emplace (it->name, cpool.size ());
+ if (insertpair.second)
+ cpool.append_cstr0 (it->name);
+ str_off = insertpair.first->second;
+ vec_off = it->index_offset;
}
else
{
@@ -23566,50 +23487,23 @@ write_hash_table (struct mapped_symtab *symtab,
vec_off = 0;
}

- str_off = MAYBE_SWAP (str_off);
- vec_off = MAYBE_SWAP (vec_off);
-
- obstack_grow (output, &str_off, sizeof (str_off));
- obstack_grow (output, &vec_off, sizeof (vec_off));
+ output.append_data (MAYBE_SWAP (str_off));
+ output.append_data (MAYBE_SWAP (vec_off));
}
-
- htab_delete (str_table);
- htab_delete (symbol_hash_table);
}

-/* Struct to map psymtab to CU index in the index file. */
-struct psymtab_cu_index_map
-{
- struct partial_symtab *psymtab;
- unsigned int cu_index;
-};
-
-static hashval_t
-hash_psymtab_cu_index (const void *item)
-{
- const struct psymtab_cu_index_map *map
- = (const struct psymtab_cu_index_map *) item;
-
- return htab_hash_pointer (map->psymtab);
-}
-
-static int
-eq_psymtab_cu_index (const void *item_lhs, const void *item_rhs)
-{
- const struct psymtab_cu_index_map *lhs
- = (const struct psymtab_cu_index_map *) item_lhs;
- const struct psymtab_cu_index_map *rhs
- = (const struct psymtab_cu_index_map *) item_rhs;
-
- return lhs->psymtab == rhs->psymtab;
-}
+typedef std::unordered_map<partial_symtab *, unsigned int> psym_index_map;

/* Helper struct for building the address table. */
struct addrmap_index_data
{
+ addrmap_index_data (data_buf &addr_vec_, psym_index_map &cu_index_htab_)
+ : addr_vec (addr_vec_), cu_index_htab (cu_index_htab_)
+ {}
+
struct objfile *objfile;
- struct obstack *addr_obstack;
- htab_t cu_index_htab;
+ data_buf &addr_vec;
+ psym_index_map &cu_index_htab;

/* Non-zero if the previous_* fields are valid.
We can't write an entry until we see the next entry (since it is only then
@@ -23621,24 +23515,21 @@ struct addrmap_index_data
CORE_ADDR previous_cu_start;
};

-/* Write an address entry to OBSTACK. */
+/* Write an address entry to ADDR_VEC. */

static void
-add_address_entry (struct objfile *objfile, struct obstack *obstack,
+add_address_entry (struct objfile *objfile, data_buf &addr_vec,
CORE_ADDR start, CORE_ADDR end, unsigned int cu_index)
{
- offset_type cu_index_to_write;
- gdb_byte addr[8];
CORE_ADDR baseaddr;

baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));

- store_unsigned_integer (addr, 8, BFD_ENDIAN_LITTLE, start - baseaddr);
- obstack_grow (obstack, addr, 8);
- store_unsigned_integer (addr, 8, BFD_ENDIAN_LITTLE, end - baseaddr);
- obstack_grow (obstack, addr, 8);
- cu_index_to_write = MAYBE_SWAP (cu_index);
- obstack_grow (obstack, &cu_index_to_write, sizeof (offset_type));
+ store_unsigned_integer (addr_vec.append_space (8), 8, BFD_ENDIAN_LITTLE,
+ start - baseaddr);
+ store_unsigned_integer (addr_vec.append_space (8), 8, BFD_ENDIAN_LITTLE,
+ end - baseaddr);
+ addr_vec.append_data (MAYBE_SWAP (cu_index));
}

/* Worker function for traversing an addrmap to build the address table. */
@@ -23650,44 +23541,39 @@ add_address_entry_worker (void *datap, CORE_ADDR start_addr, void *obj)
struct partial_symtab *pst = (struct partial_symtab *) obj;

if (data->previous_valid)
- add_address_entry (data->objfile, data->addr_obstack,
+ add_address_entry (data->objfile, data->addr_vec,
data->previous_cu_start, start_addr,
data->previous_cu_index);

data->previous_cu_start = start_addr;
if (pst != NULL)
{
- struct psymtab_cu_index_map find_map, *map;
- find_map.psymtab = pst;
- map = ((struct psymtab_cu_index_map *)
- htab_find (data->cu_index_htab, &find_map));
- gdb_assert (map != NULL);
- data->previous_cu_index = map->cu_index;
+ const auto it = data->cu_index_htab.find (pst);
+ gdb_assert (it != data->cu_index_htab.cend ());
+ data->previous_cu_index = it->second;
data->previous_valid = 1;
}
else
- data->previous_valid = 0;
+ data->previous_valid = 0;

return 0;
}

-/* Write OBJFILE's address map to OBSTACK.
+/* Write OBJFILE's address map to ADDR_VEC.
CU_INDEX_HTAB is used to map addrmap entries to their CU indices
in the index file. */

static void
-write_address_map (struct objfile *objfile, struct obstack *obstack,
- htab_t cu_index_htab)
+write_address_map (struct objfile *objfile, data_buf &addr_vec,
+ psym_index_map &cu_index_htab)
{
- struct addrmap_index_data addrmap_index_data;
+ struct addrmap_index_data addrmap_index_data (addr_vec, cu_index_htab);

/* When writing the address table, we have to cope with the fact that
the addrmap iterator only provides the start of a region; we have to
wait until the next invocation to get the start of the next region. */

addrmap_index_data.objfile = objfile;
- addrmap_index_data.addr_obstack = obstack;
- addrmap_index_data.cu_index_htab = cu_index_htab;
addrmap_index_data.previous_valid = 0;

addrmap_foreach (objfile->psymtabs_addrmap, add_address_entry_worker,
@@ -23699,7 +23585,7 @@ write_address_map (struct objfile *objfile, struct obstack *obstack,
doesn't work here. To cope we pass 0xff...ff, this is a rare situation
anyway. */
if (addrmap_index_data.previous_valid)
- add_address_entry (objfile, obstack,
+ add_address_entry (objfile, addr_vec,
addrmap_index_data.previous_cu_start, (CORE_ADDR) -1,
addrmap_index_data.previous_cu_index);
}
@@ -23746,7 +23632,7 @@ symbol_kind (struct partial_symbol *psym)

static void
write_psymbols (struct mapped_symtab *symtab,
- htab_t psyms_seen,
+ std::unordered_set<partial_symbol *> &psyms_seen,
struct partial_symbol **psymp,
int count,
offset_type cu_index,
@@ -23755,43 +23641,33 @@ write_psymbols (struct mapped_symtab *symtab,
for (; count-- > 0; ++psymp)
{
struct partial_symbol *psym = *psymp;
- void **slot;

if (SYMBOL_LANGUAGE (psym) == language_ada)
error (_("Ada is not currently supported by the index"));

/* Only add a given psymbol once. */
- slot = htab_find_slot (psyms_seen, psym, INSERT);
- if (!*slot)
+ if (psyms_seen.insert (psym).second)
{
gdb_index_symbol_kind kind = symbol_kind (psym);

- *slot = psym;
add_index_entry (symtab, SYMBOL_SEARCH_NAME (psym),
is_static, kind, cu_index);
}
}
}

-/* Write the contents of an ("unfinished") obstack to FILE. Throw an
- exception if there is an error. */
-
-static void
-write_obstack (FILE *file, struct obstack *obstack)
-{
- if (fwrite (obstack_base (obstack), 1, obstack_object_size (obstack),
- file)
- != obstack_object_size (obstack))
- error (_("couldn't data write to file"));
-}
-
/* A helper struct used when iterating over debug_types. */
struct signatured_type_index_data
{
+ signatured_type_index_data (data_buf &types_list_,
+ std::unordered_set<partial_symbol *> &psyms_seen_)
+ : types_list (types_list_), psyms_seen (psyms_seen_)
+ {}
+
struct objfile *objfile;
struct mapped_symtab *symtab;
- struct obstack *types_list;
- htab_t psyms_seen;
+ data_buf &types_list;
+ std::unordered_set<partial_symbol *> &psyms_seen;
int cu_index;
};

@@ -23805,7 +23681,6 @@ write_one_signatured_type (void **slot, void *d)
= (struct signatured_type_index_data *) d;
struct signatured_type *entry = (struct signatured_type *) *slot;
struct partial_symtab *psymtab = entry->per_cu.v.psymtab;
- gdb_byte val[8];

write_psymbols (info->symtab,
info->psyms_seen,
@@ -23820,14 +23695,14 @@ write_one_signatured_type (void **slot, void *d)
psymtab->n_static_syms, info->cu_index,
1);

- store_unsigned_integer (val, 8, BFD_ENDIAN_LITTLE,
+ store_unsigned_integer (info->types_list.append_space (8), 8,
+ BFD_ENDIAN_LITTLE,
to_underlying (entry->per_cu.sect_off));
- obstack_grow (info->types_list, val, 8);
- store_unsigned_integer (val, 8, BFD_ENDIAN_LITTLE,
+ store_unsigned_integer (info->types_list.append_space (8), 8,
+ BFD_ENDIAN_LITTLE,
to_underlying (entry->type_offset_in_tu));
- obstack_grow (info->types_list, val, 8);
- store_unsigned_integer (val, 8, BFD_ENDIAN_LITTLE, entry->signature);
- obstack_grow (info->types_list, val, 8);
+ store_unsigned_integer (info->types_list.append_space (8), 8,
+ BFD_ENDIAN_LITTLE, entry->signature);

++info->cu_index;

@@ -23841,7 +23716,7 @@ static void
recursively_write_psymbols (struct objfile *objfile,
struct partial_symtab *psymtab,
struct mapped_symtab *symtab,
- htab_t psyms_seen,
+ std::unordered_set<partial_symbol *> &psyms_seen,
offset_type cu_index)
{
int i;
@@ -23863,22 +23738,25 @@ recursively_write_psymbols (struct objfile *objfile,
1);
}

+/* Closes FILE on scope exit. */
+struct file_closer
+{
+ explicit file_closer (FILE *file)
+ : m_file (file)
+ {}
+
+ ~file_closer ()
+ { fclose (m_file); }
+
+private:
+ FILE *m_file;
+};
+
/* Create an index file for OBJFILE in the directory DIR. */

static void
write_psymtabs_to_index (struct objfile *objfile, const char *dir)
{
- struct cleanup *cleanup;
- char *filename;
- struct obstack contents, addr_obstack, constant_pool, symtab_obstack;
- struct obstack cu_list, types_cu_list;
- int i;
- FILE *out_file;
- struct mapped_symtab *symtab;
- offset_type val, size_of_contents, total_len;
- struct stat st;
- struct psymtab_cu_index_map *psymtab_cu_index_map;
-
if (dwarf2_per_objfile->using_index)
error (_("Cannot use an index to create the index"));

@@ -23888,58 +23766,39 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
if (!objfile->psymtabs || !objfile->psymtabs_addrmap)
return;

+ struct stat st;
if (stat (objfile_name (objfile), &st) < 0)
perror_with_name (objfile_name (objfile));

- filename = concat (dir, SLASH_STRING, lbasename (objfile_name (objfile)),
- INDEX_SUFFIX, (char *) NULL);
- cleanup = make_cleanup (xfree, filename);
+ std::string filename (std::string (dir) + SLASH_STRING
+ + lbasename (objfile_name (objfile)) + INDEX_SUFFIX);

- out_file = gdb_fopen_cloexec (filename, "wb");
+ FILE *out_file = gdb_fopen_cloexec (filename.c_str (), "wb");
if (!out_file)
- error (_("Can't open `%s' for writing"), filename);
-
- gdb::unlinker unlink_file (filename);
-
- symtab = create_mapped_symtab ();
- make_cleanup (cleanup_mapped_symtab, symtab);
+ error (_("Can't open `%s' for writing"), filename.c_str ());

- obstack_init (&addr_obstack);
- make_cleanup_obstack_free (&addr_obstack);
+ file_closer close_out_file (out_file);
+ gdb::unlinker unlink_file (filename.c_str ());

- obstack_init (&cu_list);
- make_cleanup_obstack_free (&cu_list);
-
- obstack_init (&types_cu_list);
- make_cleanup_obstack_free (&types_cu_list);
-
- htab_up psyms_seen (htab_create_alloc (100, htab_hash_pointer,
- htab_eq_pointer,
- NULL, xcalloc, xfree));
+ mapped_symtab symtab;
+ data_buf cu_list;
+ std::unordered_set<partial_symbol *> psyms_seen;

/* While we're scanning CU's create a table that maps a psymtab pointer
(which is what addrmap records) to its index (which is what is recorded
in the index file). This will later be needed to write the address
table. */
- htab_up cu_index_htab (htab_create_alloc (100,
- hash_psymtab_cu_index,
- eq_psymtab_cu_index,
- NULL, xcalloc, xfree));
- psymtab_cu_index_map = XNEWVEC (struct psymtab_cu_index_map,
- dwarf2_per_objfile->n_comp_units);
- make_cleanup (xfree, psymtab_cu_index_map);
+ psym_index_map cu_index_htab;
+ cu_index_htab.reserve (dwarf2_per_objfile->n_comp_units);

/* The CU list is already sorted, so we don't need to do additional
work here. Also, the debug_types entries do not appear in
all_comp_units, but only in their own hash table. */
- for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
+ for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
{
struct dwarf2_per_cu_data *per_cu
= dwarf2_per_objfile->all_comp_units[i];
struct partial_symtab *psymtab = per_cu->v.psymtab;
- gdb_byte val[8];
- struct psymtab_cu_index_map *map;
- void **slot;

/* CU of a shared file from 'dwz -m' may be unused by this main file.
It may be referenced from a local scope but in such case it does not
@@ -23948,36 +23807,32 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
continue;

if (psymtab->user == NULL)
- recursively_write_psymbols (objfile, psymtab, symtab,
- psyms_seen.get (), i);
+ recursively_write_psymbols (objfile, psymtab, &symtab,
+ psyms_seen, i);

- map = &psymtab_cu_index_map[i];
- map->psymtab = psymtab;
- map->cu_index = i;
- slot = htab_find_slot (cu_index_htab.get (), map, INSERT);
- gdb_assert (slot != NULL);
- gdb_assert (*slot == NULL);
- *slot = map;
+ const auto insertpair = cu_index_htab.emplace (psymtab, i);
+ gdb_assert (insertpair.second);

- store_unsigned_integer (val, 8, BFD_ENDIAN_LITTLE,
+ store_unsigned_integer (cu_list.append_space (8), 8,
+ BFD_ENDIAN_LITTLE,
to_underlying (per_cu->sect_off));
- obstack_grow (&cu_list, val, 8);
- store_unsigned_integer (val, 8, BFD_ENDIAN_LITTLE, per_cu->length);
- obstack_grow (&cu_list, val, 8);
+ store_unsigned_integer (cu_list.append_space (8), 8,
+ BFD_ENDIAN_LITTLE, per_cu->length);
}

/* Dump the address map. */
- write_address_map (objfile, &addr_obstack, cu_index_htab.get ());
+ data_buf addr_vec;
+ write_address_map (objfile, addr_vec, cu_index_htab);

/* Write out the .debug_type entries, if any. */
+ data_buf types_cu_list;
if (dwarf2_per_objfile->signatured_types)
{
- struct signatured_type_index_data sig_data;
+ signatured_type_index_data sig_data (types_cu_list,
+ psyms_seen);

sig_data.objfile = objfile;
- sig_data.symtab = symtab;
- sig_data.types_list = &types_cu_list;
- sig_data.psyms_seen = psyms_seen.get ();
+ sig_data.symtab = &symtab;
sig_data.cu_index = dwarf2_per_objfile->n_comp_units;
htab_traverse_noresize (dwarf2_per_objfile->signatured_types,
write_one_signatured_type, &sig_data);
@@ -23985,63 +23840,49 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)

/* Now that we've processed all symbols we can shrink their cu_indices
lists. */
- uniquify_cu_indices (symtab);
+ uniquify_cu_indices (&symtab);

- obstack_init (&constant_pool);
- make_cleanup_obstack_free (&constant_pool);
- obstack_init (&symtab_obstack);
- make_cleanup_obstack_free (&symtab_obstack);
- write_hash_table (symtab, &symtab_obstack, &constant_pool);
+ data_buf symtab_vec, constant_pool;
+ write_hash_table (&symtab, symtab_vec, constant_pool);

- obstack_init (&contents);
- make_cleanup_obstack_free (&contents);
- size_of_contents = 6 * sizeof (offset_type);
- total_len = size_of_contents;
+ data_buf contents;
+ const offset_type size_of_contents = 6 * sizeof (offset_type);
+ offset_type total_len = size_of_contents;

/* The version number. */
- val = MAYBE_SWAP (8);
- obstack_grow (&contents, &val, sizeof (val));
+ contents.append_data (MAYBE_SWAP (8));

/* The offset of the CU list from the start of the file. */
- val = MAYBE_SWAP (total_len);
- obstack_grow (&contents, &val, sizeof (val));
- total_len += obstack_object_size (&cu_list);
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += cu_list.size ();

/* The offset of the types CU list from the start of the file. */
- val = MAYBE_SWAP (total_len);
- obstack_grow (&contents, &val, sizeof (val));
- total_len += obstack_object_size (&types_cu_list);
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += types_cu_list.size ();

/* The offset of the address table from the start of the file. */
- val = MAYBE_SWAP (total_len);
- obstack_grow (&contents, &val, sizeof (val));
- total_len += obstack_object_size (&addr_obstack);
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += addr_vec.size ();

/* The offset of the symbol table from the start of the file. */
- val = MAYBE_SWAP (total_len);
- obstack_grow (&contents, &val, sizeof (val));
- total_len += obstack_object_size (&symtab_obstack);
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += symtab_vec.size ();

/* The offset of the constant pool from the start of the file. */
- val = MAYBE_SWAP (total_len);
- obstack_grow (&contents, &val, sizeof (val));
- total_len += obstack_object_size (&constant_pool);
-
- gdb_assert (obstack_object_size (&contents) == size_of_contents);
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += constant_pool.size ();

- write_obstack (out_file, &contents);
- write_obstack (out_file, &cu_list);
- write_obstack (out_file, &types_cu_list);
- write_obstack (out_file, &addr_obstack);
- write_obstack (out_file, &symtab_obstack);
- write_obstack (out_file, &constant_pool);
+ gdb_assert (contents.size () == size_of_contents);

- fclose (out_file);
+ contents.file_write (out_file);
+ cu_list.file_write (out_file);
+ types_cu_list.file_write (out_file);
+ addr_vec.file_write (out_file);
+ symtab_vec.file_write (out_file);
+ constant_pool.file_write (out_file);

/* We want to keep the file. */
unlink_file.keep ();
-
- do_cleanups (cleanup);
}

/* Implementation of the `save gdb-index' command.
--
2.5.5