[PATCHv2,1/2] gdb: Remove unneeded parameter from set_breakpoint_location_function

Message ID 4b2521dc5cecc0c83728219638aab3fc46133153.1562003938.git.andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess July 1, 2019, 6:02 p.m. UTC
  The explicit_loc parameter in set_breakpoint_location_function is not
useful.  This parameter is set from two possible fields of the
symtab_and_line used to create the breakpoint; the explicit_pc field,
and the explicit_line field.

First, the explicit_line field, this is not currently set for any
breakpoint command, so will never be true.

Next, the explicit_pc field.  This can be true but will never be true
at the same time that the sal->msymbol field is also true - the
sal->msymbol is only ever set in linespec.c:minsym_found, which
doesn't allow for explicitly setting the pc.

The result of this is that if we are setting a breakpoint on an
msymbol that could turn out to be an ifunc, then we will not also have
either an explicit_pc or an explicit_line, this check can therefore be
removed.

There should be no user visible changes after this commit.

gdb/ChangeLog:

	* breakpoint.c (set_breakpoint_location_function): Remove
	explicit_loc parameter.
	(momentary_breakpoint_from_master): Update call to
	set_breakpoint_location_function.
	(add_location_to_breakpoint): Likewise.
---
 gdb/ChangeLog    |  8 ++++++++
 gdb/breakpoint.c | 14 +++++---------
 2 files changed, 13 insertions(+), 9 deletions(-)
  

Comments

Pedro Alves July 3, 2019, 10:13 p.m. UTC | #1
On 7/1/19 7:02 PM, Andrew Burgess wrote:
> The explicit_loc parameter in set_breakpoint_location_function is not
> useful.  This parameter is set from two possible fields of the
> symtab_and_line used to create the breakpoint; the explicit_pc field,
> and the explicit_line field.
> 
> First, the explicit_line field, this is not currently set for any
> breakpoint command, so will never be true.
> 
> Next, the explicit_pc field.  This can be true but will never be true
> at the same time that the sal->msymbol field is also true - the
> sal->msymbol is only ever set in linespec.c:minsym_found, which
> doesn't allow for explicitly setting the pc.
> 
> The result of this is that if we are setting a breakpoint on an
> msymbol that could turn out to be an ifunc, then we will not also have
> either an explicit_pc or an explicit_line, this check can therefore be
> removed.
> 
> There should be no user visible changes after this commit.
> 
> gdb/ChangeLog:
> 
> 	* breakpoint.c (set_breakpoint_location_function): Remove
> 	explicit_loc parameter.
> 	(momentary_breakpoint_from_master): Update call to
> 	set_breakpoint_location_function.
> 	(add_location_to_breakpoint): Likewise.

OK.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8422db8b571..9baef38a3de 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7096,12 +7096,10 @@  set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
   return add_to_breakpoint_chain (std::move (b));
 }
 
-/* Initialize loc->function_name.  EXPLICIT_LOC says no indirect function
-   resolutions should be made as the user specified the location explicitly
-   enough.  */
+/* Initialize loc->function_name.  */
 
 static void
-set_breakpoint_location_function (struct bp_location *loc, int explicit_loc)
+set_breakpoint_location_function (struct bp_location *loc)
 {
   gdb_assert (loc->owner != NULL);
 
@@ -7113,8 +7111,7 @@  set_breakpoint_location_function (struct bp_location *loc, int explicit_loc)
 
       if (loc->msymbol != NULL
 	  && (MSYMBOL_TYPE (loc->msymbol) == mst_text_gnu_ifunc
-	      || MSYMBOL_TYPE (loc->msymbol) == mst_data_gnu_ifunc)
-	  && !explicit_loc)
+	      || MSYMBOL_TYPE (loc->msymbol) == mst_data_gnu_ifunc))
 	{
 	  struct breakpoint *b = loc->owner;
 
@@ -8482,7 +8479,7 @@  momentary_breakpoint_from_master (struct breakpoint *orig,
 
   copy = set_raw_breakpoint_without_location (orig->gdbarch, type, ops);
   copy->loc = allocate_bp_location (copy);
-  set_breakpoint_location_function (copy->loc, 1);
+  set_breakpoint_location_function (copy->loc);
 
   copy->loc->gdbarch = orig->loc->gdbarch;
   copy->loc->requested_address = orig->loc->requested_address;
@@ -8587,8 +8584,7 @@  add_location_to_breakpoint (struct breakpoint *b,
   loc->msymbol = sal->msymbol;
   loc->objfile = sal->objfile;
 
-  set_breakpoint_location_function (loc,
-				    sal->explicit_pc || sal->explicit_line);
+  set_breakpoint_location_function (loc);
 
   /* While by definition, permanent breakpoints are already present in the
      code, we don't mark the location as inserted.  Normally one would expect