Add name_of_main and language_of_main to the DWARF index
Commit Message
This patch adds a new section to the DWARF index containing the name
and the language of the main function symbol, gathered from
`cooked_index::get_main`, if available. Currently, for lack of a better name,
this section is called the "shortcut table" (suggestions for a better name are
appreciated). The way this name is both saved and applied upon an index being
loaded in mirrors how it is done in `cooked_index_functions`, more specifically,
the full name of the main function symbol is saved and `set_objfile_main_name`
is used to apply it after it is loaded.
The main use case for this patch is in improving startup times when dealing with
large binaries. Currently, when an index is used, GDB has to expand symtabs
until it finds out what the language of the main function symbol is. For some
large executables, this may take a considerable amount of time to complete,
slowing down startup. This patch bypasses that operation by having both the name
and language of the main function symbol be provided ahead of time by the index.
In my testing (a binary with about 1.8GB worth of DWARF data) this change brings
startup time down from about 34 seconds to about 1.5 seconds.
(I feel like I might've changed too much about the index format by adding a
breaking change. If there's a better way to do this it'd be glad to hear about
it.)
---
gdb/dwarf2/index-write.c | 47 +++++++++++++++++++++++++++++++++----
gdb/dwarf2/read-gdb-index.c | 44 +++++++++++++++++++++++++++++++++-
2 files changed, 85 insertions(+), 6 deletions(-)
Comments
>>>>> Matheus Branco Borella via Gdb-patches <gdb-patches@sourceware.org> writes:
Hi. Thank you for the patch. I think it's a good addition and is
fundamentally fine. There are a few nits below.
For a patch of this size, I think we will need a copyright assignment.
The form is here:
http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
Normally this is a pretty quick process.
> This patch adds a new section to the DWARF index containing the name
> and the language of the main function symbol, gathered from
> `cooked_index::get_main`, if available. Currently, for lack of a better name,
> this section is called the "shortcut table" (suggestions for a better name are
> appreciated).
Seems fine to me :)
> In my testing (a binary with about 1.8GB worth of DWARF data) this change brings
> startup time down from about 34 seconds to about 1.5 seconds.
Very nice.
> (I feel like I might've changed too much about the index format by adding a
> breaking change. If there's a better way to do this it'd be glad to hear about
> it.)
The index format is documented in gdb/doc/gdb.texinfo. So, your patch
will at least need an update to that file. I suspect a brief entry in
gdb/NEWS describing the change would also be good.
There's a note on the compatibility issue inline, below.
> /* The version number. */
> - contents.append_offset (8);
> + contents.append_offset (9);
Someday we should probably use a #define for this.
Not your problem though.
> +/* Write shortcut information. */
> +
> +static void
> +write_shortcuts_table (cooked_index *table, data_buf& shortcuts,
gdb style puts the "&" next to "shortcuts".
There's a few of these I think.
> + lang = main_info->per_cu->lang ();
...
> + shortcuts.append_uint (4, BFD_ENDIAN_LITTLE, lang);
I think it would be better to use a DW_LANG_ constant here -- it's
better not to expose gdb's enum language values to the world, whereas
the DWARF values are already fixed.
Those aren't really preserved exactly in the reader, but mapping back
from the gdb language to DWARF would be fine (probably).
> + ++i;
> +
> + const gdb_byte *shortcut_table = addr + metadata[i];
> + const gdb_byte *shortcut_table_end = addr + metadata[i + 1];
> + map->shortcut_table
> + = gdb::array_view<const gdb_byte> (shortcut_table, shortcut_table_end);
This section probably has to be conditional on 'version >= 8.
Otherwise a new gdb will fail with an older version of the index.
> +/* Sets the name and language of the main function from the shortcut table. */
> +
> +static void
> +set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile,
> + mapped_gdb_index *index)
> +{
> + auto ptr = index->shortcut_table.data ();
> + const auto lang = extract_unsigned_integer (ptr, 4, BFD_ENDIAN_LITTLE);
This code should probably check the size of the data, both for safety
and also because that's a decent way to handle the index version issue.
Tom
@@ -1080,14 +1080,15 @@ write_gdbindex_1 (FILE *out_file,
const data_buf &types_cu_list,
const data_buf &addr_vec,
const data_buf &symtab_vec,
- const data_buf &constant_pool)
+ const data_buf &constant_pool,
+ const data_buf &shortcuts)
{
data_buf contents;
- const offset_type size_of_header = 6 * sizeof (offset_type);
+ const offset_type size_of_header = 7 * sizeof (offset_type);
offset_type total_len = size_of_header;
/* The version number. */
- contents.append_offset (8);
+ contents.append_offset (9);
/* The offset of the CU list from the start of the file. */
contents.append_offset (total_len);
@@ -1105,6 +1106,10 @@ write_gdbindex_1 (FILE *out_file,
contents.append_offset (total_len);
total_len += symtab_vec.size ();
+ /* The offset of the shortcut table from the start of the file. */
+ contents.append_offset (total_len);
+ total_len += shortcuts.size ();
+
/* The offset of the constant pool from the start of the file. */
contents.append_offset (total_len);
total_len += constant_pool.size ();
@@ -1116,6 +1121,7 @@ write_gdbindex_1 (FILE *out_file,
types_cu_list.file_write (out_file);
addr_vec.file_write (out_file);
symtab_vec.file_write (out_file);
+ shortcuts.file_write (out_file);
constant_pool.file_write (out_file);
assert_file_size (out_file, total_len);
@@ -1193,6 +1199,34 @@ write_cooked_index (cooked_index *table,
}
}
+/* Write shortcut information. */
+
+static void
+write_shortcuts_table (cooked_index *table, data_buf& shortcuts,
+ data_buf& cpool)
+{
+ const auto main_info = table->get_main ();
+ size_t main_name_offset = 0;
+ language lang = language_unknown;
+
+ if (main_info != nullptr)
+ {
+ lang = main_info->per_cu->lang ();
+
+ if (lang != language_unknown)
+ {
+ auto_obstack obstack;
+ const auto main_name = main_info->full_name (&obstack, true);
+
+ main_name_offset = cpool.size ();
+ cpool.append_cstr0 (main_name);
+ }
+ }
+
+ shortcuts.append_uint (4, BFD_ENDIAN_LITTLE, lang);
+ shortcuts.append_offset (main_name_offset);
+}
+
/* Write contents of a .gdb_index section for OBJFILE into OUT_FILE.
If OBJFILE has an associated dwz file, write contents of a .gdb_index
section for that dwz file into DWZ_OUT_FILE. If OBJFILE does not have an
@@ -1270,11 +1304,14 @@ write_gdbindex (dwarf2_per_bfd *per_bfd, cooked_index *table,
write_hash_table (&symtab, symtab_vec, constant_pool);
+ data_buf shortcuts;
+ write_shortcuts_table (table, shortcuts, constant_pool);
+
write_gdbindex_1(out_file, objfile_cu_list, types_cu_list, addr_vec,
- symtab_vec, constant_pool);
+ symtab_vec, constant_pool, shortcuts);
if (dwz_out_file != NULL)
- write_gdbindex_1 (dwz_out_file, dwz_cu_list, {}, {}, {}, {});
+ write_gdbindex_1 (dwz_out_file, dwz_cu_list, {}, {}, {}, {}, {});
else
gdb_assert (dwz_cu_list.empty ());
}
@@ -88,6 +88,9 @@ struct mapped_gdb_index final : public mapped_index_base
/* A pointer to the constant pool. */
gdb::array_view<const gdb_byte> constant_pool;
+ /* The shortcut table data. */
+ gdb::array_view<const gdb_byte> shortcut_table;
+
/* Return the index into the constant pool of the name of the IDXth
symbol in the symbol table. */
offset_type symbol_name_index (offset_type idx) const
@@ -166,6 +169,7 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
mapped_gdb_index *index = (gdb::checked_static_cast<mapped_gdb_index *>
(per_objfile->per_bfd->index_table.get ()));
+
gdb_printf (".gdb_index: version %d\n", index->version);
gdb_printf ("\n");
}
@@ -583,7 +587,7 @@ to use the section anyway."),
/* Indexes with higher version than the one supported by GDB may be no
longer backward compatible. */
- if (version > 8)
+ if (version > 9)
return 0;
map->version = version;
@@ -608,6 +612,12 @@ to use the section anyway."),
map->symbol_table
= offset_view (gdb::array_view<const gdb_byte> (symbol_table,
symbol_table_end));
+ ++i;
+
+ const gdb_byte *shortcut_table = addr + metadata[i];
+ const gdb_byte *shortcut_table_end = addr + metadata[i + 1];
+ map->shortcut_table
+ = gdb::array_view<const gdb_byte> (shortcut_table, shortcut_table_end);
++i;
map->constant_pool = buffer.slice (metadata[i]);
@@ -763,6 +773,36 @@ create_addrmap_from_gdb_index (dwarf2_per_objfile *per_objfile,
= new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, &mutable_map);
}
+/* Sets the name and language of the main function from the shortcut table. */
+
+static void
+set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile,
+ mapped_gdb_index *index)
+{
+ auto ptr = index->shortcut_table.data ();
+ const auto lang = extract_unsigned_integer (ptr, 4, BFD_ENDIAN_LITTLE);
+ if (lang >= nr_languages)
+ {
+ complaint (_(".gdb_index shortcut table has invalid main language %u"),
+ (unsigned) lang);
+ return;
+ }
+ if (lang == language_unknown)
+ {
+ /* Don't bother if the language for the main symbol was not known or if
+ * there was no main symbol at all when the index was built. */
+ return;
+ }
+ ptr += 4;
+
+ const auto name_offset = extract_unsigned_integer (ptr,
+ sizeof (offset_type),
+ BFD_ENDIAN_LITTLE);
+ const auto name = (const char*) (index->constant_pool.data () + name_offset);
+
+ set_objfile_main_name (per_objfile->objfile, name, (enum language) lang);
+}
+
/* See read-gdb-index.h. */
int
@@ -848,6 +888,8 @@ dwarf2_read_gdb_index
create_addrmap_from_gdb_index (per_objfile, map.get ());
+ set_main_name_from_gdb_index (per_objfile, map.get ());
+
per_bfd->index_table = std::move (map);
per_bfd->quick_file_names_table =
create_quick_file_names_table (per_bfd->all_units.size ());