[Ltrace-devel] [PATCH 0/1] Attempt to fix libdl tracing when attaching to PIDs

[Petr Machata]

Joe Damato <ice799 at gmail.com> writes:

> I noticed that ltrace was unable to trace symbols from runtime loaded
> dynamic objects when attaching to a PID. I've written a small (and
> ugly) patch to try to fix this. Let met know if there is a better way
> to do this and I'll be happy to make the change and submit a fix.

Oh, I think it was approximately the right thing.  We simply need to do
the same things on attach that we do after hitting _start.  I extracted
this to a separate function (process_hit_start), and call it from
open_pid and entry_breakpoint_on_hit.

Since there is no other way to get on dyn_addr in open_pid anyway, we
simply look for the main library, and read it there, like you did in
your patch.  That means we don't need to track that information at entry
breakpoint anymore, and we can get rid of struct entry_breakpoint.

A test case and this patch are sitting on pmachata/dlattach.  Tell me
what you think.

Thanks,
PM

---
 breakpoints.c |   38 +++++++++++---------------------------
 proc.c        |   21 ++++++++++++++++++++-
 proc.h        |    4 ++++
 3 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/breakpoints.c b/breakpoints.c
index 305bdae..a0c6b89 100644
--- a/breakpoints.c
+++ b/breakpoints.c
@@ -343,45 +343,29 @@ disable_all_breakpoints(struct process *proc)
 		  NULL, disable_bp_cb, proc);
 }
 
-/* XXX This is not currently properly supported.  On clone, this is
- * just sliced.  Hopefully at the point that clone is done, this
- * breakpoint is not necessary anymore.  If this use case ends up
- * being important, we need to add a clone and destroy callbacks to
- * breakpoints, and we should also probably drop arch_breakpoint_data
- * so that we don't end up with two different customization mechanisms
- * for one structure.  */
-struct entry_breakpoint {
-	struct breakpoint super;
-	arch_addr_t dyn_addr;
-};
-
 static void
-entry_breakpoint_on_hit(struct breakpoint *a, struct process *proc)
+entry_breakpoint_on_hit(struct breakpoint *bp, struct process *proc)
 {
-	struct entry_breakpoint *bp = (void *)a;
 	if (proc == NULL || proc->leader == NULL)
 		return;
-	arch_addr_t dyn_addr = bp->dyn_addr;
-	delete_breakpoint(proc, bp->super.addr);
-	linkmap_init(proc, dyn_addr);
-	arch_dynlink_done(proc);
+	delete_breakpoint(proc, bp->addr);
+	process_hit_start(proc);
 }
 
 int
 entry_breakpoint_init(struct process *proc,
-		      struct entry_breakpoint *bp, arch_addr_t addr,
+		      struct breakpoint *bp, arch_addr_t addr,
 		      struct library *lib)
 {
 	assert(addr != 0);
-	int err = breakpoint_init(&bp->super, proc, addr, NULL);
+	int err = breakpoint_init(bp, proc, addr, NULL);
 	if (err < 0)
 		return err;
 
 	static struct bp_callbacks entry_callbacks = {
 		.on_hit = entry_breakpoint_on_hit,
 	};
-	bp->super.cbs = &entry_callbacks;
-	bp->dyn_addr = lib->dyn_addr;
+	bp->cbs = &entry_callbacks;
 	return 0;
 }
 
@@ -402,7 +386,7 @@ breakpoints_init(struct process *proc)
 	assert(proc->filename != NULL);
 
 	struct library *lib = ltelf_read_main_binary(proc, proc->filename);
-	struct entry_breakpoint *entry_bp = NULL;
+	struct breakpoint *entry_bp = NULL;
 	int bp_state = 0;
 	int result = -1;
 	switch ((int)(lib != NULL)) {
@@ -410,9 +394,9 @@ breakpoints_init(struct process *proc)
 		switch (bp_state) {
 		case 2:
 			proc_remove_library(proc, lib);
-			proc_remove_breakpoint(proc, &entry_bp->super);
+			proc_remove_breakpoint(proc, entry_bp);
 		case 1:
-			breakpoint_destroy(&entry_bp->super);
+			breakpoint_destroy(entry_bp);
 		}
 		library_destroy(lib);
 		free(entry_bp);
@@ -432,11 +416,11 @@ breakpoints_init(struct process *proc)
 	} else {
 		++bp_state;
 
-		if ((result = proc_add_breakpoint(proc, &entry_bp->super)) < 0)
+		if ((result = proc_add_breakpoint(proc, entry_bp)) < 0)
 			goto fail;
 		++bp_state;
 
-		if ((result = breakpoint_turn_on(&entry_bp->super, proc)) < 0)
+		if ((result = breakpoint_turn_on(entry_bp, proc)) < 0)
 			goto fail;
 	}
 	proc_add_library(proc, lib);
diff --git a/proc.c b/proc.c
index 4de3282..e3badd5 100644
--- a/proc.c
+++ b/proc.c
@@ -508,6 +508,25 @@ start_one_pid(struct process *proc, void *data)
 	return CBS_CONT;
 }
 
+static enum callback_status
+is_main(struct process *proc, struct library *lib, void *data)
+{
+	return CBS_STOP_IF(lib->type == LT_LIBTYPE_MAIN);
+}
+
+void
+process_hit_start(struct process *proc)
+{
+	struct process *leader = proc->leader;
+	assert(leader != NULL);
+
+	struct library *mainlib
+		= proc_each_library(leader, NULL, is_main, NULL);
+	assert(mainlib != NULL);
+	linkmap_init(leader, mainlib->dyn_addr);
+	arch_dynlink_done(leader);
+}
+
 void
 open_pid(pid_t pid)
 {
@@ -564,7 +583,7 @@ open_pid(pid_t pid)
 
 	/* XXX Is there a way to figure out whether _start has
 	 * actually already been hit?  */
-	arch_dynlink_done(leader);
+	process_hit_start(leader);
 
 	/* Done.  Continue everyone.  */
 	each_task(leader, NULL, start_one_pid, NULL);
diff --git a/proc.h b/proc.h
index e46b235..e8032fa 100644
--- a/proc.h
+++ b/proc.h
@@ -181,6 +181,10 @@ struct process *each_task(struct process *proc, struct process *start_after,
 
 void change_process_leader(struct process *proc, struct process *leader);
 
+/* Prepare those parts of process initialization that need to be done
+ * after _start is hit (i.e. after dynamic linking was done).  */
+void process_hit_start(struct process *proc);
+
 /* Remove process from the list of traced processes, drop any events
  * in the event queue, destroy it and free memory.  */
 void remove_process(struct process *proc);
-- 
1.7.6.5