* [RFA 0/4] Improved linker-debugger interface
@ 2012-07-12 12:34 Gary Benson
2012-07-12 12:35 ` [RFA 1/4] " Gary Benson
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-12 12:34 UTC (permalink / raw)
To: gdb-patches
Hi all,
This patch series implements an improved debug interface with the
runtime linker to help address the following bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=658851
aka http://sources.redhat.com/bugzilla/show_bug.cgi?id=2328
"_dl_debug_state() RT_CONSISTENT called too early"
https://bugzilla.redhat.com/show_bug.cgi?id=698001
"improve GDB performance on an application performing
a lot of object loading."
http://sourceware.org/bugzilla/show_bug.cgi?id=11839
"gdb does not detect calls to dlmopen"
The current linker-debugger interface has a structure (r_debug)
containing a list of loaded libraries, and an empty function
(_dl_debug_state) for debuggers to set breakpoints on and which
the linker calls both before and after modifying this list.
The problems with the current interface are as follows:
- There is one place where glibc calls _dl_debug_state earlier than
Solaris libc. This is #658851. It is unlikely that glibc will
ever be changed to make it compatible with Solaris libc, which
means GDB reports libraries as loaded and ready before they
really are.
- This interface was presumably invented before dlmopen() was, so
there's only provision in it for one namespace. In glibc each
namespace has it's own r_debug structure, but there is no way for
the linker to communicate the addresses of the others to the
debugger. This is PR 11839.
- In normal use GDB only needs to stop _after_ the list is modified.
Because _dl_debug_state is called both before and after, GDB stops
twice as often as it needs to. There is also no provision for
communicating what has changed, so GDB must load the entire list
of loaded libraries at every stop. This is #698001.
- When stop-on-solib-events is set, however, it is useful to stop
both before and after library loads.
My proposed solution is to insert a number of named probes into glibc.
My current setup has a probe everywhere _dl_debug_state is called, and
an extra pair to surround relocation events. New probes can be added
as and when necessary without breaking the interface, and likewise new
arguments can be added to existing probes.
This approach solves the various problems like so:
- Debuggers can pick and choose which probes to set breakpoints
on. By using the "relocation completed" probe instead of the
one mirroring _dl_debug_state debuggers can stop after relocations
have occurred, matching the behaviour of Solaris libc.
- All probes have namespace id and r_debug address arguments,
allowing debuggers to see namespaces other than the default.
- When stop-on-solib-events is unset, GDB does not have to stop
before changes are made, only after. By disabling the "before"
breakpoints the number of stops made can be halved.
- Probes adding new libraries may optionally supply the address
of the link-map entry of the first newly added library. This
enables debuggers to skip past libraries they already saw.
This patch series modifies GDB to search for named probes in
the runtime linker, and to use them instead of _dl_debug_state
if found. If the probes are not found then GDB will fall back
to its previous behaviour. When probes are used, GDB stops
after relocation. Stops before changes are made will be
inhibited when stop-on-solib-events is off, and if the linker
supplies the information to allow incremental updating then
GDB will use it. I've not done anything on the GDB side to
address the dlmopen() issue, but it's now possible to fix it
using the data supplied by the new interface.
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFA 1/4] Improved linker-debugger interface
2012-07-12 12:34 [RFA 0/4] Improved linker-debugger interface Gary Benson
@ 2012-07-12 12:35 ` Gary Benson
2012-07-12 12:36 ` [RFA 3/4] " Gary Benson
` (3 subsequent siblings)
4 siblings, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-12 12:35 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 418 bytes --]
Hi all,
This patch implements the probes-based runtime linker-debugger
interface in glibc using SystemTap probes. I'll shortly send
this to libc-alpha@sourceware.org, so if you have comments or
reviews it would probably be best if you could find the thread
over there and reply to that instead.
Note that this patch contains a description of the interfaces,
both new and old.
Cheers,
Gary
--
http://gbenson.net/
[-- Attachment #2: glibc-patch-3.patch --]
[-- Type: text/plain, Size: 11160 bytes --]
2012-07-12 Gary Benson <gbenson@redhat.com>
[BZ #14298]
* elf/rtld.c: Include <stap-probe.h>.
(dl_main): Added static probes "init_start" and "init_complete".
* elf/dl-load.c: Include <stap-probe.h>.
(lose): Take new parameter "nsid".
Added static probe "map_complete".
(_dl_map_object_from_fd): Pass namespace id to lose.
Added static probe "map_start".
(open_verify): Pass namespace id to lose.
* elf/dl-open.c: Include <stap-probe.h>.
(dl_open_worker) Added static probes "map_complete", "reloc_start"
and "reloc_complete".
* elf/dl-close.c: Include <stap-probe.h>.
(_dl_close_worker): Added static probes "unmap_start" and
"unmap_complete".
* elf/rtld-debugger-interface.txt: New file documenting the above.
diff --git a/elf/rtld-debugger-interface.txt b/elf/rtld-debugger-interface.txt
new file mode 100644
index 0000000..61bc99e
--- /dev/null
+++ b/elf/rtld-debugger-interface.txt
@@ -0,0 +1,122 @@
+Standard debugger interface
+===========================
+
+The run-time linker exposes a rendezvous structure to allow debuggers
+to interface with it. This structure, r_debug, is defined in link.h.
+If the executable's dynamic section has a DT_DEBUG element, the
+run-time linker sets that element's value to the address where this
+structure can be found.
+
+The r_debug structure contains (amongst others) the following fields:
+
+ struct link_map *r_map:
+ A linked list of loaded objects.
+
+ enum { RT_CONSISTENT, RT_ADD, RT_DELETE } r_state:
+ The current state of the r_map list. RT_CONSISTENT means that r_map
+ is not currently being modified and may safely be inspected. RT_ADD
+ means that an object is being added to r_map, and that the list is
+ not guaranteed to be consistent. Likewise RT_DELETE means that an
+ object is being removed from the list.
+
+ ElfW(Addr) r_brk:
+ The address of a function internal to the run-time linker which is
+ called whenever r_state is changed. The debugger should set a
+ breakpoint at this address if it wants to notice mapping changes.
+
+This protocol is widely supported, but somewhat limited in that it
+has no provision to provide access to multiple namespaces, and that
+the notifications (via r_brk) only refer to changes to r_map--the
+debugger is notified that a new object has been added, for instance,
+but there is no way for the debugger to discover whether any of the
+objects in the link-map have been relocated or not.
+
+
+Probe-based debugger interface
+==============================
+
+Systemtap is a dynamic tracing/instrumenting tool available on Linux.
+Probes that are not fired at run time have close to zero overhead.
+glibc contains a number of probes that debuggers can set breakpoints
+on in order to notice certain events.
+
+All rtld probes have the following arguments:
+
+ arg1: Lmid_t lmid:
+ The link-map ID of the link-map list that the object was loaded
+ into. This will be LM_ID_BASE for the application's main link-map
+ list, or some other value for different namespaces.
+
+ arg2: struct r_debug *r_debug:
+ A pointer to the r_debug structure containing the link-map list
+ that the object was loaded into. This will be the value stored in
+ DT_DEBUG for the application's main link-map list, or some other
+ value for different namespaces.
+
+map_complete and reloc_complete may have the following additional
+argument:
+
+ arg3: struct link_map *new:
+ A pointer which, if not NULL, points to the entry in the specified
+ r_debug structure's link-map list corresponding to the first new
+ object to have been mapped or relocated, with new->l_next pointing
+ to the link-map of the next new object to have been mapped or
+ relocated, and so on. Note that because `new' is an entry in a
+ larger list, new->l_prev (if not NULL) will point to what was the
+ last link-map in the link-map list prior to the new objects being
+ mapped or relocated.
+
+The following probes are available:
+
+ init_start:
+ This is called once, when the linker is about to fill in the main
+ r_debug structure at application startup. init_start always has
+ lmid set to LM_ID_BASE and r_debug set to the value stored in
+ DT_DEBUG. r_debug is not guaranteed to be consistent until
+ init_complete is fired.
+
+ init_complete:
+ This is called once, when the linker has filled in the main
+ r_debug structure at application startup. init_complete always
+ has lmid set to LM_ID_BASE and r_debug set to the value stored
+ in DT_DEBUG. The r_debug structure is consistent and may be
+ inspected, and all objects in the link-map are guaranteed to
+ have been relocated.
+
+ map_start:
+ The linker is about to map new objects into the specified
+ namespace. The namespace's r_debug structure is not guaranteed
+ to be consistent until a corresponding map_complete is fired.
+
+ map_complete:
+ The linker has finished mapping new objects into the specified
+ namespace. The namespace's r_debug structure is consistent and
+ may be inspected, although objects in the namespace's link-map
+ are not guaranteed to have been relocated.
+
+ map_failed:
+ The linker failed while attempting to map new objects into
+ the specified namespace. The namespace's r_debug structure
+ is consistent and may be inspected.
+
+ reloc_start:
+ The linker is about to relocate all unrelocated objects in the
+ specified namespace. The namespace's r_debug structure is not
+ guaranteed to be consistent until a corresponding reloc_complete
+ is fired.
+
+ reloc_complete:
+ The linker has relocated all objects in the specified namespace.
+ The namespace's r_debug structure is consistent and may be
+ inspected, and all objects in the namespace's link-map are
+ guaranteed to have been relocated.
+
+ unmap_start:
+ The linker is about to remove objects from the specified
+ namespace. The namespace's r_debug structure is not guaranteed to
+ be consistent until a corresponding unmap_complete is fired.
+
+ unmap_complete:
+ The linker has finished removing objects into the specified
+ namespace. The namespace's r_debug structure is consistent and
+ may be inspected.
diff --git a/elf/rtld.c b/elf/rtld.c
index 6bcf224..06c4220 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -39,6 +39,7 @@
#include <dl-osinfo.h>
#include <dl-procinfo.h>
#include <tls.h>
+#include <stap-probe.h>
#include <stackinfo.h>
#include <assert.h>
@@ -1683,6 +1684,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
/* We start adding objects. */
r->r_state = RT_ADD;
_dl_debug_state ();
+ LIBC_PROBE (init_start, 2, LM_ID_BASE, r);
/* Auditing checkpoint: we are ready to signal that the initial map
is being constructed. */
@@ -2402,6 +2404,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
r = _dl_debug_initialize (0, LM_ID_BASE);
r->r_state = RT_CONSISTENT;
_dl_debug_state ();
+ LIBC_PROBE (init_complete, 2, LM_ID_BASE, r);
#ifndef MAP_COPY
/* We must munmap() the cache file. */
diff --git a/elf/dl-load.c b/elf/dl-load.c
index fe83f87..43e1269 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -35,6 +35,7 @@
#include <stackinfo.h>
#include <caller.h>
#include <sysdep.h>
+#include <stap-probe.h>
#include <dl-dst.h>
@@ -882,7 +883,7 @@ _dl_init_paths (const char *llp)
static void
__attribute__ ((noreturn, noinline))
lose (int code, int fd, const char *name, char *realname, struct link_map *l,
- const char *msg, struct r_debug *r)
+ const char *msg, struct r_debug *r, Lmid_t nsid)
{
/* The file might already be closed. */
if (fd != -1)
@@ -896,6 +897,7 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l,
{
r->r_state = RT_CONSISTENT;
_dl_debug_state ();
+ LIBC_PROBE (map_failed, 2, nsid, r);
}
_dl_signal_error (code, name, NULL, msg);
@@ -934,7 +936,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
errval = errno;
call_lose:
lose (errval, fd, name, realname, l, errstring,
- make_consistent ? r : NULL);
+ make_consistent ? r : NULL, nsid);
}
/* Look again to see if the real name matched another already loaded. */
@@ -1041,6 +1043,7 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp,
linking has not been used before. */
r->r_state = RT_ADD;
_dl_debug_state ();
+ LIBC_PROBE (map_start, 2, nsid, r);
make_consistent = true;
}
else
@@ -1736,7 +1739,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
name = strdupa (realname);
free (realname);
}
- lose (errval, fd, name, NULL, NULL, errstring, NULL);
+ lose (errval, fd, name, NULL, NULL, errstring, NULL, 0);
}
/* See whether the ELF header is what we expect. */
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 9fe0a7f..9c788c2 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -32,6 +32,7 @@
#include <caller.h>
#include <sysdep-cancel.h>
#include <tls.h>
+#include <stap-probe.h>
#include <dl-dst.h>
@@ -291,6 +292,7 @@ dl_open_worker (void *a)
struct r_debug *r = _dl_debug_initialize (0, args->nsid);
r->r_state = RT_CONSISTENT;
_dl_debug_state ();
+ LIBC_PROBE (map_complete, 3, args->nsid, r, new);
/* Print scope information. */
if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_SCOPES, 0))
@@ -376,10 +378,19 @@ dl_open_worker (void *a)
}
}
+ int relocation_in_progress = 0;
+
for (size_t i = nmaps; i-- > 0; )
{
l = maps[i];
+ if (! relocation_in_progress)
+ {
+ /* Notify the debugger that relocations are about to happen. */
+ LIBC_PROBE (reloc_start, 2, args->nsid, r);
+ relocation_in_progress = 1;
+ }
+
#ifdef SHARED
if (__builtin_expect (GLRO(dl_profile) != NULL, 0))
{
@@ -544,6 +555,10 @@ cannot load any more object with static TLS"));
}
}
+ /* Notify the debugger all new objects have been relocated. */
+ if (relocation_in_progress)
+ LIBC_PROBE (reloc_complete, 3, args->nsid, r, new);
+
/* Run the initializer functions of new objects. */
_dl_init (new, args->argc, args->argv, args->env);
diff --git a/elf/dl-close.c b/elf/dl-close.c
index a250ea5..45b2187 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -31,6 +31,7 @@
#include <sys/mman.h>
#include <sysdep-cancel.h>
#include <tls.h>
+#include <stap-probe.h>
/* Type of the constructor functions. */
@@ -468,6 +469,7 @@ _dl_close_worker (struct link_map *map)
struct r_debug *r = _dl_debug_initialize (0, nsid);
r->r_state = RT_DELETE;
_dl_debug_state ();
+ LIBC_PROBE (unmap_start, 2, nsid, r);
if (unload_global)
{
@@ -737,6 +739,7 @@ _dl_close_worker (struct link_map *map)
/* Notify the debugger those objects are finalized and gone. */
r->r_state = RT_CONSISTENT;
_dl_debug_state ();
+ LIBC_PROBE (unmap_complete, 2, nsid, r);
/* Recheck if we need to retry, release the lock. */
out:
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFA 2/4] Improved linker-debugger interface
2012-07-12 12:34 [RFA 0/4] Improved linker-debugger interface Gary Benson
` (2 preceding siblings ...)
2012-07-12 12:36 ` [RFA 4/4] " Gary Benson
@ 2012-07-12 12:36 ` Gary Benson
2012-07-13 9:41 ` Gary Benson
2012-07-18 14:08 ` [RFA 0/4] " Gary Benson
4 siblings, 1 reply; 19+ messages in thread
From: Gary Benson @ 2012-07-12 12:36 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]
Hi all,
This patch implements the basics of the improved runtime linker
interface in GDB--everything except the incremental library
list loading, basically. It is an updated version of the patch
I posted here in May:
http://www.cygwin.com/ml/gdb-patches/2012-05/msg00130.html
The changes I've made since then are as follows:
- All references to SystemTap have been removed; the patch
now uses GDB's generic probe API, and can work with any
system which allows named probes with arguments.
- The probe names have been changed. The previous version
prefixed the names with "rtld_", but I didn't realise at
the time that probes exist within providers which serve
as namespaces (the probes are all in the "rtld" provider).
Some Fedora and RHEL systems have the original, prefixed
probe names, so the patch makes GDB try these names if
the unprefixed probes are not found. Not doing this
would cause GDB to regress PR 2328 on these systems.
- I've used ARRAY_SIZE to define NUM_PROBES rather than
calculating it explicitly.
- I've made a new function, free_probes, to collect some
code I'd repeated in several places.
- I've made a new function, solib_event_probe_at, which is
used only once in this patch, but which is also used by
the last patch in this series.
- I've written a simple testcase to check that solib-event
stops work when probes are used. (It will XFAIL if probes
are not available).
- Various formatting nits were fixed.
I did not suffix the "mandatory" field of struct probe_info
with "_p", as the consensus seemed to be that it was better
without. I also didn't make it a bitfield as I wasn't sure
if that was desirable, but I can easily do so if necessary.
Cheers,
Gary
--
http://gbenson.net/
[-- Attachment #2: rtld-stap-2.patch --]
[-- Type: text/plain, Size: 21172 bytes --]
gdb/
2012-07-12 Gary Benson <gbenson@redhat.com>
* infrun.c (set_stop_on_solib_events): New function.
(_initialize_infrun): Use the above for "set stop-on-solib-events".
* solib-svr4.c (probe_info): New struct.
(probe_info): New static variable.
(NUM_PROBES): New definition.
(svr4_info): New fields "using_probes" and "probes".
(free_probes): New function.
(svr4_pspace_data_cleanup): Free probes.
(probe_and_info): New struct.
(solib_event_probe_at): New function.
(svr4_update_solib_event_breakpoint): Likewise.
(svr4_update_solib_event_breakpoints): Likewise.
(svr4_create_solib_event_breakpoints): Likewise.
(enable_break): Free probes before creating breakpoints.
Use svr4_create_solib_event_breakpoints to create breakpoints.
(_initialize_svr4_solib): Initialise svr4_so_ops.update_breakpoints.
* solib.h (update_solib_breakpoints): New function definition.
* solib.c (update_solib_breakpoints): New function.
* solist.h (target_so_ops): New field "update_breakpoints".
gdb/testsuite
2012-07-12 Gary Benson <gbenson@redhat.com>
* gdb.base/break-interp.exp (solib_bp): New constant.
(reach_1): Use the above instead of "_dl_debug_state".
(test_attach): Likewise.
(test_ld): Likewise.
* gdb.base/break-probes.exp: New file.
* gdb.base/break-probes.c: Likewise.
* gdb.base/break-probes-solib.c: Likewise.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 11f981f..89fbcfc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -361,6 +361,16 @@ static struct symbol *step_start_function;
/* Nonzero if we want to give control to the user when we're notified
of shared library events by the dynamic linker. */
int stop_on_solib_events;
+
+/* Enable or disable optional shared library event breakpoints
+ as appropriate when the above flag is changed. */
+
+static void
+set_stop_on_solib_events (char *args, int from_tty, struct cmd_list_element *c)
+{
+ update_solib_breakpoints ();
+}
+
static void
show_stop_on_solib_events (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -7204,7 +7214,7 @@ Show stopping for shared library events."), _("\
If nonzero, gdb will give control to the user when the dynamic linker\n\
notifies gdb of shared library events. The most common event of interest\n\
to the user would be loading/unloading of a new library."),
- NULL,
+ set_stop_on_solib_events,
show_stop_on_solib_events,
&setlist, &showlist);
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 307e483..c111f04 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -47,6 +47,8 @@
#include "auxv.h"
#include "exceptions.h"
+#include "probe.h"
+
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
static void svr4_relocate_main_executable (void);
@@ -92,6 +94,32 @@ static const char * const solib_break_names[] =
NULL
};
+/* A list of named probes which, if present in the dynamic linker,
+ allow more fine-grained breakpoints to be placed on shared library
+ events. */
+
+struct probe_info
+{
+ /* The name of the probe. */
+ const char *name;
+
+ /* Nonzero if this probe must be stopped at even when
+ stop-on-solib-events is off. */
+ int mandatory;
+};
+
+static const struct probe_info probe_info[] =
+{
+ { "init_start", 0 },
+ { "init_complete", 1 },
+ { "map_start", 0 },
+ { "reloc_complete", 1 },
+ { "unmap_start", 0 },
+ { "unmap_complete", 1 },
+};
+
+#define NUM_PROBES ARRAY_SIZE (probe_info)
+
static const char * const bkpt_names[] =
{
"_start",
@@ -313,17 +341,41 @@ struct svr4_info
CORE_ADDR interp_text_sect_high;
CORE_ADDR interp_plt_sect_low;
CORE_ADDR interp_plt_sect_high;
+
+ /* Nonzero if we are using the probes-based interface. */
+ int using_probes;
+
+ /* Named probes in the dynamic linker. */
+ VEC (probe_p) *probes[NUM_PROBES];
};
/* Per-program-space data key. */
static const struct program_space_data *solib_svr4_pspace_data;
+/* Free any allocated probe vectors. */
+
+static void
+free_probes (struct svr4_info *info)
+{
+ int i;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ VEC_free (probe_p, info->probes[i]);
+
+ memset (info->probes, 0, sizeof (info->probes));
+}
+
static void
svr4_pspace_data_cleanup (struct program_space *pspace, void *arg)
{
struct svr4_info *info;
info = program_space_data (pspace, solib_svr4_pspace_data);
+ if (info == NULL)
+ return;
+
+ free_probes (info);
+
xfree (info);
}
@@ -1400,6 +1452,168 @@ exec_entry_point (struct bfd *abfd, struct target_ops *targ)
targ);
}
+/* A probe and its associated information structure. */
+
+struct probe_and_info
+{
+ /* The probe. */
+ struct probe *probe;
+
+ /* The probe_info from which the probe was created. */
+ const struct probe_info *info;
+};
+
+/* Get the solib event probe at the specified location, and the
+ probe_info the probe was created with. Returns NULL if no solib
+ event probe exists at that location. */
+
+static struct probe_and_info *
+solib_event_probe_at (struct bp_location *loc, struct probe_and_info *result)
+{
+ struct svr4_info *info = get_svr4_info ();
+ int i;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ {
+ struct probe *probe;
+ int ix;
+
+ for (ix = 0; VEC_iterate (probe_p, info->probes[i], ix, probe); ++ix)
+ {
+ if (loc->pspace == current_program_space
+ && loc->address == probe->address)
+ {
+ result->info = &probe_info[i];
+ result->probe = probe;
+
+ return result;
+ }
+ }
+ }
+
+ return NULL;
+}
+
+/* Helper function for svr4_update_solib_event_breakpoints. */
+
+static int
+svr4_update_solib_event_breakpoint (struct breakpoint *b, void *arg)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct bp_location *loc;
+
+ if (b->type != bp_shlib_event)
+ return 0;
+
+ for (loc = b->loc; loc; loc = loc->next)
+ {
+ struct probe_and_info buf, *pi;
+
+ pi = solib_event_probe_at (loc, &buf);
+ if (pi != NULL)
+ {
+ if (!pi->info->mandatory)
+ b->enable_state = (stop_on_solib_events
+ ? bp_enabled : bp_disabled);
+
+ return 0;
+ }
+ }
+
+ return 0;
+}
+
+/* Enable or disable optional solib event breakpoints as appropriate.
+ Called whenever stop_on_solib_events is changed. */
+
+static void
+svr4_update_solib_event_breakpoints (void)
+{
+ struct svr4_info *info = get_svr4_info ();
+
+ if (info->using_probes)
+ iterate_over_breakpoints (svr4_update_solib_event_breakpoint, NULL);
+}
+
+/* Both the SunOS and the SVR4 dynamic linkers call a marker function
+ before and after mapping and unmapping shared libraries. The sole
+ purpose of this method is to allow debuggers to set a breakpoint so
+ they can track these changes.
+
+ Some versions of the glibc dynamic linker contain named probes
+ to allow more fine grained stopping. Given the address of the
+ original marker function, this function attempts to find these
+ probes, and if found, sets breakpoints on those instead. If the
+ probes aren't found, a single breakpoint is set on the original
+ marker function. */
+
+static void
+svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch, CORE_ADDR address)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct obj_section *os;
+
+ os = find_pc_section (address);
+ if (os != NULL)
+ {
+ int with_prefix;
+
+ for (with_prefix = 0; with_prefix <= 1; with_prefix++)
+ {
+ int all_probes_found = 1;
+ int i;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ {
+ char name[32] = { '\0' };
+
+ /* Fedora 17, RHEL 6.2, and RHEL 6.3 shipped with an
+ early version of the probes code in which the probes'
+ names were prefixed with "rtld_". The locations and
+ arguments of the probes are otherwise the same, so we
+ check for the prefixed version if the unprefixed
+ probes are not found. */
+
+ if (with_prefix)
+ strncat (name, "rtld_", sizeof (name));
+
+ strncat (name, probe_info[i].name, sizeof (name));
+
+ info->probes[i] = find_probes_in_objfile (os->objfile, "rtld",
+ name);
+
+ if (!VEC_length (probe_p, info->probes[i]))
+ {
+ free_probes (info);
+ all_probes_found = 0;
+ break;
+ }
+ }
+
+ if (all_probes_found)
+ {
+ info->using_probes = 1;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ {
+ struct probe *probe;
+ int ix;
+
+ for (ix = 0;
+ VEC_iterate (probe_p, info->probes[i], ix, probe);
+ ++ix)
+ create_solib_event_breakpoint (gdbarch, probe->address);
+ }
+
+ svr4_update_solib_event_breakpoints ();
+ return;
+ }
+ }
+ }
+
+ create_solib_event_breakpoint (gdbarch, address);
+}
+
/* Helper function for gdb_bfd_lookup_symbol. */
static int
@@ -1452,6 +1666,9 @@ enable_break (struct svr4_info *info, int from_tty)
info->interp_text_sect_low = info->interp_text_sect_high = 0;
info->interp_plt_sect_low = info->interp_plt_sect_high = 0;
+ free_probes (info);
+ info->using_probes = 0;
+
/* If we already have a shared library list in the target, and
r_debug contains r_brk, set the breakpoint there - this should
mean r_brk has already been relocated. Assume the dynamic linker
@@ -1483,7 +1700,7 @@ enable_break (struct svr4_info *info, int from_tty)
That knowledge is encoded in the address, if it's Thumb the low bit
is 1. However, we've stripped that info above and it's not clear
what all the consequences are of passing a non-addr_bits_remove'd
- address to create_solib_event_breakpoint. The call to
+ address to svr4_create_solib_event_breakpoints. The call to
find_pc_section verifies we know about the address and have some
hope of computing the right kind of breakpoint to use (via
symbol info). It does mean that GDB needs to be pointed at a
@@ -1521,7 +1738,7 @@ enable_break (struct svr4_info *info, int from_tty)
+ bfd_section_size (tmp_bfd, interp_sect);
}
- create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);
return 1;
}
}
@@ -1676,7 +1893,8 @@ enable_break (struct svr4_info *info, int from_tty)
if (sym_addr != 0)
{
- create_solib_event_breakpoint (target_gdbarch, load_addr + sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch,
+ load_addr + sym_addr);
xfree (interp_name);
return 1;
}
@@ -1702,7 +1920,7 @@ enable_break (struct svr4_info *info, int from_tty)
sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch,
sym_addr,
¤t_target);
- create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);
return 1;
}
}
@@ -1718,7 +1936,7 @@ enable_break (struct svr4_info *info, int from_tty)
sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch,
sym_addr,
¤t_target);
- create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);
return 1;
}
}
@@ -2494,4 +2712,5 @@ _initialize_svr4_solib (void)
svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
svr4_so_ops.same = svr4_same;
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
+ svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
}
diff --git a/gdb/solib.h b/gdb/solib.h
index 7a2ff84..65e3857 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -91,4 +91,8 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
void *),
void *data);
+/* Enable or disable optional solib event breakpoints as appropriate. */
+
+extern void update_solib_breakpoints (void);
+
#endif /* SOLIB_H */
diff --git a/gdb/solib.c b/gdb/solib.c
index 90439ba..dda0130 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1209,6 +1209,18 @@ no_shared_libraries (char *ignored, int from_tty)
objfile_purge_solibs ();
}
+/* See solib.h. */
+
+void
+update_solib_breakpoints (void)
+{
+ struct target_so_ops *ops = solib_ops (target_gdbarch);
+
+ if (ops->update_breakpoints != NULL)
+ ops->update_breakpoints ();
+}
+
+
/* Reload shared libraries, but avoid reloading the same symbol file
we already have loaded. */
diff --git a/gdb/solist.h b/gdb/solist.h
index 7413e3b..0d9046d 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -149,6 +149,13 @@ struct target_so_ops
core file (in particular, for readonly sections). */
int (*keep_data_in_core) (CORE_ADDR vaddr,
unsigned long size);
+
+ /* Enable or disable optional solib event breakpoints as
+ appropriate. This should be called whenever
+ stop_on_solib_events is changed. This pointer can be
+ NULL, in which case no enabling or disabling is necessary
+ for this target. */
+ void (*update_breakpoints) (void);
};
/* Free the memory associated with a (so_list *). */
diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
index 4012e66..7aa6537 100644
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -109,12 +109,19 @@ proc strip_debug {dest} {
}
}
+# The marker function for the standard runtime linker interface is
+# _dl_debug_state. The probes-based interface has no specific marker
+# function; the probe we will stop on (init_start) is in dl_main so we
+# check for that.
+
+set solib_bp {(_dl_debug_state|dl_main)}
+
# Implementation of reach.
proc reach_1 {func command displacement} {
- global gdb_prompt expect_out
+ global gdb_prompt expect_out solib_bp
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
# Breakpoint on _dl_debug_state can have problems due to its overlap
# with the existing internal breakpoint from GDB.
gdb_test_no_output "set stop-on-solib-events 1"
@@ -142,21 +149,21 @@ proc reach_1 {func command displacement} {
exp_continue
}
-re "Breakpoint \[0-9\]+, \\.?(__GI_)?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
fail $test
} else {
pass $test
}
}
-re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?(__GI_)?$func \\(\\).*\r\n$gdb_prompt $" {
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
fail $test
} else {
pass $test
}
}
-re "Stopped due to (spurious )?shared library event.*\r\n$gdb_prompt $" {
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
if {$debug_state_count == 0} {
# First stop does not yet relocate the _start function
# descriptor on ppc64.
@@ -175,7 +182,7 @@ proc reach_1 {func command displacement} {
fail $test_displacement
}
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
gdb_test_no_output "set stop-on-solib-events 0"
}
}
@@ -357,7 +364,7 @@ proc test_attach {file displacement {relink_args ""}} {
}
proc test_ld {file ifmain trynosym displacement} {
- global srcdir subdir gdb_prompt expect_out inferior_exited_re
+ global srcdir subdir gdb_prompt expect_out inferior_exited_re solib_bp
# First test normal `file'-command loaded $FILE with symbols.
@@ -385,9 +392,9 @@ proc test_ld {file ifmain trynosym displacement} {
gdb_test_no_output "set args ${objdir}/${subdir}/$binfile_test" "set args OBJDIR/${subdir}/$binfile_test"
}
- reach "_dl_debug_state" "run" $displacement
+ reach $solib_bp "run" $displacement
- gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?_dl_debug_state\\M.*" "dl bt"
+ gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?$solib_bp\\M.*" "dl bt"
if $ifmain {
reach "main" continue "NONE"
@@ -399,7 +406,7 @@ proc test_ld {file ifmain trynosym displacement} {
# Try re-run if the new PIE displacement takes effect.
gdb_test "kill" "" "kill" {Kill the program being debugged\? \(y or n\) } "y"
- reach "_dl_debug_state" "run" $displacement
+ reach $solib_bp "run" $displacement
if $ifmain {
test_core $file $displacement
@@ -431,7 +438,7 @@ proc test_ld {file ifmain trynosym displacement} {
gdb_test "exec-file $file" "exec-file $escapedfile" "load"
if $ifmain {
- reach "_dl_debug_state" run $displacement
+ reach $solib_bp run $displacement
# Use two separate gdb_test_multiple statements to avoid timeouts due
# to slow processing of wildcard capturing long output
diff --git a/gdb/testsuite/gdb.base/break-probes-solib.c b/gdb/testsuite/gdb.base/break-probes-solib.c
new file mode 100644
index 0000000..9979ee7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-probes-solib.c
@@ -0,0 +1,24 @@
+/* Copyright 2012 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/>. */
+
+#include <stdio.h>
+
+int
+foo (int n)
+{
+ printf ("foo %d\n", n);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/break-probes.c b/gdb/testsuite/gdb.base/break-probes.c
new file mode 100644
index 0000000..a778099
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-probes.c
@@ -0,0 +1,26 @@
+/* Copyright 2012 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/>. */
+
+#include <dlfcn.h>
+
+int
+main ()
+{
+ void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+
+ dlclose (handle);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/break-probes.exp b/gdb/testsuite/gdb.base/break-probes.exp
new file mode 100644
index 0000000..94b125a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-probes.exp
@@ -0,0 +1,76 @@
+# Copyright 2012 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/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+ return 0
+}
+
+standard_testfile
+
+set libname $testfile-solib
+set srcfile_lib $srcdir/$subdir/$libname.c
+set binfile_lib $objdir/$subdir/$libname.so
+
+set normal_bp "_dl_debug_state"
+set probes_bp "dl_main"
+
+if { [gdb_compile_shlib $srcfile_lib $binfile_lib \
+ [list additional_flags=-fPIC]] != "" } {
+ untested "Could not compile $binfile_lib."
+ return -1
+}
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile \
+ [list additional_flags=-DSHLIB_NAME\=\"$binfile_lib\" libs=-ldl]] } {
+ return -1
+}
+
+# Enable stop-on-solib-events
+gdb_test_no_output "set stop-on-solib-events 1"
+
+# Run to the first stop
+gdb_test "run" ".*Stopped due to shared library event.*"
+
+# XFAIL if we are not using probes
+set test "ensure using probes"
+set using_probes 0
+gdb_test_multiple "bt" $test {
+ -re "#0 +\[^\r\n\]*\\m(__GI_)?$normal_bp\\M.*$gdb_prompt $" {
+ xfail $test
+ }
+ -re "#0 +\[^\r\n\]*\\m(__GI_)?$probes_bp\\M.*$gdb_prompt $" {
+ pass $test
+ set using_probes 1
+ }
+}
+
+if { $using_probes } {
+ # Run til it loads our library
+ set test "run til our library loads"
+ set loaded_library 0
+ while { !$loaded_library } {
+ gdb_test_multiple "c" $test {
+ -re "Inferior loaded $binfile_lib\\M.*$gdb_prompt $" {
+ pass $test
+ set loaded_library 1
+ }
+ -re "Stopped due to shared library event\\M.*$gdb_prompt $" {
+ }
+ }
+ }
+
+ # Call something to ensure that relocation occurred
+ gdb_test "call foo(23)" "foo 23.*\\\$.* = .*"
+}
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFA 4/4] Improved linker-debugger interface
2012-07-12 12:34 [RFA 0/4] Improved linker-debugger interface Gary Benson
2012-07-12 12:35 ` [RFA 1/4] " Gary Benson
2012-07-12 12:36 ` [RFA 3/4] " Gary Benson
@ 2012-07-12 12:36 ` Gary Benson
2012-07-13 9:42 ` Gary Benson
2012-07-12 12:36 ` [RFA 2/4] " Gary Benson
2012-07-18 14:08 ` [RFA 0/4] " Gary Benson
4 siblings, 1 reply; 19+ messages in thread
From: Gary Benson @ 2012-07-12 12:36 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 835 bytes --]
Hi all,
This patch builds on the probes-based runtime linker interface,
adding incremental library list loading. The list of currently
loaded libraries is cached in the program space, and updated as
necessary whenever a solib event breakpoint is hit. I've tried
to make it fail as gracefully as possible for maximum forward
compatibility: if something happens that it does not understand
then the cache will be wiped and not repopulated until an event
occurs that it does understand. Until such time GDB will use
its previous behaviour of walking the entire link map each time
a solib event occurs.
I have of course tested this locally, but I don't have a real
world example of an application using thousands of shared
libraries, so I'd really appreciate feedback from those of you
that do.
Cheers,
Gary
--
http://gbenson.net/
[-- Attachment #2: rtld-stap-3.patch --]
[-- Type: text/plain, Size: 25805 bytes --]
gdb/
2012-07-12 Gary Benson <gbenson@redhat.com>
* breakpoint.h (handle_solib_event): Moved function definition
to solib.h, and added a new parameter.
* breakpoint.c (handle_solib_event): Moved function to solib.c
and added a new parameter.
(bpstat_stop_status): Pass new argument to handle_solib_event.
* solib.h (breakpoint.h): New include.
(handle_solib_event): Moved function definition from breakpoint.h
and added a new parameter.
* solib.c (handle_solib_event): Moved function from breakpoint.c
and added a new parameter.
* infrun.c (handle_inferior_event): Pass new argument to
handle_solib_event.
* solist.h (breakpoint.h): New include.
(target_so_ops): New field "handle_solib_event".
* solib-svr4.c (svr4_free_library_list): New forward declaration.
(probe_action): New enum.
(probe_info): New field "action".
(svr4_info): New field "solib_cache".
(free_solib_cache): New function.
(svr4_pspace_data_cleanup): Call the above.
(svr4_copy_library_list): New function.
(svr4_read_so_list): New parameter "prev_lm".
Changed return type from void to int.
Return nonzero on success, zero on error.
(svr4_current_sos): Return cached list if available.
Add new argument to calls to svr4_read_so_list.
(solib_event_probe_action): New function.
(solib_cache_update_full): Likewise.
(solib_cache_update_incremental): Likewise.
(svr4_handle_solib_event): Likewise.
(svr4_solib_create_inferior_hook): Free any cached solibs.
(_initialize_svr4_solib): Initialise svr4_so_ops.handle_solib_event.
gdb/testsuite
2012-07-12 Gary Benson <gbenson@redhat.com>
* gdb.base/info-shared.exp: New file.
* gdb.base/info-shared.c: Likewise.
* gdb.base/info-shared-solib1.c: Likewise.
* gdb.base/info-shared-solib2.c: Likewise.
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 4e4f875..f678702 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1515,6 +1515,4 @@ extern int user_breakpoint_p (struct breakpoint *);
/* Attempt to determine architecture of location identified by SAL. */
extern struct gdbarch *get_sal_arch (struct symtab_and_line sal);
-extern void handle_solib_event (void);
-
#endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6b9faf3..51c98a7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5205,7 +5205,7 @@ bpstat_stop_status (struct address_space *aspace,
{
if (bs->breakpoint_at && bs->breakpoint_at->type == bp_shlib_event)
{
- handle_solib_event ();
+ handle_solib_event (bs);
break;
}
}
@@ -5301,25 +5301,6 @@ handle_jit_event (void)
target_terminal_inferior ();
}
-/* Handle an solib event by calling solib_add. */
-
-void
-handle_solib_event (void)
-{
- clear_program_space_solib_cache (current_inferior ()->pspace);
-
- /* Check for any newly added shared libraries if we're supposed to
- be adding them automatically. Switch terminal for any messages
- produced by breakpoint_re_set. */
- target_terminal_ours_for_output ();
-#ifdef SOLIB_ADD
- SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
-#else
- solib_add (NULL, 0, ¤t_target, auto_solib_add);
-#endif
- target_terminal_inferior ();
-}
-
/* Prepare WHAT final decision for infrun. */
/* Decide what infrun needs to do with this bpstat. */
diff --git a/gdb/solib.h b/gdb/solib.h
index 65e3857..6e92ddc 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -21,6 +21,9 @@
#ifndef SOLIB_H
#define SOLIB_H
+/* For bpstat. */
+#include "breakpoint.h"
+
/* Forward decl's for prototypes */
struct so_list;
struct target_ops;
@@ -91,6 +94,13 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
void *),
void *data);
+/* Handle an solib event by calling solib_add. Targets which handle
+ solib events using breakpoints must pass a valid bpstat. Targets
+ which handle solib events using some other mechanism should pass
+ NULL. */
+
+extern void handle_solib_event (bpstat bs);
+
/* Enable or disable optional solib event breakpoints as appropriate. */
extern void update_solib_breakpoints (void);
diff --git a/gdb/solib.c b/gdb/solib.c
index dda0130..072fe4d 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1212,6 +1212,30 @@ no_shared_libraries (char *ignored, int from_tty)
/* See solib.h. */
void
+handle_solib_event (bpstat bs)
+{
+ struct target_so_ops *ops = solib_ops (target_gdbarch);
+
+ if (ops->handle_solib_event != NULL)
+ ops->handle_solib_event (bs);
+
+ clear_program_space_solib_cache (current_inferior ()->pspace);
+
+ /* Check for any newly added shared libraries if we're supposed to
+ be adding them automatically. Switch terminal for any messages
+ produced by breakpoint_re_set. */
+ target_terminal_ours_for_output ();
+#ifdef SOLIB_ADD
+ SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
+#else
+ solib_add (NULL, 0, ¤t_target, auto_solib_add);
+#endif
+ target_terminal_inferior ();
+}
+
+/* See solib.h. */
+
+void
update_solib_breakpoints (void)
{
struct target_so_ops *ops = solib_ops (target_gdbarch);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 89fbcfc..7c621ca 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3318,7 +3318,7 @@ handle_inferior_event (struct execution_control_state *ecs)
context_switch (ecs->ptid);
regcache = get_thread_regcache (ecs->ptid);
- handle_solib_event ();
+ handle_solib_event (NULL);
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (regcache),
diff --git a/gdb/solist.h b/gdb/solist.h
index 0d9046d..ea580a9 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -23,6 +23,8 @@
#define SO_NAME_MAX_PATH_SIZE 512 /* FIXME: Should be dynamic */
/* For domain_enum domain. */
#include "symtab.h"
+/* For bpstat. */
+#include "breakpoint.h"
/* Forward declaration for target specific link map information. This
struct is opaque to all but the target specific file. */
@@ -150,6 +152,13 @@ struct target_so_ops
int (*keep_data_in_core) (CORE_ADDR vaddr,
unsigned long size);
+ /* Target-specific handling of solib events. For targets which
+ handle solib events using breakpoints a valid bpstat must be
+ passed. Targets which handle solib events using some other
+ mechanism should pass NULL. This pointer can be NULL, in which
+ case no specific handling is necessary for this target. */
+ void (*handle_solib_event) (bpstat bs);
+
/* Enable or disable optional solib event breakpoints as
appropriate. This should be called whenever
stop_on_solib_events is changed. This pointer can be
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index c111f04..ebb8c3c 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -52,6 +52,7 @@
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
static void svr4_relocate_main_executable (void);
+static void svr4_free_library_list (void *p_list);
/* Link map info to include in an allocated so_list entry. */
@@ -94,6 +95,25 @@ static const char * const solib_break_names[] =
NULL
};
+/* What to do with the link_map cache. */
+
+enum probe_action
+ {
+ /* No action is required. The cache is still valid. */
+ LM_CACHE_NO_ACTION,
+
+ /* Something went wrong. The cache may be invalid and must be
+ cleared. Do not attempt further caching at this stop. */
+ LM_CACHE_INVALIDATE,
+
+ /* The cache should be reloaded now. */
+ LM_CACHE_RELOAD,
+
+ /* Attempt to incrementally update the cache. If the update
+ fails or is not possible, fall back to LM_CACHE_RELOAD. */
+ LM_CACHE_UPDATE_OR_RELOAD
+ };
+
/* A list of named probes which, if present in the dynamic linker,
allow more fine-grained breakpoints to be placed on shared library
events. */
@@ -106,16 +126,20 @@ struct probe_info
/* Nonzero if this probe must be stopped at even when
stop-on-solib-events is off. */
int mandatory;
+
+ /* What to do with the link_map cache when a breakpoint at this
+ probe is hit. */
+ enum probe_action action;
};
static const struct probe_info probe_info[] =
{
- { "init_start", 0 },
- { "init_complete", 1 },
- { "map_start", 0 },
- { "reloc_complete", 1 },
- { "unmap_start", 0 },
- { "unmap_complete", 1 },
+ { "init_start", 0, LM_CACHE_NO_ACTION },
+ { "init_complete", 1, LM_CACHE_RELOAD },
+ { "map_start", 0, LM_CACHE_NO_ACTION },
+ { "reloc_complete", 1, LM_CACHE_UPDATE_OR_RELOAD },
+ { "unmap_start", 0, LM_CACHE_NO_ACTION },
+ { "unmap_complete", 1, LM_CACHE_RELOAD },
};
#define NUM_PROBES ARRAY_SIZE (probe_info)
@@ -347,6 +371,10 @@ struct svr4_info
/* Named probes in the dynamic linker. */
VEC (probe_p) *probes[NUM_PROBES];
+
+ /* List of objects loaded from the inferior, used by the
+ probes-based interface to support incremental updates. */
+ struct so_list *solib_cache;
};
/* Per-program-space data key. */
@@ -365,6 +393,18 @@ free_probes (struct svr4_info *info)
memset (info->probes, 0, sizeof (info->probes));
}
+/* Free any cached solibs. */
+
+static void
+free_solib_cache (struct svr4_info *info)
+{
+ if (info->solib_cache == NULL)
+ return;
+
+ svr4_free_library_list (&info->solib_cache);
+ info->solib_cache = NULL;
+}
+
static void
svr4_pspace_data_cleanup (struct program_space *pspace, void *arg)
{
@@ -375,6 +415,7 @@ svr4_pspace_data_cleanup (struct program_space *pspace, void *arg)
return;
free_probes (info);
+ free_solib_cache (info);
xfree (info);
}
@@ -1036,6 +1077,36 @@ svr4_free_library_list (void *p_list)
}
}
+/* Copy library list. */
+
+static struct so_list *
+svr4_copy_library_list (struct so_list *src)
+{
+ struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
+ struct so_list *dst = NULL;
+ struct so_list **link = &dst;
+
+ while (src != NULL)
+ {
+ struct so_list *new;
+
+ new = XZALLOC (struct so_list);
+
+ memcpy (new, src, sizeof (struct so_list));
+
+ new->lm_info = xmalloc (lmo->link_map_size);
+ memcpy (new->lm_info, src->lm_info, lmo->link_map_size);
+
+ new->next = NULL;
+ *link = new;
+ link = &new->next;
+
+ src = src->next;
+ }
+
+ return dst;
+}
+
#ifdef HAVE_LIBEXPAT
#include "xml-support.h"
@@ -1215,15 +1286,17 @@ svr4_default_sos (void)
return new;
}
-/* Read the whole inferior libraries chain starting at address LM. Add the
- entries to the tail referenced by LINK_PTR_PTR. Ignore the first entry if
- IGNORE_FIRST and set global MAIN_LM_ADDR according to it. */
+/* Read the whole inferior libraries chain starting at address LM.
+ Expect the first entry in the chain's previous entry to be PREV_LM.
+ Add the entries to the tail referenced by LINK_PTR_PTR. Ignore the
+ first entry if IGNORE_FIRST and set global MAIN_LM_ADDR according
+ to it. Returns nonzero upon success. */
-static void
-svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
- int ignore_first)
+static int
+svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
+ struct so_list ***link_ptr_ptr, int ignore_first)
{
- CORE_ADDR prev_lm = 0, next_lm;
+ CORE_ADDR next_lm;
for (; lm != 0; prev_lm = lm, lm = next_lm)
{
@@ -1240,7 +1313,7 @@ svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
if (new->lm_info == NULL)
{
do_cleanups (old_chain);
- break;
+ return 0;
}
next_lm = new->lm_info->l_next;
@@ -1251,7 +1324,7 @@ svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
paddress (target_gdbarch, prev_lm),
paddress (target_gdbarch, new->lm_info->l_prev));
do_cleanups (old_chain);
- break;
+ return 0;
}
/* For SVR4 versions, the first entry in the link map is for the
@@ -1297,6 +1370,8 @@ svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
**link_ptr_ptr = new;
*link_ptr_ptr = &new->next;
}
+
+ return 1;
}
/* Implement the "current_sos" target_so_ops method. */
@@ -1333,6 +1408,10 @@ svr4_current_sos (void)
info = get_svr4_info ();
+ /* If we have a cached result then return a copy. */
+ if (info->solib_cache != NULL)
+ return svr4_copy_library_list (info->solib_cache);
+
/* Always locate the debug struct, in case it has moved. */
info->debug_base = 0;
locate_base (info);
@@ -1355,7 +1434,7 @@ svr4_current_sos (void)
`struct so_list' nodes. */
lm = solib_svr4_r_map (info);
if (lm)
- svr4_read_so_list (lm, &link_ptr, ignore_first);
+ svr4_read_so_list (lm, 0, &link_ptr, ignore_first);
/* On Solaris, the dynamic linker is not in the normal list of
shared objects, so make sure we pick it up too. Having
@@ -1363,7 +1442,7 @@ svr4_current_sos (void)
for skipping dynamic linker resolver code. */
lm = solib_svr4_r_ldsomap (info);
if (lm)
- svr4_read_so_list (lm, &link_ptr, 0);
+ svr4_read_so_list (lm, 0, &link_ptr, 0);
discard_cleanups (back_to);
@@ -1494,6 +1573,146 @@ solib_event_probe_at (struct bp_location *loc, struct probe_and_info *result)
return NULL;
}
+/* Decide what action to take when the specified solib event probe is
+ hit. */
+
+static enum probe_action
+solib_event_probe_action (struct probe_and_info *pi)
+{
+ enum probe_action action;
+ int update;
+ struct obj_section *os;
+ unsigned probe_argc;
+ struct svr4_info *info;
+ CORE_ADDR debug_base;
+
+ action = pi->info->action;
+ if (action == LM_CACHE_NO_ACTION || action == LM_CACHE_INVALIDATE)
+ return action;
+
+ gdb_assert (action == LM_CACHE_RELOAD
+ || action == LM_CACHE_UPDATE_OR_RELOAD);
+
+ os = find_pc_section (pi->probe->address);
+ if (os == NULL)
+ return LM_CACHE_INVALIDATE;
+
+ /* Check that an appropriate number of arguments has been supplied.
+ We expect:
+ arg1: Lmid_t lmid (mandatory)
+ arg2: struct r_debug *r_debug (mandatory)
+ arg3: struct link_map *new (optional, for incremental updates) */
+ probe_argc = get_probe_argument_count (os->objfile, pi->probe);
+ if (probe_argc == 2)
+ action = LM_CACHE_RELOAD;
+ else if (probe_argc < 2)
+ return LM_CACHE_INVALIDATE;
+
+ /* We only currently support the global namespace (PR gdb/11839).
+ If the probe's r_debug doesn't match the global r_debug then
+ this event refers to some other namespace and must be ignored. */
+ info = get_svr4_info ();
+
+ /* Always locate the debug struct, in case it has moved. */
+ info->debug_base = 0;
+ locate_base (info);
+
+ debug_base = value_as_address (evaluate_probe_argument (os->objfile,
+ pi->probe, 1));
+
+ if (debug_base != info->debug_base)
+ return LM_CACHE_NO_ACTION;
+
+ return action;
+}
+
+/* Populate the solib cache with by reading the entire list of shared
+ objects from the inferior. */
+
+static void
+solib_cache_update_full (void)
+{
+ struct svr4_info *info = get_svr4_info ();
+
+ gdb_assert (info->solib_cache == NULL);
+ info->solib_cache = svr4_current_sos ();
+}
+
+/* Update the solib cache starting from the link-map supplied by the
+ linker in the probe's third argument. Returns nonzero if the list
+ was successfully updated, or zero to indicate failure. */
+
+static int
+solib_cache_update_incremental (struct probe_and_info *pi)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct so_list *tail, **link;
+ struct obj_section *os;
+ CORE_ADDR lm;
+
+ if (info->solib_cache == NULL)
+ return 0;
+
+ tail = info->solib_cache;
+ while (tail->next)
+ tail = tail->next;
+ link = &tail->next;
+
+ os = find_pc_section (pi->probe->address);
+ if (os == NULL)
+ return 0;
+
+ lm = value_as_address (evaluate_probe_argument (os->objfile,
+ pi->probe, 2));
+
+ if (lm == 0)
+ return 0;
+
+ return svr4_read_so_list (lm, tail->lm_info->lm_addr, &link, 0);
+}
+
+/* Update the solib cache as appropriate when using the probes-based
+ linker interface. Do nothing if using the standard interface. */
+
+static void
+svr4_handle_solib_event (bpstat bs)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct probe_and_info buf, *pi;
+ enum probe_action action;
+
+ /* It is possible that this function will be called incorrectly
+ by the handle_solib_event in handle_inferior_event if GDB goes
+ fully multi-target. */
+ gdb_assert (bs != NULL);
+
+ if (!info->using_probes)
+ return;
+
+ pi = solib_event_probe_at (bs->bp_location_at, &buf);
+ if (pi == NULL)
+ action = LM_CACHE_INVALIDATE; /* Should never happen. */
+ else
+ action = solib_event_probe_action (pi);
+
+ if (action == LM_CACHE_NO_ACTION)
+ return;
+
+ if (action == LM_CACHE_UPDATE_OR_RELOAD)
+ {
+ if (solib_cache_update_incremental (pi))
+ return;
+
+ action = LM_CACHE_RELOAD;
+ }
+
+ free_solib_cache (info);
+ if (action == LM_CACHE_INVALIDATE)
+ return;
+
+ solib_cache_update_full ();
+}
+
/* Helper function for svr4_update_solib_event_breakpoints. */
static int
@@ -2446,6 +2665,9 @@ svr4_solib_create_inferior_hook (int from_tty)
info = get_svr4_info ();
+ /* Free any solibs cached by the probes-based linker interface. */
+ free_solib_cache (info);
+
/* Relocate the main executable if necessary. */
svr4_relocate_main_executable ();
@@ -2712,5 +2934,6 @@ _initialize_svr4_solib (void)
svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
svr4_so_ops.same = svr4_same;
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
+ svr4_so_ops.handle_solib_event = svr4_handle_solib_event;
svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
}
diff --git a/gdb/testsuite/gdb.base/info-shared-solib1.c b/gdb/testsuite/gdb.base/info-shared-solib1.c
new file mode 100644
index 0000000..9979ee7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared-solib1.c
@@ -0,0 +1,24 @@
+/* Copyright 2012 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/>. */
+
+#include <stdio.h>
+
+int
+foo (int n)
+{
+ printf ("foo %d\n", n);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-shared-solib2.c b/gdb/testsuite/gdb.base/info-shared-solib2.c
new file mode 100644
index 0000000..d4ed1e6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared-solib2.c
@@ -0,0 +1,24 @@
+/* Copyright 2012 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/>. */
+
+#include <stdio.h>
+
+int
+bar (int n)
+{
+ printf ("bar %d\n", n);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-shared.c b/gdb/testsuite/gdb.base/info-shared.c
new file mode 100644
index 0000000..d699a11
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared.c
@@ -0,0 +1,48 @@
+/* Copyright 2012 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/>. */
+
+#include <dlfcn.h>
+
+void
+stop ()
+{
+}
+
+int
+main ()
+{
+ void *handle1, *handle2;
+ void (*func)(int);
+
+ handle1 = dlopen (SHLIB1_NAME, RTLD_LAZY);
+ stop ();
+
+ handle2 = dlopen (SHLIB2_NAME, RTLD_LAZY);
+ stop ();
+
+ func = (void (*)(int)) dlsym (handle1, "foo");
+ func (1);
+
+ func = (void (*)(int)) dlsym (handle2, "bar");
+ func (2);
+
+ dlclose (handle1);
+ stop ();
+
+ dlclose (handle2);
+ stop ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-shared.exp b/gdb/testsuite/gdb.base/info-shared.exp
new file mode 100644
index 0000000..f3a74c6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared.exp
@@ -0,0 +1,139 @@
+# Copyright 2012 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/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+ return 0
+}
+
+standard_testfile
+
+set lib1name $testfile-solib1
+set srcfile_lib1 $srcdir/$subdir/$lib1name.c
+set binfile_lib1 $objdir/$subdir/$lib1name.so
+set define1 -DSHLIB1_NAME\=\"$binfile_lib1\"
+
+set lib2name $testfile-solib2
+set srcfile_lib2 $srcdir/$subdir/$lib2name.c
+set binfile_lib2 $objdir/$subdir/$lib2name.so
+set define2 -DSHLIB2_NAME\=\"$binfile_lib2\"
+
+if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 \
+ [list additional_flags=-fPIC]] != "" } {
+ untested "Could not compile $binfile_lib1."
+ return -1
+}
+
+if { [gdb_compile_shlib $srcfile_lib2 $binfile_lib2 \
+ [list additional_flags=-fPIC]] != "" } {
+ untested "Could not compile $binfile_lib2."
+ return -1
+}
+
+set cflags "$define1 $define2"
+if { [prepare_for_testing $testfile.exp $testfile $srcfile \
+ [list additional_flags=$cflags libs=-ldl]] } {
+ return -1
+}
+
+# Run "info sharedlibrary" and check for the presence or absence of
+# our libraries.
+proc check_info_shared { test expect1 expect2 } {
+ global lib1name
+ global lib2name
+ global gdb_prompt
+
+ set actual1 0
+ set actual2 0
+
+ gdb_test_multiple "info sharedlibrary" $test {
+ -re $lib1name {
+ set actual1 1
+ exp_continue
+ }
+ -re $lib2name {
+ set actual2 1
+ exp_continue
+ }
+ -re "\r\n$gdb_prompt $" {
+ if { $actual1 == $expect1 && $actual2 == $expect2 } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ }
+}
+
+# Set up breakpoints.
+gdb_test "break stop" {Breakpoint [0-9]+ at .*}
+gdb_test_no_output "set breakpoint pending on"
+gdb_test "break foo" {Breakpoint [0-9]+ \(foo\) pending\.}
+gdb_test "break bar" {Breakpoint [0-9]+ \(bar\) pending\.}
+
+# Check neither of the libraries are loaded at the start.
+gdb_test "start" {Temporary breakpoint [0-9]+, .* in main \(\)}
+check_info_shared "info sharedlibrary #1" 0 0
+
+# Run to the first stop and check that only the first library is loaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #2" 1 0
+
+# Run to the second stop and check that both libraries are loaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #3" 1 1
+
+# Check that the next stop is in foo.
+gdb_test "c" {Breakpoint [0-9]+, .* in foo \(\) from .*}
+
+# Check that the next stop is in bar.
+gdb_test "c" {Breakpoint [0-9]+, .* in bar \(\) from .*}
+
+# Restart the inferior and make sure there are no breakpoint reset
+# errors. These can happen with the probes-based runtime linker
+# interface if the cache is not cleared correctly.
+set test "restart"
+gdb_test_multiple "run" $test {
+ -re {Start it from the beginning\? \(y or n\) } {
+ send_gdb "y\n"
+ exp_continue
+ }
+ -re {Error in re-setting breakpoint} {
+ fail $test
+ }
+ -re "\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+# We're at the first stop. Check that only the first library is loaded.
+check_info_shared "info sharedlibrary #4" 1 0
+
+# Run to the second stop and check that both libraries are loaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #5" 1 1
+
+# Check that the next stop is in foo.
+gdb_test "c" {Breakpoint [0-9]+, .* in foo \(\) from .*}
+
+# Check that the next stop is in bar.
+gdb_test "c" {Breakpoint [0-9]+, .* in bar \(\) from .*}
+
+# Run to the next stop and check that the first library has been unloaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #6" 0 1
+
+# Run to the last stop and check that both libraries are gone.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #7" 0 0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFA 3/4] Improved linker-debugger interface
2012-07-12 12:34 [RFA 0/4] Improved linker-debugger interface Gary Benson
2012-07-12 12:35 ` [RFA 1/4] " Gary Benson
@ 2012-07-12 12:36 ` Gary Benson
2012-07-17 18:01 ` Sergio Durigan Junior
2012-07-17 21:57 ` Jan Kratochvil
2012-07-12 12:36 ` [RFA 4/4] " Gary Benson
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-12 12:36 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 138 bytes --]
Hi all,
This patch encapsulates part of the probes API which the final
patch in this series uses.
Cheers,
Gary
--
http://gbenson.net/
[-- Attachment #2: rtld-stap-1.patch --]
[-- Type: text/plain, Size: 2651 bytes --]
2012-07-12 Gary Benson <gbenson@redhat.com>
* probe.h (get_probe_argument_count): New declaration.
(evaluate_probe_argument): Likewise.
* probe.c (get_probe_argument_count): New function.
(evaluate_probe_argument): Likewise.
(probe_safe_evaluate_at_pc): Use the above new functions.
diff --git a/gdb/probe.h b/gdb/probe.h
index 8d44ca2..1d29b87 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -210,6 +210,17 @@ extern void info_probes_for_ops (char *arg, int from_tty,
extern struct cmd_list_element **info_probes_cmdlist_get (void);
+/* Return the argument count of the specified probe. */
+
+extern unsigned get_probe_argument_count (struct objfile *objfile,
+ struct probe *probe);
+
+/* Evaluate argument N of the specified probe. */
+
+extern struct value *evaluate_probe_argument (struct objfile *objfile,
+ struct probe *probe,
+ unsigned n);
+
/* A convenience function that finds a probe at the PC in FRAME and
evaluates argument N, with 0 <= N < number_of_args. If there is no
probe at that location, or if the probe does not have enough arguments,
diff --git a/gdb/probe.c b/gdb/probe.c
index 77f3b13..a61f4ea 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -658,6 +658,30 @@ info_probes_command (char *arg, int from_tty)
/* See comments in probe.h. */
+unsigned
+get_probe_argument_count (struct objfile *objfile, struct probe *probe)
+{
+ gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
+
+ return objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
+ probe);
+}
+
+/* See comments in probe.h. */
+
+struct value *
+evaluate_probe_argument (struct objfile *objfile, struct probe *probe,
+ unsigned n)
+{
+ gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
+
+ return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
+ probe,
+ n);
+}
+
+/* See comments in probe.h. */
+
struct value *
probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
{
@@ -668,17 +692,12 @@ probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
probe = find_probe_by_pc (get_frame_pc (frame), &objfile);
if (!probe)
return NULL;
- gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
- n_probes
- = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
- probe);
+ n_probes = get_probe_argument_count (objfile, probe);
if (n >= n_probes)
return NULL;
- return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
- probe,
- n);
+ return evaluate_probe_argument (objfile, probe, n);
}
/* See comment in probe.h. */
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 2/4] Improved linker-debugger interface
2012-07-12 12:36 ` [RFA 2/4] " Gary Benson
@ 2012-07-13 9:41 ` Gary Benson
0 siblings, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-13 9:41 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 284 bytes --]
Gary Benson wrote:
> This patch implements the basics of the improved runtime linker
> interface in GDB--everything except the incremental library
> list loading, basically.
This is a minor update to make the testcase use standard_output_file.
Thanks,
Gary
--
http://gbenson.net/
[-- Attachment #2: rtld-stap-2.patch --]
[-- Type: text/plain, Size: 21179 bytes --]
gdb/
2012-07-12 Gary Benson <gbenson@redhat.com>
* infrun.c (set_stop_on_solib_events): New function.
(_initialize_infrun): Use the above for "set stop-on-solib-events".
* solib-svr4.c (probe_info): New struct.
(probe_info): New static variable.
(NUM_PROBES): New definition.
(svr4_info): New fields "using_probes" and "probes".
(free_probes): New function.
(svr4_pspace_data_cleanup): Free probes.
(probe_and_info): New struct.
(solib_event_probe_at): New function.
(svr4_update_solib_event_breakpoint): Likewise.
(svr4_update_solib_event_breakpoints): Likewise.
(svr4_create_solib_event_breakpoints): Likewise.
(enable_break): Free probes before creating breakpoints.
Use svr4_create_solib_event_breakpoints to create breakpoints.
(_initialize_svr4_solib): Initialise svr4_so_ops.update_breakpoints.
* solib.h (update_solib_breakpoints): New function definition.
* solib.c (update_solib_breakpoints): New function.
* solist.h (target_so_ops): New field "update_breakpoints".
gdb/testsuite
2012-07-12 Gary Benson <gbenson@redhat.com>
* gdb.base/break-interp.exp (solib_bp): New constant.
(reach_1): Use the above instead of "_dl_debug_state".
(test_attach): Likewise.
(test_ld): Likewise.
* gdb.base/break-probes.exp: New file.
* gdb.base/break-probes.c: Likewise.
* gdb.base/break-probes-solib.c: Likewise.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 11f981f..89fbcfc 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -361,6 +361,16 @@ static struct symbol *step_start_function;
/* Nonzero if we want to give control to the user when we're notified
of shared library events by the dynamic linker. */
int stop_on_solib_events;
+
+/* Enable or disable optional shared library event breakpoints
+ as appropriate when the above flag is changed. */
+
+static void
+set_stop_on_solib_events (char *args, int from_tty, struct cmd_list_element *c)
+{
+ update_solib_breakpoints ();
+}
+
static void
show_stop_on_solib_events (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
@@ -7204,7 +7214,7 @@ Show stopping for shared library events."), _("\
If nonzero, gdb will give control to the user when the dynamic linker\n\
notifies gdb of shared library events. The most common event of interest\n\
to the user would be loading/unloading of a new library."),
- NULL,
+ set_stop_on_solib_events,
show_stop_on_solib_events,
&setlist, &showlist);
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 307e483..c111f04 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -47,6 +47,8 @@
#include "auxv.h"
#include "exceptions.h"
+#include "probe.h"
+
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
static void svr4_relocate_main_executable (void);
@@ -92,6 +94,32 @@ static const char * const solib_break_names[] =
NULL
};
+/* A list of named probes which, if present in the dynamic linker,
+ allow more fine-grained breakpoints to be placed on shared library
+ events. */
+
+struct probe_info
+{
+ /* The name of the probe. */
+ const char *name;
+
+ /* Nonzero if this probe must be stopped at even when
+ stop-on-solib-events is off. */
+ int mandatory;
+};
+
+static const struct probe_info probe_info[] =
+{
+ { "init_start", 0 },
+ { "init_complete", 1 },
+ { "map_start", 0 },
+ { "reloc_complete", 1 },
+ { "unmap_start", 0 },
+ { "unmap_complete", 1 },
+};
+
+#define NUM_PROBES ARRAY_SIZE (probe_info)
+
static const char * const bkpt_names[] =
{
"_start",
@@ -313,17 +341,41 @@ struct svr4_info
CORE_ADDR interp_text_sect_high;
CORE_ADDR interp_plt_sect_low;
CORE_ADDR interp_plt_sect_high;
+
+ /* Nonzero if we are using the probes-based interface. */
+ int using_probes;
+
+ /* Named probes in the dynamic linker. */
+ VEC (probe_p) *probes[NUM_PROBES];
};
/* Per-program-space data key. */
static const struct program_space_data *solib_svr4_pspace_data;
+/* Free any allocated probe vectors. */
+
+static void
+free_probes (struct svr4_info *info)
+{
+ int i;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ VEC_free (probe_p, info->probes[i]);
+
+ memset (info->probes, 0, sizeof (info->probes));
+}
+
static void
svr4_pspace_data_cleanup (struct program_space *pspace, void *arg)
{
struct svr4_info *info;
info = program_space_data (pspace, solib_svr4_pspace_data);
+ if (info == NULL)
+ return;
+
+ free_probes (info);
+
xfree (info);
}
@@ -1400,6 +1452,168 @@ exec_entry_point (struct bfd *abfd, struct target_ops *targ)
targ);
}
+/* A probe and its associated information structure. */
+
+struct probe_and_info
+{
+ /* The probe. */
+ struct probe *probe;
+
+ /* The probe_info from which the probe was created. */
+ const struct probe_info *info;
+};
+
+/* Get the solib event probe at the specified location, and the
+ probe_info the probe was created with. Returns NULL if no solib
+ event probe exists at that location. */
+
+static struct probe_and_info *
+solib_event_probe_at (struct bp_location *loc, struct probe_and_info *result)
+{
+ struct svr4_info *info = get_svr4_info ();
+ int i;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ {
+ struct probe *probe;
+ int ix;
+
+ for (ix = 0; VEC_iterate (probe_p, info->probes[i], ix, probe); ++ix)
+ {
+ if (loc->pspace == current_program_space
+ && loc->address == probe->address)
+ {
+ result->info = &probe_info[i];
+ result->probe = probe;
+
+ return result;
+ }
+ }
+ }
+
+ return NULL;
+}
+
+/* Helper function for svr4_update_solib_event_breakpoints. */
+
+static int
+svr4_update_solib_event_breakpoint (struct breakpoint *b, void *arg)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct bp_location *loc;
+
+ if (b->type != bp_shlib_event)
+ return 0;
+
+ for (loc = b->loc; loc; loc = loc->next)
+ {
+ struct probe_and_info buf, *pi;
+
+ pi = solib_event_probe_at (loc, &buf);
+ if (pi != NULL)
+ {
+ if (!pi->info->mandatory)
+ b->enable_state = (stop_on_solib_events
+ ? bp_enabled : bp_disabled);
+
+ return 0;
+ }
+ }
+
+ return 0;
+}
+
+/* Enable or disable optional solib event breakpoints as appropriate.
+ Called whenever stop_on_solib_events is changed. */
+
+static void
+svr4_update_solib_event_breakpoints (void)
+{
+ struct svr4_info *info = get_svr4_info ();
+
+ if (info->using_probes)
+ iterate_over_breakpoints (svr4_update_solib_event_breakpoint, NULL);
+}
+
+/* Both the SunOS and the SVR4 dynamic linkers call a marker function
+ before and after mapping and unmapping shared libraries. The sole
+ purpose of this method is to allow debuggers to set a breakpoint so
+ they can track these changes.
+
+ Some versions of the glibc dynamic linker contain named probes
+ to allow more fine grained stopping. Given the address of the
+ original marker function, this function attempts to find these
+ probes, and if found, sets breakpoints on those instead. If the
+ probes aren't found, a single breakpoint is set on the original
+ marker function. */
+
+static void
+svr4_create_solib_event_breakpoints (struct gdbarch *gdbarch, CORE_ADDR address)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct obj_section *os;
+
+ os = find_pc_section (address);
+ if (os != NULL)
+ {
+ int with_prefix;
+
+ for (with_prefix = 0; with_prefix <= 1; with_prefix++)
+ {
+ int all_probes_found = 1;
+ int i;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ {
+ char name[32] = { '\0' };
+
+ /* Fedora 17, RHEL 6.2, and RHEL 6.3 shipped with an
+ early version of the probes code in which the probes'
+ names were prefixed with "rtld_". The locations and
+ arguments of the probes are otherwise the same, so we
+ check for the prefixed version if the unprefixed
+ probes are not found. */
+
+ if (with_prefix)
+ strncat (name, "rtld_", sizeof (name));
+
+ strncat (name, probe_info[i].name, sizeof (name));
+
+ info->probes[i] = find_probes_in_objfile (os->objfile, "rtld",
+ name);
+
+ if (!VEC_length (probe_p, info->probes[i]))
+ {
+ free_probes (info);
+ all_probes_found = 0;
+ break;
+ }
+ }
+
+ if (all_probes_found)
+ {
+ info->using_probes = 1;
+
+ for (i = 0; i < NUM_PROBES; i++)
+ {
+ struct probe *probe;
+ int ix;
+
+ for (ix = 0;
+ VEC_iterate (probe_p, info->probes[i], ix, probe);
+ ++ix)
+ create_solib_event_breakpoint (gdbarch, probe->address);
+ }
+
+ svr4_update_solib_event_breakpoints ();
+ return;
+ }
+ }
+ }
+
+ create_solib_event_breakpoint (gdbarch, address);
+}
+
/* Helper function for gdb_bfd_lookup_symbol. */
static int
@@ -1452,6 +1666,9 @@ enable_break (struct svr4_info *info, int from_tty)
info->interp_text_sect_low = info->interp_text_sect_high = 0;
info->interp_plt_sect_low = info->interp_plt_sect_high = 0;
+ free_probes (info);
+ info->using_probes = 0;
+
/* If we already have a shared library list in the target, and
r_debug contains r_brk, set the breakpoint there - this should
mean r_brk has already been relocated. Assume the dynamic linker
@@ -1483,7 +1700,7 @@ enable_break (struct svr4_info *info, int from_tty)
That knowledge is encoded in the address, if it's Thumb the low bit
is 1. However, we've stripped that info above and it's not clear
what all the consequences are of passing a non-addr_bits_remove'd
- address to create_solib_event_breakpoint. The call to
+ address to svr4_create_solib_event_breakpoints. The call to
find_pc_section verifies we know about the address and have some
hope of computing the right kind of breakpoint to use (via
symbol info). It does mean that GDB needs to be pointed at a
@@ -1521,7 +1738,7 @@ enable_break (struct svr4_info *info, int from_tty)
+ bfd_section_size (tmp_bfd, interp_sect);
}
- create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);
return 1;
}
}
@@ -1676,7 +1893,8 @@ enable_break (struct svr4_info *info, int from_tty)
if (sym_addr != 0)
{
- create_solib_event_breakpoint (target_gdbarch, load_addr + sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch,
+ load_addr + sym_addr);
xfree (interp_name);
return 1;
}
@@ -1702,7 +1920,7 @@ enable_break (struct svr4_info *info, int from_tty)
sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch,
sym_addr,
¤t_target);
- create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);
return 1;
}
}
@@ -1718,7 +1936,7 @@ enable_break (struct svr4_info *info, int from_tty)
sym_addr = gdbarch_convert_from_func_ptr_addr (target_gdbarch,
sym_addr,
¤t_target);
- create_solib_event_breakpoint (target_gdbarch, sym_addr);
+ svr4_create_solib_event_breakpoints (target_gdbarch, sym_addr);
return 1;
}
}
@@ -2494,4 +2712,5 @@ _initialize_svr4_solib (void)
svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
svr4_so_ops.same = svr4_same;
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
+ svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
}
diff --git a/gdb/solib.h b/gdb/solib.h
index 7a2ff84..65e3857 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -91,4 +91,8 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
void *),
void *data);
+/* Enable or disable optional solib event breakpoints as appropriate. */
+
+extern void update_solib_breakpoints (void);
+
#endif /* SOLIB_H */
diff --git a/gdb/solib.c b/gdb/solib.c
index 90439ba..dda0130 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1209,6 +1209,18 @@ no_shared_libraries (char *ignored, int from_tty)
objfile_purge_solibs ();
}
+/* See solib.h. */
+
+void
+update_solib_breakpoints (void)
+{
+ struct target_so_ops *ops = solib_ops (target_gdbarch);
+
+ if (ops->update_breakpoints != NULL)
+ ops->update_breakpoints ();
+}
+
+
/* Reload shared libraries, but avoid reloading the same symbol file
we already have loaded. */
diff --git a/gdb/solist.h b/gdb/solist.h
index 7413e3b..0d9046d 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -149,6 +149,13 @@ struct target_so_ops
core file (in particular, for readonly sections). */
int (*keep_data_in_core) (CORE_ADDR vaddr,
unsigned long size);
+
+ /* Enable or disable optional solib event breakpoints as
+ appropriate. This should be called whenever
+ stop_on_solib_events is changed. This pointer can be
+ NULL, in which case no enabling or disabling is necessary
+ for this target. */
+ void (*update_breakpoints) (void);
};
/* Free the memory associated with a (so_list *). */
diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
index 4012e66..7aa6537 100644
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -109,12 +109,19 @@ proc strip_debug {dest} {
}
}
+# The marker function for the standard runtime linker interface is
+# _dl_debug_state. The probes-based interface has no specific marker
+# function; the probe we will stop on (init_start) is in dl_main so we
+# check for that.
+
+set solib_bp {(_dl_debug_state|dl_main)}
+
# Implementation of reach.
proc reach_1 {func command displacement} {
- global gdb_prompt expect_out
+ global gdb_prompt expect_out solib_bp
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
# Breakpoint on _dl_debug_state can have problems due to its overlap
# with the existing internal breakpoint from GDB.
gdb_test_no_output "set stop-on-solib-events 1"
@@ -142,21 +149,21 @@ proc reach_1 {func command displacement} {
exp_continue
}
-re "Breakpoint \[0-9\]+, \\.?(__GI_)?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
fail $test
} else {
pass $test
}
}
-re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?(__GI_)?$func \\(\\).*\r\n$gdb_prompt $" {
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
fail $test
} else {
pass $test
}
}
-re "Stopped due to (spurious )?shared library event.*\r\n$gdb_prompt $" {
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
if {$debug_state_count == 0} {
# First stop does not yet relocate the _start function
# descriptor on ppc64.
@@ -175,7 +182,7 @@ proc reach_1 {func command displacement} {
fail $test_displacement
}
- if {$func == "_dl_debug_state"} {
+ if {$func == $solib_bp} {
gdb_test_no_output "set stop-on-solib-events 0"
}
}
@@ -357,7 +364,7 @@ proc test_attach {file displacement {relink_args ""}} {
}
proc test_ld {file ifmain trynosym displacement} {
- global srcdir subdir gdb_prompt expect_out inferior_exited_re
+ global srcdir subdir gdb_prompt expect_out inferior_exited_re solib_bp
# First test normal `file'-command loaded $FILE with symbols.
@@ -385,9 +392,9 @@ proc test_ld {file ifmain trynosym displacement} {
gdb_test_no_output "set args ${objdir}/${subdir}/$binfile_test" "set args OBJDIR/${subdir}/$binfile_test"
}
- reach "_dl_debug_state" "run" $displacement
+ reach $solib_bp "run" $displacement
- gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?_dl_debug_state\\M.*" "dl bt"
+ gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?$solib_bp\\M.*" "dl bt"
if $ifmain {
reach "main" continue "NONE"
@@ -399,7 +406,7 @@ proc test_ld {file ifmain trynosym displacement} {
# Try re-run if the new PIE displacement takes effect.
gdb_test "kill" "" "kill" {Kill the program being debugged\? \(y or n\) } "y"
- reach "_dl_debug_state" "run" $displacement
+ reach $solib_bp "run" $displacement
if $ifmain {
test_core $file $displacement
@@ -431,7 +438,7 @@ proc test_ld {file ifmain trynosym displacement} {
gdb_test "exec-file $file" "exec-file $escapedfile" "load"
if $ifmain {
- reach "_dl_debug_state" run $displacement
+ reach $solib_bp run $displacement
# Use two separate gdb_test_multiple statements to avoid timeouts due
# to slow processing of wildcard capturing long output
diff --git a/gdb/testsuite/gdb.base/break-probes-solib.c b/gdb/testsuite/gdb.base/break-probes-solib.c
new file mode 100644
index 0000000..9979ee7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-probes-solib.c
@@ -0,0 +1,24 @@
+/* Copyright 2012 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/>. */
+
+#include <stdio.h>
+
+int
+foo (int n)
+{
+ printf ("foo %d\n", n);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/break-probes.c b/gdb/testsuite/gdb.base/break-probes.c
new file mode 100644
index 0000000..a778099
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-probes.c
@@ -0,0 +1,26 @@
+/* Copyright 2012 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/>. */
+
+#include <dlfcn.h>
+
+int
+main ()
+{
+ void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+
+ dlclose (handle);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/break-probes.exp b/gdb/testsuite/gdb.base/break-probes.exp
new file mode 100644
index 0000000..94b125a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/break-probes.exp
@@ -0,0 +1,76 @@
+# Copyright 2012 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/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+ return 0
+}
+
+standard_testfile
+
+set libname $testfile-solib
+set srcfile_lib $srcdir/$subdir/$libname.c
+set binfile_lib [standard_output_file $libname.so]
+
+set normal_bp "_dl_debug_state"
+set probes_bp "dl_main"
+
+if { [gdb_compile_shlib $srcfile_lib $binfile_lib \
+ [list additional_flags=-fPIC]] != "" } {
+ untested "Could not compile $binfile_lib."
+ return -1
+}
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile \
+ [list additional_flags=-DSHLIB_NAME\=\"$binfile_lib\" libs=-ldl]] } {
+ return -1
+}
+
+# Enable stop-on-solib-events
+gdb_test_no_output "set stop-on-solib-events 1"
+
+# Run to the first stop
+gdb_test "run" ".*Stopped due to shared library event.*"
+
+# XFAIL if we are not using probes
+set test "ensure using probes"
+set using_probes 0
+gdb_test_multiple "bt" $test {
+ -re "#0 +\[^\r\n\]*\\m(__GI_)?$normal_bp\\M.*$gdb_prompt $" {
+ xfail $test
+ }
+ -re "#0 +\[^\r\n\]*\\m(__GI_)?$probes_bp\\M.*$gdb_prompt $" {
+ pass $test
+ set using_probes 1
+ }
+}
+
+if { $using_probes } {
+ # Run til it loads our library
+ set test "run til our library loads"
+ set loaded_library 0
+ while { !$loaded_library } {
+ gdb_test_multiple "c" $test {
+ -re "Inferior loaded $binfile_lib\\M.*$gdb_prompt $" {
+ pass $test
+ set loaded_library 1
+ }
+ -re "Stopped due to shared library event\\M.*$gdb_prompt $" {
+ }
+ }
+ }
+
+ # Call something to ensure that relocation occurred
+ gdb_test "call foo(23)" "foo 23.*\\\$.* = .*"
+}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 4/4] Improved linker-debugger interface
2012-07-12 12:36 ` [RFA 4/4] " Gary Benson
@ 2012-07-13 9:42 ` Gary Benson
2012-07-13 12:20 ` Gary Benson
2012-07-17 18:11 ` Sergio Durigan Junior
0 siblings, 2 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-13 9:42 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 236 bytes --]
Gary Benson wrote:
> This patch builds on the probes-based runtime linker interface,
> adding incremental library list loading.
Again, a minor update to make the testcase use standard_output_file.
Thanks,
Gary
--
http://gbenson.net/
[-- Attachment #2: rtld-stap-3.patch --]
[-- Type: text/plain, Size: 25819 bytes --]
gdb/
2012-07-12 Gary Benson <gbenson@redhat.com>
* breakpoint.h (handle_solib_event): Moved function definition
to solib.h, and added a new parameter.
* breakpoint.c (handle_solib_event): Moved function to solib.c
and added a new parameter.
(bpstat_stop_status): Pass new argument to handle_solib_event.
* solib.h (breakpoint.h): New include.
(handle_solib_event): Moved function definition from breakpoint.h
and added a new parameter.
* solib.c (handle_solib_event): Moved function from breakpoint.c
and added a new parameter.
* infrun.c (handle_inferior_event): Pass new argument to
handle_solib_event.
* solist.h (breakpoint.h): New include.
(target_so_ops): New field "handle_solib_event".
* solib-svr4.c (svr4_free_library_list): New forward declaration.
(probe_action): New enum.
(probe_info): New field "action".
(svr4_info): New field "solib_cache".
(free_solib_cache): New function.
(svr4_pspace_data_cleanup): Call the above.
(svr4_copy_library_list): New function.
(svr4_read_so_list): New parameter "prev_lm".
Changed return type from void to int.
Return nonzero on success, zero on error.
(svr4_current_sos): Return cached list if available.
Add new argument to calls to svr4_read_so_list.
(solib_event_probe_action): New function.
(solib_cache_update_full): Likewise.
(solib_cache_update_incremental): Likewise.
(svr4_handle_solib_event): Likewise.
(svr4_solib_create_inferior_hook): Free any cached solibs.
(_initialize_svr4_solib): Initialise svr4_so_ops.handle_solib_event.
gdb/testsuite
2012-07-12 Gary Benson <gbenson@redhat.com>
* gdb.base/info-shared.exp: New file.
* gdb.base/info-shared.c: Likewise.
* gdb.base/info-shared-solib1.c: Likewise.
* gdb.base/info-shared-solib2.c: Likewise.
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 4e4f875..f678702 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1515,6 +1515,4 @@ extern int user_breakpoint_p (struct breakpoint *);
/* Attempt to determine architecture of location identified by SAL. */
extern struct gdbarch *get_sal_arch (struct symtab_and_line sal);
-extern void handle_solib_event (void);
-
#endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6b9faf3..51c98a7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5205,7 +5205,7 @@ bpstat_stop_status (struct address_space *aspace,
{
if (bs->breakpoint_at && bs->breakpoint_at->type == bp_shlib_event)
{
- handle_solib_event ();
+ handle_solib_event (bs);
break;
}
}
@@ -5301,25 +5301,6 @@ handle_jit_event (void)
target_terminal_inferior ();
}
-/* Handle an solib event by calling solib_add. */
-
-void
-handle_solib_event (void)
-{
- clear_program_space_solib_cache (current_inferior ()->pspace);
-
- /* Check for any newly added shared libraries if we're supposed to
- be adding them automatically. Switch terminal for any messages
- produced by breakpoint_re_set. */
- target_terminal_ours_for_output ();
-#ifdef SOLIB_ADD
- SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
-#else
- solib_add (NULL, 0, ¤t_target, auto_solib_add);
-#endif
- target_terminal_inferior ();
-}
-
/* Prepare WHAT final decision for infrun. */
/* Decide what infrun needs to do with this bpstat. */
diff --git a/gdb/solib.h b/gdb/solib.h
index 65e3857..6e92ddc 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -21,6 +21,9 @@
#ifndef SOLIB_H
#define SOLIB_H
+/* For bpstat. */
+#include "breakpoint.h"
+
/* Forward decl's for prototypes */
struct so_list;
struct target_ops;
@@ -91,6 +94,13 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_from_symtab (bfd *abfd,
void *),
void *data);
+/* Handle an solib event by calling solib_add. Targets which handle
+ solib events using breakpoints must pass a valid bpstat. Targets
+ which handle solib events using some other mechanism should pass
+ NULL. */
+
+extern void handle_solib_event (bpstat bs);
+
/* Enable or disable optional solib event breakpoints as appropriate. */
extern void update_solib_breakpoints (void);
diff --git a/gdb/solib.c b/gdb/solib.c
index dda0130..072fe4d 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1212,6 +1212,30 @@ no_shared_libraries (char *ignored, int from_tty)
/* See solib.h. */
void
+handle_solib_event (bpstat bs)
+{
+ struct target_so_ops *ops = solib_ops (target_gdbarch);
+
+ if (ops->handle_solib_event != NULL)
+ ops->handle_solib_event (bs);
+
+ clear_program_space_solib_cache (current_inferior ()->pspace);
+
+ /* Check for any newly added shared libraries if we're supposed to
+ be adding them automatically. Switch terminal for any messages
+ produced by breakpoint_re_set. */
+ target_terminal_ours_for_output ();
+#ifdef SOLIB_ADD
+ SOLIB_ADD (NULL, 0, ¤t_target, auto_solib_add);
+#else
+ solib_add (NULL, 0, ¤t_target, auto_solib_add);
+#endif
+ target_terminal_inferior ();
+}
+
+/* See solib.h. */
+
+void
update_solib_breakpoints (void)
{
struct target_so_ops *ops = solib_ops (target_gdbarch);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 89fbcfc..7c621ca 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3318,7 +3318,7 @@ handle_inferior_event (struct execution_control_state *ecs)
context_switch (ecs->ptid);
regcache = get_thread_regcache (ecs->ptid);
- handle_solib_event ();
+ handle_solib_event (NULL);
ecs->event_thread->control.stop_bpstat
= bpstat_stop_status (get_regcache_aspace (regcache),
diff --git a/gdb/solist.h b/gdb/solist.h
index 0d9046d..ea580a9 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -23,6 +23,8 @@
#define SO_NAME_MAX_PATH_SIZE 512 /* FIXME: Should be dynamic */
/* For domain_enum domain. */
#include "symtab.h"
+/* For bpstat. */
+#include "breakpoint.h"
/* Forward declaration for target specific link map information. This
struct is opaque to all but the target specific file. */
@@ -150,6 +152,13 @@ struct target_so_ops
int (*keep_data_in_core) (CORE_ADDR vaddr,
unsigned long size);
+ /* Target-specific handling of solib events. For targets which
+ handle solib events using breakpoints a valid bpstat must be
+ passed. Targets which handle solib events using some other
+ mechanism should pass NULL. This pointer can be NULL, in which
+ case no specific handling is necessary for this target. */
+ void (*handle_solib_event) (bpstat bs);
+
/* Enable or disable optional solib event breakpoints as
appropriate. This should be called whenever
stop_on_solib_events is changed. This pointer can be
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index c111f04..ebb8c3c 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -52,6 +52,7 @@
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
static void svr4_relocate_main_executable (void);
+static void svr4_free_library_list (void *p_list);
/* Link map info to include in an allocated so_list entry. */
@@ -94,6 +95,25 @@ static const char * const solib_break_names[] =
NULL
};
+/* What to do with the link_map cache. */
+
+enum probe_action
+ {
+ /* No action is required. The cache is still valid. */
+ LM_CACHE_NO_ACTION,
+
+ /* Something went wrong. The cache may be invalid and must be
+ cleared. Do not attempt further caching at this stop. */
+ LM_CACHE_INVALIDATE,
+
+ /* The cache should be reloaded now. */
+ LM_CACHE_RELOAD,
+
+ /* Attempt to incrementally update the cache. If the update
+ fails or is not possible, fall back to LM_CACHE_RELOAD. */
+ LM_CACHE_UPDATE_OR_RELOAD
+ };
+
/* A list of named probes which, if present in the dynamic linker,
allow more fine-grained breakpoints to be placed on shared library
events. */
@@ -106,16 +126,20 @@ struct probe_info
/* Nonzero if this probe must be stopped at even when
stop-on-solib-events is off. */
int mandatory;
+
+ /* What to do with the link_map cache when a breakpoint at this
+ probe is hit. */
+ enum probe_action action;
};
static const struct probe_info probe_info[] =
{
- { "init_start", 0 },
- { "init_complete", 1 },
- { "map_start", 0 },
- { "reloc_complete", 1 },
- { "unmap_start", 0 },
- { "unmap_complete", 1 },
+ { "init_start", 0, LM_CACHE_NO_ACTION },
+ { "init_complete", 1, LM_CACHE_RELOAD },
+ { "map_start", 0, LM_CACHE_NO_ACTION },
+ { "reloc_complete", 1, LM_CACHE_UPDATE_OR_RELOAD },
+ { "unmap_start", 0, LM_CACHE_NO_ACTION },
+ { "unmap_complete", 1, LM_CACHE_RELOAD },
};
#define NUM_PROBES ARRAY_SIZE (probe_info)
@@ -347,6 +371,10 @@ struct svr4_info
/* Named probes in the dynamic linker. */
VEC (probe_p) *probes[NUM_PROBES];
+
+ /* List of objects loaded from the inferior, used by the
+ probes-based interface to support incremental updates. */
+ struct so_list *solib_cache;
};
/* Per-program-space data key. */
@@ -365,6 +393,18 @@ free_probes (struct svr4_info *info)
memset (info->probes, 0, sizeof (info->probes));
}
+/* Free any cached solibs. */
+
+static void
+free_solib_cache (struct svr4_info *info)
+{
+ if (info->solib_cache == NULL)
+ return;
+
+ svr4_free_library_list (&info->solib_cache);
+ info->solib_cache = NULL;
+}
+
static void
svr4_pspace_data_cleanup (struct program_space *pspace, void *arg)
{
@@ -375,6 +415,7 @@ svr4_pspace_data_cleanup (struct program_space *pspace, void *arg)
return;
free_probes (info);
+ free_solib_cache (info);
xfree (info);
}
@@ -1036,6 +1077,36 @@ svr4_free_library_list (void *p_list)
}
}
+/* Copy library list. */
+
+static struct so_list *
+svr4_copy_library_list (struct so_list *src)
+{
+ struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
+ struct so_list *dst = NULL;
+ struct so_list **link = &dst;
+
+ while (src != NULL)
+ {
+ struct so_list *new;
+
+ new = XZALLOC (struct so_list);
+
+ memcpy (new, src, sizeof (struct so_list));
+
+ new->lm_info = xmalloc (lmo->link_map_size);
+ memcpy (new->lm_info, src->lm_info, lmo->link_map_size);
+
+ new->next = NULL;
+ *link = new;
+ link = &new->next;
+
+ src = src->next;
+ }
+
+ return dst;
+}
+
#ifdef HAVE_LIBEXPAT
#include "xml-support.h"
@@ -1215,15 +1286,17 @@ svr4_default_sos (void)
return new;
}
-/* Read the whole inferior libraries chain starting at address LM. Add the
- entries to the tail referenced by LINK_PTR_PTR. Ignore the first entry if
- IGNORE_FIRST and set global MAIN_LM_ADDR according to it. */
+/* Read the whole inferior libraries chain starting at address LM.
+ Expect the first entry in the chain's previous entry to be PREV_LM.
+ Add the entries to the tail referenced by LINK_PTR_PTR. Ignore the
+ first entry if IGNORE_FIRST and set global MAIN_LM_ADDR according
+ to it. Returns nonzero upon success. */
-static void
-svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
- int ignore_first)
+static int
+svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
+ struct so_list ***link_ptr_ptr, int ignore_first)
{
- CORE_ADDR prev_lm = 0, next_lm;
+ CORE_ADDR next_lm;
for (; lm != 0; prev_lm = lm, lm = next_lm)
{
@@ -1240,7 +1313,7 @@ svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
if (new->lm_info == NULL)
{
do_cleanups (old_chain);
- break;
+ return 0;
}
next_lm = new->lm_info->l_next;
@@ -1251,7 +1324,7 @@ svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
paddress (target_gdbarch, prev_lm),
paddress (target_gdbarch, new->lm_info->l_prev));
do_cleanups (old_chain);
- break;
+ return 0;
}
/* For SVR4 versions, the first entry in the link map is for the
@@ -1297,6 +1370,8 @@ svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
**link_ptr_ptr = new;
*link_ptr_ptr = &new->next;
}
+
+ return 1;
}
/* Implement the "current_sos" target_so_ops method. */
@@ -1333,6 +1408,10 @@ svr4_current_sos (void)
info = get_svr4_info ();
+ /* If we have a cached result then return a copy. */
+ if (info->solib_cache != NULL)
+ return svr4_copy_library_list (info->solib_cache);
+
/* Always locate the debug struct, in case it has moved. */
info->debug_base = 0;
locate_base (info);
@@ -1355,7 +1434,7 @@ svr4_current_sos (void)
`struct so_list' nodes. */
lm = solib_svr4_r_map (info);
if (lm)
- svr4_read_so_list (lm, &link_ptr, ignore_first);
+ svr4_read_so_list (lm, 0, &link_ptr, ignore_first);
/* On Solaris, the dynamic linker is not in the normal list of
shared objects, so make sure we pick it up too. Having
@@ -1363,7 +1442,7 @@ svr4_current_sos (void)
for skipping dynamic linker resolver code. */
lm = solib_svr4_r_ldsomap (info);
if (lm)
- svr4_read_so_list (lm, &link_ptr, 0);
+ svr4_read_so_list (lm, 0, &link_ptr, 0);
discard_cleanups (back_to);
@@ -1494,6 +1573,146 @@ solib_event_probe_at (struct bp_location *loc, struct probe_and_info *result)
return NULL;
}
+/* Decide what action to take when the specified solib event probe is
+ hit. */
+
+static enum probe_action
+solib_event_probe_action (struct probe_and_info *pi)
+{
+ enum probe_action action;
+ int update;
+ struct obj_section *os;
+ unsigned probe_argc;
+ struct svr4_info *info;
+ CORE_ADDR debug_base;
+
+ action = pi->info->action;
+ if (action == LM_CACHE_NO_ACTION || action == LM_CACHE_INVALIDATE)
+ return action;
+
+ gdb_assert (action == LM_CACHE_RELOAD
+ || action == LM_CACHE_UPDATE_OR_RELOAD);
+
+ os = find_pc_section (pi->probe->address);
+ if (os == NULL)
+ return LM_CACHE_INVALIDATE;
+
+ /* Check that an appropriate number of arguments has been supplied.
+ We expect:
+ arg1: Lmid_t lmid (mandatory)
+ arg2: struct r_debug *r_debug (mandatory)
+ arg3: struct link_map *new (optional, for incremental updates) */
+ probe_argc = get_probe_argument_count (os->objfile, pi->probe);
+ if (probe_argc == 2)
+ action = LM_CACHE_RELOAD;
+ else if (probe_argc < 2)
+ return LM_CACHE_INVALIDATE;
+
+ /* We only currently support the global namespace (PR gdb/11839).
+ If the probe's r_debug doesn't match the global r_debug then
+ this event refers to some other namespace and must be ignored. */
+ info = get_svr4_info ();
+
+ /* Always locate the debug struct, in case it has moved. */
+ info->debug_base = 0;
+ locate_base (info);
+
+ debug_base = value_as_address (evaluate_probe_argument (os->objfile,
+ pi->probe, 1));
+
+ if (debug_base != info->debug_base)
+ return LM_CACHE_NO_ACTION;
+
+ return action;
+}
+
+/* Populate the solib cache with by reading the entire list of shared
+ objects from the inferior. */
+
+static void
+solib_cache_update_full (void)
+{
+ struct svr4_info *info = get_svr4_info ();
+
+ gdb_assert (info->solib_cache == NULL);
+ info->solib_cache = svr4_current_sos ();
+}
+
+/* Update the solib cache starting from the link-map supplied by the
+ linker in the probe's third argument. Returns nonzero if the list
+ was successfully updated, or zero to indicate failure. */
+
+static int
+solib_cache_update_incremental (struct probe_and_info *pi)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct so_list *tail, **link;
+ struct obj_section *os;
+ CORE_ADDR lm;
+
+ if (info->solib_cache == NULL)
+ return 0;
+
+ tail = info->solib_cache;
+ while (tail->next)
+ tail = tail->next;
+ link = &tail->next;
+
+ os = find_pc_section (pi->probe->address);
+ if (os == NULL)
+ return 0;
+
+ lm = value_as_address (evaluate_probe_argument (os->objfile,
+ pi->probe, 2));
+
+ if (lm == 0)
+ return 0;
+
+ return svr4_read_so_list (lm, tail->lm_info->lm_addr, &link, 0);
+}
+
+/* Update the solib cache as appropriate when using the probes-based
+ linker interface. Do nothing if using the standard interface. */
+
+static void
+svr4_handle_solib_event (bpstat bs)
+{
+ struct svr4_info *info = get_svr4_info ();
+ struct probe_and_info buf, *pi;
+ enum probe_action action;
+
+ /* It is possible that this function will be called incorrectly
+ by the handle_solib_event in handle_inferior_event if GDB goes
+ fully multi-target. */
+ gdb_assert (bs != NULL);
+
+ if (!info->using_probes)
+ return;
+
+ pi = solib_event_probe_at (bs->bp_location_at, &buf);
+ if (pi == NULL)
+ action = LM_CACHE_INVALIDATE; /* Should never happen. */
+ else
+ action = solib_event_probe_action (pi);
+
+ if (action == LM_CACHE_NO_ACTION)
+ return;
+
+ if (action == LM_CACHE_UPDATE_OR_RELOAD)
+ {
+ if (solib_cache_update_incremental (pi))
+ return;
+
+ action = LM_CACHE_RELOAD;
+ }
+
+ free_solib_cache (info);
+ if (action == LM_CACHE_INVALIDATE)
+ return;
+
+ solib_cache_update_full ();
+}
+
/* Helper function for svr4_update_solib_event_breakpoints. */
static int
@@ -2446,6 +2665,9 @@ svr4_solib_create_inferior_hook (int from_tty)
info = get_svr4_info ();
+ /* Free any solibs cached by the probes-based linker interface. */
+ free_solib_cache (info);
+
/* Relocate the main executable if necessary. */
svr4_relocate_main_executable ();
@@ -2712,5 +2934,6 @@ _initialize_svr4_solib (void)
svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
svr4_so_ops.same = svr4_same;
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
+ svr4_so_ops.handle_solib_event = svr4_handle_solib_event;
svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
}
diff --git a/gdb/testsuite/gdb.base/info-shared-solib1.c b/gdb/testsuite/gdb.base/info-shared-solib1.c
new file mode 100644
index 0000000..9979ee7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared-solib1.c
@@ -0,0 +1,24 @@
+/* Copyright 2012 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/>. */
+
+#include <stdio.h>
+
+int
+foo (int n)
+{
+ printf ("foo %d\n", n);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-shared-solib2.c b/gdb/testsuite/gdb.base/info-shared-solib2.c
new file mode 100644
index 0000000..d4ed1e6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared-solib2.c
@@ -0,0 +1,24 @@
+/* Copyright 2012 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/>. */
+
+#include <stdio.h>
+
+int
+bar (int n)
+{
+ printf ("bar %d\n", n);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-shared.c b/gdb/testsuite/gdb.base/info-shared.c
new file mode 100644
index 0000000..d699a11
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared.c
@@ -0,0 +1,48 @@
+/* Copyright 2012 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/>. */
+
+#include <dlfcn.h>
+
+void
+stop ()
+{
+}
+
+int
+main ()
+{
+ void *handle1, *handle2;
+ void (*func)(int);
+
+ handle1 = dlopen (SHLIB1_NAME, RTLD_LAZY);
+ stop ();
+
+ handle2 = dlopen (SHLIB2_NAME, RTLD_LAZY);
+ stop ();
+
+ func = (void (*)(int)) dlsym (handle1, "foo");
+ func (1);
+
+ func = (void (*)(int)) dlsym (handle2, "bar");
+ func (2);
+
+ dlclose (handle1);
+ stop ();
+
+ dlclose (handle2);
+ stop ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/info-shared.exp b/gdb/testsuite/gdb.base/info-shared.exp
new file mode 100644
index 0000000..f3a74c6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/info-shared.exp
@@ -0,0 +1,139 @@
+# Copyright 2012 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/>.
+
+if { [skip_shlib_tests] || [is_remote target] } {
+ return 0
+}
+
+standard_testfile
+
+set lib1name $testfile-solib1
+set srcfile_lib1 $srcdir/$subdir/$lib1name.c
+set binfile_lib1 [standard_output_file $lib1name.so]
+set define1 -DSHLIB1_NAME\=\"$binfile_lib1\"
+
+set lib2name $testfile-solib2
+set srcfile_lib2 $srcdir/$subdir/$lib2name.c
+set binfile_lib2 [standard_output_file $lib2name.so]
+set define2 -DSHLIB2_NAME\=\"$binfile_lib2\"
+
+if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 \
+ [list additional_flags=-fPIC]] != "" } {
+ untested "Could not compile $binfile_lib1."
+ return -1
+}
+
+if { [gdb_compile_shlib $srcfile_lib2 $binfile_lib2 \
+ [list additional_flags=-fPIC]] != "" } {
+ untested "Could not compile $binfile_lib2."
+ return -1
+}
+
+set cflags "$define1 $define2"
+if { [prepare_for_testing $testfile.exp $testfile $srcfile \
+ [list additional_flags=$cflags libs=-ldl]] } {
+ return -1
+}
+
+# Run "info sharedlibrary" and check for the presence or absence of
+# our libraries.
+proc check_info_shared { test expect1 expect2 } {
+ global lib1name
+ global lib2name
+ global gdb_prompt
+
+ set actual1 0
+ set actual2 0
+
+ gdb_test_multiple "info sharedlibrary" $test {
+ -re $lib1name {
+ set actual1 1
+ exp_continue
+ }
+ -re $lib2name {
+ set actual2 1
+ exp_continue
+ }
+ -re "\r\n$gdb_prompt $" {
+ if { $actual1 == $expect1 && $actual2 == $expect2 } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ }
+}
+
+# Set up breakpoints.
+gdb_test "break stop" {Breakpoint [0-9]+ at .*}
+gdb_test_no_output "set breakpoint pending on"
+gdb_test "break foo" {Breakpoint [0-9]+ \(foo\) pending\.}
+gdb_test "break bar" {Breakpoint [0-9]+ \(bar\) pending\.}
+
+# Check neither of the libraries are loaded at the start.
+gdb_test "start" {Temporary breakpoint [0-9]+, .* in main \(\)}
+check_info_shared "info sharedlibrary #1" 0 0
+
+# Run to the first stop and check that only the first library is loaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #2" 1 0
+
+# Run to the second stop and check that both libraries are loaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #3" 1 1
+
+# Check that the next stop is in foo.
+gdb_test "c" {Breakpoint [0-9]+, .* in foo \(\) from .*}
+
+# Check that the next stop is in bar.
+gdb_test "c" {Breakpoint [0-9]+, .* in bar \(\) from .*}
+
+# Restart the inferior and make sure there are no breakpoint reset
+# errors. These can happen with the probes-based runtime linker
+# interface if the cache is not cleared correctly.
+set test "restart"
+gdb_test_multiple "run" $test {
+ -re {Start it from the beginning\? \(y or n\) } {
+ send_gdb "y\n"
+ exp_continue
+ }
+ -re {Error in re-setting breakpoint} {
+ fail $test
+ }
+ -re "\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+# We're at the first stop. Check that only the first library is loaded.
+check_info_shared "info sharedlibrary #4" 1 0
+
+# Run to the second stop and check that both libraries are loaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #5" 1 1
+
+# Check that the next stop is in foo.
+gdb_test "c" {Breakpoint [0-9]+, .* in foo \(\) from .*}
+
+# Check that the next stop is in bar.
+gdb_test "c" {Breakpoint [0-9]+, .* in bar \(\) from .*}
+
+# Run to the next stop and check that the first library has been unloaded.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #6" 0 1
+
+# Run to the last stop and check that both libraries are gone.
+gdb_test "c" {Breakpoint [0-9]+, .* in stop \(\)}
+check_info_shared "info sharedlibrary #7" 0 0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 4/4] Improved linker-debugger interface
2012-07-13 9:42 ` Gary Benson
@ 2012-07-13 12:20 ` Gary Benson
2012-07-17 18:11 ` Sergio Durigan Junior
1 sibling, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-13 12:20 UTC (permalink / raw)
To: gdb-patches
Gary Benson wrote:
> +/* Decide what action to take when the specified solib event probe is
> + hit. */
> +
> +static enum probe_action
> +solib_event_probe_action (struct probe_and_info *pi)
> +{
> + enum probe_action action;
> + int update;
^^^^^^
I just noticed this variable is never used, will remove it from my tree.
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 3/4] Improved linker-debugger interface
2012-07-12 12:36 ` [RFA 3/4] " Gary Benson
@ 2012-07-17 18:01 ` Sergio Durigan Junior
2012-07-17 21:57 ` Jan Kratochvil
1 sibling, 0 replies; 19+ messages in thread
From: Sergio Durigan Junior @ 2012-07-17 18:01 UTC (permalink / raw)
To: gdb-patches
On Thursday, July 12 2012, Gary Benson wrote:
> This patch encapsulates part of the probes API which the final
> patch in this series uses.
I looked at your patch, and it seems OK to me. Thanks.
> 2012-07-12 Gary Benson <gbenson@redhat.com>
>
> * probe.h (get_probe_argument_count): New declaration.
> (evaluate_probe_argument): Likewise.
> * probe.c (get_probe_argument_count): New function.
> (evaluate_probe_argument): Likewise.
> (probe_safe_evaluate_at_pc): Use the above new functions.
>
> diff --git a/gdb/probe.h b/gdb/probe.h
> index 8d44ca2..1d29b87 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -210,6 +210,17 @@ extern void info_probes_for_ops (char *arg, int from_tty,
>
> extern struct cmd_list_element **info_probes_cmdlist_get (void);
>
> +/* Return the argument count of the specified probe. */
> +
> +extern unsigned get_probe_argument_count (struct objfile *objfile,
> + struct probe *probe);
> +
> +/* Evaluate argument N of the specified probe. */
> +
> +extern struct value *evaluate_probe_argument (struct objfile *objfile,
> + struct probe *probe,
> + unsigned n);
> +
> /* A convenience function that finds a probe at the PC in FRAME and
> evaluates argument N, with 0 <= N < number_of_args. If there is no
> probe at that location, or if the probe does not have enough arguments,
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 77f3b13..a61f4ea 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -658,6 +658,30 @@ info_probes_command (char *arg, int from_tty)
>
> /* See comments in probe.h. */
>
> +unsigned
> +get_probe_argument_count (struct objfile *objfile, struct probe *probe)
> +{
> + gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
> +
> + return objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> + probe);
> +}
> +
> +/* See comments in probe.h. */
> +
> +struct value *
> +evaluate_probe_argument (struct objfile *objfile, struct probe *probe,
> + unsigned n)
> +{
> + gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
> +
> + return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> + probe,
> + n);
> +}
> +
> +/* See comments in probe.h. */
> +
> struct value *
> probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
> {
> @@ -668,17 +692,12 @@ probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
> probe = find_probe_by_pc (get_frame_pc (frame), &objfile);
> if (!probe)
> return NULL;
> - gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
>
> - n_probes
> - = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> - probe);
> + n_probes = get_probe_argument_count (objfile, probe);
> if (n >= n_probes)
> return NULL;
>
> - return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> - probe,
> - n);
> + return evaluate_probe_argument (objfile, probe, n);
> }
>
> /* See comment in probe.h. */
--
Sergio
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 4/4] Improved linker-debugger interface
2012-07-13 9:42 ` Gary Benson
2012-07-13 12:20 ` Gary Benson
@ 2012-07-17 18:11 ` Sergio Durigan Junior
2012-07-18 14:28 ` Jan Kratochvil
2012-07-19 14:38 ` Gary Benson
1 sibling, 2 replies; 19+ messages in thread
From: Sergio Durigan Junior @ 2012-07-17 18:11 UTC (permalink / raw)
To: gdb-patches
On Friday, July 13 2012, Gary Benson wrote:
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index c111f04..ebb8c3c 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> +/* Decide what action to take when the specified solib event probe is
> + hit. */
> +
> +static enum probe_action
> +solib_event_probe_action (struct probe_and_info *pi)
> +{
> + enum probe_action action;
> + int update;
> + struct obj_section *os;
> + unsigned probe_argc;
> + struct svr4_info *info;
> + CORE_ADDR debug_base;
> +
> + action = pi->info->action;
> + if (action == LM_CACHE_NO_ACTION || action == LM_CACHE_INVALIDATE)
> + return action;
> +
> + gdb_assert (action == LM_CACHE_RELOAD
> + || action == LM_CACHE_UPDATE_OR_RELOAD);
> +
> + os = find_pc_section (pi->probe->address);
> + if (os == NULL)
> + return LM_CACHE_INVALIDATE;
> +
> + /* Check that an appropriate number of arguments has been supplied.
> + We expect:
> + arg1: Lmid_t lmid (mandatory)
> + arg2: struct r_debug *r_debug (mandatory)
> + arg3: struct link_map *new (optional, for incremental updates) */
I guess you could rename the arguments listed here to 'arg0', 'arg1' and
'arg2', because `evaluate_probe_argument' takes these numbers as
arguments. Or you could explicitly say that here. Otherwise it will
confuse the reader, IMO.
> + probe_argc = get_probe_argument_count (os->objfile, pi->probe);
> + if (probe_argc == 2)
> + action = LM_CACHE_RELOAD;
> + else if (probe_argc < 2)
> + return LM_CACHE_INVALIDATE;
This is OK...
> + /* We only currently support the global namespace (PR gdb/11839).
> + If the probe's r_debug doesn't match the global r_debug then
> + this event refers to some other namespace and must be ignored. */
> + info = get_svr4_info ();
> +
> + /* Always locate the debug struct, in case it has moved. */
> + info->debug_base = 0;
> + locate_base (info);
> +
> + debug_base = value_as_address (evaluate_probe_argument (os->objfile,
> + pi->probe, 1));
...but what would happen if `evaluate_probe_argument' returned NULL?
It's better to check this, because `value_as_address' calls `value_type'
which does not check NULL pointers.
Currently, only the SystemTap backend is implemented, and if it returns
NULL in this case it would be an error, but it's better to guard your
code IMO.
Thanks,
--
Sergio
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 3/4] Improved linker-debugger interface
2012-07-12 12:36 ` [RFA 3/4] " Gary Benson
2012-07-17 18:01 ` Sergio Durigan Junior
@ 2012-07-17 21:57 ` Jan Kratochvil
2012-07-17 23:42 ` Sergio Durigan Junior
2012-07-19 14:36 ` Gary Benson
1 sibling, 2 replies; 19+ messages in thread
From: Jan Kratochvil @ 2012-07-17 21:57 UTC (permalink / raw)
To: gdb-patches
On Thu, 12 Jul 2012 14:35:54 +0200, Gary Benson wrote:
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -210,6 +210,17 @@ extern void info_probes_for_ops (char *arg, int from_tty,
>
> extern struct cmd_list_element **info_probes_cmdlist_get (void);
>
> +/* Return the argument count of the specified probe. */
/* OBJFILE must be known to have probes support. */
> +
> +extern unsigned get_probe_argument_count (struct objfile *objfile,
> + struct probe *probe);
> +
> +/* Evaluate argument N of the specified probe. */
/* OBJFILE must be known to have probes support. N must be between
0 inclusive and get_probe_argument_count exclusive. */
> +
> +extern struct value *evaluate_probe_argument (struct objfile *objfile,
> + struct probe *probe,
> + unsigned n);
> +
> /* A convenience function that finds a probe at the PC in FRAME and
> evaluates argument N, with 0 <= N < number_of_args. If there is no
> probe at that location, or if the probe does not have enough arguments,
> diff --git a/gdb/probe.c b/gdb/probe.c
> index 77f3b13..a61f4ea 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -658,6 +658,30 @@ info_probes_command (char *arg, int from_tty)
>
> /* See comments in probe.h. */
>
> +unsigned
> +get_probe_argument_count (struct objfile *objfile, struct probe *probe)
> +{
> + gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
> +
> + return objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> + probe);
> +}
> +
> +/* See comments in probe.h. */
> +
> +struct value *
> +evaluate_probe_argument (struct objfile *objfile, struct probe *probe,
> + unsigned n)
> +{
> + gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
> +
> + return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> + probe,
> + n);
probe and n can be on single line, it is 76 columns [yes, it is copy-paste].
> +}
> +
> +/* See comments in probe.h. */
> +
> struct value *
> probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
> {
> @@ -668,17 +692,12 @@ probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
> probe = find_probe_by_pc (get_frame_pc (frame), &objfile);
> if (!probe)
> return NULL;
> - gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
>
> - n_probes
> - = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> - probe);
> + n_probes = get_probe_argument_count (objfile, probe);
> if (n >= n_probes)
> return NULL;
>
> - return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> - probe,
> - n);
> + return evaluate_probe_argument (objfile, probe, n);
> }
>
> /* See comment in probe.h. */
It is pre-approved but please do not check it in without any use, only with
some other patch depending on it.
Thanks,
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 3/4] Improved linker-debugger interface
2012-07-17 21:57 ` Jan Kratochvil
@ 2012-07-17 23:42 ` Sergio Durigan Junior
2012-07-18 7:02 ` Jan Kratochvil
2012-07-19 14:36 ` Gary Benson
1 sibling, 1 reply; 19+ messages in thread
From: Sergio Durigan Junior @ 2012-07-17 23:42 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On Tuesday, July 17 2012, Jan Kratochvil wrote:
> On Thu, 12 Jul 2012 14:35:54 +0200, Gary Benson wrote:
>> --- a/gdb/probe.h
>> +++ b/gdb/probe.h
>> @@ -210,6 +210,17 @@ extern void info_probes_for_ops (char *arg, int from_tty,
>>
>> extern struct cmd_list_element **info_probes_cmdlist_get (void);
>>
>> +/* Return the argument count of the specified probe. */
>
> /* OBJFILE must be known to have probes support. */
I was considering suggesting the removal of the `gdb_assert' calls, and
instead make it a simple check and return properly if the OBJFILE does
not support probes.
unsigned
get_probe_argument_count (struct objfile *objfile, struct probe *probe)
{
if (objfile->sf == NULL || objfile->sf->sym_probe_fns == NULL)
return 0;
return objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
probe);
}
It would cover the case when OBJFILE does not support probe and would
not be less safe.
--
Sergio
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 3/4] Improved linker-debugger interface
2012-07-17 23:42 ` Sergio Durigan Junior
@ 2012-07-18 7:02 ` Jan Kratochvil
2012-07-18 10:36 ` Gary Benson
0 siblings, 1 reply; 19+ messages in thread
From: Jan Kratochvil @ 2012-07-18 7:02 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
On Wed, 18 Jul 2012 01:42:12 +0200, Sergio Durigan Junior wrote:
> I was considering suggesting the removal of the `gdb_assert' calls, and
> instead make it a simple check and return properly if the OBJFILE does
> not support probes.
>
> unsigned
> get_probe_argument_count (struct objfile *objfile, struct probe *probe)
> {
> if (objfile->sf == NULL || objfile->sf->sym_probe_fns == NULL)
> return 0;
>
> return objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> probe);
> }
>
> It would cover the case when OBJFILE does not support probe and would
> not be less safe.
In fact why 'struct probe' does not contain its 'struct objfile *'? It does
not make sense to pass probe with different objfile. So the parameter
'objfile' should not be passed at all.
Thanks,
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 3/4] Improved linker-debugger interface
2012-07-18 7:02 ` Jan Kratochvil
@ 2012-07-18 10:36 ` Gary Benson
0 siblings, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-18 10:36 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Sergio Durigan Junior, gdb-patches
Jan Kratochvil wrote:
> On Wed, 18 Jul 2012 01:42:12 +0200, Sergio Durigan Junior wrote:
> > I was considering suggesting the removal of the `gdb_assert'
> > calls, and instead make it a simple check and return properly
> > if the OBJFILE does not support probes.
> >
> > unsigned
> > get_probe_argument_count (struct objfile *objfile, struct probe *probe)
> > {
> > if (objfile->sf == NULL || objfile->sf->sym_probe_fns == NULL)
> > return 0;
> >
> > return objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> > probe);
> > }
> >
> > It would cover the case when OBJFILE does not support probe and
> > would not be less safe.
>
> In fact why 'struct probe' does not contain its 'struct objfile *'?
> It does not make sense to pass probe with different objfile. So the
> parameter 'objfile' should not be passed at all.
That would certainly simplify my code. Sergio, is this possible to
do?
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 0/4] Improved linker-debugger interface
2012-07-12 12:34 [RFA 0/4] Improved linker-debugger interface Gary Benson
` (3 preceding siblings ...)
2012-07-12 12:36 ` [RFA 2/4] " Gary Benson
@ 2012-07-18 14:08 ` Gary Benson
4 siblings, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-18 14:08 UTC (permalink / raw)
To: gdb-patches
Gary Benson wrote:
> This patch series implements an improved debug interface with the
> runtime linker to help address the following bugs...
I'm preparing an updated version of this patchset, so please don't
spend any time reviewing it.
Thanks,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 4/4] Improved linker-debugger interface
2012-07-17 18:11 ` Sergio Durigan Junior
@ 2012-07-18 14:28 ` Jan Kratochvil
2012-07-18 15:11 ` Sergio Durigan Junior
2012-07-19 14:38 ` Gary Benson
1 sibling, 1 reply; 19+ messages in thread
From: Jan Kratochvil @ 2012-07-18 14:28 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
On Tue, 17 Jul 2012 20:11:07 +0200, Sergio Durigan Junior wrote:
> On Friday, July 13 2012, Gary Benson wrote:
> > + arg1: Lmid_t lmid (mandatory)
> > + arg2: struct r_debug *r_debug (mandatory)
> > + arg3: struct link_map *new (optional, for incremental updates) */
>
> I guess you could rename the arguments listed here to 'arg0', 'arg1' and
> 'arg2', because `evaluate_probe_argument' takes these numbers as
> arguments. Or you could explicitly say that here. Otherwise it will
> confuse the reader, IMO.
Could you clarify the 1-counting vs. 0-count in: evaluate_probe_argument,
compile_to_ax. Maybe it is not fully clear even in
sym_evaluate_probe_argument, sym_compile_to_ax.
(It is already clear in probe_safe_evaluate_at_pc.)
OK to check it in as obvious if the new text is clear enough (0 <= x < count).
> > + debug_base = value_as_address (evaluate_probe_argument (os->objfile,
> > + pi->probe, 1));
>
> ...but what would happen if `evaluate_probe_argument' returned NULL?
> It's better to check this, because `value_as_address' calls `value_type'
> which does not check NULL pointers.
>
> Currently, only the SystemTap backend is implemented, and if it returns
> NULL in this case it would be an error, but it's better to guard your
> code IMO.
Currently the API comment defines "returning a value corresponding to it.".
There is no "or NULL if evaluation error occurs" or anything like that,
therefore it IMNSHO means the returned value is non-NULL.
Therefore I find correct for Gary to assume the returned value is non-NULL.
Thanks,
Jan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 4/4] Improved linker-debugger interface
2012-07-18 14:28 ` Jan Kratochvil
@ 2012-07-18 15:11 ` Sergio Durigan Junior
0 siblings, 0 replies; 19+ messages in thread
From: Sergio Durigan Junior @ 2012-07-18 15:11 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On Wednesday, July 18 2012, Jan Kratochvil wrote:
> On Tue, 17 Jul 2012 20:11:07 +0200, Sergio Durigan Junior wrote:
>> On Friday, July 13 2012, Gary Benson wrote:
>> > + arg1: Lmid_t lmid (mandatory)
>> > + arg2: struct r_debug *r_debug (mandatory)
>> > + arg3: struct link_map *new (optional, for incremental updates) */
>>
>> I guess you could rename the arguments listed here to 'arg0', 'arg1' and
>> 'arg2', because `evaluate_probe_argument' takes these numbers as
>> arguments. Or you could explicitly say that here. Otherwise it will
>> confuse the reader, IMO.
>
> Could you clarify the 1-counting vs. 0-count in: evaluate_probe_argument,
> compile_to_ax. Maybe it is not fully clear even in
> sym_evaluate_probe_argument, sym_compile_to_ax.
> (It is already clear in probe_safe_evaluate_at_pc.)
Ok.
>> > + debug_base = value_as_address (evaluate_probe_argument (os->objfile,
>> > + pi->probe, 1));
>>
>> ...but what would happen if `evaluate_probe_argument' returned NULL?
>> It's better to check this, because `value_as_address' calls `value_type'
>> which does not check NULL pointers.
>>
>> Currently, only the SystemTap backend is implemented, and if it returns
>> NULL in this case it would be an error, but it's better to guard your
>> code IMO.
>
> Currently the API comment defines "returning a value corresponding to it.".
> There is no "or NULL if evaluation error occurs" or anything like that,
> therefore it IMNSHO means the returned value is non-NULL.
>
> Therefore I find correct for Gary to assume the returned value is non-NULL.
I will not argue.
--
Sergio
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 3/4] Improved linker-debugger interface
2012-07-17 21:57 ` Jan Kratochvil
2012-07-17 23:42 ` Sergio Durigan Junior
@ 2012-07-19 14:36 ` Gary Benson
1 sibling, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-19 14:36 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> On Thu, 12 Jul 2012 14:35:54 +0200, Gary Benson wrote:
> > --- a/gdb/probe.h
> > +++ b/gdb/probe.h
> > @@ -210,6 +210,17 @@ extern void info_probes_for_ops (char *arg, int from_tty,
> >
> > extern struct cmd_list_element **info_probes_cmdlist_get (void);
> >
> > +/* Return the argument count of the specified probe. */
>
> /* OBJFILE must be known to have probes support. */
>
> > +
> > +extern unsigned get_probe_argument_count (struct objfile *objfile,
> > + struct probe *probe);
> > +
> > +/* Evaluate argument N of the specified probe. */
>
> /* OBJFILE must be known to have probes support. N must be between
> 0 inclusive and get_probe_argument_count exclusive. */
[snip]
> > +struct value *
> > +evaluate_probe_argument (struct objfile *objfile, struct probe *probe,
> > + unsigned n)
> > +{
> > + gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
> > +
> > + return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> > + probe,
> > + n);
>
> probe and n can be on single line, it is 76 columns [yes, it is copy-paste].
[snip]
> It is pre-approved but please do not check it in without any use, only with
> some other patch depending on it.
Thank you for the review. I have made these changes (and updated the
patch with Sergio's recent changes) here:
http://www.cygwin.com/ml/gdb-patches/2012-07/msg00332.html
Does the pre-approval still count, or do the changes for Sergio's
stuff warrant another review?
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFA 4/4] Improved linker-debugger interface
2012-07-17 18:11 ` Sergio Durigan Junior
2012-07-18 14:28 ` Jan Kratochvil
@ 2012-07-19 14:38 ` Gary Benson
1 sibling, 0 replies; 19+ messages in thread
From: Gary Benson @ 2012-07-19 14:38 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches
Sergio Durigan Junior wrote:
> On Friday, July 13 2012, Gary Benson wrote:
> > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> > index c111f04..ebb8c3c 100644
> > --- a/gdb/solib-svr4.c
> > +++ b/gdb/solib-svr4.c
[snip]
> > + /* Check that an appropriate number of arguments has been supplied.
> > + We expect:
> > + arg1: Lmid_t lmid (mandatory)
> > + arg2: struct r_debug *r_debug (mandatory)
> > + arg3: struct link_map *new (optional, for incremental updates) */
>
> I guess you could rename the arguments listed here to 'arg0', 'arg1' and
> 'arg2', because `evaluate_probe_argument' takes these numbers as
> arguments. Or you could explicitly say that here. Otherwise it will
> confuse the reader, IMO.
>
> > + probe_argc = get_probe_argument_count (os->objfile, pi->probe);
> > + if (probe_argc == 2)
> > + action = LM_CACHE_RELOAD;
> > + else if (probe_argc < 2)
> > + return LM_CACHE_INVALIDATE;
>
> This is OK...
>
> > + /* We only currently support the global namespace (PR gdb/11839).
> > + If the probe's r_debug doesn't match the global r_debug then
> > + this event refers to some other namespace and must be ignored. */
> > + info = get_svr4_info ();
> > +
> > + /* Always locate the debug struct, in case it has moved. */
> > + info->debug_base = 0;
> > + locate_base (info);
> > +
> > + debug_base = value_as_address (evaluate_probe_argument (os->objfile,
> > + pi->probe, 1));
>
> ...but what would happen if `evaluate_probe_argument' returned NULL?
> It's better to check this, because `value_as_address' calls `value_type'
> which does not check NULL pointers.
>
> Currently, only the SystemTap backend is implemented, and if it returns
> NULL in this case it would be an error, but it's better to guard your
> code IMO.
Thank you for the review. I've updated the comment and added NULL
checks (amongst other things) here:
http://www.cygwin.com/ml/gdb-patches/2012-07/msg00333.html
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-07-19 14:38 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-12 12:34 [RFA 0/4] Improved linker-debugger interface Gary Benson
2012-07-12 12:35 ` [RFA 1/4] " Gary Benson
2012-07-12 12:36 ` [RFA 3/4] " Gary Benson
2012-07-17 18:01 ` Sergio Durigan Junior
2012-07-17 21:57 ` Jan Kratochvil
2012-07-17 23:42 ` Sergio Durigan Junior
2012-07-18 7:02 ` Jan Kratochvil
2012-07-18 10:36 ` Gary Benson
2012-07-19 14:36 ` Gary Benson
2012-07-12 12:36 ` [RFA 4/4] " Gary Benson
2012-07-13 9:42 ` Gary Benson
2012-07-13 12:20 ` Gary Benson
2012-07-17 18:11 ` Sergio Durigan Junior
2012-07-18 14:28 ` Jan Kratochvil
2012-07-18 15:11 ` Sergio Durigan Junior
2012-07-19 14:38 ` Gary Benson
2012-07-12 12:36 ` [RFA 2/4] " Gary Benson
2012-07-13 9:41 ` Gary Benson
2012-07-18 14:08 ` [RFA 0/4] " Gary Benson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox