OpenE2K/binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Only make a nullterminated string if we need to

Browse Source 
As of 7bb4305982, we no longer need
a nullterminated linkage_name to look up the entry in the hash table.

So this patch makes it so we only make the copy if the entry was
not found.

By auditing all callers of symbol_set_names, I found out that all cases
where the string may not be nullterminated already pass true for COPY_NAME.
So here, I am documenting that as a requirement and am removing the code
that relies on undefined behavior in symbol_set_names (it accessed the string
past the provided length to check for nulltermination). Note that the Ada
case at the beginning of symbol_set_names was already relying on this.

gdb/ChangeLog:

2019-10-29  Christian Biesinger  <cbiesinger@google.com>

	* symtab.h (symbol_set_names): Document that copy_name must be
	set to true for non-nullterminated strings.
	* symtab.c (symbol_set_names): Only make a nullterminated copy of
	linkage_name if the entry was not found and we need to demangle.

Change-Id: I183302e1f51483ff6dff0fd5c3b0f32f0f04a5d2
This commit is contained in:
Christian Biesinger 2019-10-22 17:25:50 -05:00
parent 35e65c49df
commit 0c921b219c
3 changed files with 28 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
gdb/ChangeLog
View File

@ -1,3 +1,10 @@
2019-10-29 Christian Biesinger <cbiesinger@google.com>
* symtab.h (symbol_set_names): Document that copy_name must be
set to true for non-nullterminated strings.
* symtab.c (symbol_set_names): Only make a nullterminated copy of
linkage_name if the entry was not found and we need to demangle.
2019-10-29 Christian Biesinger <cbiesinger@google.com>
* Makefile.in (HFILES_NO_SRCDIR): Add gdb_binary_search.h.

37
gdb/symtab.c
View File

@ -832,8 +832,6 @@ symbol_set_names (struct general_symbol_info *gsymbol,
struct objfile_per_bfd_storage *per_bfd)
{
struct demangled_name_entry **slot;
/* A 0-terminated copy of the linkage name. */
const char *linkage_name_copy;
if (gsymbol->language == language_ada)
{
@ -858,20 +856,7 @@ symbol_set_names (struct general_symbol_info *gsymbol,
if (per_bfd->demangled_names_hash == NULL)
create_demangled_names_hash (per_bfd);
if (linkage_name[len] != '\0')
{
char *alloc_name;
alloc_name = (char *) alloca (len + 1);
memcpy (alloc_name, linkage_name, len);
alloc_name[len] = '\0';
linkage_name_copy = alloc_name;
}
else
linkage_name_copy = linkage_name;
struct demangled_name_entry entry (gdb::string_view (linkage_name_copy, len));
struct demangled_name_entry entry (gdb::string_view (linkage_name, len));
slot = ((struct demangled_name_entry **)
htab_find_slot (per_bfd->demangled_names_hash.get (),
&entry, INSERT));
@ -882,6 +867,21 @@ symbol_set_names (struct general_symbol_info *gsymbol,
This happens to, e.g., main.init (__go_init_main). Cope. */
|| (gsymbol->language == language_go && (*slot)->demangled == nullptr))
{
/* A 0-terminated copy of the linkage name. Callers must set COPY_NAME
to true if the string might not be nullterminated. We have to make
this copy because demangling needs a nullterminated string. */
const char *linkage_name_copy;
if (copy_name)
{
char *alloc_name = (char *) alloca (len + 1);
memcpy (alloc_name, linkage_name, len);
alloc_name[len] = '\0';
linkage_name_copy = alloc_name;
}
else
linkage_name_copy = linkage_name;
gdb::unique_xmalloc_ptr<char> demangled_name_ptr
(symbol_find_demangled_name (gsymbol, linkage_name_copy));
@ -894,7 +894,7 @@ symbol_set_names (struct general_symbol_info *gsymbol,
It turns out that it is actually important to still save such
an entry in the hash table, because storing this name gives
us better bcache hit rates for partial symbols. */
if (!copy_name && linkage_name_copy == linkage_name)
if (!copy_name)
{
*slot
= ((struct demangled_name_entry *)
@ -912,7 +912,8 @@ symbol_set_names (struct general_symbol_info *gsymbol,
obstack_alloc (&per_bfd->storage_obstack,
sizeof (demangled_name_entry) + len + 1));
char *mangled_ptr = reinterpret_cast<char *> (*slot + 1);
strcpy (mangled_ptr, linkage_name_copy);
memcpy (mangled_ptr, linkage_name, len);
mangled_ptr [len] = '\0';
new (*slot) demangled_name_entry
(gdb::string_view (mangled_ptr, len));
}

3
gdb/symtab.h
View File

@ -504,7 +504,8 @@ extern void symbol_set_language (struct general_symbol_info *symbol,
(symbol)->ginfo.name = (linkage_name)
/* Set the linkage and natural names of a symbol, by demangling
the linkage name. */
the linkage name. If linkage_name may not be nullterminated,
copy_name must be set to true. */
#define SYMBOL_SET_NAMES(symbol,linkage_name,len,copy_name,objfile) \
symbol_set_names (&(symbol)->ginfo, linkage_name, len, copy_name, \
(objfile)->per_bfd)