From patchwork Sun Feb 2 01:10:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 37646 Received: (qmail 113231 invoked by alias); 2 Feb 2020 01:10:32 -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 113219 invoked by uid 89); 2 Feb 2020 01:10:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=UD:breakpoint.c, breakpoint.c, breakpointc, HX-Received:adf X-HELO: mail-wr1-f50.google.com Received: from mail-wr1-f50.google.com (HELO mail-wr1-f50.google.com) (209.85.221.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 02 Feb 2020 01:10:28 +0000 Received: by mail-wr1-f50.google.com with SMTP id z9so1061064wrs.10 for ; Sat, 01 Feb 2020 17:10:28 -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; bh=S+JsbtRnoAJr5UG4Jy0VmoBx6tqkB9xo+HXLCn00Q8c=; b=DL4gRBzcWSPkFdjFdeNpL/P5Ng2To2GFidfl1Js1f51rNiBMaiCgUVEsT3tWboZpg1 coE8VdnR5JG0laxYeNWRexpGzw0ZLAmKgt3fk/y7T1Dly7ywqEMU3rKG8yLnEPX4u9sq uCoiAlK8AHY6Q31ywfOWh+xtQ5Tl+BEflXsu0H5vqFLngU0Ipzv+wRT7houQBltpLnMv FiyPFq3Jd7POLs5g731vZ0zuHhyk4LkeKMkYgPPd/Lfk8asAADhQdGIo1aDHesRSAmsn HEk3rqt4Yyo/039Rlftf2B6KlAw98hnq5EtGKtxM1VNFy6EtLmEzCVcN8MUKRMy9Evht J9rQ== Return-Path: Received: from localhost (host86-191-239-73.range86-191.btcentralplus.com. [86.191.239.73]) by smtp.gmail.com with ESMTPSA id v17sm18207793wrt.91.2020.02.01.17.10.25 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 01 Feb 2020 17:10:25 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [RFC] gdb: Give a better error when a breakpoint can't be placed in a PIE Date: Sun, 2 Feb 2020 01:10:22 +0000 Message-Id: <20200202011022.11357-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes I'd be interested in feedback on the patch below. Do people think this is a good/useful idea? Would there be a better error message to give to the user? Or a better time/place to present the error message? All feedback welcome. Thanks, Andrew --- One issue that seems to crop up fairly regularly on help sites like stackoverflow, is a use complaining why something like this doesn't work: $ gdb app.exe (gdb) disassemble main Dump of assembler code for function main: 0x00000000000005e5 <+0>: push %rbp 0x00000000000005e6 <+1>: mov %rsp,%rbp 0x00000000000005e9 <+4>: mov $0x0,%eax 0x00000000000005ee <+9>: pop %rbp 0x00000000000005ef <+10>: retq (gdb) break *0x5e5 Breakpoint 1 at 0x5e5: file app.c, line 20. (gdb) run Starting program: app.exe Warning: Cannot insert breakpoint 1. Cannot access memory at address 0x5e5 Everything appears to be going fine until the user tries to run GDB, which can, understandably be a little confusing. The problem is that the executable was compiled as PIE, and the dynamic linker has placed main at some address that doesn't match the addresses (or offsets as they really are now) in the applications ELF. The problem is that if the user isn't really that aware of PIC/PIE, or how it works, then they might not realise they what the problem is. This commit tries to detect when the user has made this mistake, and gives a warning message that (hopefully) is more helpful, the user now sees this: (gdb) run Starting program: app.exe Warning: Cannot insert breakpoint 1. Cannot access memory at address 0x5e5 The address is located within a code region of `app.exe' which was dynamically relocated at run time. The code region `0x0000000000000500 ... 0x0000000000000662' is now located at `0x0000555555554500 ... 0x0000555555554662', you may need to adjust your breakpoints. gdb/ChangeLog: * breakpoint.c (pic_code_error_check): New function. (insert_bp_location): Call new function. gdb/testsuite/ChangeLog: * gdb.base/pie-bp-addr-error.c: New file. * gdb.base/pie-bp-addr-error.exp: New file. Change-Id: Iedea261a54ab0a6ed03ec9b8df06fdbe459f28be --- gdb/ChangeLog | 5 +++ gdb/breakpoint.c | 65 ++++++++++++++++++++++++++++ gdb/testsuite/ChangeLog | 5 +++ gdb/testsuite/gdb.base/pie-bp-addr-error.c | 22 ++++++++++ gdb/testsuite/gdb.base/pie-bp-addr-error.exp | 61 ++++++++++++++++++++++++++ 5 files changed, 158 insertions(+) create mode 100644 gdb/testsuite/gdb.base/pie-bp-addr-error.c create mode 100644 gdb/testsuite/gdb.base/pie-bp-addr-error.exp diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 5a9352c26fe..1d7926dcefb 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2405,6 +2405,69 @@ breakpoint_kind (struct bp_location *bl, CORE_ADDR *addr) return gdbarch_breakpoint_kind_from_pc (bl->gdbarch, addr); } +/* This should be called when we fail to insert a breakpoint, with ADDRESS + set to the location at which we tried to insert the breakpoint. + + This function checks to see if it is possible the user tried to place + the breakpoint within an executable section of an object file that has + been relocated at run time. If this is the case then an extra warning + is printed to TMP_ERROR_STREAM. */ + +static void +pic_code_error_check (struct ui_file *tmp_error_stream, + CORE_ADDR address) +{ + for (objfile *objfile : current_program_space->objfiles ()) + { + struct obj_section *osect; + + /* If the primary text section is not relocated then nothing should + be relocated - maybe? */ + if (objfile->text_section_offset () == 0) + continue; + + /* We know this object file was relocated, but was the breakpoint + inside one of the code sections before relocation? */ + ALL_OBJFILE_OSECTIONS (objfile, osect) + { + /* Ignore non-code sections. */ + if ((bfd_section_flags (osect->the_bfd_section) & SEC_CODE) == 0) + continue; + + /* Was ADDRESS within this section before relocation? */ + if (!(address >= bfd_section_vma (osect->the_bfd_section) + && address < (bfd_section_vma (osect->the_bfd_section) + + bfd_section_size (osect->the_bfd_section)))) + continue; + + /* Final check that this section was relocated. */ + int idx = osect - objfile->sections; + CORE_ADDR offset = objfile->section_offsets[idx]; + if (offset == 0) + continue; + + /* Let the user know that they may have made a mistake. One + final check that it might be nice to do is to only give this + warning if the user placed the breakpoint by address, that is + using 'break *0x.......', if they did anything else and we + still have this issue then I'm not sure what the user could + reasonably do about it. However, for now, we just always + give this error. */ + CORE_ADDR len = bfd_section_size (osect->the_bfd_section); + CORE_ADDR start = bfd_section_vma (osect->the_bfd_section); + fprintf_unfiltered (tmp_error_stream, "\ +The address is located within a code region of `%s' which was dynamically\n\ +relocated at run time. The code region `%s ... %s' is now located\n\ +at `%s ... %s', you may need to adjust your breakpoints.\n", + objfile_name (objfile), + core_addr_to_string (start), + core_addr_to_string (start + len), + core_addr_to_string (offset + start), + core_addr_to_string (offset + start + len)); + } + } +} + /* Insert a low-level "breakpoint" of some type. BL is the breakpoint location. Any error messages are printed to TMP_ERROR_STREAM; and DISABLED_BREAKS, and HW_BREAKPOINT_ERROR are used to report problems. @@ -2670,6 +2733,8 @@ insert_bp_location (struct bp_location *bl, "Cannot insert breakpoint %d.\n" "%s\n", bl->owner->number, message.c_str ()); + + pic_code_error_check (tmp_error_stream, bl->address); } else { diff --git a/gdb/testsuite/gdb.base/pie-bp-addr-error.c b/gdb/testsuite/gdb.base/pie-bp-addr-error.c new file mode 100644 index 00000000000..f4825c8a7c1 --- /dev/null +++ b/gdb/testsuite/gdb.base/pie-bp-addr-error.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 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 () +{ + return 0; +} diff --git a/gdb/testsuite/gdb.base/pie-bp-addr-error.exp b/gdb/testsuite/gdb.base/pie-bp-addr-error.exp new file mode 100644 index 00000000000..a54320c0d7f --- /dev/null +++ b/gdb/testsuite/gdb.base/pie-bp-addr-error.exp @@ -0,0 +1,61 @@ +# Copyright 2020 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 . + +# Check that, if the user places a breakpoint on an address within a +# PIE before relocation, then we get a suitable error. + +global inferior_spawn_id +global gdb_spawn_id + +if ![istarget *-linux*] { + continue +} + +standard_testfile .c + +set opts [list debug additional_flags=-fPIE ldflags=-pie] +if { [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] } { + return -1 +} + +# Figure out the address of main before sectionns are relocated. +set main_addr_before_reloc \ + [get_hexadecimal_valueof "&main" 0 "address of main"] + +# Start the inferior and run to main, this will relocate the sections. +if ![runto_main] { + return -1 +} + +# First confirm that main has moved. +set main_relocated_address \ + [get_hexadecimal_valueof "&main" 0 "runtime address of main"] +gdb_assert { ${main_addr_before_reloc} != ${main_relocated_address} } \ + "address of main has changed" + +# Now place a breakpoint using the before relocation address. +gdb_breakpoint "*${main_addr_before_reloc}" + +# Continue the inferior, inserting all breakpoints - which should +# error due to our use of an invalid address. +gdb_test "continue" \ + [multi_line \ + "Cannot insert breakpoint $decimal." \ + "Cannot access memory at address $hex" \ + "The address is located within a code region of `\[^\r\n\]+' which was dynamically" \ + "relocated at run time. The code region `$hex ... $hex' is now located" \ + "at `$hex ... $hex', you may need to adjust your breakpoints." \ + "" \ + "Command aborted." ]