On 2020-01-07 5:07 p.m., Tom Tromey wrote:
> I noticed a few spots in dwarf2read.c that could be improved by moving
> to unique_xmalloc_ptr or, in one case, std::vector.
>
> gdb/ChangeLog
> 2020-01-07 Tom Tromey <tromey@adacore.com>
>
> * dwarf2read.c (add_partial_symbol): Use unique_xmalloc_ptr.
> (dwarf2_compute_name, open_dwo_file): Likewise.
> (process_enumeration_scope): Use std::vector.
> (guess_partial_die_structure_name): Use unique_xmalloc_ptr.
> (partial_die_info::fixup, dwarf2_start_subfile)
> (guess_full_die_structure_name, dwarf2_name): Likewise.
> (determine_prefix): Update.
> (guess_full_die_structure_name): Make return type const.
>
> Change-Id: I1cb278c608041ef36ef1f77c7e7565c921038d08
Thanks, this LGTM. I noted some suggestions below.
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 63d533a4459..9fd2b7715b1 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -8954,13 +8954,13 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
> CORE_ADDR addr = 0;
> const char *actual_name = NULL;
> CORE_ADDR baseaddr;
> - char *built_actual_name;
>
> baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>
> - built_actual_name = partial_die_full_name (pdi, cu);
> + gdb::unique_xmalloc_ptr<char> built_actual_name
> + (partial_die_full_name (pdi, cu));
You could make partial_die_full_name return a gdb::unique_xmalloc_ptr while at
it, since this is the sole user. This can then stay an assignment:
gdb::unique_xmalloc_ptr<char> built_actual_name
= partial_die_full_name (pdi, cu);
> @@ -16566,34 +16555,26 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu)
> {
> sym = new_symbol (child_die, this_type, cu);
>
> - if ((num_fields % DW_FIELD_ALLOC_CHUNK) == 0)
> - {
> - fields = (struct field *)
> - xrealloc (fields,
> - (num_fields + DW_FIELD_ALLOC_CHUNK)
> - * sizeof (struct field));
> - }
You can actually delete DW_FIELD_ALLOC_CHUNK, defined higher in dwarf2read.c.
Simon
@@ -8954,13 +8954,13 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
CORE_ADDR addr = 0;
const char *actual_name = NULL;
CORE_ADDR baseaddr;
- char *built_actual_name;
baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
- built_actual_name = partial_die_full_name (pdi, cu);
+ gdb::unique_xmalloc_ptr<char> built_actual_name
+ (partial_die_full_name (pdi, cu));
if (built_actual_name != NULL)
- actual_name = built_actual_name;
+ actual_name = built_actual_name.get ();
if (actual_name == NULL)
actual_name = pdi->name;
@@ -9053,10 +9053,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
/* Static Variable. Skip symbols whose value we cannot know (those
without location descriptors or constant values). */
if (!has_loc && !pdi->has_const_value)
- {
- xfree (built_actual_name);
- return;
- }
+ return;
add_psymbol_to_list (actual_name,
built_actual_name != NULL,
@@ -9106,10 +9103,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
union or class entry that does not have a byte size attribute
and that has a DW_AT_declaration attribute." */
if (!pdi->has_byte_size && pdi->is_declaration)
- {
- xfree (built_actual_name);
- return;
- }
+ return;
/* NOTE: carlton/2003-10-07: See comment in new_symbol about
static vs. global. */
@@ -9134,8 +9128,6 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
default:
break;
}
-
- xfree (built_actual_name);
}
/* Read a partial die corresponding to a namespace; also, add a symbol
@@ -10912,11 +10904,10 @@ dwarf2_compute_name (const char *name,
prefix = determine_prefix (die, cu);
if (*prefix != '\0')
{
- char *prefixed_name = typename_concat (NULL, prefix, name,
- physname, cu);
+ gdb::unique_xmalloc_ptr<char> prefixed_name
+ (typename_concat (NULL, prefix, name, physname, cu));
- buf.puts (prefixed_name);
- xfree (prefixed_name);
+ buf.puts (prefixed_name.get ());
}
else
buf.puts (name);
@@ -12958,16 +12949,15 @@ open_dwo_file (struct dwarf2_per_objfile *dwarf2_per_objfile,
if (comp_dir != NULL)
{
- char *path_to_try = concat (comp_dir, SLASH_STRING,
- file_name, (char *) NULL);
+ gdb::unique_xmalloc_ptr<char> path_to_try
+ (concat (comp_dir, SLASH_STRING, file_name, (char *) NULL));
/* NOTE: If comp_dir is a relative path, this will also try the
search path, which seems useful. */
gdb_bfd_ref_ptr abfd (try_open_dwop_file (dwarf2_per_objfile,
- path_to_try,
+ path_to_try.get (),
0 /*is_dwp*/,
1 /*search_cwd*/));
- xfree (path_to_try);
if (abfd != NULL)
return abfd;
}
@@ -16548,8 +16538,7 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu)
{
struct die_info *child_die;
struct symbol *sym;
- struct field *fields = NULL;
- int num_fields = 0;
+ std::vector<struct field> fields;
const char *name;
child_die = die->child;
@@ -16566,34 +16555,26 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu)
{
sym = new_symbol (child_die, this_type, cu);
- if ((num_fields % DW_FIELD_ALLOC_CHUNK) == 0)
- {
- fields = (struct field *)
- xrealloc (fields,
- (num_fields + DW_FIELD_ALLOC_CHUNK)
- * sizeof (struct field));
- }
-
- FIELD_NAME (fields[num_fields]) = sym->linkage_name ();
- FIELD_TYPE (fields[num_fields]) = NULL;
- SET_FIELD_ENUMVAL (fields[num_fields], SYMBOL_VALUE (sym));
- FIELD_BITSIZE (fields[num_fields]) = 0;
+ fields.emplace_back ();
+ struct field &field = fields.back ();
- num_fields++;
+ FIELD_NAME (field) = sym->linkage_name ();
+ FIELD_TYPE (field) = NULL;
+ SET_FIELD_ENUMVAL (field, SYMBOL_VALUE (sym));
+ FIELD_BITSIZE (field) = 0;
}
}
child_die = sibling_die (child_die);
}
- if (num_fields)
+ if (!fields.empty ())
{
- TYPE_NFIELDS (this_type) = num_fields;
+ TYPE_NFIELDS (this_type) = fields.size ();
TYPE_FIELDS (this_type) = (struct field *)
- TYPE_ALLOC (this_type, sizeof (struct field) * num_fields);
- memcpy (TYPE_FIELDS (this_type), fields,
- sizeof (struct field) * num_fields);
- xfree (fields);
+ TYPE_ALLOC (this_type, sizeof (struct field) * fields.size ());
+ memcpy (TYPE_FIELDS (this_type), fields.data (),
+ sizeof (struct field) * fields.size ());
}
}
@@ -19345,16 +19326,15 @@ guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
if (child_pdi->tag == DW_TAG_subprogram
&& child_pdi->linkage_name != NULL)
{
- char *actual_class_name
- = language_class_name_from_physname (cu->language_defn,
- child_pdi->linkage_name);
+ gdb::unique_xmalloc_ptr<char> actual_class_name
+ (language_class_name_from_physname (cu->language_defn,
+ child_pdi->linkage_name));
if (actual_class_name != NULL)
{
struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
struct_pdi->name
= obstack_strdup (&objfile->per_bfd->storage_obstack,
- actual_class_name);
- xfree (actual_class_name);
+ actual_class_name.get ());
}
break;
}
@@ -19419,24 +19399,22 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
|| tag == DW_TAG_union_type)
&& linkage_name != NULL)
{
- char *demangled;
-
- demangled = gdb_demangle (linkage_name, DMGL_TYPES);
- if (demangled)
+ gdb::unique_xmalloc_ptr<char> demangled
+ (gdb_demangle (linkage_name, DMGL_TYPES));
+ if (demangled != nullptr)
{
const char *base;
/* Strip any leading namespaces/classes, keep only the base name.
DW_AT_name for named DIEs does not contain the prefixes. */
- base = strrchr (demangled, ':');
- if (base && base > demangled && base[-1] == ':')
+ base = strrchr (demangled.get (), ':');
+ if (base && base > demangled.get () && base[-1] == ':')
base++;
else
- base = demangled;
+ base = demangled.get ();
struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
name = obstack_strdup (&objfile->per_bfd->storage_obstack, base);
- xfree (demangled);
}
}
@@ -21716,7 +21694,7 @@ static void
dwarf2_start_subfile (struct dwarf2_cu *cu, const char *filename,
const char *dirname)
{
- char *copy = NULL;
+ gdb::unique_xmalloc_ptr<char> copy;
/* In order not to lose the line information directory,
we concatenate it to the filename when it makes sense.
@@ -21727,14 +21705,11 @@ dwarf2_start_subfile (struct dwarf2_cu *cu, const char *filename,
if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
{
- copy = concat (dirname, SLASH_STRING, filename, (char *)NULL);
- filename = copy;
+ copy.reset (concat (dirname, SLASH_STRING, filename, (char *) NULL));
+ filename = copy.get ();
}
cu->get_builder ()->start_subfile (filename);
-
- if (copy != NULL)
- xfree (copy);
}
/* Start a symtab for DWARF. NAME, COMP_DIR, LOW_PC are passed to the
@@ -22702,7 +22677,7 @@ read_type_die_1 (struct die_info *die, struct dwarf2_cu *cu)
This is the full-die version of guess_partial_die_structure_name.
In this case we know DIE has no useful parent. */
-static char *
+static const char *
guess_full_die_structure_name (struct die_info *die, struct dwarf2_cu *cu)
{
struct die_info *spec_die;
@@ -22728,33 +22703,32 @@ guess_full_die_structure_name (struct die_info *die, struct dwarf2_cu *cu)
if (linkage_name != NULL)
{
- char *actual_name
- = language_class_name_from_physname (cu->language_defn,
- linkage_name);
- char *name = NULL;
+ gdb::unique_xmalloc_ptr<char> actual_name
+ (language_class_name_from_physname (cu->language_defn,
+ linkage_name));
+ const char *name = NULL;
if (actual_name != NULL)
{
const char *die_name = dwarf2_name (die, cu);
if (die_name != NULL
- && strcmp (die_name, actual_name) != 0)
+ && strcmp (die_name, actual_name.get ()) != 0)
{
/* Strip off the class name from the full name.
We want the prefix. */
int die_name_len = strlen (die_name);
- int actual_name_len = strlen (actual_name);
+ int actual_name_len = strlen (actual_name.get ());
+ const char *ptr = actual_name.get ();
/* Test for '::' as a sanity check. */
if (actual_name_len > die_name_len + 2
- && actual_name[actual_name_len
- - die_name_len - 1] == ':')
+ && ptr[actual_name_len - die_name_len - 1] == ':')
name = obstack_strndup (
&objfile->per_bfd->storage_obstack,
- actual_name, actual_name_len - die_name_len - 2);
+ ptr, actual_name_len - die_name_len - 2);
}
}
- xfree (actual_name);
return name;
}
}
@@ -22941,7 +22915,7 @@ determine_prefix (struct die_info *die, struct dwarf2_cu *cu)
|| die->tag == DW_TAG_structure_type
|| die->tag == DW_TAG_union_type))
{
- char *name = guess_full_die_structure_name (die, cu);
+ const char *name = guess_full_die_structure_name (die, cu);
if (name != NULL)
return name;
}
@@ -23115,8 +23089,6 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510. */
if (!attr || DW_STRING (attr) == NULL)
{
- char *demangled = NULL;
-
attr = dw2_linkage_name_attr (die, cu);
if (attr == NULL || DW_STRING (attr) == NULL)
return NULL;
@@ -23124,18 +23096,17 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
/* Avoid demangling DW_STRING (attr) the second time on a second
call for the same DIE. */
if (!DW_STRING_IS_CANONICAL (attr))
- demangled = gdb_demangle (DW_STRING (attr), DMGL_TYPES);
-
- if (demangled)
{
+ gdb::unique_xmalloc_ptr<char> demangled
+ (gdb_demangle (DW_STRING (attr), DMGL_TYPES));
+
const char *base;
/* FIXME: we already did this for the partial symbol... */
DW_STRING (attr)
= obstack_strdup (&objfile->per_bfd->storage_obstack,
- demangled);
+ demangled.get ());
DW_STRING_IS_CANONICAL (attr) = 1;
- xfree (demangled);
/* Strip any leading namespaces/classes, keep only the base name.
DW_AT_name for named DIEs does not contain the prefixes. */