This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] MIPS16 FP manual call/return fixes
Jan,
> > We must have somehow established its address previously or we couldn't
> > have called it. Is it possible to cache it somehow for example?
>
> Yes, we could. In set_breakpoint_location_function is created
> bp_gnu_ifunc_resolver, so cache it into 'struct bp_location' there (or even
> into 'struct breakpoint', I do not see too much difference there)..
Well, it's associated with a specific location really, so...
> Then transfer this info when bp_gnu_ifunc_resolver_return is created from that
> bp_gnu_ifunc_resolver.
Given that we rely on having a reference to bp_gnu_ifunc_resolver from
bp_gnu_ifunc_resolver_return I've decided that we don't really have to
transfer anything here.
> I would be fine also with just some error there.
I'm not sure what you mean, we do need to return something here.
> > Thanks for raising the issue of unloading symbol tables, that's an
> > important point as to how MIPS16 and microMIPS symbols should be treated
> > in general -- here I think it will only matter in the asynchronous mode.
>
> No, even in synchronous mode. If you still do "stepi", "stepi", "stepi"...
> you will get something like:
>
> (gdb) start
> (gdb) b strcmp
> Breakpoint 2 at gnu-indirect-function resolver at 0x7ffff7aaa3d0: file ../sysdeps/x86_64/multiarch/strcmp.S, line 87.
> (gdb) b *strcmp
> Note: breakpoint 2 also set at pc 0x7ffff7aaa3d0.
> Breakpoint 3 at 0x7ffff7aaa3d0: file ../sysdeps/x86_64/multiarch/strcmp.S, line 87.
> (gdb) c
> Continuing.
> warning: Breakpoint 2 address previously adjusted from 0x004003c6 to 0x7ffff7aaa3d0.
> Breakpoint 2, strcmp () at ../sysdeps/x86_64/multiarch/strcmp.S:87
> 87 cmpl $0, __cpu_features+KIND_OFFSET(%rip)
> (gdb) maintenance info breakpoints
> Num Type Disp Enb Address What
> [...]
> 2 STT_GNU_IFUNC resolver keep y 0x00007ffff7aaa3d0 ../sysdeps/x86_64/multiarch/strcmp.S:87 inf 1
> breakpoint already hit 1 time
> 3 breakpoint keep y 0x00007ffff7aaa3d0 ../sysdeps/x86_64/multiarch/strcmp.S:87 inf 1
> breakpoint already hit 1 time
> 0 STT_GNU_IFUNC resolver return keep y 0x00007ffff7deb3be <_dl_fixup+446> inf 1 thread 1
> stop only in thread 1
>
> So you can see anything can happen now before we hit that breakpoint #0.
> I can for example unload the glibc symbol file by 'nosharedlibrary' (which has
> led to unrelated PR 14054 now).
Good point, it's turned out I was confused what the use case of this code
exactly is.
So here's the new proposal, comprising only the relevant bits (I keep it
all with the rest of the patch really, but decided not to clutter the list
with), it doesn't cause any regressions compared to the original.
I have decided to add a related_address field to bp_location; this could
get converted to an opaque member in the future in case some other
breakpoints need to carry other auxiliary information (just as we do with
some other structures). It's set to the address of the resolver as
set_breakpoint_location_function sets up a bp_gnu_ifunc_resolver
breakpoint; conveniently it already looks up the function, just did not
request its address before.
I have decided to keep the assertion you had concerns about after all.
The reason is as set_breakpoint_location_function will never create a
bp_gnu_ifunc_resolver breakpoint with more than one location assigned
anyway, see the condition enclosing:
b->type = bp_gnu_ifunc_resolver;
-- it already requires loc->next to be NULL or it doesn't select this
special breakpoint type at all (the intent to avoid such breakpoints is
commented on too; the comment even qualified for context of the patch
below, so you can just read it here). Is it ever possible for this
breakpoint to be updated later on with additional locations? What would
the conditions be if so? -- I'd like to be able to reproduce them in the
lab then.
I have tested this change with the mips-sde-elf target observing no
regressions compared to the previous proposal. I did some manual checks
running GDB under GDB to see the expected flow of changes is correct here,
using the test program used by gdb.base/gnu-ifunc.exp; this was with the
x86_64-linux-gnu target. It worked exactly as expected.
OK for this update? I believe this is the last part of the whole change
there are unresolved concerns about.
2012-05-08 Maciej W. Rozycki <macro@codesourcery.com>
gdb/
* breakpoint.h (bp_location): Add related_address member.
* breakpoint.c (set_breakpoint_location_function): Initialize it
for bp_gnu_ifunc_resolver breakpoints.
* elfread.c (elf_gnu_ifunc_resolver_return_stop): Pass the
requested function's address to gdbarch_return_value.
Maciej
gdb-mips16-calls-ifunc-new.diff
Index: gdb-fsf-trunk-quilt/gdb/elfread.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/elfread.c 2012-05-08 08:25:37.795639727 +0100
+++ gdb-fsf-trunk-quilt/gdb/elfread.c 2012-05-08 09:42:32.075560588 +0100
@@ -1020,21 +1020,15 @@ elf_gnu_ifunc_resolver_return_stop (stru
struct type *func_func_type = builtin_type (gdbarch)->builtin_func_func;
struct type *value_type = TYPE_TARGET_TYPE (func_func_type);
struct regcache *regcache = get_thread_regcache (inferior_ptid);
+ struct value *func_func;
struct value *value;
CORE_ADDR resolved_address, resolved_pc;
struct symtab_and_line sal;
struct symtabs_and_lines sals, sals_end;
+ CORE_ADDR func_func_addr;
gdb_assert (b->type == bp_gnu_ifunc_resolver_return);
- value = allocate_value (value_type);
- gdbarch_return_value (gdbarch, func_func_type, value_type, regcache,
- value_contents_raw (value), NULL);
- resolved_address = value_as_address (value);
- resolved_pc = gdbarch_convert_from_func_ptr_addr (gdbarch,
- resolved_address,
- ¤t_target);
-
while (b->related_breakpoint != b)
{
struct breakpoint *b_next = b->related_breakpoint;
@@ -1055,6 +1049,18 @@ elf_gnu_ifunc_resolver_return_stop (stru
b = b_next;
}
gdb_assert (b->type == bp_gnu_ifunc_resolver);
+ gdb_assert (b->loc->next == NULL);
+
+ func_func = allocate_value (func_func_type);
+ set_value_address (func_func, b->loc->related_address);
+
+ value = allocate_value (value_type);
+ gdbarch_return_value (gdbarch, func_func, value_type, regcache,
+ value_contents_raw (value), NULL);
+ resolved_address = value_as_address (value);
+ resolved_pc = gdbarch_convert_from_func_ptr_addr (gdbarch,
+ resolved_address,
+ ¤t_target);
gdb_assert (current_program_space == b->pspace || b->pspace == NULL);
elf_gnu_ifunc_record_cache (b->addr_string, resolved_pc);
Index: gdb-fsf-trunk-quilt/gdb/breakpoint.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/breakpoint.c 2012-05-04 19:26:24.845606346 +0100
+++ gdb-fsf-trunk-quilt/gdb/breakpoint.c 2012-05-08 08:26:49.855603092 +0100
@@ -6586,9 +6586,10 @@ set_breakpoint_location_function (struct
{
int is_gnu_ifunc;
const char *function_name;
+ CORE_ADDR func_addr;
find_pc_partial_function_gnu_ifunc (loc->address, &function_name,
- NULL, NULL, &is_gnu_ifunc);
+ &func_addr, NULL, &is_gnu_ifunc);
if (is_gnu_ifunc && !explicit_loc)
{
@@ -6609,6 +6610,9 @@ set_breakpoint_location_function (struct
/* 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;
}
}
Index: gdb-fsf-trunk-quilt/gdb/breakpoint.h
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/breakpoint.h 2012-05-04 19:26:24.845606346 +0100
+++ gdb-fsf-trunk-quilt/gdb/breakpoint.h 2012-05-08 08:26:49.875618479 +0100
@@ -418,6 +418,11 @@ struct bp_location
processor's architectual constraints. */
CORE_ADDR requested_address;
+ /* An additional address assigned with this location. This is currently
+ only used by STT_GNU_IFUNC resolver breakpoints to hold the address
+ of the resolver function. */
+ CORE_ADDR related_address;
+
/* If the location comes from a probe point, this is the probe associated
with it. */
struct probe *probe;