From patchwork Fri Dec 6 23:46:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 36581 Received: (qmail 109132 invoked by alias); 6 Dec 2019 23:46:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 109052 invoked by uid 89); 6 Dec 2019 23:46:53 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=alive, Breakpoint, objfilesc X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2019 23:46:50 +0000 Received: by mail-wm1-f66.google.com with SMTP id p9so9508638wmc.2 for ; Fri, 06 Dec 2019 15:46:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=5C9YVHGPcLnmXquvwBYzLLXYKN7r5foMptkx0+X3vtw=; b=EtY+ru8oBfa1kzc+hYQyl5VXHNGMAa4G4kd0wBk1hjdSV0xLA1BAecLtdKGIDAKev/ rkZPmIH1yEeTMKQdFWtZ8/6iOXSdlmd+5euLXMYqyN2KIMqg6aI+QLn+EzVKvmjWg1sB oh9wbKBlphI1ZA0ImPM4PGGsMXIKKtLn+mLT/WBwr+50XnuLAZ4GM99zFVYOqL0pjXDR gtcPTYQm/ZIQBrDXR7Ursz6f/oqpRNhLuEw/wlzTf0zk4OwKuH2KIGp1zo2Ts5YJIpxU Vp7K0hdcYXzRd7YMgw67DrG7KxeFvQ9OYjlLfWaVNMuLvyLNlRmBOCEdYdkoNnx77tzD CLoQ== Return-Path: Received: from localhost (host86-186-80-236.range86-186.btcentralplus.com. [86.186.80.236]) by smtp.gmail.com with ESMTPSA id w22sm4848089wmk.34.2019.12.06.15.46.47 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 06 Dec 2019 15:46:47 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb: Handle malformed ELF, symbols in non-allocatable sections Date: Fri, 6 Dec 2019 23:46:41 +0000 Message-Id: In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes 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 . */ + + .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 . */ + +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 . + +# 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 +}