Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

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><f4>: Abbrev Number: 2 (DW_TAG_subprogram)
>>>     <f5>   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

[-- Attachment #2: 0001-gdb-breakpoint-Handle-setting-breakpoint-on-label-without-address.patch --]
[-- Type: text/x-patch, Size: 14652 bytes --]

[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><f4>: Abbrev Number: 2 (DW_TAG_subprogram)
    <f5>   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  <tdevries@suse.de>

	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  <tdevries@suse.de>

	* 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<symtab_and_line> 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<symtab_and_line>
 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<symtab_and_line>
 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<symtab_and_line>
 bkpt_probe_decode_location (struct breakpoint *b,
-			    const struct event_location *location,
+			    struct event_location *location,
 			    struct program_space *search_pspace)
 {
   std::vector<symtab_and_line> 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<symtab_and_line>
 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<symtab_and_line>
 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<symtab_and_line>
 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<symtab_and_line>
 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<symtab_and_line> (*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<symtab_and_line>
    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 <http://www.gnu.org/licenses/>.  */
+
+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 <http://www.gnu.org/licenses/>.  */
+
+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"