public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
@ 2020-08-27 11:52 Tom de Vries
  2020-08-27 12:41 ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Tom de Vries @ 2020-08-27 11:52 UTC (permalink / raw)
  To: gdb-patches

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
...
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.

Any comments?

Thanks,
- Tom

[gdb/breakpoint] Handle setting breakpoint on label without address

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 | 42 ++++++++++++++++++++++++
 6 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ef8e54f634..879d61f4b9 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,
@@ -12271,7 +12271,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)
 {
@@ -12296,7 +12296,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 ();
@@ -12519,7 +12519,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)
 {
@@ -12550,7 +12550,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);
@@ -12723,7 +12723,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)
 {
@@ -12737,7 +12737,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);
@@ -12831,7 +12831,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)
 {
@@ -12862,7 +12862,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);
@@ -12874,7 +12874,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)
 {
@@ -12884,7 +12884,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.  */
@@ -12965,7 +12965,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)
 {
@@ -13035,7 +13035,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;
@@ -13718,7 +13718,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)
 {
@@ -13755,7 +13755,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..eda68a4b0a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/label-without-address.exp
@@ -0,0 +1,42 @@
+# 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
+}
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+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 L1" "Location L1 not available"

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-27 11:52 [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address Tom de Vries
@ 2020-08-27 12:41 ` Pedro Alves
  2020-08-27 13:49   ` Tom de Vries
  0 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2020-08-27 12:41 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

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, 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?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-27 12:41 ` Pedro Alves
@ 2020-08-27 13:49   ` Tom de Vries
  2020-08-28 10:31     ` Tom de Vries
  2020-08-28 13:32     ` [PATCH][gdb/breakpoint] " Pedro Alves
  0 siblings, 2 replies; 12+ messages in thread
From: Tom de Vries @ 2020-08-27 13:49 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

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.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-27 13:49   ` Tom de Vries
@ 2020-08-28 10:31     ` Tom de Vries
  2020-08-28 13:20       ` [PATCH][gdb/breakpoint, PIE] " Tom de Vries
  2020-08-28 13:32     ` [PATCH][gdb/breakpoint] " Pedro Alves
  1 sibling, 1 reply; 12+ messages in thread
From: Tom de Vries @ 2020-08-28 10:31 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

[-- 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"

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH][gdb/breakpoint, PIE] Handle setting breakpoint on label without address
  2020-08-28 10:31     ` Tom de Vries
@ 2020-08-28 13:20       ` Tom de Vries
  2020-09-03 10:34         ` [committed][PATCH][gdb/breakpoint, " Tom de Vries
  0 siblings, 1 reply; 12+ messages in thread
From: Tom de Vries @ 2020-08-28 13:20 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

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

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

On 8/28/20 12:31 PM, Tom de Vries wrote:
> 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 ).

Which is fixed by the patch below.

Any comments?

Thanks,
- Tom



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

[gdb/breakpoint, PIE] Handle setting breakpoint on label without address

When adding:
...
if ![runto_main] then {
    fail "can't run to main"
    return 0
}
...
to test-case gdb.base/label-without-address.exp and running it with target
board unix/-fPIE/-pie, we run into:
...
(gdb) break main:L1^M
Breakpoint 2 at 0x555555554000: file label-without-address.c, line 22.^M
...
That is, for a label with optimized-out address, we set a breakpoint at the
relocation base.

The root cause is that the dwarf reader, despite finding that attribute
DW_AT_low_pc is missing, still tags the L1 symbol as having LOC_LABEL, which
means it has a valid address, which defaults to 0.

Fix this by instead tagging the L1 symbol with LOC_OPTIMIZED_OUT.

Tested on x86_64-linux.

gdb/ChangeLog:

2020-08-28  Tom de Vries  <tdevries@suse.de>

	PR breakpoint/26546
	* dwarf2/read.c (new_symbol): Tag label symbol without DW_AT_low_pc as
	LOC_OPTIMIZED_OUT instead of LOC_LABEL.

gdb/testsuite/ChangeLog:

2020-08-28  Tom de Vries  <tdevries@suse.de>

	PR breakpoint/26546
	* gdb.base/label-without-address.exp: Runto main first.

---
 gdb/dwarf2/read.c                                | 4 +++-
 gdb/testsuite/gdb.base/label-without-address.exp | 5 +++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 0ac8533263..b37f7e7a2f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -21447,10 +21447,12 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	      addr = attr->value_as_address ();
 	      addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
 	      SET_SYMBOL_VALUE_ADDRESS (sym, addr);
+	      SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
 	    }
+	  else
+	    SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
 	  SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
 	  SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
-	  SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
 	  add_symbol_to_list (sym, cu->list_in_scope);
 	  break;
 	case DW_TAG_subprogram:
diff --git a/gdb/testsuite/gdb.base/label-without-address.exp b/gdb/testsuite/gdb.base/label-without-address.exp
index 0fcb1fd19a..c688149cf3 100644
--- a/gdb/testsuite/gdb.base/label-without-address.exp
+++ b/gdb/testsuite/gdb.base/label-without-address.exp
@@ -19,6 +19,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
     return -1
 }
 
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
 set supported 0
 gdb_test_multiple "l main:L1" "" {
     -wrap -re "No label \"L1\" defined in function \"main\"\." {

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-27 13:49   ` Tom de Vries
  2020-08-28 10:31     ` Tom de Vries
@ 2020-08-28 13:32     ` Pedro Alves
  2020-08-28 13:53       ` Tom de Vries
  1 sibling, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2020-08-28 13:32 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 8/27/20 2: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 don't understand the "not useful" remark.  If a user does gets
the error, they'll probably do:

  "b 3",

and they'll get a breakpoint at line 5, no?

That's very likely what a user would do after the error.

IMO GDB should do that for the user.

So far I don't agree with your patch.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-28 13:32     ` [PATCH][gdb/breakpoint] " Pedro Alves
@ 2020-08-28 13:53       ` Tom de Vries
  2020-08-28 14:30         ` Tom de Vries
  2020-08-28 15:14         ` Pedro Alves
  0 siblings, 2 replies; 12+ messages in thread
From: Tom de Vries @ 2020-08-28 13:53 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 8/28/20 3:32 PM, Pedro Alves wrote:
> On 8/27/20 2: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 don't understand the "not useful" remark.  If a user does gets
> the error, they'll probably do:
> 
>   "b 3",
> 
> and they'll get a breakpoint at line 5, no?
> 
> That's very likely what a user would do after the error.
> 
> IMO GDB should do that for the user.
> 
> So far I don't agree with your patch.
> 

I see what you mean, but let's try this counter-example:
...
 cat -n test.c
     1  int
     2  main (void)
     3  {
     4    goto L2;
     5
     6   L3:
     7    return 0;
     8
     9   L1:
    10    (void)0;
    11    return 1;
    12
    13   L2:
    14    goto L3;
    15  }
    16
...
compiled like this:
...
$ gcc test.c -g
...

With the patch, we're not able to set a breakpoint at L1, and setting
the breakpoint at the corresponding line, line 9:
...
$ gdb a.out
Reading symbols from a.out...
(gdb) b main:L1
Location main:L1 not available
(gdb) b 9
Breakpoint 1 at 0x40049c: file test.c, line 14.
(gdb)
...
yields a breakpoint at line 14, a piece of code that's not reachable
from L1.

To me, label L1 and line 14 are unrelated enough to convince me to not
do this automatically.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-28 13:53       ` Tom de Vries
@ 2020-08-28 14:30         ` Tom de Vries
  2020-08-28 15:23           ` Pedro Alves
  2020-08-28 15:14         ` Pedro Alves
  1 sibling, 1 reply; 12+ messages in thread
From: Tom de Vries @ 2020-08-28 14:30 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 8/28/20 3:53 PM, Tom de Vries wrote:
> On 8/28/20 3:32 PM, Pedro Alves wrote:
>> On 8/27/20 2: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 don't understand the "not useful" remark.  If a user does gets
>> the error, they'll probably do:
>>
>>   "b 3",
>>
>> and they'll get a breakpoint at line 5, no?
>>
>> That's very likely what a user would do after the error.
>>
>> IMO GDB should do that for the user.
>>
>> So far I don't agree with your patch.
>>
> 
> I see what you mean, but let's try this counter-example:
> ...
>  cat -n test.c
>      1  int
>      2  main (void)
>      3  {
>      4    goto L2;
>      5
>      6   L3:
>      7    return 0;
>      8
>      9   L1:
>     10    (void)0;
>     11    return 1;
>     12
>     13   L2:
>     14    goto L3;
>     15  }
>     16
> ...
> compiled like this:
> ...
> $ gcc test.c -g
> ...
> 
> With the patch, we're not able to set a breakpoint at L1, and setting
> the breakpoint at the corresponding line, line 9:
> ...
> $ gdb a.out
> Reading symbols from a.out...
> (gdb) b main:L1
> Location main:L1 not available
> (gdb) b 9
> Breakpoint 1 at 0x40049c: file test.c, line 14.
> (gdb)
> ...
> yields a breakpoint at line 14, a piece of code that's not reachable
> from L1.
> 
> To me, label L1 and line 14 are unrelated enough to convince me to not
> do this automatically.
> 

FWIW, lldb does the same:
...
$ lldb a.out
(lldb) target create "a.out"
Current executable set to 'a.out' (x86_64).
(lldb) b main:L1
Breakpoint 1: no locations (pending).
WARNING:  Unable to resolve breakpoint to any actual locations.
(lldb) b 9
Breakpoint 2: where = a.out`main + 5 at test.c:14, address =
0x000000000040049c
(lldb)
...

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-28 13:53       ` Tom de Vries
  2020-08-28 14:30         ` Tom de Vries
@ 2020-08-28 15:14         ` Pedro Alves
  2020-08-28 16:15           ` Tom de Vries
  1 sibling, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2020-08-28 15:14 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 8/28/20 2:53 PM, Tom de Vries wrote:

> I see what you mean, but let's try this counter-example:
> ...
>  cat -n test.c
>      1  int
>      2  main (void)
>      3  {
>      4    goto L2;
>      5
>      6   L3:
>      7    return 0;
>      8
>      9   L1:
>     10    (void)0;
>     11    return 1;
>     12
>     13   L2:
>     14    goto L3;
>     15  }
>     16
> ...
> compiled like this:
> ...
> $ gcc test.c -g
> ...
> 
> With the patch, we're not able to set a breakpoint at L1, and setting
> the breakpoint at the corresponding line, line 9:
> ...
> $ gdb a.out
> Reading symbols from a.out...
> (gdb) b main:L1
> Location main:L1 not available
> (gdb) b 9
> Breakpoint 1 at 0x40049c: file test.c, line 14.
> (gdb)
> ...
> yields a breakpoint at line 14, a piece of code that's not reachable
> from L1.
> 
> To me, label L1 and line 14 are unrelated enough to convince me to not
> do this automatically.

Well, OK.  All the code after line 7 is unreacheable.

Then using that rationale, why don't we reject a breakpoint at line 9 too?
Line 14 is not reacheable from line 9 either.  Can you justify the
difference in a small sentence/paragraph?

It would be perhaps nice if "break" accepted an option to prevent the
move-to-following-insn -- for all kinds of breakpoints -- like lldb's
"break set -m" option:

       -m <boolean> ( --move-to-nearest-code <boolean> )
            Move breakpoints to nearest code. If not set the target.move-to-nearest-codesetting is used.

BTW, I noticed that, setting the breakpoint at the label _after_ running
to main "works":

 Reading symbols from testsuite/outputs/gdb.base/label-without-address/label-without-address...
 (gdb) b main:L1
 Location main:L1 not available
 (gdb) start
 Temporary breakpoint 1 at 0x1131: file testsuite/gdb.base/label-without-address.c, line 21.
 Starting program: testsuite/outputs/gdb.base/label-without-address/label-without-address 

 Temporary breakpoint 1, main () at testsuite/gdb.base/label-without-address.c:21
 21        return 0;
 (gdb) b main:L1
 Breakpoint 2 at 0x555555554000: file testsuite/gdb.base/label-without-address.c, line 22.
 (gdb) info breakpoints 
 Num     Type           Disp Enb Address            What
 2       breakpoint     keep y   0x0000555555554000 main:L1

I see the same with your more complicated example.  What's going on here?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-28 14:30         ` Tom de Vries
@ 2020-08-28 15:23           ` Pedro Alves
  0 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2020-08-28 15:23 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 8/28/20 3:30 PM, Tom de Vries wrote:
> On 8/28/20 3:53 PM, Tom de Vries wrote:
>> On 8/28/20 3:32 PM, Pedro Alves wrote:
>>> On 8/27/20 2: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 don't understand the "not useful" remark.  If a user does gets
>>> the error, they'll probably do:
>>>
>>>   "b 3",
>>>
>>> and they'll get a breakpoint at line 5, no?
>>>
>>> That's very likely what a user would do after the error.
>>>
>>> IMO GDB should do that for the user.
>>>
>>> So far I don't agree with your patch.
>>>
>>
>> I see what you mean, but let's try this counter-example:
>> ...
>>  cat -n test.c
>>      1  int
>>      2  main (void)
>>      3  {
>>      4    goto L2;
>>      5
>>      6   L3:
>>      7    return 0;
>>      8
>>      9   L1:
>>     10    (void)0;
>>     11    return 1;
>>     12
>>     13   L2:
>>     14    goto L3;
>>     15  }
>>     16
>> ...
>> compiled like this:
>> ...
>> $ gcc test.c -g
>> ...
>>
>> With the patch, we're not able to set a breakpoint at L1, and setting
>> the breakpoint at the corresponding line, line 9:
>> ...
>> $ gdb a.out
>> Reading symbols from a.out...
>> (gdb) b main:L1
>> Location main:L1 not available
>> (gdb) b 9
>> Breakpoint 1 at 0x40049c: file test.c, line 14.
>> (gdb)
>> ...
>> yields a breakpoint at line 14, a piece of code that's not reachable
>> from L1.
>>
>> To me, label L1 and line 14 are unrelated enough to convince me to not
>> do this automatically.
>>
> 
> FWIW, lldb does the same:
> ...
> $ lldb a.out
> (lldb) target create "a.out"
> Current executable set to 'a.out' (x86_64).
> (lldb) b main:L1
> Breakpoint 1: no locations (pending).
> WARNING:  Unable to resolve breakpoint to any actual locations.
> (lldb) b 9
> Breakpoint 2: where = a.out`main + 5 at test.c:14, address =
> 0x000000000040049c
> (lldb)
> ...

Well, I don't think you can claim that because it doesn't look
like lldb understands labels at all.  Tweak the testcase to:

    18  int
    19  main (void)
    20  {
    21   L1:
    22    return 0;
    23  }

And then:

 (lldb) b main:L1
 Breakpoint 1: no locations (pending).
 WARNING:  Unable to resolve breakpoint to any actual locations.

And you get the same message with any random nonexistent label name:

 (lldb) b main:FOOBAR
 Breakpoint 2: no locations (pending).
 WARNING:  Unable to resolve breakpoint to any actual locations.
 (lldb) 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address
  2020-08-28 15:14         ` Pedro Alves
@ 2020-08-28 16:15           ` Tom de Vries
  0 siblings, 0 replies; 12+ messages in thread
From: Tom de Vries @ 2020-08-28 16:15 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 8/28/20 5:14 PM, Pedro Alves wrote:
> On 8/28/20 2:53 PM, Tom de Vries wrote:
> 
>> I see what you mean, but let's try this counter-example:
>> ...
>>  cat -n test.c
>>      1  int
>>      2  main (void)
>>      3  {
>>      4    goto L2;
>>      5
>>      6   L3:
>>      7    return 0;
>>      8
>>      9   L1:
>>     10    (void)0;
>>     11    return 1;
>>     12
>>     13   L2:
>>     14    goto L3;
>>     15  }
>>     16
>> ...
>> compiled like this:
>> ...
>> $ gcc test.c -g
>> ...
>>
>> With the patch, we're not able to set a breakpoint at L1, and setting
>> the breakpoint at the corresponding line, line 9:
>> ...
>> $ gdb a.out
>> Reading symbols from a.out...
>> (gdb) b main:L1
>> Location main:L1 not available
>> (gdb) b 9
>> Breakpoint 1 at 0x40049c: file test.c, line 14.
>> (gdb)
>> ...
>> yields a breakpoint at line 14, a piece of code that's not reachable
>> from L1.
>>
>> To me, label L1 and line 14 are unrelated enough to convince me to not
>> do this automatically.
> 
> Well, OK.  All the code after line 7 is unreacheable.
> 

Um, not sure what you mean here, line 13/14 is reachable.

> Then using that rationale, why don't we reject a breakpoint at line 9 too?

Well, I did wonder if we can somehow deduce something 'irregular' from
the line table in this case and reject the breakpoint at line 9, while
having the regular and helpful:
...
3 int main (void) {
4
5   i = 3;
(gdb) b 4
...
still work and set a breakpoint in line 5, but I didn't manage to come
up with something.

So I think this is something we have to live with because the generic
case is useful.

> Line 14 is not reacheable from line 9 either.  Can you justify the
> difference in a small sentence/paragraph?
> 

Let me try.

My argument is basically is that the behaviour "this line doesn't work,
let me try a different line" is simple enough for the user to follow
(although arguably, we could be more verbose about it).

The behaviour "this label doesn't work, let me try the corresponding
line, hey the line doesn't work, let me try a different line" is too
elaborate IMO.

The behaviour "this label doesn't work, let me try the corresponding
line, if the line works" sounds still ok to me, but I don't think it
will ever be triggered.  In this situation, I expect the compiler to be
able to generate a label with address itself.

> It would be perhaps nice if "break" accepted an option to prevent the
> move-to-following-insn -- for all kinds of breakpoints -- like lldb's
> "break set -m" option:
> 
>        -m <boolean> ( --move-to-nearest-code <boolean> )
>             Move breakpoints to nearest code. If not set the target.move-to-nearest-codesetting is used.
> 

Oh, that's interesting.  In general I'm in favor of putting things which
may help the user in certain situations but can backfire in others under
user control.

> BTW, I noticed that, setting the breakpoint at the label _after_ running
> to main "works":
> 
>  Reading symbols from testsuite/outputs/gdb.base/label-without-address/label-without-address...
>  (gdb) b main:L1
>  Location main:L1 not available
>  (gdb) start
>  Temporary breakpoint 1 at 0x1131: file testsuite/gdb.base/label-without-address.c, line 21.
>  Starting program: testsuite/outputs/gdb.base/label-without-address/label-without-address 
> 
>  Temporary breakpoint 1, main () at testsuite/gdb.base/label-without-address.c:21
>  21        return 0;
>  (gdb) b main:L1
>  Breakpoint 2 at 0x555555554000: file testsuite/gdb.base/label-without-address.c, line 22.
>  (gdb) info breakpoints 
>  Num     Type           Disp Enb Address            What
>  2       breakpoint     keep y   0x0000555555554000 main:L1
> 
> I see the same with your more complicated example.  What's going on here?
> 

This is what you get when you're generate a PIE.  It looks like it
works, but it doesn't, that breakpoint address is incorrect, you're
running into PR26546, for which I submitted a patch.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [committed][PATCH][gdb/breakpoint, PIE] Handle setting breakpoint on label without address
  2020-08-28 13:20       ` [PATCH][gdb/breakpoint, PIE] " Tom de Vries
@ 2020-09-03 10:34         ` Tom de Vries
  0 siblings, 0 replies; 12+ messages in thread
From: Tom de Vries @ 2020-09-03 10:34 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 8/28/20 3:20 PM, Tom de Vries wrote:
> [ was: Re: [PATCH][gdb/breakpoint] Handle setting breakpoint on label
> without address ]
> 
> On 8/28/20 12:31 PM, Tom de Vries wrote:
>> 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 ).
> Which is fixed by the patch below.
> 
> Any comments?
> 

I've committed this.

Thanks,
- Tom
> 
> 0001-gdb-breakpoint-PIE-Handle-setting-breakpoint-on-label-without-address.patch
> 
> [gdb/breakpoint, PIE] Handle setting breakpoint on label without address
> 
> When adding:
> ...
> if ![runto_main] then {
>     fail "can't run to main"
>     return 0
> }
> ...
> to test-case gdb.base/label-without-address.exp and running it with target
> board unix/-fPIE/-pie, we run into:
> ...
> (gdb) break main:L1^M
> Breakpoint 2 at 0x555555554000: file label-without-address.c, line 22.^M
> ...
> That is, for a label with optimized-out address, we set a breakpoint at the
> relocation base.
> 
> The root cause is that the dwarf reader, despite finding that attribute
> DW_AT_low_pc is missing, still tags the L1 symbol as having LOC_LABEL, which
> means it has a valid address, which defaults to 0.
> 
> Fix this by instead tagging the L1 symbol with LOC_OPTIMIZED_OUT.
> 
> Tested on x86_64-linux.
> 
> gdb/ChangeLog:
> 
> 2020-08-28  Tom de Vries  <tdevries@suse.de>
> 
> 	PR breakpoint/26546
> 	* dwarf2/read.c (new_symbol): Tag label symbol without DW_AT_low_pc as
> 	LOC_OPTIMIZED_OUT instead of LOC_LABEL.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-08-28  Tom de Vries  <tdevries@suse.de>
> 
> 	PR breakpoint/26546
> 	* gdb.base/label-without-address.exp: Runto main first.
> 
> ---
>  gdb/dwarf2/read.c                                | 4 +++-
>  gdb/testsuite/gdb.base/label-without-address.exp | 5 +++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 0ac8533263..b37f7e7a2f 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -21447,10 +21447,12 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>  	      addr = attr->value_as_address ();
>  	      addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
>  	      SET_SYMBOL_VALUE_ADDRESS (sym, addr);
> +	      SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
>  	    }
> +	  else
> +	    SYMBOL_ACLASS_INDEX (sym) = LOC_OPTIMIZED_OUT;
>  	  SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
>  	  SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
> -	  SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
>  	  add_symbol_to_list (sym, cu->list_in_scope);
>  	  break;
>  	case DW_TAG_subprogram:
> diff --git a/gdb/testsuite/gdb.base/label-without-address.exp b/gdb/testsuite/gdb.base/label-without-address.exp
> index 0fcb1fd19a..c688149cf3 100644
> --- a/gdb/testsuite/gdb.base/label-without-address.exp
> +++ b/gdb/testsuite/gdb.base/label-without-address.exp
> @@ -19,6 +19,11 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
>      return -1
>  }
>  
> +if ![runto_main] then {
> +    fail "can't run to main"
> +    return 0
> +}
> +
>  set supported 0
>  gdb_test_multiple "l main:L1" "" {
>      -wrap -re "No label \"L1\" defined in function \"main\"\." {
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-09-03 10:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 11:52 [PATCH][gdb/breakpoint] Handle setting breakpoint on label without address Tom de Vries
2020-08-27 12:41 ` Pedro Alves
2020-08-27 13:49   ` Tom de Vries
2020-08-28 10:31     ` Tom de Vries
2020-08-28 13:20       ` [PATCH][gdb/breakpoint, PIE] " Tom de Vries
2020-09-03 10:34         ` [committed][PATCH][gdb/breakpoint, " Tom de Vries
2020-08-28 13:32     ` [PATCH][gdb/breakpoint] " Pedro Alves
2020-08-28 13:53       ` Tom de Vries
2020-08-28 14:30         ` Tom de Vries
2020-08-28 15:23           ` Pedro Alves
2020-08-28 15:14         ` Pedro Alves
2020-08-28 16:15           ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).