Re: [PATCH 1/2] gdb: Add soname to build-id mapping for core files

[Tom Tromey <tom@tromey.com>]

>>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

Aaron> Since commit aa2d5a422 gdb has been able to read executable and shared
Aaron> library build-ids within core files.

Aaron> Expand this functionality so that each core file bfd maintains a map of
Aaron> soname to build-id for each shared library referenced in the core file.

Aaron> This feature may be used to verify that gdb has found the correct shared
Aaron> libraries for core files and to facilitate downloading shared libaries via
Aaron> debuginfod.

Thanks for working on this.

Aaron> +	    gdb::unique_xmalloc_ptr<char> soname =
Aaron> +	      gdb_bfd_read_elf_soname (bfd->filename);

The '=' should go after the line break.

Aaron> +/* Mapping of a core file's shared library sonames to their respective
Aaron> +   build-ids.  Added to the registries of core file bfds.  */
Aaron> +
Aaron> +typedef std::unordered_map<std::string, std::string> soname_build_id_map;
Aaron> +
Aaron> +/* Key used to associate a soname_build_id_map to a core file bfd.  */
Aaron> +
Aaron> +static const struct bfd_data *cbfd_soname_build_id_data_key;

I think it's better to use the template form, like

static const struct bfd_key<soname_build_id_map> cbfd_soname_build_id_data_key;

More comments on this below.

Aaron> +  soname_build_id_map *mapptr = (soname_build_id_map *)
Aaron> +    bfd_data (abfd.get (), cbfd_soname_build_id_data_key);

... this cast won't be needed:

soname_build_id_map *mapptr = cbfd_soname_build_id_data_key.get (abfd.get ());

Aaron> +  if (mapptr == nullptr)
Aaron> +    {
Aaron> +      mapptr = new soname_build_id_map ();
Aaron> +      set_bfd_data (abfd.get (), cbfd_soname_build_id_data_key, mapptr);

mapptr = cbfd_soname_build_id_data_key.emplace (abfd.get ());

Aaron> +/* See solib.h.  */
Aaron> +
Aaron> +gdb::unique_xmalloc_ptr<char>
Aaron> +get_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd, const char *soname)
Aaron> +{

Why does this return a unique_xmalloc_ptr?

Aaron> +/* Free memory allocated for a soname_build_id_map.  */
Aaron> +
Aaron> +static void
Aaron> +delete_soname_build_id_map (struct bfd *abfd, void *mapptr)
Aaron> +{
Aaron> +  if (mapptr != nullptr)
Aaron> +    delete (soname_build_id_map *) mapptr;
Aaron> +}

With the template form, this can go away entirely.

Aaron>  _initialize_solib ()
Aaron>  {
Aaron>    solib_data = gdbarch_data_register_pre_init (solib_init);
Aaron> +  cbfd_soname_build_id_data_key = (const struct bfd_data *)
Aaron> +    register_bfd_data_with_cleanup (nullptr, delete_soname_build_id_map);

This won't be needed with the template form.

Tom