From patchwork Mon Jun 12 16:08:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 20948 Received: (qmail 18131 invoked by alias); 12 Jun 2017 16:08:18 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 18109 invoked by uid 89); 12 Jun 2017 16:08:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_STOCKGEN, SPF_HELO_PASS, T_RP_MATCHES_RCVD, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=observe, Meanwhile X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Jun 2017 16:08:10 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D971C80F9A; Mon, 12 Jun 2017 16:08:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D971C80F9A Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D971C80F9A Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 103D653CDD; Mon, 12 Jun 2017 16:08:07 +0000 (UTC) Subject: [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer. To: Jan Kratochvil , gdb-patches@sourceware.org References: <149582312757.15869.18345460438195439402.stgit@host1.jankratochvil.net> <149582313928.15869.12647134810146005233.stgit@host1.jankratochvil.net> Cc: Victor Leschuk From: Pedro Alves Message-ID: <8efc0742-1014-4fe0-6948-f40a9c5c4975@redhat.com> Date: Mon, 12 Jun 2017 17:08:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <149582313928.15869.12647134810146005233.stgit@host1.jankratochvil.net> On 05/26/2017 07:25 PM, Jan Kratochvil wrote: > gdb/ChangeLog > 2017-05-26 Jan Kratochvil > > Code cleanup: C++ify .gdb_index producer. > * common/common-defs.h (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 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 > + struct _MakeUniq > + { typedef unique_ptr<_Tp> __single_object; }; > + template > + 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 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 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 Code cleanup: C++ify .gdb_index producer. * dwarf2read.c: Include and . (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 + + Code cleanup: C++ify .gdb_index producer. + * dwarf2read.c: Include and . + (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 * 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 #include #include +#include +#include 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 (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) /* 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 +static void +file_write (FILE *file, const std::vector &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 + void append_data (const T &data) + { + std::copy (reinterpret_cast (&data), + reinterpret_cast (&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 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 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> 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 & 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 &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 +class vector_hasher { - offset_type i; - htab_t symbol_hash_table; - htab_t str_table; +public: + size_t operator () (const std::vector &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, offset_type, + vector_hasher> + symbol_hash_table; + + /* We add all the index vectors to the constant pool first, to + ensure alignment is ok. */ + for (const std::unique_ptr &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 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 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 &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 &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 &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 &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 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.