[review v3] Defer minimal symbol name-setting
Pedro Alves (Code Review)
gerrit@gnutoolchain-gerrit.osci.io
Fri Nov 22 16:15:00 GMT 2019
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/166
......................................................................
Patch Set 3: Code-Review+2
(2 comments)
| --- gdb/minsyms.c
| +++ gdb/minsyms.c
| @@ -1348,17 +1353,28 @@ minimal_symbol_reader::install ()
| The strings themselves are also located in the storage_obstack
| of this objfile. */
|
| if (m_objfile->per_bfd->minimal_symbol_count != 0)
| clear_minimal_symbol_hash_tables (m_objfile);
|
| m_objfile->per_bfd->minimal_symbol_count = mcount;
| m_objfile->per_bfd->msymbols = std::move (msym_holder);
|
| + msymbols = m_objfile->per_bfd->msymbols.get ();
| + for (int i = 0; i < mcount; ++i)
| + {
| + if (!msymbols[i].name_set)
| + {
| + symbol_set_names (&msymbols[i], msymbols[i].name,
| + false, m_objfile->per_bfd);
| + msymbols[i].name_set = 1;
| + }
| + }
PS3, Line 1371:
I was wondering whether we could do without the name_set field by
doing this symbol_set_names loop only over the new minsyms, before
appending them to the objfile's preexisting minsyms list, but I guess
the need for deduplication shows that we'd be demangling more than
necessary.
| +
| build_minimal_symbol_hash_tables (m_objfile);
| }
| }
|
| /* Check if PC is in a shared library trampoline code stub.
| Return minimal symbol for the trampoline entry or NULL if PC is not
| in a trampoline code stub. */
|
| --- gdb/symtab.h
| +++ gdb/symtab.h
| @@ -683,19 +683,23 @@ struct minimal_symbol : public general_symbol_info
| the object file format may not carry that piece of information. */
| unsigned int has_size : 1;
|
| /* For data symbols only, if this is set, then the symbol might be
| subject to copy relocation. In this case, a minimal symbol
| matching the symbol's linkage name is first looked for in the
| main objfile. If found, then that address is used; otherwise the
| address in this symbol is used. */
|
| unsigned maybe_copied : 1;
|
| + /* Non-zero if this symbol ever had its demangled name set (even if
| + it was set to NULL). */
| + unsigned int name_set : 1;
PS3, Line 696:
Don't change it, since the other fields are the same, but,
I'm wondering whether nowadays with C++ we shouldn't be writing
bool name_set : 1;
and then use true/false instead of 0/1.
I assume that it compiles down to the same.
| +
| /* Minimal symbols with the same hash key are kept on a linked
| list. This is the link. */
|
| struct minimal_symbol *hash_next;
|
| /* Minimal symbols are stored in two different hash tables. This is
| the `next' pointer for the demangled hash table. */
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I4fe3993b99fb3a43968067806e294d48e377fd76
Gerrit-Change-Number: 166
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 22 Nov 2019 16:15:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
More information about the Gdb-patches
mailing list