[PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index

[Andrew Burgess <aburgess@redhat.com>]

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