[PATCH] [dwarf2read] Fix crash when loading dwp files: calculate num_sections based on actual section indices, not just the number of sections.

Jordan Rupprecht via gdb-patches
2019-02-23T00:54:14Z

When loading dwp files, we create an array of elf sections indexed by the section index in the dwp file. The size of this array is calculated by section_count + 1 (the +1 handling the null section). However, when loading the bfd file, strtab/symtab sections are not added to the list, nor do they increment section_count, so section_count is actually smaller than the number of sections.

This happens to work when using GNU dwp, which lays out .debug section first, with sections like .shstrtab coming at the end. Other tools, like llvm-dwp, put .strtab first, and gdb crashes when loading those dwp files.

For instance, with the current state of gdb, loading a file like this:
$ readelf -SW <file.dwp>
[ 0] <empty>
[ 1] .debug_foo PROGBITS ...
[ 2] .strtab STRTAB ...

... results in section_count = 2 (.debug is the only thing placed into bfd->sections, so section_count + 1 == 2), and sectp->this_idx = 1 when mapping over .debug_foo in dwarf2_locate_common_dwp_sections, which passes the assertion that 1 < 2.

However, using a dwp file produced by llvm-dwp:
$ readelf -SW <file.dwp>
[ 0] <empty>
[ 1] .strtab STRTAB ...
[ 2] .debug_foo PROGBITS ...
... results in section_count = 2 (.debug is the only thing placed into bfd->sections, so section_count + 1 == 2), and sectp->this_idx = 2 when mapping over .debug_foo in dwarf2_locate_common_dwp_sections, which fails the assertion that 2 < 2.

This patch changes the calculation of section_count to look at the actual highest this_idx value we see instead of assuming that all strtab/symtab sections come at the end.
---
gdb/ChangeLog | 6 ++++++
gdb/dwarf2read.c | 17 +++++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6a551dfa64..c789b34890 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-02-22 Jordan Rupprecht <rupprecht@google.com>
+
+ * dwarf2read.c (get_largest_section_index): new function
+ (open_and_init_dwp_file): Call get_largest_section_index to
+ calculate dwp_file->num_sections.
+
2019-02-22 Simon Marchi <simon.marchi@polymtl.ca>

* MAINTAINERS: Update my email address.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 98f46e0416..07a9d4ea5d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13182,6 +13182,18 @@ open_dwp_file (struct dwarf2_per_objfile *dwarf2_per_objfile,
return NULL;
}

+/* Return the largest section index */
+static int
+get_largest_section_index(bfd *bfd)
+{
+ int max_sec_idx = 0;
+ asection *sectp;
+ for (sectp = bfd->sections; sectp != NULL; sectp = sectp->next)
+ max_sec_idx = std::max (max_sec_idx, elf_section_data (sectp)->this_idx);
+
+ return max_sec_idx;
+}
+
/* Initialize the use of the DWP file for the current objfile.
By convention the name of the DWP file is ${objfile}.dwp.
The result is NULL if it can't be found. */
@@ -13230,8 +13242,9 @@ open_and_init_dwp_file (struct dwarf2_per_objfile *dwarf2_per_objfile)
std::unique_ptr<struct dwp_file> dwp_file
(new struct dwp_file (name, std::move (dbfd)));

- /* +1: section 0 is unused */
- dwp_file->num_sections = bfd_count_sections (dwp_file->dbfd) + 1;
+ /* +1: so that dwp_file->elf_sections[max_idx] is valid */
+ dwp_file->num_sections =
+ get_largest_section_index (dwp_file->dbfd.get()) + 1;
dwp_file->elf_sections =
OBSTACK_CALLOC (&objfile->objfile_obstack,
dwp_file->num_sections, asection *);
--
2.21.0.rc0.258.g878e2cd30e-goog