[PATCH 1/2] gdb/testsuite: Allow DWARF assembler to create multiple line tables

[PATCH 1/2] gdb/testsuite: Allow DWARF assembler to create multiple line tables

[Andrew Burgess]

Fixes a bug in the DWARF assembler that prevents multiple line tables
from being created in a test.  We currently don't initialise a couple
of flags, as a result we will only ever generate one end of file list,
and one end of header, in the first line table.  Any additional line
tables will be missing these parts, and will therefore be corrupt.

This fix will be required for a later commit.  There should be no
change in the testsuite after this commit.

gdb/testsuite/ChangeLog:

	* lib/dwarf.exp (Dwarf::lines): Reset _line_saw_program and
	_line_saw_file.

Change-Id: Id7123f217a036f26ee32d608db3064dd43164596
---
 gdb/testsuite/ChangeLog     | 5 +++++
 gdb/testsuite/lib/dwarf.exp | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 75d19abf802..c7f7cee722d 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1309,6 +1309,8 @@ namespace eval Dwarf {
 	set is_64 0
 	set _unit_version 4
 	set _unit_addr_size default
+	set _line_saw_program 0
+	set _line_saw_file 0
 
 	foreach { name value } $options {
 	    switch -exact -- $name {
-- 
2.14.5

[PATCH 2/2] gdb: Handle malformed ELF, symbols in non-allocatable sections

[Andrew Burgess]

I ended up debugging a malformed ELF where a section containing
executable code was not correctly marked as allocatable.  Before
realising the ELF was corrupted I tried to place a breakpoint on a
symbol in the non-allocatable, executable section, and GDB crashed.

Though trying to debug such an ELF clearly isn't going to go well I
would prefer, as far as possible, that any input, no matter how
corrupted, not crash GDB.

The crash occurs when trying to set a breakpoint on the name of a
function from the corrupted section.  GDB converts the symbol to a
symtab_and_line, and looks up a suitable section for this.

The problem is that the section is actually an obj_section, which is
stored in the table within the objfile, and we only initialise this
table for allocatable sections (see add_to_objfile_sections_full in
objfiles.c).  So, if the symbol is in a non-allocatable section then
we end up referencing an uninitialised obj_section.

Later we call get_sal_arch on the symtab_and_line, which calls
get_objfile_arch, which uses the objfile from the uninitialised
obj_section, which will be nullptr, at which point GDB crashes.

The fix I propose here is that when we setup the section references on
msymbols, we should check if the bfd_section being referenced is
allocatable or not.  If it is not then we should set the section
reference back to the default 0 section (see how MSYMBOL_OBJ_SECTION
and SYMBOL_OBJ_SECTION treat the 0 section index).

With this fix in place GDB no longer crashes.  Instead GDB creates the
breakpoint at the non-allocated address, and then fails, with an
error, when it tries to insert the breakpoint.

gdb/ChangeLog:

	* elfread.c (record_minimal_symbol): Set section index to 0 for
	non-allocatable sections.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/dw2-bad-elf-other.S: New file.
	* gdb.dwarf2/dw2-bad-elf.c: New file.
	* gdb.dwarf2/dw2-bad-elf.exp: New file.

Change-Id: Ie05436ab4c6a71440304d20ee639dfb021223f8b
---
 gdb/ChangeLog                                |   5 +
 gdb/elfread.c                                |  13 +-
 gdb/testsuite/ChangeLog                      |   6 +
 gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S |  29 +++++
 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c       |  21 +++
 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp     | 183 +++++++++++++++++++++++++++
 6 files changed, 253 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 44b793d8f14..fef2acb2ce5 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -210,11 +210,16 @@ record_minimal_symbol (minimal_symbol_reader &reader,
       || ms_type == mst_text_gnu_ifunc)
     address = gdbarch_addr_bits_remove (gdbarch, address);
 
+  /* We only setup section information for allocatable sections.  Usually
+     we'd only expect to find msymbols for allocatable sections, but if the
+     ELF is malformed then this might not be the case.  In that case don't
+     create an msymbol that references an uninitialised section object.  */
+  int section_index = 0;
+  if ((bfd_section_flags (bfd_section) & SEC_ALLOC) == SEC_ALLOC)
+    section_index = gdb_bfd_section_index (objfile->obfd, bfd_section);
+
   struct minimal_symbol *result
-    = reader.record_full (name, copy_name, address,
-			  ms_type,
-			  gdb_bfd_section_index (objfile->obfd,
-						 bfd_section));
+    = reader.record_full (name, copy_name, address, ms_type, section_index);
   if ((objfile->flags & OBJF_MAINLINE) == 0
       && (ms_type == mst_data || ms_type == mst_bss))
     result->maybe_copied = 1;
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S
new file mode 100644
index 00000000000..c0f0e603274
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S
@@ -0,0 +1,29 @@
+/* Copyright 2019 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/>.  */
+
+	.section ".other", "x"
+	.global some_func, some_func_end
+	.type   some_func, @function
+	nop
+	nop
+	nop
+	nop
+some_func:
+	.rept 64
+	.byte 0
+	.endr
+	.size some_func,.-some_func
+some_func_end:
+	nop
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c
new file mode 100644
index 00000000000..a8b27239a70
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c
@@ -0,0 +1,21 @@
+/* Copyright 2019 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/>.  */
+
+int
+main ()
+{
+  asm ("main_label: .globl main_label");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp
new file mode 100644
index 00000000000..b2f55e05694
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp
@@ -0,0 +1,183 @@
+# Copyright 2019 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/>.
+
+# Checks for a bug where a baddly formed ELF would cause GDB to crash.
+# A section containing executable code, for which there was DWARF is
+# accidentally marked as non-alloctable, GDB becomes unhappy.
+#
+# This test creates some fake DWARF pointing at some symbols in a
+# non-allocatable section that is still marked as executable.  We then
+# start GDB and try to place a breakpoint on the symbol in the
+# non-allocatable section.
+#
+# It is not expected that the final debug experience really makes
+# sense, the symbol is in a non-allocatable section after all, but GDB
+# absolutely shouldn't crash.  All we try to do after placing the
+# breakpoint is check that GDB is still alive.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile dw2-bad-elf.c dw2-bad-elf-other.S dw2-bad-elf-dwarf.S
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile3]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+
+    declare_labels ranges_label_1 ranges_label_2 L1 L2
+
+    set main_result [function_range main ${srcdir}/${subdir}/${srcfile}]
+    set main_start [lindex $main_result 0]
+    set main_length [lindex $main_result 1]
+
+    set int_size [get_sizeof "int" 4]
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C}
+	    {DW_AT_name     dw2-bad-elf.c}
+	    {DW_AT_comp_dir ${srcdir}/${subdir}}
+	    {stmt_list $L1 DW_FORM_sec_offset}
+	    {ranges ${ranges_label_1} DW_FORM_sec_offset}
+	    {DW_AT_low_pc   0 addr}
+	} {
+	    declare_labels integer_label
+
+            DW_TAG_subprogram {
+                {name main}
+                {low_pc $main_start addr}
+                {high_pc $main_length data8}
+		{DW_AT_type :$integer_label}
+		{DW_AT_decl_file 1 data1}
+		{DW_AT_decl_line 10 data1}
+	    }
+
+            integer_label: DW_TAG_base_type {
+                {DW_AT_byte_size $int_size DW_FORM_sdata}
+                {DW_AT_encoding  @DW_ATE_signed}
+                {DW_AT_name      integer}
+	    }
+	}
+    }
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C}
+	    {DW_AT_name     dw2-bad-elf-other.c}
+	    {DW_AT_comp_dir ${srcdir}/${subdir}}
+	    {stmt_list $L2 DW_FORM_sec_offset}
+	    {ranges ${ranges_label_2} DW_FORM_sec_offset}
+	    {DW_AT_low_pc   0 addr}
+	} {
+	    declare_labels integer_label
+
+            DW_TAG_subprogram {
+                {name some_func}
+                {low_pc some_func addr}
+                {high_pc some_func_end addr}
+		{DW_AT_type :$integer_label}
+		{DW_AT_decl_file 2 data1}
+		{DW_AT_decl_line 5 data1}
+	    }
+
+            integer_label: DW_TAG_base_type {
+                {DW_AT_byte_size $int_size DW_FORM_sdata}
+                {DW_AT_encoding  @DW_ATE_signed}
+                {DW_AT_name      integer}
+	    }
+	}
+    }
+
+    ranges {is_64 [is_64_target]} {
+	ranges_label_1: sequence {
+	    {base [lindex $main_result 0]}
+	    {range 0 [lindex $main_result 1]}
+	}
+	ranges_label_2: sequence {
+	    {base some_func}
+	    {range 0 64}
+	}
+    }
+
+    lines {version 2} L1 {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	# Line data doens't need to be correct, just present.
+	program {
+	    {DW_LNE_set_address [lindex $main_result 0]}
+	    {DW_LNS_advance_line 10}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc [lindex $main_result 1]}
+	    {DW_LNS_advance_line 19}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+	}
+    }
+
+    lines {version 2} L2 {
+	include_dir "${srcdir}/${subdir}"
+	file_name "dw2-bad-elf-other.c" 1
+
+	# Line data doens't need to be correct, just present.
+	program {
+	    {DW_LNE_set_address some_func}
+	    {DW_LNS_advance_line 5}
+	    {DW_LNS_copy}
+	    {DW_LNS_advance_pc 64}
+	    {DW_LNS_advance_line 8}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+	}
+    }
+}
+
+if { [build_executable ${testfile}.exp ${testfile} \
+	  [list $srcfile $srcfile2 $asm_file] {nodebug}] } {
+    return -1
+}
+
+# Attempt to place a breakpoint on 'some_func', then check GDB is
+# still alive.  This test can optionally set a breakpoint on 'main'
+# first (based on GOTO_MAIN), the original bug behaved differently
+# when there was already a breakpoint set.
+proc run_test { goto_main } {
+    global binfile decimal hex
+
+    clean_restart ${binfile}
+
+    if { $goto_main } {
+	if ![runto_main] {
+	    return -1
+	}
+    }
+
+    # Place a breakpoint.
+    gdb_test "break some_func" \
+	"Breakpoint $decimal at $hex: file .*dw2-bad-elf-other\\.c, line 6\\."
+
+    # Check GDB is still alive.
+    gdb_test "echo hello\\n" "hello"
+}
+
+# Run the tests.
+foreach_with_prefix goto_main { 0 1 } {
+    run_test $goto_main
+}
-- 
2.14.5

[PATCH 0/2] Fix bug when debugging a corrupted ELF

[Andrew Burgess]

Patch #1 is a minor bug fix in the DWARF assembler library which lets
me write a test required for patch #2 which is the actual issue I'm
trying to fix.

---

Andrew Burgess (2):
  gdb/testsuite: Allow DWARF assembler to create multiple line tables
  gdb: Handle malformed ELF, symbols in non-allocatable sections

 gdb/ChangeLog                                |   5 +
 gdb/elfread.c                                |  13 +-
 gdb/testsuite/ChangeLog                      |  11 ++
 gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S |  29 +++++
 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c       |  21 +++
 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp     | 183 +++++++++++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp                  |   2 +
 7 files changed, 260 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-bad-elf-other.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-bad-elf.exp

-- 
2.14.5

Re: [PATCH 1/2] gdb/testsuite: Allow DWARF assembler to create multiple line tables

[Kevin Buettner]

On Fri,  6 Dec 2019 23:46:40 +0000
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> Fixes a bug in the DWARF assembler that prevents multiple line tables
> from being created in a test.  We currently don't initialise a couple
> of flags, as a result we will only ever generate one end of file list,
> and one end of header, in the first line table.  Any additional line
> tables will be missing these parts, and will therefore be corrupt.
> 
> This fix will be required for a later commit.  There should be no
> change in the testsuite after this commit.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* lib/dwarf.exp (Dwarf::lines): Reset _line_saw_program and
> 	_line_saw_file.

LGTM.

Kevin

Re: [PATCH 2/2] gdb: Handle malformed ELF, symbols in non-allocatable sections

[Kevin Buettner]

On Fri,  6 Dec 2019 23:46:41 +0000
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> I ended up debugging a malformed ELF where a section containing
> executable code was not correctly marked as allocatable.  Before
> realising the ELF was corrupted I tried to place a breakpoint on a
> symbol in the non-allocatable, executable section, and GDB crashed.
> 
> Though trying to debug such an ELF clearly isn't going to go well I
> would prefer, as far as possible, that any input, no matter how
> corrupted, not crash GDB.
> 
> The crash occurs when trying to set a breakpoint on the name of a
> function from the corrupted section.  GDB converts the symbol to a
> symtab_and_line, and looks up a suitable section for this.
> 
> The problem is that the section is actually an obj_section, which is
> stored in the table within the objfile, and we only initialise this
> table for allocatable sections (see add_to_objfile_sections_full in
> objfiles.c).  So, if the symbol is in a non-allocatable section then
> we end up referencing an uninitialised obj_section.
> 
> Later we call get_sal_arch on the symtab_and_line, which calls
> get_objfile_arch, which uses the objfile from the uninitialised
> obj_section, which will be nullptr, at which point GDB crashes.
> 
> The fix I propose here is that when we setup the section references on
> msymbols, we should check if the bfd_section being referenced is
> allocatable or not.  If it is not then we should set the section
> reference back to the default 0 section (see how MSYMBOL_OBJ_SECTION
> and SYMBOL_OBJ_SECTION treat the 0 section index).
> 
> With this fix in place GDB no longer crashes.  Instead GDB creates the
> breakpoint at the non-allocated address, and then fails, with an
> error, when it tries to insert the breakpoint.
> 
> gdb/ChangeLog:
> 
> 	* elfread.c (record_minimal_symbol): Set section index to 0 for
> 	non-allocatable sections.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.dwarf2/dw2-bad-elf-other.S: New file.
> 	* gdb.dwarf2/dw2-bad-elf.c: New file.
> 	* gdb.dwarf2/dw2-bad-elf.exp: New file.

LGTM.

Kevin