Fix setting breakpoints on ifunc functions after they're already resolved
authorPedro Alves <palves@redhat.com>
Thu, 26 Apr 2018 12:01:26 +0000 (13:01 +0100)
committerPedro Alves <palves@redhat.com>
Thu, 26 Apr 2018 12:06:21 +0000 (13:06 +0100)
This fixes setting breakpoints on ifunc functions by name after the
ifunc has already been resolved.

In that case, if you have debug info for the ifunc resolver, without
the fix, then gdb puts a breakpoint past the prologue of the resolver,
instead of setting a breakpoint at the ifunc target:

  break gnu_ifunc
  Breakpoint 4 at 0x7ffff7bd36f2: file src/gdb/testsuite/gdb.base/gnu-ifunc-lib.c, line 34.
  (gdb) continue
  Continuing.
  [Inferior 1 (process 13300) exited normally]
  (gdb)

above we should have stopped at "final", but didn't because we never
resolved the ifunc to the final location.

If you don't have debug info for the resolver, GDB manages to resolve
the ifunc target, but, it should be setting a breakpoint after the
prologue of the final function, and instead what you get is that GDB
sets a breakpoint on the first address of the target function.  With
the gnu-ifunc.exp tests added by a later patch, we get, without the
fix:

  (gdb) break gnu_ifunc
  Breakpoint 4 at 0x400753
  (gdb) continue
  Continuing.

  Breakpoint 4, final (arg=1) at src/gdb/testsuite/gdb.base/gnu-ifunc-final.c:20
  20 {

vs, fixed:

  (gdb) break gnu_ifunc
  Breakpoint 4 at 0x40075a: file src/gdb/testsuite/gdb.base/gnu-ifunc-final.c, line 21.
  (gdb) continue
  Continuing.

  Breakpoint 4, final (arg=2) at src/gdb/testsuite/gdb.base/gnu-ifunc-final.c:21
  21   return arg + 1;
  (gdb)

Fix the problems above by moving the ifunc target resolving to
linespec.c, before we skip a function's prologue.  We need to save
something in the sal, so that set_breakpoint_location_function knows
that it needs to create a bp_gnu_ifunc_resolver bp_location.  Might as
well just save a pointer to the minsym.

gdb/ChangeLog:
2018-04-26  Pedro Alves  <palves@redhat.com>

* breakpoint.c (set_breakpoint_location_function): Don't resolve
ifunc targets here.  Instead, if we have an ifunc minsym, use its
address/name.
(add_location_to_breakpoint): Store the minsym and the objfile in
the breakpoint location.
* breakpoint.h (bp_location) <msymbol, objfile>: New fields.
* linespec.c (minsym_found): Resolve GNU ifunc targets here.
Record the minsym in the sal.
* symtab.h (symtab_and_line) <msymbol>: New field.

gdb/ChangeLog
gdb/breakpoint.c
gdb/breakpoint.h
gdb/linespec.c
gdb/symtab.h

index 9de1b109aa2d2ba2bcac5aaf6454d5184a9d0212..5901a15d14bbd057d75972c77921b096670f5318 100644 (file)
@@ -1,3 +1,15 @@
+2018-04-26  Pedro Alves  <palves@redhat.com>
+
+       * breakpoint.c (set_breakpoint_location_function): Don't resolve
+       ifunc targets here.  Instead, if we have an ifunc minsym, use its
+       address/name.
+       (add_location_to_breakpoint): Store the minsym and the objfile in
+       the breakpoint location.
+       * breakpoint.h (bp_location) <msymbol, objfile>: New fields.
+       * linespec.c (minsym_found): Resolve GNU ifunc targets here.
+       Record the minsym in the sal.
+       * symtab.h (symtab_and_line) <msymbol>: New field.
+
 2018-04-26  Pedro Alves  <palves@redhat.com>
 
        * elfread.c (elf_gnu_ifunc_resolve_by_got): Don't write to *ADDR_P
index 11b89bcf88202d7b7ac63e762ec230b9f395574b..4e514d26be7a33d3cdea14374081d45306582cca 100644 (file)
@@ -7156,37 +7156,29 @@ set_breakpoint_location_function (struct bp_location *loc, int explicit_loc)
       || loc->owner->type == bp_hardware_breakpoint
       || is_tracepoint (loc->owner))
     {
-      int is_gnu_ifunc;
       const char *function_name;
-      CORE_ADDR func_addr;
 
-      find_pc_partial_function_gnu_ifunc (loc->address, &function_name,
-                                         &func_addr, NULL, &is_gnu_ifunc);
-
-      if (is_gnu_ifunc && !explicit_loc)
+      if (loc->msymbol != NULL
+         && MSYMBOL_TYPE (loc->msymbol) == mst_text_gnu_ifunc
+         && !explicit_loc)
        {
          struct breakpoint *b = loc->owner;
 
-         gdb_assert (loc->pspace == current_program_space);
-         if (gnu_ifunc_resolve_name (function_name,
-                                     &loc->requested_address))
-           {
-             /* Recalculate ADDRESS based on new REQUESTED_ADDRESS.  */
-             loc->address = adjust_breakpoint_address (loc->gdbarch,
-                                                       loc->requested_address,
-                                                       b->type);
-           }
-         else if (b->type == bp_breakpoint && b->loc == loc
-                  && loc->next == NULL && b->related_breakpoint == b)
+         function_name = MSYMBOL_LINKAGE_NAME (loc->msymbol);
+
+         if (b->type == bp_breakpoint && b->loc == loc
+             && loc->next == NULL && b->related_breakpoint == b)
            {
              /* Create only the whole new breakpoint of this type but do not
                 mess more complicated breakpoints with multiple locations.  */
              b->type = bp_gnu_ifunc_resolver;
              /* Remember the resolver's address for use by the return
                 breakpoint.  */
-             loc->related_address = func_addr;
+             loc->related_address = loc->address;
            }
        }
+      else
+       find_pc_partial_function (loc->address, &function_name, NULL, NULL);
 
       if (function_name)
        loc->function_name = xstrdup (function_name);
@@ -8650,6 +8642,8 @@ add_location_to_breakpoint (struct breakpoint *b,
   loc->line_number = sal->line;
   loc->symtab = sal->symtab;
   loc->symbol = sal->symbol;
+  loc->msymbol = sal->msymbol;
+  loc->objfile = sal->objfile;
 
   set_breakpoint_location_function (loc,
                                    sal->explicit_pc || sal->explicit_line);
index 062e390469ae082bbc883c19c1694790a87c2c50..1b045ae092cf322839a26d5a7e4d207c8ebeff86 100644 (file)
@@ -490,6 +490,14 @@ public:
      ascertain when an event location was set at a different location than
      the one originally selected by parsing, e.g., inlined symbols.  */
   const struct symbol *symbol = NULL;
+
+  /* Similarly, the minimal symbol found by the location parser, if
+     any.  This may be used to ascertain if the location was
+     originally set on a GNU ifunc symbol.  */
+  const minimal_symbol *msymbol = NULL;
+
+  /* The objfile the symbol or minimal symbol were found in.  */
+  const struct objfile *objfile = NULL;
 };
 
 /* The possible return values for print_bpstat, print_it_normal,
index 8951c1e8c4736026f247dd85120bf4d8b037fe14..8a52f5efdef790659ada4ed4367f131611b933bd 100644 (file)
@@ -4244,10 +4244,24 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
              struct minimal_symbol *msymbol,
              std::vector<symtab_and_line> *result)
 {
-  struct symtab_and_line sal;
+  bool want_start_sal;
 
   CORE_ADDR func_addr;
-  if (msymbol_is_function (objfile, msymbol, &func_addr))
+  bool is_function = msymbol_is_function (objfile, msymbol, &func_addr);
+
+  if (is_function)
+    {
+      const char *msym_name = MSYMBOL_LINKAGE_NAME (msymbol);
+
+      if (MSYMBOL_TYPE (msymbol) == mst_text_gnu_ifunc)
+       want_start_sal = gnu_ifunc_resolve_name (msym_name, &func_addr);
+      else
+       want_start_sal = true;
+    }
+
+  symtab_and_line sal;
+
+  if (is_function && want_start_sal)
     {
       sal = find_pc_sect_line (func_addr, NULL, 0);
 
@@ -4270,7 +4284,13 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
   else
     {
       sal.objfile = objfile;
-      sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
+      sal.msymbol = msymbol;
+      /* Store func_addr, not the minsym's address in case this was an
+        ifunc that hasn't been resolved yet.  */
+      if (is_function)
+       sal.pc = func_addr;
+      else
+       sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
       sal.pspace = current_program_space;
     }
 
index 94b6b24bd5dd87b74eb1a17fcebcd364332d2b33..017a69063303d58d8fd7c3e7348ddbd0a794a62d 100644 (file)
@@ -1765,6 +1765,7 @@ struct symtab_and_line
   struct symtab *symtab = NULL;
   struct symbol *symbol = NULL;
   struct obj_section *section = NULL;
+  struct minimal_symbol *msymbol = NULL;
   /* Line number.  Line numbers start at 1 and proceed through symtab->nlines.
      0 is never a valid line number; it is used to indicate that line number
      information is not available.  */