* [PATCH 1/6] gdb: a number of fixes for .gdb_index parsing
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
@ 2026-10-18 20:50 ` Andrew Burgess
2026-10-20 18:39 ` Simon Marchi
2026-10-18 20:50 ` [PATCH 2/6] gdb: handle incorrect CU size in .gdb_index Andrew Burgess
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2026-10-18 20:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
I recently posted a patch which Simon pointed out during review caused
GDB to crash when running with cc-with-gdb-index board. The problem
was that I was now generating an invalid .gdb_index section.
This got me thinking, rather than crashing, GDB should really be
rejecting the index as invalid. GDB should not treat the .gdb_index
as trusted input. Even if we take the position that GDB will always
generate valid .gdb_index sections, there's nothing to say that some
other tool might try to generate .gdb_index sections, and do it wrong.
We also write the .gdb_index into the index cache, disk corruption
could result in an invalid .gdb_index being read back in later.
This commit includes a number of fixes which I detail below, but the
most important thing here is the addition of the 'gdb_index' proc to
testsuite/lib/dwarf.exp. This proc started as a copy of 'proc
debug_names' but updated to emit .gdb_index sections. This proc has
been further modified to allow invalid indexes to be generated. I
believe all of the features I've added so far are documented in the
comments, so I'll not repeat it all here, but the general idea is to
allow every offset to be overridden to allow for invalid offsets to be
encoded into the .gdb_index. There are also options to allow parts of
the index to be elided completely.
Not every override feature of 'proc gdb_index' has a matching test
right now. If I'd done that then I was worried that the GDB side
fixes would be too large for a single patch. Further patches in this
series will make use of the currently unused features.
Currently, in dwarf2/read-gdb-index.c, I have the following fixes:
In mapped_gdb_index::build_name_components:
(1) If the name index in the .gdb_index's symbol table is out of
bounds of the constant pool then ignore this symbol table entry.
(2) Add an assert that the components list will contain at least
one entry, even an empty string will result in a single
component, the empty string.
(3) When processing the CU vector, if the declared CU vector length
is larger than the constant pool then ignore this CU vector.
(4) If we end up with an empty component string after splitting a
name from the index, then don't try to call .back() on the empty
string.
In read_gdb_index_from_buffer:
(5) If the symbol table is not empty, but the constant pool doesn't
end with a null character then reject this index. A non-empty
symbol table means that there must be strings in the constant
pool's string table. The strings in this table are
null-terminated. And the string table is at the end of the
constant pool. Therefore, the last byte in the constant pool
must be a null character.
If this is not the case then reading strings from the constant
pool might not be null-terminated, and might read outside the
constant pool.
Placing this check here means that we can later on assume that
any string read from the constant pool is null-terminated
without reading outside the constant pool.
In mapped_gdb_index::set_main_name:
(6) If the main function name is outside the bounds of the constant
pool then warn, and ignore the main function name.
(7) Assert that the constant pool is null-terminated, see (5) above.
---
gdb/dwarf2/read-gdb-index.c | 85 ++-
.../gdb.dwarf2/gdb-index-corrupted.c | 47 ++
.../gdb.dwarf2/gdb-index-corrupted.exp | 367 ++++++++++++
gdb/testsuite/lib/dwarf.exp | 531 ++++++++++++++++++
4 files changed, 1022 insertions(+), 8 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.c
create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 324984c0de2..86512c4ab9c 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -187,8 +187,9 @@ struct mapped_gdb_index
const char *symbol_name_at (offset_type idx,
dwarf2_per_objfile *per_objfile) const
{
- return (const char *) (this->constant_pool.data ()
- + symbol_name_index (idx));
+ offset_type name_index = symbol_name_index (idx);
+ gdb_assert (name_index < this->constant_pool.size ());
+ return (const char *) (this->constant_pool.data () + name_index);
}
size_t symbol_name_count () const
@@ -218,6 +219,17 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
if (this->symbol_name_slot_invalid (idx))
continue;
+ /* If the name offset is out of range for the constant pool then
+ we cannot get a valid string. We just skip this symbol. */
+ if (this->symbol_name_index (idx) >= this->constant_pool.size ())
+ {
+ complaint (_(".gdb_index entry %s has bad name offset %s [in module %s]"),
+ pulongest (idx),
+ pulongest (this->symbol_name_index (idx)),
+ objfile_name (per_objfile->objfile));
+ continue;
+ }
+
const char *name = this->symbol_name_at (idx, per_objfile);
/* This code only knows how to break apart components of C++
@@ -256,9 +268,25 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
lang = (components.size () == 1) ? language_unknown : language_go;
}
+ /* Even if NAME was the empty string then COMPONENTS will contain one
+ entry, the empty string. */
+ gdb_assert (components.size () > 0);
+
std::vector<cooked_index_entry *> these_entries;
offset_view vec (constant_pool.slice (symbol_vec_index (idx)));
offset_type vec_len = vec[0];
+
+ /* Bounds check VEC_LEN against the CONSTANT_POOL. */
+ if (vec_len >= vec.size ())
+ {
+ /* The vector count would exceed the available space in the
+ constant pool. This is a corrupt .gdb_index. We could
+ treat everything up to the end of the constant pool as a
+ valid vector, but we have no reason to think that's not
+ also wrong. Instead just pretend this vector is empty. */
+ vec_len = 0;
+ }
+
for (offset_type vec_idx = 0; vec_idx < vec_len; ++vec_idx)
{
offset_type cu_index_and_attrs = vec[vec_idx + 1];
@@ -347,9 +375,15 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
if (components.size () > 1)
{
std::string_view penultimate = components[components.size () - 2];
- std::string_view prefix (name, &penultimate.back () + 1 - name);
-
- need_parents.emplace_back (prefix, std::move (these_entries));
+ /* When parsing the DIE, GDB strips out entries with an empty name,
+ so in a well formed index PENULTIMATE should never be empty.
+ But the index is external data, so we cannot assume it is well
+ formed, ignore any empty prefixes. */
+ if (penultimate.size () > 0)
+ {
+ std::string_view prefix (name, &penultimate.back () + 1 - name);
+ need_parents.emplace_back (prefix, std::move (these_entries));
+ }
}
}
@@ -502,6 +536,25 @@ read_gdb_index_from_buffer (const char *filename,
symbol_table));
}
+ if (!map->symbol_table.empty ())
+ {
+ gdb_assert (!map->constant_pool.empty ());
+
+ if (map->constant_pool[map->constant_pool.size () - 1] != '\0')
+ {
+ static bool warning_printed = false;
+ if (!warning_printed)
+ {
+ warning (_("skipping .gdb_index in %ps, null byte at end of "
+ "constant pool is missing"),
+ styled_string (file_name_style.style (),
+ filename));
+ warning_printed = true;
+ }
+ return false;
+ }
+ }
+
return true;
}
@@ -707,10 +760,26 @@ mapped_gdb_index::set_main_name (dwarf2_per_objfile *per_objfile)
}
ptr += 4;
+ const ULONGEST name_offset
+ = extract_unsigned_integer (ptr, sizeof (offset_type), BFD_ENDIAN_LITTLE);
+
+ if (name_offset >= this->constant_pool.size ())
+ {
+ complaint (_(".gdb_index shortcut table has invalid main name offset %s"),
+ pulongest (name_offset));
+ return;
+ }
+
+ /* If the symbol table is empty then we ignore the .gdb_index in
+ dwarf2_read_gdb_index. If the symbol table is not empty, and doesn't
+ end in a '\0' byte then we also ignore the .gdb_index in
+ read_gdb_index_from_buffer. What this means is that, by the time we
+ get here, we know that the constant pool ends with '\0' and so the
+ MAIN_NAME will be a correctly terminated C style string. */
+ gdb_assert ((this->constant_pool)[(this->constant_pool).size () - 1]
+ == '\0');
+
main_lang = dwarf_lang_to_enum_language (dw_lang);
- const auto name_offset = extract_unsigned_integer (ptr,
- sizeof (offset_type),
- BFD_ENDIAN_LITTLE);
main_name = (const char *) (this->constant_pool.data () + name_offset);
}
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.c b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.c
new file mode 100644
index 00000000000..520df0ce8c4
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2026 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Dummy _start function, to be used with -nostartfile. */
+
+#include <stdlib.h>
+
+volatile int global_var = 0;
+
+void
+_start (void)
+{
+ asm ("_start_label: .globl _start_label");
+
+ /* Some work so that the body of the function is not too small. */
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+ global_var = global_var + 1;
+
+ exit (0);
+}
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
new file mode 100644
index 00000000000..a57b3c26341
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -0,0 +1,367 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test that GDB handles a corrupt .gdb_index gracefully. This test
+# checks a number of different scenarios, check the 'gdb_index' calls
+# within the 'run_test' proc for details.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+# This test installs its own hand-crafted .gdb_index into the
+# executable. The 'readnow' board forces GDB to ignore the index and
+# read the full debug info, making this test irrelevant.
+require !readnow
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" "${testfile}" \
+ [list ${srcfile}] {debug ldflags=-nostartfiles}]} {
+ return
+}
+
+set index [have_index $binfile]
+if { ![string eq $index ""] } {
+ unsupported "cache created at compile time"
+ return
+}
+
+set func_info_vars \
+ [get_func_info _start [list debug ldflags=-nostartfiles]]
+
+# TESTNAME is a string that is used to identify the test to run. This
+# string is used to control how the gdb_index is created, and then
+# what checks are performed on the executable after loading it.
+proc run_test { testname } {
+ set asm_file [standard_output_file ${testname}.S]
+ Dwarf::assemble {
+ filename $asm_file
+ add_dummy_cus 0
+ } {
+ upvar testname testname
+ global func_info_vars
+ foreach var $func_info_vars {
+ global $var
+ }
+
+ cu { label cu_label } {
+ compile_unit {
+ DW_AT_language @DW_LANG_C_plus_plus
+ } {
+ subprogram {
+ DW_AT_name _start
+ DW_AT_low_pc $_start_start DW_FORM_addr
+ DW_AT_high_pc $_start_end DW_FORM_addr
+ DW_AT_external 1 DW_FORM_flag
+ }
+ }
+ }
+
+ aranges {} cu_label {
+ arange {} $_start_start $_start_len
+ }
+
+ if { $testname eq "empty-component" } {
+ # GDB tries to split entries within the index based on
+ # '::' by default. Create an index entry with an empty
+ # component and ensure that this doesn't trigger undefined
+ # behaviour in GDB.
+ gdb_index {} {
+ cu cu_label
+ name "::method" {{cu_label function global}}
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "main-name-offset-1" } {
+ # Add main function details to a basic index, but encode
+ # an out of range name offset using the special string
+ # 'end'. Ensure that GDB doesn't crash and instead just
+ # ignores the main function details.
+ #
+ # Using 'end' here means that the constant pool index used
+ # for the main entry will point at the first byte after
+ # the constant pool. This is the off-by-one boundary
+ # case.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ main DW_LANG_C99 end
+ }
+ } elseif { $testname eq "main-name-offset-2" } {
+ # Add main function details to a basic index, but encode
+ # an out of range name offset. Ensure that GDB doesn't
+ # crash and instead just ignores the main function details.
+ #
+ # This is very similar to the 'main-name-offset-1' test,
+ # but here the name offset is huge, far past the end of
+ # the constant pool. This is far more likely to cause GDB
+ # to crash.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ main DW_LANG_C99 0xffffff00
+ }
+ } elseif { $testname eq "main-language" } {
+ # Add main function details to a basic index, but encode
+ # an invalid language number. Ensure that GDB ignores the
+ # main function details.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ main 0xfffff0 _start
+ }
+ } elseif { $testname eq "no-null" } {
+ # Create a .gdb_index without a null character at the end
+ # of the string table. This is invalid and GDB should just
+ # ignore this .gdb_index.
+ gdb_index {
+ no_null true
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "skip-constant-pool" } {
+ # Create a .gdb_index with a non-empty symbol table but an
+ # empty constant pool. GDB should ignore this .gdb_index.
+ gdb_index {
+ skip_index_part { constant_pool }
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "long-cu-vector" } {
+ # Create a .gdb_index containing a CU vector in the
+ # constant pool where the vector length is invalid.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {99999 {cu_label function global}}
+ }
+ } elseif { $testname eq "name-offset-1" } {
+ # Add a symbol to the index whose name offset in the symbol
+ # table is out of range. The slot still has a valid CU
+ # vector, so GDB won't treat it as an empty slot and will
+ # try to read the name. Ensure GDB doesn't crash and
+ # instead just ignores this one symbol.
+ #
+ # The 'end' offset is exactly the size of the constant pool,
+ # that is, it points at the first byte after the pool. This
+ # is the off-by-one boundary case.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}} end
+ }
+ } elseif { $testname eq "name-offset-2" } {
+ # Just like 'name-offset-1', but here the name offset is
+ # huge, far past the end of the constant pool. This is far
+ # more likely to cause GDB to crash.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}} 0xffffff00
+ }
+ } elseif { $testname eq "valid-index" } {
+ # Generate a well-formed index that includes a valid main
+ # function (valid language, with the name pointing at the
+ # '_start' symbol). This is a sanity check that index
+ # generation works, at one point this got broken and this
+ # wasn't spotted straight away as every index was broken
+ # in some subtle way.
+ #
+ # The C language is used, rather than C++, for the main
+ # function as for C++/Ada GDB only honours an IS_MAIN entry
+ # whose name is literally "main".
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}}
+ main DW_LANG_C99 _start
+ }
+ } else {
+ perror "invalid testname '$testname' during .gdb_index creation"
+ }
+ }
+
+ set testfile ${::testfile}-${testname}
+ set binfile [standard_output_file $testfile]
+ if {[build_executable "build executable" \
+ $testfile "$asm_file $::srcfile" \
+ [list ldflags=-nostartfiles]] == -1} {
+ return
+ }
+
+ clean_restart
+
+ gdb_test_no_output "maint set dwarf synchronous on"
+ gdb_test_no_output "set complaints 100"
+
+ # Loading the binary should not crash despite whatever weird
+ # things are in the .gdb_index.
+ gdb_load $binfile
+
+ # If we're using the cc-with-gnu-debuglink board then the debug
+ # information will have been moved to a separate file.
+ if { [section_get $binfile ".gnu_debuglink"] eq "" } {
+ set suffix ""
+ } else {
+ # If we are using a debuglink then assume the debug is moved
+ # into a file with a .debug suffix.
+ set suffix ".debug"
+ }
+
+ # Setup the expected results based on the testname.
+ if { $testname eq "empty-component" } {
+ set expect_gdb_index_ignored false
+ set expect_start true
+ set expect_method true
+ set expect_main false
+ set expect_foo false
+ set expect_name_count 2
+ set expect_warning_re ""
+ } elseif { $testname eq "main-name-offset-1" \
+ || $testname eq "main-name-offset-2" \
+ || $testname eq "main-language" \
+ || $testname eq "long-cu-vector" \
+ || $testname eq "name-offset-1" \
+ || $testname eq "name-offset-2" } {
+ set expect_gdb_index_ignored false
+ set expect_start true
+ set expect_method false
+ set expect_main false
+ set expect_foo false
+ set expect_name_count 1
+ if { $testname eq "main-language" } {
+ set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index shortcut table has invalid main language $::decimal\r\n"
+ } elseif { $testname eq "main-name-offset-1" \
+ || $testname eq "main-name-offset-2" } {
+ set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index shortcut table has invalid main name offset $::decimal\r\n"
+ } elseif { $testname eq "name-offset-1" \
+ || $testname eq "name-offset-2" } {
+ set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index entry $::decimal has bad name offset $::decimal \\\[in module \[^\r\n\]+\\\]\r\n"
+ } else {
+ set expect_warning_re ""
+ }
+ } elseif { $testname eq "valid-index" } {
+ # This is a well-formed index, so GDB should see _start and
+ # should recognise it as the main function.
+ set expect_gdb_index_ignored false
+ set expect_start true
+ set expect_method false
+ set expect_main true
+ set expect_foo true
+ set expect_name_count 2
+ set expect_warning_re ""
+ } elseif { $testname eq "no-null"
+ || $testname eq "skip-constant-pool" } {
+ set expect_gdb_index_ignored true
+ if { $testname eq "no-null" } {
+ set expect_warning_re "\r\nwarning: skipping .gdb_index in \[^\r\n\]+, null byte at end of constant pool is missing\r\n"
+ } else {
+ set expect_warning_re ""
+ }
+
+ # No need to set any of the other EXPECTED_* variables as they
+ # will never be used.
+ } else {
+ perror "invalid testname '$testname' during index check"
+ }
+
+ # Check the expected warning was emitted while loading the
+ # executable. The GDB_FILE_CMD_MSG global was set by the call to
+ # gdb_load above.
+ if { $expect_warning_re ne "" } {
+ gdb_assert { [regexp $expect_warning_re $::gdb_file_cmd_msg] } \
+ "index rejected with expected warning"
+ } else {
+ gdb_assert { ![regexp "\r\nDuring symbol reading: " \
+ $::gdb_file_cmd_msg] \
+ && ![regexp "\r\nwarning: " \
+ $::gdb_file_cmd_msg] } \
+ "index loaded without any warnings or complaints"
+ }
+
+ # Check if GDB used a .gdb_index section, or ignored it.
+ set index [get_index_type [file tail $binfile]$suffix]
+ if { $expect_gdb_index_ignored } {
+ gdb_assert { [string equal $index "cooked"] } ".gdb_index ignored"
+
+ # Ensure that an attempt to expand all symtabs doesn't crash GDB,
+ # even when the index is corrupted as it is in this test.
+ gdb_test_no_output "maint expand-symtabs" "expand all symtabs"
+
+ return
+ } else {
+ gdb_assert { [string equal $index "gdb"] } ".gdb_index used"
+ }
+
+ # Take a look at the details of the loaded .gdb_index and check
+ # that they match the expected results.
+ set saw_start false
+ set saw_method false
+ set saw_foo false
+ set saw_main ""
+ set name_count 0
+ gdb_test_multiple "maint print objfiles" "" -lbl {
+ -re "^\r\n\\s+name:\\s+(\[^\r\n\]+)(?=\r\n)" {
+ incr name_count
+ set name $expect_out(1,string)
+ if { $name eq "_start" } {
+ set saw_start true
+ } elseif { $name eq "method" } {
+ set saw_method true
+ } elseif { $name eq "foo" } {
+ set saw_foo true
+ }
+ exp_continue
+ }
+ -re "^\r\n\\s+main: \\(\\(cooked_index_entry \\*\\) ($::decimal|$::hex)\\)\[^\r\n\]*(?=\r\n)" {
+ set offset $expect_out(1,string)
+ if { $offset eq "0" } {
+ set saw_main false
+ } else {
+ set saw_main true
+ }
+ exp_continue
+ }
+ -re "^\r\n$::gdb_prompt $" {
+ gdb_assert { $saw_start == $expect_start \
+ && $saw_method == $expect_method \
+ && $saw_main == $expect_main \
+ && $name_count == $expect_name_count \
+ && $saw_foo == $expect_foo } \
+ $gdb_test_name
+ }
+ }
+
+ # Ensure that an attempt to expand all symtabs doesn't crash GDB,
+ # even when the index is corrupted as it is in this test.
+ gdb_test_no_output "maint expand-symtabs" "expand all symtabs"
+}
+
+set tests { empty-component \
+ main-name-offset-1 \
+ main-name-offset-2 \
+ main-language \
+ no-null \
+ skip-constant-pool \
+ long-cu-vector \
+ name-offset-1 \
+ name-offset-2 \
+ valid-index }
+
+foreach_with_prefix test $tests {
+ run_test $test
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b3c77864719..7a82f3915d3 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -585,6 +585,11 @@ namespace eval Dwarf {
# otherwise false.
variable _cu_is_fission
+ # The total lengths of each CU/TU unit as an associative array,
+ # keyed by the unit's label. This is used by the .gdb_index
+ # generator (see 'gdb_index') to emit a correct CU length.
+ variable _cu_lengths
+
# When assembling, this is used to stash the level in which any
# code should be evaluated.
variable _level
@@ -1796,6 +1801,7 @@ namespace eval Dwarf {
variable _cu_addr_size
variable _cu_offset_size
variable _cu_is_fission
+ variable _cu_lengths
variable _level
# Establish the defaults.
@@ -1858,6 +1864,9 @@ namespace eval Dwarf {
set start_label [_compute_label "cu${cu_num}_start"]
set end_label [_compute_label "cu${cu_num}_end"]
+ # Record the length of this CU.
+ set _cu_lengths($_cu_label) "$end_label - $_cu_label"
+
if { $label != "" } {
upvar $label my_label
set my_label $_cu_label
@@ -1955,6 +1964,7 @@ namespace eval Dwarf {
variable _cu_addr_size
variable _cu_offset_size
variable _cu_is_fission
+ variable _cu_lengths
variable _level
# Establish the defaults.
@@ -2018,6 +2028,9 @@ namespace eval Dwarf {
set start_label [_compute_label "cu${cu_num}_start"]
set end_label [_compute_label "cu${cu_num}_end"]
+ # Record the length of this TU.
+ set _cu_lengths($_cu_label) "$end_label - $_cu_label"
+
if { $label != "" } {
upvar $label my_label
set my_label $_cu_label
@@ -3757,6 +3770,524 @@ namespace eval Dwarf {
debug_names_end:
}
+ # Compute the .gdb_index hash for NAME. This must match
+ # mapped_index_string_hash (index version >= 5) from
+ # dwarf2/index-common.c.
+ proc _gdb_index_hash { name } {
+ set r 0
+ foreach c [split $name ""] {
+ scan $c %c code
+ if {$code >= 65 && $code <= 90} {
+ set code [expr {$code + 32}]
+ }
+ set r [expr {($r * 67 + $code - 113) & 0xFFFFFFFF}]
+ }
+ return $r
+ }
+
+ # Pack a CU vector entry for .gdb_index. Returns a uint32 with
+ # the CU index in bits 0-23, symbol kind in bits 28-30, and the
+ # static/global flag in bit 31.
+ proc _gdb_index_pack_entry { cu_index kind_str scope_str } {
+ set kinds {none 0 type 1 variable 2 function 3 other 4}
+ set kind_val [dict get $kinds $kind_str]
+ set static_bit [expr {$scope_str eq "static" ? 1 : 0}]
+ return [expr {($static_bit << 31) | ($kind_val << 28)
+ | ($cu_index & 0xFFFFFF)}]
+ }
+
+ # Emit a .gdb_index section.
+ #
+ # OPTIONS is a list with an even number of elements containing
+ # option-name and option-value pairs.
+ # Current options are:
+ # version N - the .gdb_index version number
+ # (default 9)
+ # no_null true|false
+ # - when true, omit inserting '\0' bytes after each string
+ # in the string table. Failing to do this results in an
+ # invalid index. This exists only for testing GDB's error
+ # handling.
+ # (default false)
+ # skip_index_part <parts>
+ # - skip generating some parts of the .gdb_index. <parts>
+ # is a list of part names, each name identifies part of
+ # the .gdb_index to not generate. When a part is skipped
+ # its content is not emitted into the index, but the
+ # offset to where the content would have been is still
+ # placed into the index header, the next header offset
+ # will be at the same location though as the skipped part
+ # has zero length. This option is only useful for
+ # generating invalid .gdb_index sections for testing GDB's
+ # error handling. The supported part names are:
+ # 'constant_pool'.
+ # (default {} -- no .gdb_index parts are skipped)
+ #
+ # BODY is Tcl code that emits the parts which make up the body of
+ # the .gdb_index section. It is evaluated in the caller's
+ # context. The following commands are available for the BODY
+ # section:
+ #
+ # cu <cu-label> [<length>]
+ # -- add a CU by its label. The optional <length> overrides the
+ # CU length written into the CU list. Normally this length is
+ # computed automatically to match the actual size of the CU in
+ # .debug_info; providing a (bogus) override allows GDB's
+ # handling of an inconsistent CU length to be tested.
+ #
+ # name <name> <entries> [<name_offset>]
+ # -- add symbol <name> with its CU vector to the constant pool:
+ #
+ # The <entries> is a list of {<cu_ref> <kind> <scope>} tuples
+ # where:
+ #
+ # <cu_ref> is a CU label as passed to the 'cu' command;
+ # this is used to calculate the index into the CU list
+ # within the index.
+ #
+ # Or, this can be a string 'CU-<index>' where <index>
+ # is a decimal value which is used as the index.
+ # This exists for testing, it allows invalid indexes
+ # to be generated.
+ #
+ # <kind> is 'none', 'type', 'variable', 'function', or 'other'.
+ #
+ # <scope> is 'global' or 'static'
+ #
+ # The optional <name_offset> overrides the name offset that is
+ # written into this symbol's slot in the symbol table. Normally
+ # this offset is computed automatically so that it points at the
+ # string <name> in the constant pool. Providing an override
+ # value allows an invalid (e.g. out of range) offset to be
+ # encoded for testing GDB's error handling. The value can be a
+ # raw number, or the keyword 'end' which resolves to the exact
+ # size of the constant pool (i.e. one byte past its end). The
+ # string <name> is still emitted into the constant pool.
+ #
+ # main <language> <name>
+ # -- add symbol <name> of <language> to the index as the entry point
+ # symbol.
+ #
+ # For a well formed index the <name> should be something
+ # declared with the 'name' command above, and is used to
+ # compute the offset into the constant pool's string table.
+ #
+ # For testing purposes the <name> can also be a hex or
+ # decimal value, in which case this is used as the offset,
+ # without being checked. This allows for offsets that
+ # point outside the constant pool.
+ #
+ # If <name> is the string 'end' then this also generates an
+ # invalid offset, this offset points exactly to the first
+ # byte outside the constant pool and is used to test for
+ # off-by-one errors in GDB. This does mean that we cannot
+ # generate a fake index including a function called 'end',
+ # but this felt like an acceptable tradeoff.
+ #
+ # The <language> can be any valid DW_LANG_* constant, or for
+ # testing purposes, any integer, even if this is not a valid
+ # language.
+ #
+ # Example:
+ # gdb_index {} {
+ # cu cu_label
+ # name "object" {{cu_label type global}}
+ # name "object::method" {{cu_label function global}}
+ # name "some_function" {{cu_label function global}}
+ # main DW_LANG_C99 some_function
+ # }
+ proc gdb_index { options body } {
+ global decimal
+ variable _cu_lengths
+
+ parse_options {
+ { version 9 }
+ { no_null false }
+ { skip_index_part {} }
+ }
+
+ # Validate the names passed to skip_index_part to catch typos in
+ # tests.
+ foreach part $skip_index_part {
+ if { [lsearch -exact { constant_pool } $part] == -1 } {
+ error "gdb_index: unknown skip_index_part '$part'"
+ }
+ }
+
+ # A list of two entry lists describing CUs. Each sub-list has the
+ # form {label length}. The <label> is a CU label, created when the
+ # CU itself was created. The <length> is optional, and will usually
+ # be the empty string, but can be set to a number to override the
+ # automatically calculated length.
+ variable _gdb_index_cus
+ set _gdb_index_cus []
+
+ # A list of lists. Each sub-list has the form: {name entries
+ # offset}, where <name> is a string, entries is a list of CU
+ # descriptors, and <offset> is usually the empty string, but can be
+ # a number which overrides the offset from the CU vector to the
+ # <name> string. See _gdb_index_name for more details.
+ variable _gdb_index_names
+ set _gdb_index_names []
+
+ # The value to encode as the main function language. The
+ # default value 0 is what the .gdb_index uses to indicate no
+ # main language information is present.
+ variable _gdb_index_main_lang
+ set _gdb_index_main_lang 0
+
+ # The main function name.
+ variable _gdb_index_main_name
+ set _gdb_index_main_name ""
+
+ # The index into the constant pool's string table of the main name
+ # string. For testing this can sometimes be set to an invalid
+ # value, so don't rely on this always pointing into the string
+ # table.
+ variable _gdb_index_main_name_idx
+ set _gdb_index_main_name_idx -1
+
+ # Create an entry in the CU list within the index. LABEL is created
+ # when the CU itself is created (see the 'label' option for 'proc
+ # cu'). The CU list also encodes the length of the CU. Usually
+ # this is done automatically, but LENGTH, which is a raw number, can
+ # be used to override this for testing invalid lengths appearing in
+ # the index.
+ proc _gdb_index_cu { label {length ""} } {
+ variable _gdb_index_cus
+ lappend _gdb_index_cus [list $label $length]
+ }
+
+ # Implement the 'name' command. Record a string NAME into the
+ # constant pool's string table, and use ENTRIES_LIST to generate a
+ # CU vector in the constant pool's CU vectors table.
+ #
+ # The CU vector contains the offset of NAME in the string table.
+ # Normally this is calculated automatically, but the optional
+ # NAME_OFFSET can be used to override this offset for testing.
+ # NAME_OFFSET can be a raw number, in which case this is used as the
+ # offset, or it can be the string 'end', which resolves to the exact
+ # size of the constant pool (i.e. one byte past its end). This is
+ # useful for testing for off-by-one errors in GDB.
+ proc _gdb_index_name { name entries_list {name_offset ""} } {
+ variable _gdb_index_names
+ lappend _gdb_index_names [list $name $entries_list $name_offset]
+ }
+
+ # Setup the name and language of the main function, as recorded in
+ # the .gdb_index shortcut table. The LANG can be a DW_LANG_*
+ # constant, but can also be a raw number, this allows invalid
+ # numbers to be used for testing.
+ #
+ # The NAME should be a string and should match a name passed to the
+ # 'name' command (_gdb_index_name), the offset of that name within
+ # the string table will then be calculated automatically.
+ #
+ # The NAME can also be a raw number, in which case this is used as
+ # the string table offset, this allows invalid offsets to be encoded
+ # for testing purposes.
+ #
+ # Finally, NAME can be the keyword 'end', which resolves to the
+ # exact size of the constant pool (i.e. one byte past its end).
+ # This is useful for testing for off-by-one errors in GDB. A main
+ # function literally named 'end' cannot be encoded into the index
+ # using this proc.
+ proc _gdb_index_main { lang name } {
+ variable _gdb_index_main_lang
+ variable _gdb_index_main_name
+ variable _gdb_index_main_name_idx
+ variable _constants
+
+ if {[regexp -- "^($::hex|$::decimal)$" $lang]} {
+ set _gdb_index_main_lang $lang
+ } else {
+ set _gdb_index_main_lang $_constants($lang)
+ }
+
+ if {[regexp -- "^($::hex|$::decimal)$" $name]} {
+ set _gdb_index_main_name_idx $name
+ } elseif { $name eq "end" } {
+ set _gdb_index_main_name_idx "end"
+ } else {
+ set _gdb_index_main_name $name
+ }
+ }
+
+ with_override Dwarf::cu Dwarf::_gdb_index_cu {
+ with_override Dwarf::name Dwarf::_gdb_index_name {
+ with_override Dwarf::main Dwarf::_gdb_index_main {
+ variable _level
+ uplevel $_level $body
+ }}}
+
+ set num_names [llength $_gdb_index_names]
+
+ # Compute hash for each name and record the index of the main
+ # function if we find a matching name.
+ set name_idx 0
+ set hashes []
+ foreach entry $_gdb_index_names {
+ lassign $entry name entries_list
+ lappend hashes [_gdb_index_hash $name]
+ if { $_gdb_index_main_lang != 0 } {
+ if { $_gdb_index_main_name eq $name } {
+ set _gdb_index_main_name_idx $name_idx
+ }
+ }
+ incr name_idx
+ }
+
+ # If we have a main function language recorded, but no index then
+ # something has gone wrong. Maybe the string passed to
+ # _gdb_index_main didn't match any string passed to _gdb_index_name?
+ if { $_gdb_index_main_lang != 0 && $_gdb_index_main_name_idx == -1 } {
+ perror "failed to find main function '$_gdb_index_main_name'"
+ }
+
+ # If we have a main function defined, but the index version is less
+ # than 9, then this is a mistake, earlier index versions didn't
+ # record this information.
+ if { $_gdb_index_main_lang != 0 && $version < 9 } {
+ perror "index version $version doesn't include main function details"
+ }
+
+ # Determine hash table size: smallest power of 2 where
+ # occupancy is below 75%. Minimum size is 2.
+ set table_size 2
+ while { 4 * $num_names >= 3 * $table_size } {
+ set table_size [expr {$table_size * 2}]
+ }
+
+ # Assign names to hash table slots using open addressing.
+ for {set i 0} {$i < $table_size} {incr i} {
+ set slot_occupied($i) 0
+ }
+
+ set name_idx 0
+ foreach hash_val $hashes {
+ set index [expr {$hash_val & ($table_size - 1)}]
+ set step [expr {(($hash_val * 17) & ($table_size - 1)) | 1}]
+ while { $slot_occupied($index) } {
+ set index [expr {($index + $step) & ($table_size - 1)}]
+ }
+ set slot_occupied($index) 1
+ set slot_name_idx($index) $name_idx
+ incr name_idx
+ }
+
+ # Create labels for each name's CU vector and string in the
+ # constant pool.
+ set cu_vec_labels []
+ set str_labels []
+ for {set i 0} {$i < $num_names} {incr i} {
+ lappend cu_vec_labels [new_label "gdb_index_cu_vec"]
+ lappend str_labels [new_label "gdb_index_str"]
+ }
+
+ # Emit the section.
+ _section ".gdb_index"
+
+ declare_labels gdb_index_start gdb_index_cu_list \
+ gdb_index_tu_list gdb_index_addr_table \
+ gdb_index_symtab gdb_index_constant_pool \
+ gdb_index_constant_pool_end
+
+ if { $version >= 9 } {
+ declare_labels gdb_index_shortcut
+ }
+
+ # If the main function was given by symbol name, then
+ # _GDB_INDEX_MAIN_NAME_IDX currently holds the index of that name in
+ # the names list. Now that the string labels and the constant pool
+ # label exist, translate it into the offset of the name's string
+ # within the constant pool, so the shortcut table emission below can
+ # use _GDB_INDEX_MAIN_NAME_IDX directly. (For a raw numeric offset,
+ # or the 'end' keyword, it already holds the value to emit.)
+ #
+ # After this we no longer need _GDB_INDEX_MAIN_NAME; unset it to
+ # avoid accidental uses below.
+ if { $_gdb_index_main_name ne "" } {
+ set str_label [lindex $str_labels $_gdb_index_main_name_idx]
+ set _gdb_index_main_name_idx "$str_label - $gdb_index_constant_pool"
+ }
+ unset _gdb_index_main_name
+
+ gdb_index_start:
+
+ # Header.
+ _op .4byte $version "Version"
+ _op .4byte "$gdb_index_cu_list - $gdb_index_start" \
+ "CU list offset"
+ _op .4byte "$gdb_index_tu_list - $gdb_index_start" \
+ "TU list offset"
+ _op .4byte "$gdb_index_addr_table - $gdb_index_start" \
+ "Address table offset"
+ _op .4byte "$gdb_index_symtab - $gdb_index_start" \
+ "Symbol table offset"
+ if { $version >= 9 } {
+ _op .4byte "$gdb_index_shortcut - $gdb_index_start" \
+ "Shortcut table offset"
+ }
+ _op .4byte "$gdb_index_constant_pool - $gdb_index_start" \
+ "Constant pool offset"
+
+ # CU list.
+ gdb_index_cu_list:
+ foreach cu_rec $_gdb_index_cus {
+ lassign $cu_rec cu length
+ upvar $cu tmp
+ _op_offset 8 $tmp "CU offset"
+ # Record the CU's length into the index. In a well formed
+ # index the length recorded here must match the actual
+ # length of the CU in .debug_info. We allow the length to
+ # be overridden, for testing purposes, by passing a bogus
+ # length to the 'cu' command. If that was done then use
+ # that length instead.
+ if { $length ne "" } {
+ set cu_length $length
+ } elseif { [info exists _cu_lengths($tmp)] } {
+ set cu_length $_cu_lengths($tmp)
+ } else {
+ error "gdb_index: unknown length for CU '$cu'"
+ }
+ _op_offset 8 "$cu_length" "CU length"
+ }
+
+ # TU list (empty for now).
+ gdb_index_tu_list:
+
+ # Address table (empty for now).
+ gdb_index_addr_table:
+
+ # Symbol table.
+ gdb_index_symtab:
+ for {set i 0} {$i < $table_size} {incr i} {
+ if {$slot_occupied($i)} {
+ set idx $slot_name_idx($i)
+ set str_label [lindex $str_labels $idx]
+ set vec_label [lindex $cu_vec_labels $idx]
+
+ # The name offset in the symbol table normally points at the
+ # string emitted in the constant pool, but it can be
+ # overridden (e.g. to an out of range value) for testing.
+ # See the 'name' command in the gdb_index documentation for
+ # details.
+ set name_offset [lindex [lindex $_gdb_index_names $idx] 2]
+ if { $name_offset eq "" } {
+ _op .4byte \
+ "$str_label - $gdb_index_constant_pool" \
+ "name offset"
+ } elseif { $name_offset eq "end" } {
+ _op .4byte \
+ "$gdb_index_constant_pool_end - $gdb_index_constant_pool" \
+ "name offset"
+ } else {
+ _op .4byte "$name_offset" "name offset"
+ }
+ _op .4byte \
+ "$vec_label - $gdb_index_constant_pool" \
+ "CU vector offset"
+ } else {
+ _op .4byte 0 "empty slot"
+ _op .4byte 0 "empty slot"
+ }
+ }
+
+ # Shortcut table (version 9 only).
+ if { $version >= 9 } {
+ gdb_index_shortcut:
+ _op .4byte $_gdb_index_main_lang "Language"
+ if { $_gdb_index_main_lang == 0 } {
+ _op .4byte 0 "Name offset"
+ } elseif { $_gdb_index_main_name_idx eq "end" } {
+ _op .4byte \
+ "$gdb_index_constant_pool_end - $gdb_index_constant_pool" \
+ "Name offset"
+ } else {
+ _op .4byte "$_gdb_index_main_name_idx" \
+ "Name offset"
+ }
+ }
+
+ # The constant pool is CU vectors first, and then name
+ # strings.
+ gdb_index_constant_pool:
+
+ if { [lsearch -exact $skip_index_part constant_pool] != -1 } {
+ # We've been asked to not generate the constant pool. Just make
+ # sure all of the required labels are defined. This results in
+ # an invalid .gdb_index. We only support this as an option for
+ # error testing.
+ for {set i 0} {$i < $num_names} {incr i} {
+ define_label [lindex $cu_vec_labels $i]
+ define_label [lindex $str_labels $i]
+ }
+ } else {
+ # Generate the .gdb_index constant pool.
+ set name_idx 0
+ foreach entry $_gdb_index_names {
+ lassign $entry name entries_list
+ define_label [lindex $cu_vec_labels $name_idx]
+
+ # Each entry in the ENTRIES_LIST is itself a list
+ # containing the details needed to create an entry in
+ # the CU vector. Each CU vector starts with a count
+ # of the entries in the list.
+ #
+ # If the first entry in ENTRIES_LIST is a number then
+ # this overrides the entries count. This allows
+ # invalid entry counts to be generated for testing
+ # reasons.
+ set first [lindex $entries_list 0]
+ if {[string is integer -strict $first]} {
+ set num_entries $first
+ set entries_list [lrange $entries_list 1 end]
+ } else {
+ set num_entries [llength $entries_list]
+ }
+
+ _op .4byte $num_entries "CU vector length"
+ foreach cu_entry $entries_list {
+ lassign $cu_entry cu_ref kind scope
+ if { [regexp "^CU-($decimal)$" $cu_ref dummy cu_index] } {
+ # Explicit CU index.
+ } else {
+ # Each element of _gdb_index_cus is a {label length}
+ # pair. Find the index of the one whose label matches
+ # CU_REF.
+ set cu_index \
+ [lsearch -index 0 -exact $_gdb_index_cus $cu_ref]
+ if { $cu_index == -1 } {
+ error "gdb_index: unknown CU reference: $cu_ref"
+ }
+ }
+ set packed [_gdb_index_pack_entry $cu_index $kind $scope]
+ _op .4byte $packed "CU index and attributes"
+ }
+ incr name_idx
+ }
+
+ set name_idx 0
+ foreach entry $_gdb_index_names {
+ lassign $entry name entries_list
+ define_label [lindex $str_labels $name_idx]
+ if { $no_null } {
+ # This generates a corrupt index with no '\0' between
+ # strings or at the end of the string table. This
+ # only exists for testing error handling in this case.
+ _op .ascii [_nz_quote $name]
+ } else {
+ _op .ascii [_quote $name]
+ }
+ incr name_idx
+ }
+ }
+
+ # Mark the end of the constant pool. This allows a name offset of
+ # 'end' to resolve to the exact size of the constant pool.
+ gdb_index_constant_pool_end:
+ }
+
# Add the strings in ARGS to the .debug_str section, and create a
# .debug_str_offsets section pointing to those strings.
# Current options are:
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/6] gdb: a number of fixes for .gdb_index parsing
2026-10-18 20:50 ` [PATCH 1/6] gdb: a number of fixes " Andrew Burgess
@ 2026-10-20 18:39 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2026-10-20 18:39 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
I focused on the non-test code (don't really have the time or mental
energy to read nearly a thousand lines of spicy tcl code right now), but
I also noted a few comments in the testsuite parts that came from
Claude.
On 10/18/26 4:50 PM, Andrew Burgess wrote:
> I recently posted a patch which Simon pointed out during review caused
> GDB to crash when running with cc-with-gdb-index board. The problem
> was that I was now generating an invalid .gdb_index section.
>
> This got me thinking, rather than crashing, GDB should really be
> rejecting the index as invalid. GDB should not treat the .gdb_index
> as trusted input. Even if we take the position that GDB will always
> generate valid .gdb_index sections, there's nothing to say that some
> other tool might try to generate .gdb_index sections, and do it wrong.
> We also write the .gdb_index into the index cache, disk corruption
> could result in an invalid .gdb_index being read back in later.
>
> This commit includes a number of fixes which I detail below, but the
> most important thing here is the addition of the 'gdb_index' proc to
> testsuite/lib/dwarf.exp. This proc started as a copy of 'proc
> debug_names' but updated to emit .gdb_index sections. This proc has
> been further modified to allow invalid indexes to be generated. I
> believe all of the features I've added so far are documented in the
> comments, so I'll not repeat it all here, but the general idea is to
> allow every offset to be overridden to allow for invalid offsets to be
> encoded into the .gdb_index. There are also options to allow parts of
> the index to be elided completely.
>
> Not every override feature of 'proc gdb_index' has a matching test
> right now. If I'd done that then I was worried that the GDB side
> fixes would be too large for a single patch. Further patches in this
> series will make use of the currently unused features.
>
> Currently, in dwarf2/read-gdb-index.c, I have the following fixes:
>
> In mapped_gdb_index::build_name_components:
>
> (1) If the name index in the .gdb_index's symbol table is out of
> bounds of the constant pool then ignore this symbol table entry.
>
> (2) Add an assert that the components list will contain at least
> one entry, even an empty string will result in a single
> component, the empty string.
>
> (3) When processing the CU vector, if the declared CU vector length
> is larger than the constant pool then ignore this CU vector.
>
> (4) If we end up with an empty component string after splitting a
> name from the index, then don't try to call .back() on the empty
> string.
>
> In read_gdb_index_from_buffer:
>
> (5) If the symbol table is not empty, but the constant pool doesn't
> end with a null character then reject this index. A non-empty
> symbol table means that there must be strings in the constant
> pool's string table. The strings in this table are
> null-terminated. And the string table is at the end of the
> constant pool. Therefore, the last byte in the constant pool
> must be a null character.
>
> If this is not the case then reading strings from the constant
> pool might not be null-terminated, and might read outside the
> constant pool.
Claude pointed out that according to the doc / spec of the format,
nothing would prevent a producer to use non-zero padding at the end of
the section... We can disregard that and assume it doesn't happen in
real life, but I wanted to point it out because I believe it's
technically true.
Or we could change the spec of the format to say that any padding must
be made of '\0' bytes.
>
> Placing this check here means that we can later on assume that
> any string read from the constant pool is null-terminated
> without reading outside the constant pool.
>
> In mapped_gdb_index::set_main_name:
>
> (6) If the main function name is outside the bounds of the constant
> pool then warn, and ignore the main function name.
>
> (7) Assert that the constant pool is null-terminated, see (5) above.
> ---
> gdb/dwarf2/read-gdb-index.c | 85 ++-
> .../gdb.dwarf2/gdb-index-corrupted.c | 47 ++
> .../gdb.dwarf2/gdb-index-corrupted.exp | 367 ++++++++++++
> gdb/testsuite/lib/dwarf.exp | 531 ++++++++++++++++++
> 4 files changed, 1022 insertions(+), 8 deletions(-)
> create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.c
> create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
>
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index 324984c0de2..86512c4ab9c 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -187,8 +187,9 @@ struct mapped_gdb_index
> const char *symbol_name_at (offset_type idx,
> dwarf2_per_objfile *per_objfile) const
> {
> - return (const char *) (this->constant_pool.data ()
> - + symbol_name_index (idx));
> + offset_type name_index = symbol_name_index (idx);
> + gdb_assert (name_index < this->constant_pool.size ());
> + return (const char *) (this->constant_pool.data () + name_index);
> }
>
> size_t symbol_name_count () const
> @@ -218,6 +219,17 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
> if (this->symbol_name_slot_invalid (idx))
> continue;
>
> + /* If the name offset is out of range for the constant pool then
> + we cannot get a valid string. We just skip this symbol. */
> + if (this->symbol_name_index (idx) >= this->constant_pool.size ())
> + {
> + complaint (_(".gdb_index entry %s has bad name offset %s [in module %s]"),
Line slightly too long.
> + pulongest (idx),
> + pulongest (this->symbol_name_index (idx)),
> + objfile_name (per_objfile->objfile));
> + continue;
> + }
> +
> const char *name = this->symbol_name_at (idx, per_objfile);
>
> /* This code only knows how to break apart components of C++
> @@ -256,9 +268,25 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
> lang = (components.size () == 1) ? language_unknown : language_go;
> }
>
> + /* Even if NAME was the empty string then COMPONENTS will contain one
> + entry, the empty string. */
> + gdb_assert (components.size () > 0);
> +
> std::vector<cooked_index_entry *> these_entries;
> offset_view vec (constant_pool.slice (symbol_vec_index (idx)));
> offset_type vec_len = vec[0];
> +
> + /* Bounds check VEC_LEN against the CONSTANT_POOL. */
> + if (vec_len >= vec.size ())
> + {
> + /* The vector count would exceed the available space in the
> + constant pool. This is a corrupt .gdb_index. We could
> + treat everything up to the end of the constant pool as a
> + valid vector, but we have no reason to think that's not
> + also wrong. Instead just pretend this vector is empty. */
> + vec_len = 0;
Just wondering, why set vec_len to 0, instead of complaint + continue?
> + }
Perhaps work for a subsequent patch, but maybe you (or I) could
reorganize this so that the bounds check is done before creating the
view, such that `vec` has exactly the right length (VEC_LEN elements).
I think that right now it's a view from the start of the vector until
the end of the constant pool. The VEC view could also exclude the
length itself, and then the for loop just below could become a simple
ranged for loop.
No need to change your patch, it is an improvement already.
> +
> for (offset_type vec_idx = 0; vec_idx < vec_len; ++vec_idx)
> {
> offset_type cu_index_and_attrs = vec[vec_idx + 1];
> @@ -347,9 +375,15 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
> if (components.size () > 1)
> {
> std::string_view penultimate = components[components.size () - 2];
> - std::string_view prefix (name, &penultimate.back () + 1 - name);
> -
> - need_parents.emplace_back (prefix, std::move (these_entries));
> + /* When parsing the DIE, GDB strips out entries with an empty name,
> + so in a well formed index PENULTIMATE should never be empty.
> + But the index is external data, so we cannot assume it is well
> + formed, ignore any empty prefixes. */
> + if (penultimate.size () > 0)
> + {
> + std::string_view prefix (name, &penultimate.back () + 1 - name);
> + need_parents.emplace_back (prefix, std::move (these_entries));
> + }
Given this recent fix:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8f7fb58377a9a50c668190a4a740a4078355076d
Is it still possible for split_name to return an empty component? For
the DOT_STYLE style I think it's no longer possible, but maybe it is for
the CXX style. If some invalid input to split_name results in an empty
component, then maybe we should change split_name to not do that
instead. It would be consistent with my fix. If there is no legitimate
case for split_name to return an empty component, then here we could
assert that the component is not empty.
When thinking about that (when I wrote the fix above), I was wondering
if maybe splitting "::foo::bar" should yield an empty first component,
The presence of that empty component could be a sign that the name was
"fully qualified", meaning it started with "::". But as far as I could
tell, no caller relied on that, so splitting "::foo::bar" could safely
return {"foo", "bar"}, I think.
> }
> }
>
> @@ -502,6 +536,25 @@ read_gdb_index_from_buffer (const char *filename,
> symbol_table));
> }
>
> + if (!map->symbol_table.empty ())
> + {
> + gdb_assert (!map->constant_pool.empty ());
> +
> + if (map->constant_pool[map->constant_pool.size () - 1] != '\0')
Hmm, std::span has a back() method, should we add it to array_view?
> + {
> + static bool warning_printed = false;
Not sure I would use a static flag here (which means we will print the
warning just once for the whole GDB session, even if you load multiple
objfiles that have the same problem). And it's not like this case
actually happens in real life... I would just unconditionally print the
warning.
> + if (!warning_printed)
> + {
> + warning (_("skipping .gdb_index in %ps, null byte at end of "
> + "constant pool is missing"),
> + styled_string (file_name_style.style (),
> + filename));
> + warning_printed = true;
> + }
> + return false;
> + }
> + }
Can you add a comment to tell why this last null byte check is useful?
Like you explained in the commit message.
> +
> return true;
> }
>
> @@ -707,10 +760,26 @@ mapped_gdb_index::set_main_name (dwarf2_per_objfile *per_objfile)
> }
> ptr += 4;
>
> + const ULONGEST name_offset
> + = extract_unsigned_integer (ptr, sizeof (offset_type), BFD_ENDIAN_LITTLE);
> +
> + if (name_offset >= this->constant_pool.size ())
> + {
> + complaint (_(".gdb_index shortcut table has invalid main name offset %s"),
> + pulongest (name_offset));
Maybe add the "in module %" part to the message?
> + return;
> + }
> +
> + /* If the symbol table is empty then we ignore the .gdb_index in
> + dwarf2_read_gdb_index. If the symbol table is not empty, and doesn't
> + end in a '\0' byte then we also ignore the .gdb_index in
> + read_gdb_index_from_buffer. What this means is that, by the time we
I wonder if this last sentence means to say "if the constant pool
doesn't end in a '\0' byte", rather than the symbol table.
> + get here, we know that the constant pool ends with '\0' and so the
> + MAIN_NAME will be a correctly terminated C style string. */
> + gdb_assert ((this->constant_pool)[(this->constant_pool).size () - 1]
> + == '\0');
I'm not sure this assert is really needed though... like why do it here
but not in build_name_components where we read all the symbols name? It
seems a bit random.
(and if you leave if there: the parentheses around this->constant_pool
are redundant, both times, but really it would be nicer with a back()
method)
> + # Check the expected warning was emitted while loading the
> + # executable. The GDB_FILE_CMD_MSG global was set by the call to
> + # gdb_load above.
> + if { $expect_warning_re ne "" } {
> + gdb_assert { [regexp $expect_warning_re $::gdb_file_cmd_msg] } \
> + "index rejected with expected warning"
In some cases the index is accepted with a warning, so the test name
should maybe be just "got expected warning".
> + # A list of two entry lists describing CUs. Each sub-list has the
> + # form {label length}. The <label> is a CU label, created when the
> + # CU itself was created. The <length> is optional, and will usually
> + # be the empty string, but can be set to a number to override the
> + # automatically calculated length.
> + variable _gdb_index_cus
> + set _gdb_index_cus []
> +
> + # A list of lists. Each sub-list has the form: {name entries
> + # offset}, where <name> is a string, entries is a list of CU
> + # descriptors, and <offset> is usually the empty string, but can be
> + # a number which overrides the offset from the CU vector to the
> + # <name> string. See _gdb_index_name for more details.
CU vector -> symbol table, I think.
Also, if you named it "name_offset" it would be a bit clearer than just
"offset".
> + variable _gdb_index_names
> + set _gdb_index_names []
> +
> + # The value to encode as the main function language. The
> + # default value 0 is what the .gdb_index uses to indicate no
> + # main language information is present.
> + variable _gdb_index_main_lang
> + set _gdb_index_main_lang 0
> +
> + # The main function name.
> + variable _gdb_index_main_name
> + set _gdb_index_main_name ""
> +
> + # The index into the constant pool's string table of the main name
> + # string. For testing this can sometimes be set to an invalid
> + # value, so don't rely on this always pointing into the string
> + # table.
> + variable _gdb_index_main_name_idx
> + set _gdb_index_main_name_idx -1
> +
> + # Create an entry in the CU list within the index. LABEL is created
> + # when the CU itself is created (see the 'label' option for 'proc
> + # cu'). The CU list also encodes the length of the CU. Usually
> + # this is done automatically, but LENGTH, which is a raw number, can
> + # be used to override this for testing invalid lengths appearing in
> + # the index.
> + proc _gdb_index_cu { label {length ""} } {
> + variable _gdb_index_cus
> + lappend _gdb_index_cus [list $label $length]
> + }
> +
> + # Implement the 'name' command. Record a string NAME into the
> + # constant pool's string table, and use ENTRIES_LIST to generate a
> + # CU vector in the constant pool's CU vectors table.
> + #
> + # The CU vector contains the offset of NAME in the string table.
Should this say:
The symbol table contains the offset of NAME in the constant pool.
?
> + set num_names [llength $_gdb_index_names]
> +
> + # Compute hash for each name and record the index of the main
> + # function if we find a matching name.
> + set name_idx 0
> + set hashes []
> + foreach entry $_gdb_index_names {
> + lassign $entry name entries_list
> + lappend hashes [_gdb_index_hash $name]
> + if { $_gdb_index_main_lang != 0 } {
> + if { $_gdb_index_main_name eq $name } {
Can you join these two ifs together?
Claude tells me to tell you:
Nit: dwarf.exp:4127: when main is given a raw offset,
_gdb_index_main_name is "", so a name "" entry would overwrite
_gdb_index_main_name_idx. Only check names when _gdb_index_main_name
ne "".
... which I guess could happen in theory. Might as well gate the main
name check on that.
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/6] gdb: handle incorrect CU size in .gdb_index
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
2026-10-18 20:50 ` [PATCH 1/6] gdb: a number of fixes " Andrew Burgess
@ 2026-10-18 20:50 ` Andrew Burgess
2026-10-20 18:50 ` Simon Marchi
2026-10-18 20:50 ` [PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index Andrew Burgess
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2026-10-18 20:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
The CU list within the .gdb_index contains the offset of a CU and the
length of the CU.
The length field is not needed when parsing the .gdb_index, it is
currently only used within GDB to pre-fill the length field of the
dwarf2_per_cu object. This pre-filled length is then validated in
dwarf2_per_cu::set_length when the real CU header is read from the
.debug_info.
Unfortunately, this validation is currently done using a gdb_assert,
which is the wrong approach. Both the .gdb_index and the CU header
are untrusted external input. Either of them (or both of them) might
be wrong, and though they should have the same value, this is not
guaranteed.
I cannot just replace the gdb_assert in dwarf2_per_cu::set_length with
a call to complaint, there are other paths where the length is
pre-filled from the actual CU header, in this case there should be no
possibility of a mismatch, and so gdb_assert is the correct approach.
I could add a new member variable to dwarf2_per_cu that tracks if the
pre-filled length came from the real CU header or from the .gdb_index,
then we could switch on this to either call gdb_assert or complaint.
But this seemed excessive when complaints are usually turned off by
default.
Instead, I propose we just ignore the CU length from the .gdb_index,
and pass 0 instead when initially creating the dwarf2_per_cu object.
A length of 0 indicates that the CU length is unknown. When GDB reads
the actual CU header it will then set the length at this point.
The CU length in the .gdb_index is no longer being used, but I am not
proposing that we change the .gdb_index format, though we could in
theory bump the index version number and remove the CU length, saving
a little space. However, changing the index format means more work
for downstream tools that read or write .gdb_index sections, so
avoiding that churn seems best to me.
---
gdb/dwarf2/read-gdb-index.c | 27 ++++++++++++++++---
.../gdb.dwarf2/gdb-index-corrupted.exp | 14 +++++++++-
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 86512c4ab9c..e0895fb6aa8 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -573,11 +573,32 @@ create_cus_from_gdb_index_list (dwarf2_per_bfd *per_bfd,
sect_offset sect_off
= (sect_offset) extract_unsigned_integer (cu_list, 8, BFD_ENDIAN_LITTLE);
- ULONGEST length = extract_unsigned_integer (cu_list + 8, 8, BFD_ENDIAN_LITTLE);
+ /* Each entry in the CU list is a pair of (offset, length), but
+ we deliberately ignore the length as this information is not
+ needed. When the real CU contents are read they also contain
+ a length. The length stored in the .gdb_index and the real
+ length in the CU header should match, but both the .gdb_index
+ and the real CU are untrusted input, so could be corrupt, in
+ which case the lengths might not match.
+
+ If we pre-fill the length here then the dwarf2_per_cu would
+ need to track that the length came from the .gdb_index so
+ that later on, when the length is read from the real CU, if
+ the lengths don't match GDB should use the real length and
+ warn about the mismatch.
+
+ This feels like a lot of work just for a length field that we
+ can safely ignore, especially when the complaint mechanism,
+ through which we would report the length mismatch, is off
+ by default. Instead, pass 0 as the length to allocate_per_cu
+ below, this indicates that the length is unknown, and can be
+ filled in later when the real CU header is read. The length
+ from the .gdb_index is ignored completely. */
cu_list += 2 * 8;
- dwarf2_per_cu_up per_cu = per_bfd->allocate_per_cu (section, sect_off,
- length, is_dwz);
+ /* Pass 0 for the length, see comment above. */
+ dwarf2_per_cu_up per_cu
+ = per_bfd->allocate_per_cu (section, sect_off, 0, is_dwz);
units.emplace_back (per_cu.get ());
per_bfd->add_unit (std::move (per_cu));
}
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index a57b3c26341..356b12f97ce 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -173,6 +173,16 @@ proc run_test { testname } {
name _start {{cu_label function global}}
name foo {{cu_label function global}} 0xffffff00
}
+ } elseif { $testname eq "bad-cu-length" } {
+ # Create an otherwise valid index, but record a bogus length
+ # for the CU in the index's CU list; one that doesn't match the
+ # actual length of the CU in .debug_info. GDB should handle
+ # this gracefully rather than crashing when it later comes to
+ # expand the CU.
+ gdb_index {} {
+ cu cu_label 0x1f
+ name _start {{cu_label function global}}
+ }
} elseif { $testname eq "valid-index" } {
# Generate a well-formed index that includes a valid main
# function (valid language, with the name pointing at the
@@ -236,7 +246,8 @@ proc run_test { testname } {
|| $testname eq "main-language" \
|| $testname eq "long-cu-vector" \
|| $testname eq "name-offset-1" \
- || $testname eq "name-offset-2" } {
+ || $testname eq "name-offset-2" \
+ || $testname eq "bad-cu-length" } {
set expect_gdb_index_ignored false
set expect_start true
set expect_method false
@@ -360,6 +371,7 @@ set tests { empty-component \
long-cu-vector \
name-offset-1 \
name-offset-2 \
+ bad-cu-length \
valid-index }
foreach_with_prefix test $tests {
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/6] gdb: handle incorrect CU size in .gdb_index
2026-10-18 20:50 ` [PATCH 2/6] gdb: handle incorrect CU size in .gdb_index Andrew Burgess
@ 2026-10-20 18:50 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2026-10-20 18:50 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 10/18/26 4:50 PM, Andrew Burgess wrote:
> The CU list within the .gdb_index contains the offset of a CU and the
> length of the CU.
>
> The length field is not needed when parsing the .gdb_index, it is
> currently only used within GDB to pre-fill the length field of the
> dwarf2_per_cu object. This pre-filled length is then validated in
> dwarf2_per_cu::set_length when the real CU header is read from the
> .debug_info.
>
> Unfortunately, this validation is currently done using a gdb_assert,
> which is the wrong approach. Both the .gdb_index and the CU header
> are untrusted external input. Either of them (or both of them) might
> be wrong, and though they should have the same value, this is not
> guaranteed.
>
> I cannot just replace the gdb_assert in dwarf2_per_cu::set_length with
> a call to complaint, there are other paths where the length is
> pre-filled from the actual CU header, in this case there should be no
> possibility of a mismatch, and so gdb_assert is the correct approach.
>
> I could add a new member variable to dwarf2_per_cu that tracks if the
> pre-filled length came from the real CU header or from the .gdb_index,
> then we could switch on this to either call gdb_assert or complaint.
>
> But this seemed excessive when complaints are usually turned off by
> default.
>
> Instead, I propose we just ignore the CU length from the .gdb_index,
> and pass 0 instead when initially creating the dwarf2_per_cu object.
> A length of 0 indicates that the CU length is unknown. When GDB reads
> the actual CU header it will then set the length at this point.
>
> The CU length in the .gdb_index is no longer being used, but I am not
> proposing that we change the .gdb_index format, though we could in
> theory bump the index version number and remove the CU length, saving
> a little space. However, changing the index format means more work
> for downstream tools that read or write .gdb_index sections, so
> avoiding that churn seems best to me.
I think this is ok, dwarf2_find_containing_unit (one of the only
things that uses a unit's length) is ready for this.
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
2026-10-18 20:50 ` [PATCH 1/6] gdb: a number of fixes " Andrew Burgess
2026-10-18 20:50 ` [PATCH 2/6] gdb: handle incorrect CU size in .gdb_index Andrew Burgess
@ 2026-10-18 20:50 ` Andrew Burgess
2026-10-20 19:00 ` Simon Marchi
2026-10-18 20:50 ` [PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index Andrew Burgess
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2026-10-18 20:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Within the .gdb_index, the symbol table contains indexes that point to
CU vectors within the constant pool. Unfortunately, these indexes are
not validated against the size of the constant pool, so a malformed
.gdb_index can cause GDB to read outside the bounds of the .gdb_index.
This commit extends the dwarf.exp gdb_index proc to allow for the CU
offset to be overridden, this is then used to add tests that expose
this bug.
Finally, mapped_gdb_index::build_name_components is updated to sanity
check the CU offset before it is used. If we find an invalid offset
then that symbol will be ignored.
To improve GDB's ability to catch errors I also added an assert to
offset_view::operator[] that the whole offset_type is within the view.
Without the build_name_components fix this assert will be triggered by
one of the new tests.
---
gdb/dwarf2/read-gdb-index.c | 22 +++++++-
.../gdb.dwarf2/gdb-index-corrupted.exp | 54 +++++++++++++++++++
gdb/testsuite/lib/dwarf.exp | 47 ++++++++++++----
3 files changed, 111 insertions(+), 12 deletions(-)
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index e0895fb6aa8..343ad90847f 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -102,6 +102,11 @@ class offset_view
/* Extract the INDEXth offset_type from the array. */
offset_type operator[] (size_t index) const
{
+ /* Ensure the whole offset_type is within the underlying bytes. If GDB
+ is compiled with libc++ assert then the access of M_BYTES will be
+ checked, but this only guarantees that the first byte is within the
+ array_view as it's a gdb_byte view. */
+ gdb_assert (index < size ());
const gdb_byte *bytes = &m_bytes[index * sizeof (offset_type)];
return (offset_type) extract_unsigned_integer (bytes,
sizeof (offset_type),
@@ -273,7 +278,22 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
gdb_assert (components.size () > 0);
std::vector<cooked_index_entry *> these_entries;
- offset_view vec (constant_pool.slice (symbol_vec_index (idx)));
+
+ offset_type vec_index = symbol_vec_index (idx);
+ /* The CU vector starts with an offset_type length word. Check that
+ VEC_INDEX lands inside CONSTANT_POOL, and that there is at least an
+ offset_type worth of bytes beyond this in CONSTANT_POOL to be
+ read. */
+ if (vec_index >= constant_pool.size ()
+ || constant_pool.size () - vec_index < sizeof (offset_type))
+ {
+ complaint (_(".gdb_index entry %s has bad CU vector offset %s "
+ "[in module %s]"),
+ pulongest (idx), pulongest (vec_index),
+ objfile_name (per_objfile->objfile));
+ continue;
+ }
+ offset_view vec (constant_pool.slice (vec_index));
offset_type vec_len = vec[0];
/* Bounds check VEC_LEN against the CONSTANT_POOL. */
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index 356b12f97ce..33c00874002 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -173,6 +173,50 @@ proc run_test { testname } {
name _start {{cu_label function global}}
name foo {{cu_label function global}} 0xffffff00
}
+ } elseif { $testname eq "cu-vector-offset-1" } {
+ # Add a symbol whose CU vector offset in the symbol table is out
+ # of range. The slot has a valid name offset, so GDB won't
+ # treat it as an empty slot and will try to read the CU vector.
+ # Ensure GDB doesn't crash and instead just ignores this one
+ # symbol.
+ #
+ # The 'end' offset is exactly the size of the constant pool,
+ # that is, it points at the first byte after the pool. This is
+ # the off-by-one boundary case.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}} "" end
+ }
+ } elseif { $testname eq "cu-vector-offset-2" } {
+ # Just like 'cu-vector-offset-1', but here the CU vector offset
+ # is huge, far past the end of the constant pool. This is far
+ # more likely to cause GDB to crash.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}} "" 0xffffff00
+ }
+ } elseif { $testname eq "cu-vector-offset-3" } {
+ # Like cu-vector-offset-1/-2, but aim the CU vector offset at
+ # the last byte of the constant pool. That byte offset is in
+ # range, but there is no room there for the 4-byte CU vector
+ # length word that GDB reads. This ensures that GDB is
+ # correctly validating the entire length of its reads.
+ #
+ # Work out the constant pool size from its contents. There are
+ # two symbols that need adding to the constant pool, '_start'
+ # and 'foo'. For each symbol we have a CU vector (a 4-byte
+ # count and a 4-byte offset), then we have both symbol strings
+ # plus their null terminating character.
+ set pool_size \
+ [expr {2 * (4 + 4) + [string length "_start"] + 1 \
+ + [string length "foo"] + 1}]
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}} "" [expr {$pool_size - 1}]
+ }
} elseif { $testname eq "bad-cu-length" } {
# Create an otherwise valid index, but record a bogus length
# for the CU in the index's CU list; one that doesn't match the
@@ -247,6 +291,9 @@ proc run_test { testname } {
|| $testname eq "long-cu-vector" \
|| $testname eq "name-offset-1" \
|| $testname eq "name-offset-2" \
+ || $testname eq "cu-vector-offset-1" \
+ || $testname eq "cu-vector-offset-2" \
+ || $testname eq "cu-vector-offset-3" \
|| $testname eq "bad-cu-length" } {
set expect_gdb_index_ignored false
set expect_start true
@@ -262,6 +309,10 @@ proc run_test { testname } {
} elseif { $testname eq "name-offset-1" \
|| $testname eq "name-offset-2" } {
set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index entry $::decimal has bad name offset $::decimal \\\[in module \[^\r\n\]+\\\]\r\n"
+ } elseif { $testname eq "cu-vector-offset-1" \
+ || $testname eq "cu-vector-offset-2" \
+ || $testname eq "cu-vector-offset-3" } {
+ set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index entry $::decimal has bad CU vector offset $::decimal \\\[in module \[^\r\n\]+\\\]\r\n"
} else {
set expect_warning_re ""
}
@@ -371,6 +422,9 @@ set tests { empty-component \
long-cu-vector \
name-offset-1 \
name-offset-2 \
+ cu-vector-offset-1 \
+ cu-vector-offset-2 \
+ cu-vector-offset-3 \
bad-cu-length \
valid-index }
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 7a82f3915d3..b59e598a163 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -3835,7 +3835,7 @@ namespace eval Dwarf {
# .debug_info; providing a (bogus) override allows GDB's
# handling of an inconsistent CU length to be tested.
#
- # name <name> <entries> [<name_offset>]
+ # name <name> <entries> [<name_offset> [<cuvec_offset>]]
# -- add symbol <name> with its CU vector to the constant pool:
#
# The <entries> is a list of {<cu_ref> <kind> <scope>} tuples
@@ -3862,7 +3862,17 @@ namespace eval Dwarf {
# encoded for testing GDB's error handling. The value can be a
# raw number, or the keyword 'end' which resolves to the exact
# size of the constant pool (i.e. one byte past its end). The
- # string <name> is still emitted into the constant pool.
+ # string <name> is still emitted into the constant pool. Pass
+ # an empty string to keep the automatically computed offset.
+ #
+ # Similarly, the optional <cuvec_offset> overrides the CU vector
+ # offset written into this symbol's slot in the symbol table.
+ # Normally this offset is computed to point at this symbol's CU
+ # vector in the constant pool but an override allows an out of
+ # range offset to be encoded. The value can be a raw number or
+ # the keyword 'end' which has the same meaning as for
+ # <name_offset> above. The CU vector itself is still emitted
+ # into the constant pool.
#
# main <language> <name>
# -- add symbol <name> of <language> to the index as the entry point
@@ -3923,10 +3933,9 @@ namespace eval Dwarf {
set _gdb_index_cus []
# A list of lists. Each sub-list has the form: {name entries
- # offset}, where <name> is a string, entries is a list of CU
- # descriptors, and <offset> is usually the empty string, but can be
- # a number which overrides the offset from the CU vector to the
- # <name> string. See _gdb_index_name for more details.
+ # name_offset cuvec_offset}. See the 'name' command (proc
+ # _gdb_index_name) in the gdb_index documentation for details of
+ # what each field means.
variable _gdb_index_names
set _gdb_index_names []
@@ -3969,9 +3978,11 @@ namespace eval Dwarf {
# offset, or it can be the string 'end', which resolves to the exact
# size of the constant pool (i.e. one byte past its end). This is
# useful for testing for off-by-one errors in GDB.
- proc _gdb_index_name { name entries_list {name_offset ""} } {
+ proc _gdb_index_name { name entries_list {name_offset ""} \
+ {cuvec_offset ""} } {
variable _gdb_index_names
- lappend _gdb_index_names [list $name $entries_list $name_offset]
+ lappend _gdb_index_names \
+ [list $name $entries_list $name_offset $cuvec_offset]
}
# Setup the name and language of the main function, as recorded in
@@ -4184,9 +4195,23 @@ namespace eval Dwarf {
} else {
_op .4byte "$name_offset" "name offset"
}
- _op .4byte \
- "$vec_label - $gdb_index_constant_pool" \
- "CU vector offset"
+
+ # The CU vector offset normally points at this symbol's CU
+ # vector in the constant pool, but it too can be overridden
+ # (e.g. to an out of range value) for testing. See the
+ # 'name' command in the gdb_index documentation for details.
+ set cuvec_offset [lindex [lindex $_gdb_index_names $idx] 3]
+ if { $cuvec_offset eq "" } {
+ _op .4byte \
+ "$vec_label - $gdb_index_constant_pool" \
+ "CU vector offset"
+ } elseif { $cuvec_offset eq "end" } {
+ _op .4byte \
+ "$gdb_index_constant_pool_end - $gdb_index_constant_pool" \
+ "CU vector offset"
+ } else {
+ _op .4byte "$cuvec_offset" "CU vector offset"
+ }
} else {
_op .4byte 0 "empty slot"
_op .4byte 0 "empty slot"
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index
2026-10-18 20:50 ` [PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index Andrew Burgess
@ 2026-10-20 19:00 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2026-10-20 19:00 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 10/18/26 4:50 PM, Andrew Burgess wrote:
> Within the .gdb_index, the symbol table contains indexes that point to
> CU vectors within the constant pool. Unfortunately, these indexes are
> not validated against the size of the constant pool, so a malformed
> .gdb_index can cause GDB to read outside the bounds of the .gdb_index.
>
> This commit extends the dwarf.exp gdb_index proc to allow for the CU
> offset to be overridden, this is then used to add tests that expose
> this bug.
>
> Finally, mapped_gdb_index::build_name_components is updated to sanity
> check the CU offset before it is used. If we find an invalid offset
> then that symbol will be ignored.
>
> To improve GDB's ability to catch errors I also added an assert to
> offset_view::operator[] that the whole offset_type is within the view.
> Without the build_name_components fix this assert will be triggered by
> one of the new tests.
Again, I skipped the test part for now.
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index e0895fb6aa8..343ad90847f 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -102,6 +102,11 @@ class offset_view
> /* Extract the INDEXth offset_type from the array. */
> offset_type operator[] (size_t index) const
> {
> + /* Ensure the whole offset_type is within the underlying bytes. If GDB
> + is compiled with libc++ assert then the access of M_BYTES will be
Not sure if you meant to say libstdc++ here (maybe clang's libc++ has
the same asserts, I don't know).
> + checked, but this only guarantees that the first byte is within the
> + array_view as it's a gdb_byte view. */
> + gdb_assert (index < size ());
> const gdb_byte *bytes = &m_bytes[index * sizeof (offset_type)];
> return (offset_type) extract_unsigned_integer (bytes,
> sizeof (offset_type),
> @@ -273,7 +278,22 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
> gdb_assert (components.size () > 0);
>
> std::vector<cooked_index_entry *> these_entries;
> - offset_view vec (constant_pool.slice (symbol_vec_index (idx)));
> +
> + offset_type vec_index = symbol_vec_index (idx);
> + /* The CU vector starts with an offset_type length word. Check that
> + VEC_INDEX lands inside CONSTANT_POOL, and that there is at least an
> + offset_type worth of bytes beyond this in CONSTANT_POOL to be
> + read. */
> + if (vec_index >= constant_pool.size ()
> + || constant_pool.size () - vec_index < sizeof (offset_type))
Can this be simplified to just:
if (vec_index + sizeof (offset_type) > constant_pool.size ())
{
// bad
}
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
` (2 preceding siblings ...)
2026-10-18 20:50 ` [PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index Andrew Burgess
@ 2026-10-18 20:50 ` Andrew Burgess
2026-10-20 19:09 ` Simon Marchi
2026-10-18 20:50 ` [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets Andrew Burgess
2026-10-18 20:50 ` [PATCH 6/6] gdb: extend .gdb_index tests to skip all data blocks Andrew Burgess
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2026-10-18 20:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
I spotted that when reading a .gdb_index, GDB could read outside the
address table in the function create_addrmap_from_gdb_index.
Currently GDB starts with a pointer ITER and checks that this pointer
is within the address table. GDB then reads 20 bytes from the address
table. There is no check that there are 20 bytes between ITER and
END, the end of the address table. This means that a malformed index
could cause GDB to read outside the address table.
Fix this by checking at the top of the address table reading loop that
there are 20 bytes remaining. Reject any .gdb_index with a malformed
address table.
I've then extended the gdb_index proc in dwarf.exp to allow for the
creation of address tables. I added a new 'addr' command to add
properly formed entries to the address table, and 'bad_addr' to add
malformed entries to the address table.
I did consider just overloading 'addr' to do both jobs, but I didn't
like that. I felt it was less clear if a test intended to create a
bad address table entry, or just had a mistake in the test.
While I was in the area I added tests for some of the pre-existing
error handling within the address table parsing loop. None of these
error handling cases were being tested before now.
---
gdb/dwarf2/read-gdb-index.c | 11 ++
.../gdb.dwarf2/gdb-index-corrupted.exp | 114 +++++++++++++++++-
gdb/testsuite/lib/dwarf.exp | 99 ++++++++++++---
3 files changed, 209 insertions(+), 15 deletions(-)
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 343ad90847f..83e628d3afb 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -733,6 +733,17 @@ create_addrmap_from_gdb_index (dwarf2_per_objfile *per_objfile,
const gdb_byte *end = iter + index->address_table.size ();
while (iter < end)
{
+ /* Ensure we're not about to read outside the bounds of the address
+ table, if we did then we'd interpret the symbol table as address
+ map data. */
+ if (iter + 20 > end)
+ {
+ warning (_(".gdb_index address table has invalid size (%s bytes),"
+ " ignoring .gdb_index"),
+ pulongest (index->address_table.size ()));
+ return false;
+ }
+
ULONGEST hi, lo, cu_index;
lo = extract_unsigned_integer (iter, 8, BFD_ENDIAN_LITTLE);
iter += 8;
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index 33c00874002..58ac13ce2ee 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -227,6 +227,55 @@ proc run_test { testname } {
cu cu_label 0x1f
name _start {{cu_label function global}}
}
+ } elseif { $testname eq "addr-truncated-1" } {
+ # Create an address table with a truncated entry (missing the
+ # high address and CU index). The address table size is not a
+ # multiple of 20 bytes (the entry size), so GDB should reject
+ # this .gdb_index.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ bad_addr $_start_start
+ }
+ } elseif { $testname eq "addr-truncated-2" } {
+ # Like addr-truncated-1 above, but with both low and high
+ # addresses (missing only the CU index now). This creates a
+ # 16-byte entry instead of 20. Again, GDB should reject this
+ # .gdb_index.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ bad_addr $_start_start $_start_end
+ }
+ } elseif { $testname eq "addr-valid" } {
+ # Create a valid address table, just to ensure that the address
+ # table creation mechanism within the gdb_index proc works
+ # correctly. GDB should accept this .gdb_index, and later on we
+ # check that we see the expected address map entries.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ addr $_start_start $_start_end cu_label
+ }
+ } elseif { $testname eq "addr-swapped" } {
+ # Create an address table with the low and high addresses the
+ # wrong way around. GDB should reject this .gdb_index and give
+ # an appropriate warning message.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ addr $_start_end $_start_start cu_label
+ }
+ } elseif { $testname eq "addr-overlap" } {
+ # Create an address table with multiple address ranges. Two of
+ # the address ranges overlap. GDB should reject this index.
+ gdb_index {} {
+ cu cu_label
+ name _start {{cu_label function global}}
+ name foo {{cu_label function global}}
+ addr $_start_start "$_start_end - 2" cu_label
+ addr "$_start_end - 4" $_start_end cu_label
+ }
} elseif { $testname eq "valid-index" } {
# Generate a well-formed index that includes a valid main
# function (valid language, with the name pointing at the
@@ -284,6 +333,7 @@ proc run_test { testname } {
set expect_main false
set expect_foo false
set expect_name_count 2
+ set expect_addr_map {}
set expect_warning_re ""
} elseif { $testname eq "main-name-offset-1" \
|| $testname eq "main-name-offset-2" \
@@ -316,6 +366,36 @@ proc run_test { testname } {
} else {
set expect_warning_re ""
}
+ set expect_addr_map {}
+ } elseif { $testname eq "addr-valid" } {
+ set expect_start true
+ set expect_method false
+ set expect_main false
+ set expect_foo false
+ set expect_name_count 1
+ set start [get_hexadecimal_valueof "$::_start_start" "*default*" \
+ "get address for start of _start"]
+ set end [get_hexadecimal_valueof "$::_start_end" "*default*" \
+ "get address for end of _start"]
+ set expect_addr_map \
+ [list [list [format "0x%x" $start] [format "0x%x" $end]]]
+ set expect_gdb_index_ignored false
+ set expect_warning_re ""
+ } elseif { $testname eq "addr-truncated-1" \
+ || $testname eq "addr-truncated-2" \
+ || $testname eq "addr-swapped" \
+ || $testname eq "addr-overlap" } {
+ set expect_gdb_index_ignored true
+ if { $testname eq "addr-truncated-1" \
+ || $testname eq "addr-truncated-2" } {
+ set expect_warning_re "\r\nwarning: \\.gdb_index address table has invalid size \\($::decimal bytes\\), ignoring \\.gdb_index\r\n"
+ } elseif { $testname eq "addr-swapped" } {
+ set expect_warning_re "\r\nwarning: \\.gdb_index address table has invalid range \\($::hex - $::hex\\), ignoring \\.gdb_index\r\n"
+ } elseif { $testname eq "addr-overlap" } {
+ set expect_warning_re "\r\nwarning: \\.gdb_index address table has a range \\($::hex - $::hex\\) that overlaps with an earlier range, ignoring \\.gdb_index\r\n"
+ } else {
+ perror "invalid testname '$testname' during index check"
+ }
} elseif { $testname eq "valid-index" } {
# This is a well-formed index, so GDB should see _start and
# should recognise it as the main function.
@@ -325,6 +405,7 @@ proc run_test { testname } {
set expect_main true
set expect_foo true
set expect_name_count 2
+ set expect_addr_map {}
set expect_warning_re ""
} elseif { $testname eq "no-null"
|| $testname eq "skip-constant-pool" } {
@@ -376,6 +457,9 @@ proc run_test { testname } {
set saw_foo false
set saw_main ""
set name_count 0
+ set addr_map {}
+ set addr_map_entry_start ""
+ set in_address_maps false
gdb_test_multiple "maint print objfiles" "" -lbl {
-re "^\r\n\\s+name:\\s+(\[^\r\n\]+)(?=\r\n)" {
incr name_count
@@ -398,12 +482,35 @@ proc run_test { testname } {
}
exp_continue
}
+ -re "^\r\n\\s+address maps:(?=\r\n)" {
+ set in_address_maps true
+ exp_continue
+ }
+ -re "^\r\n\\s+\\\[($::hex)\\\] \\(\\(dwarf2_per_cu \\*\\) (0|$::hex)\\)(?=\r\n)" {
+ if { $in_address_maps } {
+ set start [format "0x%x" $expect_out(1,string)]
+ set obj $expect_out(2,string)
+
+ # Finish any existing entry.
+ if { $addr_map_entry_start ne "" } {
+ lappend addr_map [list $addr_map_entry_start $start]
+ set addr_map_entry_start ""
+ }
+
+ # Create the start of a new entry.
+ if { $obj ne "0" } {
+ set addr_map_entry_start $start
+ }
+ }
+ exp_continue
+ }
-re "^\r\n$::gdb_prompt $" {
gdb_assert { $saw_start == $expect_start \
&& $saw_method == $expect_method \
&& $saw_main == $expect_main \
&& $name_count == $expect_name_count \
- && $saw_foo == $expect_foo } \
+ && $saw_foo == $expect_foo \
+ && $addr_map == $expect_addr_map } \
$gdb_test_name
}
}
@@ -426,6 +533,11 @@ set tests { empty-component \
cu-vector-offset-2 \
cu-vector-offset-3 \
bad-cu-length \
+ addr-truncated-1 \
+ addr-truncated-2 \
+ addr-swapped \
+ addr-overlap \
+ addr-valid \
valid-index }
foreach_with_prefix test $tests {
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b59e598a163..25b88b226e0 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -3898,6 +3898,28 @@ namespace eval Dwarf {
# testing purposes, any integer, even if this is not a valid
# language.
#
+ # addr <low> <high> <cu_ref>
+ # -- add an address table entry mapping an address range to a CU.
+ #
+ # <low> and <high> are address expressions, usually label names
+ # that the linker will resolve from get_func_info, but can also
+ # be raw hex/decimal values.
+ #
+ # <cu_ref> is a CU label, or 'CU-<index>' for a raw CU index,
+ # just like in the 'name' command above.
+ #
+ # Emits a 20-byte entry: 8-byte low, 8-byte high, 4-byte CU
+ # index.
+ #
+ # bad_addr <low> [<high>]
+ # -- add a truncated address table entry for testing.
+ #
+ # The <low> and <high> are defined as in the 'addr' command
+ # above. The <high> argument is optional. Emits 8 bytes if only
+ # <low> is provided, 16 bytes if both are provided. In either
+ # case the address table entry created is shorter than it should
+ # be.
+ #
# Example:
# gdb_index {} {
# cu cu_label
@@ -3939,6 +3961,12 @@ namespace eval Dwarf {
variable _gdb_index_names
set _gdb_index_names []
+ # A list of address table entries. Each entry is either:
+ # {addr <low> <high> <cu_ref>} for a full 20-byte entry.
+ # {bad_addr <low> <high>} or {bad_addr <low>} for a truncated entry.
+ variable _gdb_index_addrs
+ set _gdb_index_addrs []
+
# The value to encode as the main function language. The
# default value 0 is what the .gdb_index uses to indicate no
# main language information is present.
@@ -3967,6 +3995,21 @@ namespace eval Dwarf {
lappend _gdb_index_cus [list $label $length]
}
+ # Resolve CU_REF to a CU index. CU_REF is either a CU label (as
+ # passed to the 'cu' command) or a string 'CU-<index>' where <index>
+ # is used directly.
+ proc _gdb_index_resolve_cu_ref { cu_ref } {
+ variable _gdb_index_cus
+ if { [regexp "^CU-($::decimal)$" $cu_ref _ cu_idx] } {
+ return $cu_idx
+ }
+ set cu_idx [lsearch -index 0 -exact $_gdb_index_cus $cu_ref]
+ if { $cu_idx == -1 } {
+ error "gdb_index: unknown CU reference '$cu_ref'"
+ }
+ return $cu_idx
+ }
+
# Implement the 'name' command. Record a string NAME into the
# constant pool's string table, and use ENTRIES_LIST to generate a
# CU vector in the constant pool's CU vectors table.
@@ -3985,6 +4028,18 @@ namespace eval Dwarf {
[list $name $entries_list $name_offset $cuvec_offset]
}
+ # Add a full address table entry mapping an address range to a CU.
+ proc _gdb_index_addr { low high cu_ref } {
+ variable _gdb_index_addrs
+ lappend _gdb_index_addrs [list addr $low $high $cu_ref]
+ }
+
+ # Add a truncated address table entry for testing.
+ proc _gdb_index_bad_addr { low {high ""} } {
+ variable _gdb_index_addrs
+ lappend _gdb_index_addrs [list bad_addr $low $high]
+ }
+
# Setup the name and language of the main function, as recorded in
# the .gdb_index shortcut table. The LANG can be a DW_LANG_*
# constant, but can also be a raw number, this allows invalid
@@ -4027,9 +4082,11 @@ namespace eval Dwarf {
with_override Dwarf::cu Dwarf::_gdb_index_cu {
with_override Dwarf::name Dwarf::_gdb_index_name {
with_override Dwarf::main Dwarf::_gdb_index_main {
+ with_override Dwarf::addr Dwarf::_gdb_index_addr {
+ with_override Dwarf::bad_addr Dwarf::_gdb_index_bad_addr {
variable _level
uplevel $_level $body
- }}}
+ }}}}}
set num_names [llength $_gdb_index_names]
@@ -4167,8 +4224,33 @@ namespace eval Dwarf {
# TU list (empty for now).
gdb_index_tu_list:
- # Address table (empty for now).
+ # Address table.
gdb_index_addr_table:
+ foreach addr_entry $_gdb_index_addrs {
+ lassign $addr_entry entry_type low high
+
+ if { $entry_type eq "addr" } {
+ set cu_ref [lindex $addr_entry 3]
+
+ set cu_idx [_gdb_index_resolve_cu_ref $cu_ref]
+
+ _op .8byte $low "Low address"
+ _op .8byte $high "High address"
+ _op .4byte $cu_idx "CU index"
+ } elseif { $entry_type eq "bad_addr" } {
+ _op_offset 8 "$low" "Low address"
+ # Optionally omit high address to create a truncated entry.
+ if { $high ne "" } {
+ _op_offset 8 "$high" "High address"
+ } else {
+ _op "" "" "Skip high address"
+ }
+ # Always omit CU index to create a truncated entry.
+ _op "" "" "Skip CU index"
+ } else {
+ error "gdb_index: unknown address entry type '$entry_type'"
+ }
+ }
# Symbol table.
gdb_index_symtab:
@@ -4274,18 +4356,7 @@ namespace eval Dwarf {
_op .4byte $num_entries "CU vector length"
foreach cu_entry $entries_list {
lassign $cu_entry cu_ref kind scope
- if { [regexp "^CU-($decimal)$" $cu_ref dummy cu_index] } {
- # Explicit CU index.
- } else {
- # Each element of _gdb_index_cus is a {label length}
- # pair. Find the index of the one whose label matches
- # CU_REF.
- set cu_index \
- [lsearch -index 0 -exact $_gdb_index_cus $cu_ref]
- if { $cu_index == -1 } {
- error "gdb_index: unknown CU reference: $cu_ref"
- }
- }
+ set cu_index [_gdb_index_resolve_cu_ref $cu_ref]
set packed [_gdb_index_pack_entry $cu_index $kind $scope]
_op .4byte $packed "CU index and attributes"
}
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index
2026-10-18 20:50 ` [PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index Andrew Burgess
@ 2026-10-20 19:09 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2026-10-20 19:09 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 10/18/26 4:50 PM, Andrew Burgess wrote:
> I spotted that when reading a .gdb_index, GDB could read outside the
> address table in the function create_addrmap_from_gdb_index.
> Currently GDB starts with a pointer ITER and checks that this pointer
> is within the address table. GDB then reads 20 bytes from the address
> table. There is no check that there are 20 bytes between ITER and
> END, the end of the address table. This means that a malformed index
> could cause GDB to read outside the address table.
>
> Fix this by checking at the top of the address table reading loop that
> there are 20 bytes remaining. Reject any .gdb_index with a malformed
> address table.
>
> I've then extended the gdb_index proc in dwarf.exp to allow for the
> creation of address tables. I added a new 'addr' command to add
> properly formed entries to the address table, and 'bad_addr' to add
> malformed entries to the address table.
>
> I did consider just overloading 'addr' to do both jobs, but I didn't
> like that. I felt it was less clear if a test intended to create a
> bad address table entry, or just had a mistake in the test.
>
> While I was in the area I added tests for some of the pre-existing
> error handling within the address table parsing loop. None of these
> error handling cases were being tested before now.
> ---
> gdb/dwarf2/read-gdb-index.c | 11 ++
> .../gdb.dwarf2/gdb-index-corrupted.exp | 114 +++++++++++++++++-
> gdb/testsuite/lib/dwarf.exp | 99 ++++++++++++---
> 3 files changed, 209 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index 343ad90847f..83e628d3afb 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -733,6 +733,17 @@ create_addrmap_from_gdb_index (dwarf2_per_objfile *per_objfile,
> const gdb_byte *end = iter + index->address_table.size ();
> while (iter < end)
> {
> + /* Ensure we're not about to read outside the bounds of the address
> + table, if we did then we'd interpret the symbol table as address
> + map data. */
> + if (iter + 20 > end)
> + {
> + warning (_(".gdb_index address table has invalid size (%s bytes),"
> + " ignoring .gdb_index"),
> + pulongest (index->address_table.size ()));
> + return false;
> + }
We could maybe just check once that the size of the table % 20 == 0.
Claude complained that 'iter + 20 > end' forms a pointer past end, which
I guess might be UB, in theory? It suggested changing it to
'end - iter < 20', or the modulo approach, I think the latter is nicer.
> @@ -4167,8 +4224,33 @@ namespace eval Dwarf {
> # TU list (empty for now).
> gdb_index_tu_list:
>
> - # Address table (empty for now).
> + # Address table.
> gdb_index_addr_table:
> + foreach addr_entry $_gdb_index_addrs {
> + lassign $addr_entry entry_type low high
> +
> + if { $entry_type eq "addr" } {
> + set cu_ref [lindex $addr_entry 3]
> +
> + set cu_idx [_gdb_index_resolve_cu_ref $cu_ref]
> +
> + _op .8byte $low "Low address"
> + _op .8byte $high "High address"
> + _op .4byte $cu_idx "CU index"
> + } elseif { $entry_type eq "bad_addr" } {
> + _op_offset 8 "$low" "Low address"
Claude commented:
Nit: dwarf.exp:4307: addr uses _op .8byte while bad_addr and the CU
list use _op_offset 8. On 32-bit targets, .8byte <label> needs a
64-bit relocation that gas may refuse. Use _op_offset 8 for both.
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
` (3 preceding siblings ...)
2026-10-18 20:50 ` [PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index Andrew Burgess
@ 2026-10-18 20:50 ` Andrew Burgess
2026-10-20 19:23 ` Simon Marchi
2026-10-18 20:50 ` [PATCH 6/6] gdb: extend .gdb_index tests to skip all data blocks Andrew Burgess
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2026-10-18 20:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
The .gdb_index section has a header. The header is a list of offsets
into the .gdb_index section. Currently GDB has basically no error
checking of these offsets, the offsets can point outside the
.gdb_index section and GDB will usually try to do something bad as a
result.
Most often a bad offset will result in GDB trying to allocate a huge
amount of memory. Indeed, I don't recommend running these new tests
on an unpatched GDB; I managed to bring down my machine due to
excessive memory allocation.
I've added a mechanism to the gdb_index proc (in lib/dwarf.exp) to
allow the various .gdb_index header offsets to be overridden. Then I
made use of this in gdb.dwarf2/gdb-index-corrupted.exp to override
each offset in turn.
I also added a new test gdb.dwarf2/gdb-index-tiny-header.exp which
tests with a .gdb_index sized between 0 and 28 bytes (inclusive).
This covers indexes that are too small to even hold a valid version
number field, and indexes upto the size of the full header block. All
of these indexes are too small to hold useful data so GDB should
reject them all (without crashing).
The changes in gdb/dwarf2/read-gdb-index.c are fairly straightforward;
check the .gdb_index is big enough for the version field, then use the
version field to check that we have all of the offsets, and that the
offsets point within the .gdb_index section.
Also, as the data areas that the .gdb_index header offsets point to
are arranged in offset order, I also check that offsets are always
equal or increasing in value.
---
gdb/dwarf2/read-gdb-index.c | 59 +++++++++
.../gdb.dwarf2/gdb-index-corrupted.exp | 59 +++++++++
| 49 +++++++
| 121 ++++++++++++++++++
gdb/testsuite/lib/dwarf.exp | 79 ++++++++++--
5 files changed, 355 insertions(+), 12 deletions(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S
create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 83e628d3afb..5959b0ba6bc 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -481,6 +481,22 @@ read_gdb_index_from_buffer (const char *filename,
const gdb_byte *addr = &buffer[0];
offset_view metadata (buffer);
+ /* Emit a warning that the .gdb_index section is too small. */
+ auto invalid_index_size_warning = [&] ()
+ {
+ warning (_(".gdb_index section in %ps is too small (%s bytes),"
+ " ignoring .gdb_index"),
+ styled_string (file_name_style.style (), filename),
+ pulongest (buffer.size ()));
+ return false;
+ };
+
+ /* Ensure that there is at least a version number available for reading.
+ We cannot do a minimum buffer size check until we know what version of
+ index we have, so first just check we can read that safely. */
+ if (buffer.size () < sizeof (offset_type))
+ return invalid_index_size_warning ();
+
/* Version check. */
offset_type version = metadata[0];
/* GDB now requires the symbol attributes, which were added in
@@ -513,6 +529,49 @@ read_gdb_index_from_buffer (const char *filename,
map->version = version;
+ /* The .gdb_index header is a version number followed by a series of
+ offsets into the .gdb_index section. Each header offset points to a
+ data block, and the offset order, and the data block order are the
+ same. This means that each offset must be greater than, or equal to
+ the previous. The first offset should point to the first byte after
+ the .gdb_index header. None of the header offsets should point outside
+ of the .gdb_index section.
+
+ If any of these requirements are not met then GDB can end either
+ crashing by accessing invalid memory, or trying to allocate a huge
+ amount of memory to hold an incorrectly sized data block.
+
+ So, before we make use of the header, let's sanity check it.
+
+ First, how many offsets do we expect in the header? Version 9 added
+ the shortcut table offset. */
+ const size_t offset_count = version >= 9 ? 6 : 5;
+
+ /* Ensure the buffer is large enough to contain the whole header (the
+ version word plus OFFSET_COUNT offset words) before reading any
+ offsets. */
+ if (metadata.size () < offset_count + 1)
+ return invalid_index_size_warning ();
+
+ /* Each offset must not be smaller than the end of the header, no larger
+ than the whole .gdb_index section, and no smaller than the preceding
+ offset. Initially set PREV to the header size to enforce the lower
+ bound on the first offset. */
+ size_t prev = (offset_count + 1) * sizeof (offset_type);
+ for (size_t idx = 1; idx <= offset_count; ++idx)
+ {
+ size_t off = metadata[idx];
+ if (off < prev || off > buffer.size ())
+ {
+ warning (_("skipping .gdb_index section in %ps, an invalid offset (%s)"
+ " was found in the header"),
+ styled_string (file_name_style.style (), filename),
+ hex_string (off));
+ return false;
+ }
+ prev = off;
+ }
+
int i = 1;
*cu_list = addr + metadata[i];
*cu_list_elements = (metadata[i + 1] - metadata[i]) / 8;
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index 58ac13ce2ee..865e5e50f97 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -276,6 +276,48 @@ proc run_test { testname } {
addr $_start_start "$_start_end - 2" cu_label
addr "$_start_end - 4" $_start_end cu_label
}
+ } elseif { $testname eq "header-cu-list-invalid" } {
+ # Create an index with an out-of-bounds CU list offset.
+ # GDB should detect this and reject the index.
+ gdb_index { header_override {cu_list 0xffffffff} } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "header-tu-list-invalid" } {
+ # Create an index with an out-of-bounds TU list offset.
+ # GDB should detect this and reject the index.
+ gdb_index { header_override {tu_list 0xffffffff} } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "header-address-table-invalid" } {
+ # Create an index with an out-of-bounds address table offset.
+ # GDB should detect this and reject the index.
+ gdb_index { header_override {address_table 0xffffffff} } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "header-symbol-table-invalid" } {
+ # Create an index with an out-of-bounds symbol table offset.
+ # GDB should detect this and reject the index.
+ gdb_index { header_override {symbol_table 0xffffffff} } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "header-shortcut-invalid" } {
+ # Create an index with an out-of-bounds shortcut table offset.
+ # GDB should detect this and reject the index.
+ gdb_index { header_override {shortcut 0xffffffff} } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "header-constant-pool-invalid" } {
+ # Create an index with an out-of-bounds constant pool offset.
+ # GDB should detect this and reject the index.
+ gdb_index { header_override {constant_pool 0xffffffff} } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
} elseif { $testname eq "valid-index" } {
# Generate a well-formed index that includes a valid main
# function (valid language, with the name pointing at the
@@ -367,6 +409,17 @@ proc run_test { testname } {
set expect_warning_re ""
}
set expect_addr_map {}
+ } elseif { $testname eq "header-cu-list-invalid" \
+ || $testname eq "header-tu-list-invalid" \
+ || $testname eq "header-address-table-invalid" \
+ || $testname eq "header-symbol-table-invalid" \
+ || $testname eq "header-shortcut-invalid" \
+ || $testname eq "header-constant-pool-invalid" } {
+ # Each of these tests puts an out-of-range offset in one field of
+ # the index header. GDB should detect the invalid offset, emit a
+ # warning, and reject the index.
+ set expect_gdb_index_ignored true
+ set expect_warning_re "\r\nwarning: skipping \\.gdb_index section in \[^\r\n\]*, an invalid offset \\($::hex\\) was found in the header\r\n"
} elseif { $testname eq "addr-valid" } {
set expect_start true
set expect_method false
@@ -538,6 +591,12 @@ set tests { empty-component \
addr-swapped \
addr-overlap \
addr-valid \
+ header-cu-list-invalid \
+ header-tu-list-invalid \
+ header-address-table-invalid \
+ header-symbol-table-invalid \
+ header-shortcut-invalid \
+ header-constant-pool-invalid \
valid-index }
foreach_with_prefix test $tests {
--git a/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S
new file mode 100644
index 00000000000..3df0fa666f9
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S
@@ -0,0 +1,49 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2026 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Create a .gdb_index section that is too small to hold a valid header.
+ The size of the section, in bytes, is given by the INDEX_SIZE macro
+ which the exp script sets when this file is assembled.
+
+ A valid version 9 header is 28 bytes: a 4-byte version word followed by
+ six 4-byte offsets. Any section smaller than this is invalid and GDB
+ should reject it rather than reading beyond the end of the section.
+
+ The .gdb_index format is always little-endian, so we build the section
+ byte by byte rather than with directives like .4byte, whose encoding
+ would depend on the host's endianness. The first byte is 0x09, the low
+ byte of a little-endian version 9 word; any remaining bytes are zero.
+ For sizes of 4 or more this yields a valid version word (0x09 0x00 0x00
+ 0x00) so that GDB gets past its version check and on to the header size
+ check, with the trailing zeros standing in for a truncated set of header
+ offsets. For sizes below 4 there is not even room for the full version
+ word. */
+
+ .section .gdb_index
+#if INDEX_SIZE > 0
+ /* Low byte of a little-endian version 9 word. */
+ .byte 0x09
+# if INDEX_SIZE > 1
+ /* Rest of the truncated header. GDB checks the header size
+ before checking that the header contents are valid, so
+ filling with zeros, which is invalid, doesn't really
+ matter. */
+ .fill (INDEX_SIZE - 1), 1, 0
+# endif
+#endif
+
+ .section .note.GNU-stack,"",%progbits
--git a/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp
new file mode 100644
index 00000000000..a99aa3b69d7
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp
@@ -0,0 +1,121 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB gracefully handles a .gdb_index section that is too small
+# to contain a valid header. A valid version 9 header is 28 bytes, a 4-byte
+# version word followed by six 4-byte offsets, so the section is built at
+# every size from 0 up to 28 bytes in turn. Each of these should cause GDB
+# to ignore the index and fall back to building a cooked index from the
+# DWARF, rather than reading beyond the end of the index section. The size
+# 28 case is interesting as this is big enough to hold a valid header, but
+# every offset in the header is 0, which is invalid, so GDB will reject the
+# index for that reason.
+#
+# The sizes split into a few interesting ranges:
+# 0 bytes -- an empty section, silently ignored.
+# 1 to 3 bytes -- too small even for the version word.
+# 4 to 27 bytes -- a valid version word, but a truncated set of header
+# offsets.
+# 28 bytes -- a full set of header offsets, but they are all zero,
+# which is invalid.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+# This test installs its own hand-crafted .gdb_index into the executable.
+# The 'readnow' board forces GDB to ignore the index and read the full debug
+# info, making this test irrelevant.
+require !readnow
+
+standard_testfile main.c gdb-index-tiny-header.S
+
+# This test relies on adding a .gdb_index. If we are using a board that
+# adds an index at compile time then this test is not going to work. So
+# build an executable at this point and check if it has an index already, if
+# it does, then we cannot carry on.
+if {[prepare_for_testing "failed to prepare" ${testfile} $srcfile]} {
+ return
+}
+
+set index [have_index $binfile]
+if { ![string eq $index ""] } {
+ unsupported "cache created at compile time"
+ return
+}
+
+# Build and test one variant of the tiny .gdb_index section. SIZE is the
+# number of bytes in the generated section, it is passed to the assembler
+# source via the INDEX_SIZE macro.
+proc run_test { size } {
+ set exe "${::binfile}-${size}"
+
+ if {[build_executable "failed to build" $exe \
+ [list $::srcfile $::srcfile2] \
+ [list debug additional_flags=-DINDEX_SIZE=$size]] == -1} {
+ return
+ }
+
+ clean_restart
+
+ # Read the index synchronously as part of loading the file, so that any
+ # problem shows up during the 'file' command below.
+ gdb_test_no_output "maint set dwarf synchronous on"
+
+ # Loading the executable. gdb_load returns -1 if GDB reports an
+ # internal error.
+ set res [gdb_load $exe]
+ gdb_assert { $res == 0 } "load executable"
+ if { $res != 0 } {
+ return
+ }
+
+ # A non-empty but too-small section should be rejected with a warning.
+ # A zero byte index section is empty and is silently ignored.
+ if { $size == 28 } {
+ set warning_re \
+ [string cat \
+ "\r\nwarning: skipping \\.gdb_index section in \[^\r\n\]+, an invalid offset \\(0x0\\) was found in the header\r\n"]
+ gdb_assert { [regexp $warning_re $::gdb_file_cmd_msg] } \
+ "index rejected due to invalid offset"
+ } elseif { $size > 0 } {
+ set warning_re \
+ [string cat \
+ "\r\nwarning: \\.gdb_index section in \[^\r\n\]* is too " \
+ "small \\($::decimal bytes\\), ignoring \\.gdb_index\r\n"]
+ gdb_assert { [regexp $warning_re $::gdb_file_cmd_msg] } \
+ "index rejected due to header size"
+ } else {
+ # The SIZE is 0, there should be no warning in this case.
+ gdb_assert { ![regexp "warning: " $::gdb_file_cmd_msg] } \
+ "no warning seen"
+ }
+
+ # The tiny .gdb_index is invalid, so GDB should ignore it and use a
+ # cooked index built from the DWARF instead.
+ set index [get_index_type [file tail $exe]]
+ gdb_assert { [string equal $index "cooked"] } ".gdb_index ignored"
+
+ # Expanding all symtabs should not crash GDB.
+ gdb_test_no_output "maint expand-symtabs" "expand all symtabs"
+}
+
+# Test every size from an empty section up to exactly the length of a valid
+# version 9 header (28 bytes).
+for { set s 0 } { $s <= 28 } { incr s } {
+ with_test_prefix "size $s" {
+ run_test $s
+ }
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 25b88b226e0..b2b23167671 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -3822,6 +3822,19 @@ namespace eval Dwarf {
# error handling. The supported part names are:
# 'constant_pool'.
# (default {} -- no .gdb_index parts are skipped)
+ # header_override <dict>
+ # - a dict specifying overrides for header fields. Each
+ # key is a header field name (cu_list, tu_list,
+ # address_table, symbol_table, shortcut, constant_pool)
+ # and the value is either:
+ # - a number (hex or decimal) to replace the field
+ # - the string 'skip' to omit the field from the header
+ # Any field not mentioned uses its computed value.
+ # Examples:
+ # {address_table 0xffffffff}
+ # {address_table 0x100 symbol_table 0x200}
+ # {shortcut skip constant_pool skip}
+ # (default {} -- no overrides)
#
# BODY is Tcl code that emits the parts which make up the body of
# the .gdb_index section. It is evaluated in the caller's
@@ -3936,6 +3949,7 @@ namespace eval Dwarf {
{ version 9 }
{ no_null false }
{ skip_index_part {} }
+ { header_override {} }
}
# Validate the names passed to skip_index_part to catch typos in
@@ -3946,6 +3960,16 @@ namespace eval Dwarf {
}
}
+ # Validate the names passed to header_override to catch typos in
+ # tests.
+ foreach header [dict keys $header_override] {
+ if { [lsearch -exact { cu_list tu_list address_table symbol_table \
+ shortcut constant_pool } \
+ $header] == -1 } {
+ error "gdb_index: unknown header_override '$header'"
+ }
+ }
+
# A list of two entry lists describing CUs. Each sub-list has the
# form {label length}. The <label> is a CU label, created when the
# CU itself was created. The <length> is optional, and will usually
@@ -4182,22 +4206,53 @@ namespace eval Dwarf {
gdb_index_start:
+ # Helper proc to emit a header field with optional override.
+ proc _emit_header_field { field_name label computed_value } {
+ upvar header_override header_override
+
+ if { [dict exists $header_override $field_name] } {
+ set override [dict get $header_override $field_name]
+ if { $override eq "skip" } {
+ # Don't emit this field
+ return
+ } else {
+ # Emit the override value
+ _op .4byte $override "$label (override)"
+ }
+ } else {
+ # Emit the computed value
+ _op .4byte $computed_value $label
+ }
+ }
+
# Header.
_op .4byte $version "Version"
- _op .4byte "$gdb_index_cu_list - $gdb_index_start" \
- "CU list offset"
- _op .4byte "$gdb_index_tu_list - $gdb_index_start" \
- "TU list offset"
- _op .4byte "$gdb_index_addr_table - $gdb_index_start" \
- "Address table offset"
- _op .4byte "$gdb_index_symtab - $gdb_index_start" \
- "Symbol table offset"
+
+ # CU list offset.
+ _emit_header_field cu_list "CU list offset" \
+ "$gdb_index_cu_list - $gdb_index_start"
+
+ # TU list offset.
+ _emit_header_field tu_list "TU list offset" \
+ "$gdb_index_tu_list - $gdb_index_start"
+
+ # Address table offset.
+ _emit_header_field address_table "Address table offset" \
+ "$gdb_index_addr_table - $gdb_index_start"
+
+ # Symbol table offset.
+ _emit_header_field symbol_table "Symbol table offset" \
+ "$gdb_index_symtab - $gdb_index_start"
+
+ # Shortcut table offset (version 9+).
if { $version >= 9 } {
- _op .4byte "$gdb_index_shortcut - $gdb_index_start" \
- "Shortcut table offset"
+ _emit_header_field shortcut "Shortcut table offset" \
+ "$gdb_index_shortcut - $gdb_index_start"
}
- _op .4byte "$gdb_index_constant_pool - $gdb_index_start" \
- "Constant pool offset"
+
+ # Constant pool offset.
+ _emit_header_field constant_pool "Constant pool offset" \
+ "$gdb_index_constant_pool - $gdb_index_start"
# CU list.
gdb_index_cu_list:
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets
2026-10-18 20:50 ` [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets Andrew Burgess
@ 2026-10-20 19:23 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2026-10-20 19:23 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 10/18/26 4:50 PM, Andrew Burgess wrote:
> The .gdb_index section has a header. The header is a list of offsets
> into the .gdb_index section. Currently GDB has basically no error
> checking of these offsets, the offsets can point outside the
> .gdb_index section and GDB will usually try to do something bad as a
> result.
>
> Most often a bad offset will result in GDB trying to allocate a huge
> amount of memory. Indeed, I don't recommend running these new tests
> on an unpatched GDB; I managed to bring down my machine due to
> excessive memory allocation.
>
> I've added a mechanism to the gdb_index proc (in lib/dwarf.exp) to
> allow the various .gdb_index header offsets to be overridden. Then I
> made use of this in gdb.dwarf2/gdb-index-corrupted.exp to override
> each offset in turn.
>
> I also added a new test gdb.dwarf2/gdb-index-tiny-header.exp which
> tests with a .gdb_index sized between 0 and 28 bytes (inclusive).
> This covers indexes that are too small to even hold a valid version
> number field, and indexes upto the size of the full header block. All
> of these indexes are too small to hold useful data so GDB should
> reject them all (without crashing).
>
> The changes in gdb/dwarf2/read-gdb-index.c are fairly straightforward;
> check the .gdb_index is big enough for the version field, then use the
> version field to check that we have all of the offsets, and that the
> offsets point within the .gdb_index section.
>
> Also, as the data areas that the .gdb_index header offsets point to
> are arranged in offset order, I also check that offsets are always
> equal or increasing in value.
Claude suggested this as follow-up:
- Pre-existing, non-blocking (follow-up): the CU list size isn't
checked to be a multiple of 16, nor the TU list a multiple of 24.
create_cus_from_gdb_index_list (i += 2 over size/8 elements) and the
TU loop (read-gdb-index.c:721, i += 3) then read up to 16 bytes
past the end of the block. For the dwz index nothing has to follow
the CU list, so the read can go past the end of the section. This
patch is the natural place to add that check.
I think that makes sense, it could be a follow-up. We could decide to
reject the index entirely, or just ignore the incomplete entries at the
end.
> @@ -513,6 +529,49 @@ read_gdb_index_from_buffer (const char *filename,
>
> map->version = version;
>
> + /* The .gdb_index header is a version number followed by a series of
> + offsets into the .gdb_index section. Each header offset points to a
> + data block, and the offset order, and the data block order are the
> + same. This means that each offset must be greater than, or equal to
> + the previous. The first offset should point to the first byte after
> + the .gdb_index header. None of the header offsets should point outside
> + of the .gdb_index section.
> +
> + If any of these requirements are not met then GDB can end either
> + crashing by accessing invalid memory, or trying to allocate a huge
Missing "up", "can end up either crashing".
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/6] gdb: extend .gdb_index tests to skip all data blocks
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
` (4 preceding siblings ...)
2026-10-18 20:50 ` [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets Andrew Burgess
@ 2026-10-18 20:50 ` Andrew Burgess
2026-10-20 19:32 ` Simon Marchi
5 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2026-10-18 20:50 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
We already had a skip-constant-pool test in
gdb.dwarf2/gdb-index-corrupted.exp. I added this early on as I could
see that skipping the constant-pool was going to trigger some bugs, I
added that test and fixed the bugs.
This commit extends the skip_index_part mechanism, that previously
only allowed skipping of the constant pool, to allow skipping of all
the index parts.
There was only a single bug exposed by this set of tests. When the CU
list is missing GDB would still try to use the index, however, thanks
to earlier patches in this series, every symbol in the symbol table
was ignored as its CU index was outside the (non-existent) CU list.
We already have some code in read_gdb_index_from_buffer
(dwarf2/read-gdb-index.c) which clears the symbol table if the
constant pool is empty. This results in GDB ignoring the index. I
have extended this logic to also check the CU list. Now, if either
the CU list or the constant pool are empty, and the symbol table is
not empty, then we clear the symbol table, which results in GDB
ignoring the index.
For every other part of the index, either that part can legitimately
be empty, or GDB already handled that part being empty. But having
the extra testing is a good thing I think.
---
gdb/dwarf2/read-gdb-index.c | 8 +-
.../gdb.dwarf2/gdb-index-corrupted.exp | 79 ++++++++++++++++-
gdb/testsuite/lib/dwarf.exp | 85 ++++++++++++-------
3 files changed, 138 insertions(+), 34 deletions(-)
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 5959b0ba6bc..d624ba1779c 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -606,10 +606,12 @@ read_gdb_index_from_buffer (const char *filename,
map->constant_pool = buffer.slice (metadata[i]);
- if (map->constant_pool.empty () && !map->symbol_table.empty ())
+ if ((map->constant_pool.empty () || *cu_list_elements == 0)
+ && !map->symbol_table.empty ())
{
- /* An empty constant pool implies that all symbol table entries are
- empty. Make map->symbol_table.empty () == true. */
+ /* An empty constant pool or an empty CU list implies that all symbol
+ table entries are empty. Recreate the symbol table as empty so
+ this index will be ignored. */
map->symbol_table
= offset_view (gdb::array_view<const gdb_byte> (symbol_table,
symbol_table));
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index 865e5e50f97..56cdf895033 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -141,6 +141,55 @@ proc run_test { testname } {
cu cu_label
name _start {{cu_label function global}}
}
+ } elseif { $testname eq "skip-cu-list" } {
+ # Create a .gdb_index with an empty CU list, but with a symbol
+ # that refers to a CU. The CU reference is now out of range, so
+ # GDB should ignore this .gdb_index.
+ gdb_index {
+ skip_index_part { cu_list }
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "skip-tu-list" } {
+ # Skip the TU list. The TU list is always empty anyway, so this
+ # produces a perfectly valid .gdb_index; it is a sanity check
+ # that skipping an already-empty part changes nothing.
+ gdb_index {
+ skip_index_part { tu_list }
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "skip-address-table" } {
+ # Skip the address table. An empty address table is valid, so
+ # GDB should still use this .gdb_index.
+ gdb_index {
+ skip_index_part { address_table }
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "skip-symbol-table" } {
+ # Skip the symbol table. GDB treats an index with an empty
+ # symbol table as unusable and should ignore this .gdb_index.
+ gdb_index {
+ skip_index_part { symbol_table }
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ }
+ } elseif { $testname eq "skip-shortcut" } {
+ # Skip the shortcut table, but record a main function. With no
+ # shortcut table GDB cannot find the main function, but the
+ # index is otherwise valid and should still be used.
+ gdb_index {
+ skip_index_part { shortcut }
+ } {
+ cu cu_label
+ name _start {{cu_label function global}}
+ main DW_LANG_C99 _start
+ }
} elseif { $testname eq "long-cu-vector" } {
# Create a .gdb_index containing a CU vector in the
# constant pool where the vector length is invalid.
@@ -386,7 +435,14 @@ proc run_test { testname } {
|| $testname eq "cu-vector-offset-1" \
|| $testname eq "cu-vector-offset-2" \
|| $testname eq "cu-vector-offset-3" \
- || $testname eq "bad-cu-length" } {
+ || $testname eq "bad-cu-length" \
+ || $testname eq "skip-tu-list" \
+ || $testname eq "skip-address-table" \
+ || $testname eq "skip-shortcut" } {
+ # These produce an index that GDB still uses and in which the symbol
+ # _start is visible, but with no recognised main function. For the
+ # skip-shortcut test a main function was recorded, but GDB cannot
+ # find it with the shortcut table missing.
set expect_gdb_index_ignored false
set expect_start true
set expect_method false
@@ -461,7 +517,21 @@ proc run_test { testname } {
set expect_addr_map {}
set expect_warning_re ""
} elseif { $testname eq "no-null"
- || $testname eq "skip-constant-pool" } {
+ || $testname eq "skip-constant-pool"
+ || $testname eq "skip-cu-list"
+ || $testname eq "skip-symbol-table" } {
+ # For the three skip-* cases, these parts of the .gdb_index are
+ # critical. If the symbol table is missing (or empty) then the
+ # index contains no useful symbols, so GDB ignores the index. If
+ # either of the constant pool or the CU list are missing (or empty)
+ # then any symbols in the symbol list cannot be instantiated, so the
+ # symbol table is effectively empty, again GDB ignores the index in
+ # these cases.
+ #
+ # The no-null case means that the constant pool is malformed
+ # (missing its trailing null byte). GDB ignores the constant pool
+ # in this case, effectively making it empty. This renders the
+ # symbol table useless, so GDB ignores the index.
set expect_gdb_index_ignored true
if { $testname eq "no-null" } {
set expect_warning_re "\r\nwarning: skipping .gdb_index in \[^\r\n\]+, null byte at end of constant pool is missing\r\n"
@@ -579,6 +649,11 @@ set tests { empty-component \
main-language \
no-null \
skip-constant-pool \
+ skip-cu-list \
+ skip-tu-list \
+ skip-address-table \
+ skip-symbol-table \
+ skip-shortcut \
long-cu-vector \
name-offset-1 \
name-offset-2 \
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b2b23167671..d0333c34fc9 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -3820,7 +3820,8 @@ namespace eval Dwarf {
# has zero length. This option is only useful for
# generating invalid .gdb_index sections for testing GDB's
# error handling. The supported part names are:
- # 'constant_pool'.
+ # 'cu_list', 'tu_list', 'address_table', 'symbol_table',
+ # 'shortcut' (version 9 only) and 'constant_pool'.
# (default {} -- no .gdb_index parts are skipped)
# header_override <dict>
# - a dict specifying overrides for header fields. Each
@@ -3955,7 +3956,8 @@ namespace eval Dwarf {
# Validate the names passed to skip_index_part to catch typos in
# tests.
foreach part $skip_index_part {
- if { [lsearch -exact { constant_pool } $part] == -1 } {
+ if { [lsearch -exact { cu_list tu_list address_table symbol_table \
+ shortcut constant_pool } $part] == -1 } {
error "gdb_index: unknown skip_index_part '$part'"
}
}
@@ -4256,31 +4258,44 @@ namespace eval Dwarf {
# CU list.
gdb_index_cu_list:
- foreach cu_rec $_gdb_index_cus {
- lassign $cu_rec cu length
- upvar $cu tmp
- _op_offset 8 $tmp "CU offset"
- # Record the CU's length into the index. In a well formed
- # index the length recorded here must match the actual
- # length of the CU in .debug_info. We allow the length to
- # be overridden, for testing purposes, by passing a bogus
- # length to the 'cu' command. If that was done then use
- # that length instead.
- if { $length ne "" } {
- set cu_length $length
- } elseif { [info exists _cu_lengths($tmp)] } {
- set cu_length $_cu_lengths($tmp)
- } else {
- error "gdb_index: unknown length for CU '$cu'"
+ if { [lsearch -exact $skip_index_part cu_list] != -1 } {
+ # We've been asked to not generate the CU list. This results in
+ # an invalid .gdb_index. We only support this as an option for
+ # error testing.
+ } else {
+ foreach cu_rec $_gdb_index_cus {
+ lassign $cu_rec cu length
+ upvar $cu tmp
+ _op_offset 8 $tmp "CU offset"
+ # Record the CU's length into the index. In a well formed
+ # index the length recorded here must match the actual
+ # length of the CU in .debug_info. We allow the length to
+ # be overridden, for testing purposes, by passing a bogus
+ # length to the 'cu' command. If that was done then use
+ # that length instead.
+ if { $length ne "" } {
+ set cu_length $length
+ } elseif { [info exists _cu_lengths($tmp)] } {
+ set cu_length $_cu_lengths($tmp)
+ } else {
+ error "gdb_index: unknown length for CU '$cu'"
+ }
+ _op_offset 8 "$cu_length" "CU length"
}
- _op_offset 8 "$cu_length" "CU length"
}
- # TU list (empty for now).
+ # TU list. This is always empty for now. Skipping it (via
+ # SKIP_INDEX_PART) is therefore a no-op so we don't even check.
gdb_index_tu_list:
# Address table.
gdb_index_addr_table:
+ if { [lsearch -exact $skip_index_part address_table] != -1 } {
+ # We've been asked to not generate the address table. An empty
+ # address table is actually valid, this exists for completeness
+ # and for error testing.
+ set _gdb_index_addrs {}
+ }
foreach addr_entry $_gdb_index_addrs {
lassign $addr_entry entry_type low high
@@ -4309,6 +4324,12 @@ namespace eval Dwarf {
# Symbol table.
gdb_index_symtab:
+ if { [lsearch -exact $skip_index_part symbol_table] != -1 } {
+ # We've been asked to not generate the symbol table. GDB treats
+ # an index with an empty symbol table as unusable. This exists
+ # for error testing.
+ set table_size 0
+ }
for {set i 0} {$i < $table_size} {incr i} {
if {$slot_occupied($i)} {
set idx $slot_name_idx($i)
@@ -4358,16 +4379,22 @@ namespace eval Dwarf {
# Shortcut table (version 9 only).
if { $version >= 9 } {
gdb_index_shortcut:
- _op .4byte $_gdb_index_main_lang "Language"
- if { $_gdb_index_main_lang == 0 } {
- _op .4byte 0 "Name offset"
- } elseif { $_gdb_index_main_name_idx eq "end" } {
- _op .4byte \
- "$gdb_index_constant_pool_end - $gdb_index_constant_pool" \
- "Name offset"
+ if { [lsearch -exact $skip_index_part shortcut] != -1 } {
+ # We've been asked to not generate the shortcut table. GDB
+ # copes with a missing shortcut table by not recording a
+ # main function. This exists for error testing.
} else {
- _op .4byte "$_gdb_index_main_name_idx" \
- "Name offset"
+ _op .4byte $_gdb_index_main_lang "Language"
+ if { $_gdb_index_main_lang == 0 } {
+ _op .4byte 0 "Name offset"
+ } elseif { $_gdb_index_main_name_idx eq "end" } {
+ _op .4byte \
+ "$gdb_index_constant_pool_end - $gdb_index_constant_pool" \
+ "Name offset"
+ } else {
+ _op .4byte "$_gdb_index_main_name_idx" \
+ "Name offset"
+ }
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] gdb: extend .gdb_index tests to skip all data blocks
2026-10-18 20:50 ` [PATCH 6/6] gdb: extend .gdb_index tests to skip all data blocks Andrew Burgess
@ 2026-10-20 19:32 ` Simon Marchi
0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2026-10-20 19:32 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
On 10/18/26 4:50 PM, Andrew Burgess wrote:
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index 5959b0ba6bc..d624ba1779c 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -606,10 +606,12 @@ read_gdb_index_from_buffer (const char *filename,
>
> map->constant_pool = buffer.slice (metadata[i]);
>
> - if (map->constant_pool.empty () && !map->symbol_table.empty ())
> + if ((map->constant_pool.empty () || *cu_list_elements == 0)
> + && !map->symbol_table.empty ())
Is an index with no CUs necessarily invalid? What if there are some
TUs, or some CUs in the dwz file?
It seems to me like we could let those go through, and if the index is
really invalid in some way, then one of the checks you added in the
previous patches will trigger and reject the index.
> {
> - /* An empty constant pool implies that all symbol table entries are
> - empty. Make map->symbol_table.empty () == true. */
> + /* An empty constant pool or an empty CU list implies that all symbol
> + table entries are empty. Recreate the symbol table as empty so
> + this index will be ignored. */
> map->symbol_table
> = offset_view (gdb::array_view<const gdb_byte> (symbol_table,
> symbol_table));
I wonder why this rejects the index by clearing symbol_table, instead of
just returning false.
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread