From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id E7684387090E for ; Fri, 28 Aug 2020 10:31:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E7684387090E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id B25B3B1A0; Fri, 28 Aug 2020 10:31:46 +0000 (UTC) Subject: Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address From: Tom de Vries To: Pedro Alves , gdb-patches@sourceware.org References: <20200827115217.GA17450@delia.home> <23b43acc-9846-a03c-5207-3ae80efc1d6f@palves.net> <29930a75-5de9-43ed-6a24-2909eb70ec66@suse.de> Autocrypt: addr=tdevries@suse.de; keydata= xsBNBF0ltCcBCADDhsUnMMdEXiHFfqJdXeRvgqSEUxLCy/pHek88ALuFnPTICTwkf4g7uSR7 HvOFUoUyu8oP5mNb4VZHy3Xy8KRZGaQuaOHNhZAT1xaVo6kxjswUi3vYgGJhFMiLuIHdApoc u5f7UbV+egYVxmkvVLSqsVD4pUgHeSoAcIlm3blZ1sDKviJCwaHxDQkVmSsGXImaAU+ViJ5l CwkvyiiIifWD2SoOuFexZyZ7RUddLosgsO0npVUYbl6dEMq2a5ijGF6/rBs1m3nAoIgpXk6P TCKlSWVW6OCneTaKM5C387972qREtiArTakRQIpvDJuiR2soGfdeJ6igGA1FZjU+IsM5ABEB AAHNH1RvbSBkZSBWcmllcyA8dGRldnJpZXNAc3VzZS5kZT7CwKsEEwEIAD4WIQSsnSe5hKbL MK1mGmjuhV2rbOJEoAUCXSW0JwIbAwUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAh CRDuhV2rbOJEoBYhBKydJ7mEpsswrWYaaO6FXats4kSgc48H/Ra2lq5p3dHsrlQLqM7N68Fo eRDf3PMevXyMlrCYDGLVncQwMw3O/AkousktXKQ42DPJh65zoXB22yUt8m0g12xkLax98KFJ 5NyUloa6HflLl+wQL/uZjIdNUQaHQLw3HKwRMVi4l0/Jh/TygYG1Dtm8I4o708JS4y8GQxoQ UL0z1OM9hyM3gI2WVTTyprsBHy2EjMOu/2Xpod95pF8f90zBLajy6qXEnxlcsqreMaqmkzKn 3KTZpWRxNAS/IH3FbGQ+3RpWkNGSJpwfEMVCeyK5a1n7yt1podd1ajY5mA1jcaUmGppqx827 8TqyteNe1B/pbiUt2L/WhnTgW1NC1QDOwE0EXSW0JwEIAM99H34Bu4MKM7HDJVt864MXbx7B 1M93wVlpJ7Uq+XDFD0A0hIal028j+h6jA6bhzWto4RUfDl/9mn1StngNVFovvwtfzbamp6+W pKHZm9X5YvlIwCx131kTxCNDcF+/adRW4n8CU3pZWYmNVqhMUiPLxElA6QhXTtVBh1RkjCZQ Kmbd1szvcOfaD8s+tJABJzNZsmO2hVuFwkDrRN8Jgrh92a+yHQPd9+RybW2l7sJv26nkUH5Z 5s84P6894ebgimcprJdAkjJTgprl1nhgvptU5M9Uv85Pferoh2groQEAtRPlCGrZ2/2qVNe9 XJfSYbiyedvApWcJs5DOByTaKkcAEQEAAcLAkwQYAQgAJhYhBKydJ7mEpsswrWYaaO6FXats 4kSgBQJdJbQnAhsMBQkDwmcAACEJEO6FXats4kSgFiEErJ0nuYSmyzCtZhpo7oVdq2ziRKD3 twf7BAQBZ8TqR812zKAD7biOnWIJ0McV72PFBxmLIHp24UVe0ZogtYMxSWKLg3csh0yLVwc7 H3vldzJ9AoK3Qxp0Q6K/rDOeUy3HMqewQGcqrsRRh0NXDIQk5CgSrZslPe47qIbe3O7ik/MC q31FNIAQJPmKXX25B115MMzkSKlv4udfx7KdyxHrTSkwWZArLQiEZj5KG4cCKhIoMygPTA3U yGaIvI/BGOtHZ7bEBVUCFDFfOWJ26IOCoPnSVUvKPEOH9dv+sNy7jyBsP5QxeTqwxC/1ZtNS DUCSFQjqA6bEGwM22dP8OUY6SC94x1G81A9/xbtm9LQxKm0EiDH8KBMLfQ== Message-ID: <79ab7968-bd35-aa3a-dd8b-37076609043d@suse.de> Date: Fri, 28 Aug 2020 12:31:12 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <29930a75-5de9-43ed-6a24-2909eb70ec66@suse.de> Content-Type: multipart/mixed; boundary="------------AE4C89F1F6A917EE44C6FA1F" Content-Language: en-US X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Aug 2020 10:31:17 -0000 This is a multi-part message in MIME format. --------------AE4C89F1F6A917EE44C6FA1F Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 8/27/20 3:49 PM, Tom de Vries wrote: > On 8/27/20 2:41 PM, Pedro Alves wrote: >> On 8/27/20 12:52 PM, Tom de Vries wrote: >>> Hi, >>> >>> Consider test-case test.c: >>> ... >>> $ cat test.c >>> int main (void) { >>> return 0; >>> L1: >>> (void)0; >>> } >>> ... >>> >>> Compiled with debug info: >>> ... >>> $ gcc test.c -g >>> ... >>> >>> When attempting to set a breakpoint at L1, which is a label without address: >>> ... >>> <1>: Abbrev Number: 2 (DW_TAG_subprogram) >>> DW_AT_name : main >>> <2><115>: Abbrev Number: 3 (DW_TAG_label) >>> <116> DW_AT_name : L1 >>> <119> DW_AT_decl_file : 1 >>> <11a> DW_AT_decl_line : 5 >>> <2><11b>: Abbrev Number: 0 >> >> Is this a debug info bug, > > Strictly speaking, this is a debug info bug. The standard says that: > ... > The label entry has a DW_AT_low_pc attribute whose value is the address > of the first executable instruction for the location identified by the > label in the source program. > ... > > But I interpret the missing DW_AT_low_pc attribute as: there is a label > in the source, but the corresponding code has been optimized out. > >> or is the debug info telling us that the >> address of the label is the same as the line number's address? >> >> How about looking up the line number address instead of throwing >> an error? >> > > Well, in this particular case, that wouldn't help. > > With L1 at line 3: > ... > $ cat -n test.c > 1 int main (void) { > 2 return 0; > 3 L1: > 4 (void)0; > 5 } > 6 > ... > there's no corresponding address: > ... > $ readelf -wL a.out > CU: test.c: > File name Line number Starting address > View Stmt > test.c 1 0x400497 > x > test.c 2 0x40049b > x > test.c 5 0x4004a0 > x > test.c - 0x4004a2 > ... > > My suspicion is that this won't be useful in general. > I've pushed this as attached below, with the test-case updated to work around PR26546 - "[pie] Setting breakpoint on missing label sets breakpoint at offset 0 in NULL section" ( https://sourceware.org/bugzilla/show_bug.cgi?id=26546 ). Thanks, - Tom --------------AE4C89F1F6A917EE44C6FA1F Content-Type: text/x-patch; charset=UTF-8; name="0001-gdb-breakpoint-Handle-setting-breakpoint-on-label-without-address.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-gdb-breakpoint-Handle-setting-breakpoint-on-label-witho"; filename*1="ut-address.patch" [gdb/breakpoint] Handle setting breakpoint on label without address Consider test-case test.c: ... $ cat test.c int main (void) { return 0; L1: (void)0; } ... Compiled with debug info: ... $ gcc test.c -g ... When attempting to set a breakpoint at L1, which is a label without address: ... <1>: Abbrev Number: 2 (DW_TAG_subprogram) DW_AT_name : main <2><115>: Abbrev Number: 3 (DW_TAG_label) <116> DW_AT_name : L1 <119> DW_AT_decl_file : 1 <11a> DW_AT_decl_line : 5 <2><11b>: Abbrev Number: 0 ... we run into an internal-error: ... $ gdb -batch a.out -ex "b main:L1" linespec.c:3233: internal-error: void \ decode_line_full(const event_location*, int, program_space*, symtab*, \ int, linespec_result*, const char*, const char*): \ Assertion `result.size () == 1 || canonical->pre_expanded' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. ... Fix this by detecting the error condition in decode_line_full instead, and throwing an error, such that we have instead: ... (gdb) b main:L1 Location main:L1 not available (gdb) ... Unfortunately, to call event_location_to_string, which is used to get the location name in the error message, we need to pass a non-const struct event_location, because the call may cache the string in the struct (See EL_STRING). So, we change the prototype of decode_line_full accordingly, and everywhere this propages to. Tested on x86_64-linux. gdb/ChangeLog: 2020-08-27 Tom de Vries PR breakpoint/26544 * breakpoint.c (parse_breakpoint_sals): Remove const from struct event_location. (create_breakpoint): Same. (base_breakpoint_decode_location): Same. (bkpt_create_sals_from_location): Same. (bkpt_decode_location): Same. (bkpt_probe_create_sals_from_location): Same. (bkpt_probe_decode_location): Same. (tracepoint_create_sals_from_location): Same. (tracepoint_decode_location): Same. (tracepoint_probe_decode_location): Same. (strace_marker_create_sals_from_location): Same. (strace_marker_decode_location): Same. (create_sals_from_location_default): Same. (decode_location_default): Same. * breakpoint.h (struct breakpoint_ops): Same. (create_breakpoint): Same. * linespec.h (decode_line_full): Same. * linespec.c (decode_line_full): Same. Throw error if result.size () == 0. gdb/testsuite/ChangeLog: 2020-08-27 Tom de Vries * gdb.base/label-without-address.c: New test. * gdb.base/label-without-address.exp: New file. --- gdb/breakpoint.c | 36 +++++++++++------------ gdb/breakpoint.h | 6 ++-- gdb/linespec.c | 6 +++- gdb/linespec.h | 2 +- gdb/testsuite/gdb.base/label-without-address.c | 24 +++++++++++++++ gdb/testsuite/gdb.base/label-without-address.exp | 37 ++++++++++++++++++++++++ 6 files changed, 88 insertions(+), 23 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 8f75618bc9..670cba0057 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -89,7 +89,7 @@ static void map_breakpoint_numbers (const char *, static void breakpoint_re_set_default (struct breakpoint *); static void - create_sals_from_location_default (const struct event_location *location, + create_sals_from_location_default (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted); @@ -104,7 +104,7 @@ static void create_breakpoints_sal_default (struct gdbarch *, int, int, int, unsigned); static std::vector decode_location_default - (struct breakpoint *b, const struct event_location *location, + (struct breakpoint *b, struct event_location *location, struct program_space *search_pspace); static int can_use_hardware_watchpoint @@ -8948,7 +8948,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch, the caller's responsibility to free them. */ static void -parse_breakpoint_sals (const struct event_location *location, +parse_breakpoint_sals (struct event_location *location, struct linespec_result *canonical) { struct symtab_and_line cursal; @@ -9213,7 +9213,7 @@ breakpoint_ops_for_event_location (const struct event_location *location, int create_breakpoint (struct gdbarch *gdbarch, - const struct event_location *location, + struct event_location *location, const char *cond_string, int thread, const char *extra_string, int parse_extra, @@ -12266,7 +12266,7 @@ base_breakpoint_print_recreate (struct breakpoint *b, struct ui_file *fp) static void base_breakpoint_create_sals_from_location - (const struct event_location *location, + (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -12291,7 +12291,7 @@ base_breakpoint_create_breakpoints_sal (struct gdbarch *gdbarch, static std::vector base_breakpoint_decode_location (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { internal_error_pure_virtual_called (); @@ -12514,7 +12514,7 @@ bkpt_print_recreate (struct breakpoint *tp, struct ui_file *fp) } static void -bkpt_create_sals_from_location (const struct event_location *location, +bkpt_create_sals_from_location (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -12545,7 +12545,7 @@ bkpt_create_breakpoints_sal (struct gdbarch *gdbarch, static std::vector bkpt_decode_location (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { return decode_location_default (b, location, search_pspace); @@ -12718,7 +12718,7 @@ bkpt_probe_remove_location (struct bp_location *bl, } static void -bkpt_probe_create_sals_from_location (const struct event_location *location, +bkpt_probe_create_sals_from_location (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -12732,7 +12732,7 @@ bkpt_probe_create_sals_from_location (const struct event_location *location, static std::vector bkpt_probe_decode_location (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { std::vector sals = parse_probes (location, search_pspace, NULL); @@ -12826,7 +12826,7 @@ tracepoint_print_recreate (struct breakpoint *self, struct ui_file *fp) } static void -tracepoint_create_sals_from_location (const struct event_location *location, +tracepoint_create_sals_from_location (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -12857,7 +12857,7 @@ tracepoint_create_breakpoints_sal (struct gdbarch *gdbarch, static std::vector tracepoint_decode_location (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { return decode_location_default (b, location, search_pspace); @@ -12869,7 +12869,7 @@ struct breakpoint_ops tracepoint_breakpoint_ops; static void tracepoint_probe_create_sals_from_location - (const struct event_location *location, + (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -12879,7 +12879,7 @@ tracepoint_probe_create_sals_from_location static std::vector tracepoint_probe_decode_location (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { /* We use the same method for breakpoint on probes. */ @@ -12960,7 +12960,7 @@ dprintf_after_condition_true (struct bpstats *bs) markers (`-m'). */ static void -strace_marker_create_sals_from_location (const struct event_location *location, +strace_marker_create_sals_from_location (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -13030,7 +13030,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch, static std::vector strace_marker_decode_location (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { struct tracepoint *tp = (struct tracepoint *) b; @@ -13713,7 +13713,7 @@ breakpoint_re_set_default (struct breakpoint *b) calls parse_breakpoint_sals. Return 1 for success, zero for failure. */ static void -create_sals_from_location_default (const struct event_location *location, +create_sals_from_location_default (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted) { @@ -13750,7 +13750,7 @@ create_breakpoints_sal_default (struct gdbarch *gdbarch, static std::vector decode_location_default (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace) { struct linespec_result canonical; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 347aeb75f3..a5fead9149 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -609,7 +609,7 @@ struct breakpoint_ops `create_sals_from_location_default'. This function is called inside `create_breakpoint'. */ - void (*create_sals_from_location) (const struct event_location *location, + void (*create_sals_from_location) (struct event_location *location, struct linespec_result *canonical, enum bptype type_wanted); @@ -636,7 +636,7 @@ struct breakpoint_ops This function is called inside `location_to_sals'. */ std::vector (*decode_location) (struct breakpoint *b, - const struct event_location *location, + struct event_location *location, struct program_space *search_pspace); /* Return true if this breakpoint explains a signal. See @@ -1386,7 +1386,7 @@ enum breakpoint_create_flags Returns true if any breakpoint was created; false otherwise. */ extern int create_breakpoint (struct gdbarch *gdbarch, - const struct event_location *location, + struct event_location *location, const char *cond_string, int thread, const char *extra_string, int parse_extra, diff --git a/gdb/linespec.c b/gdb/linespec.c index 4a634e3aff..e8f3d594c3 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -3201,7 +3201,7 @@ event_location_to_sals (linespec_parser *parser, /* See linespec.h. */ void -decode_line_full (const struct event_location *location, int flags, +decode_line_full (struct event_location *location, int flags, struct program_space *search_pspace, struct symtab *default_symtab, int default_line, struct linespec_result *canonical, @@ -3230,6 +3230,10 @@ decode_line_full (const struct event_location *location, int flags, location); state = PARSER_STATE (&parser); + if (result.size () == 0) + throw_error (NOT_SUPPORTED_ERROR, _("Location %s not available"), + event_location_to_string (location)); + gdb_assert (result.size () == 1 || canonical->pre_expanded); canonical->pre_expanded = 1; diff --git a/gdb/linespec.h b/gdb/linespec.h index 2a34f60f05..9c2c8988fb 100644 --- a/gdb/linespec.h +++ b/gdb/linespec.h @@ -124,7 +124,7 @@ extern std::vector strcmp sense) to FILTER will be returned; all others will be filtered out. */ -extern void decode_line_full (const struct event_location *location, int flags, +extern void decode_line_full (struct event_location *location, int flags, struct program_space *search_pspace, struct symtab *default_symtab, int default_line, struct linespec_result *canonical, diff --git a/gdb/testsuite/gdb.base/label-without-address.c b/gdb/testsuite/gdb.base/label-without-address.c new file mode 100644 index 0000000000..f0d4a42268 --- /dev/null +++ b/gdb/testsuite/gdb.base/label-without-address.c @@ -0,0 +1,24 @@ +/* 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 (void) +{ + return 0; + L1: + (void)0; +} diff --git a/gdb/testsuite/gdb.base/label-without-address.exp b/gdb/testsuite/gdb.base/label-without-address.exp new file mode 100644 index 0000000000..0fcb1fd19a --- /dev/null +++ b/gdb/testsuite/gdb.base/label-without-address.exp @@ -0,0 +1,37 @@ +# 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 . */ + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +set supported 0 +gdb_test_multiple "l main:L1" "" { + -wrap -re "No label \"L1\" defined in function \"main\"\." { + unsupported $gdb_test_name + } + -wrap -re "L1:\r\n.*" { + pass $gdb_test_name + set supported 1 + } +} + +if { ! $supported } { + return -1 +} + +gdb_test "break main:L1" "Location main:L1 not available" --------------AE4C89F1F6A917EE44C6FA1F--