[Ltrace-devel] -l reintroduced

Edgar E. Iglesias edgar.iglesias at gmail.com
Fri Sep 28 12:40:55 UTC 2012


On Fri, Sep 28, 2012 at 01:00:19AM +0200, Petr Machata wrote:
> Hi list,
> 
> during the development of libs branch (support for tracing inter-library
> calls), I broke support for -l.  This is now back.  -l takes as an
> argument a library pattern, the same that -e and -x take after a @.  It
> only takes a library pattern though, not the full expression, so -l X
> means library X, not symbol X.  -l means: trace any PLT entry that is
> implemented by a library matched by this pattern.  E.g.:
> 
> $ ./ltrace -l libselinux\* -F <(echo 'int lgetfilecon(string, +string*);') -- ls -Z e*.o
> ls->lgetfilecon("execute_program.o", "unconfined_u:object_r:user_home_"...) = 37
> ls->lgetfilecon("expr.o", "unconfined_u:object_r:user_home_"...)            = 37
> -rw-rw-r--. petr petr unconfined_u:object_r:user_home_t:s0 execute_program.o
> -rw-rw-r--. petr petr unconfined_u:object_r:user_home_t:s0 expr.o
> +++ exited (status 0) +++
> 
> With this code come two ways of postponing breakpoint enablement.  The
> one I need for this to work is latent symbol.  The one I hope will be
> useful for MIPS is delayed symbol.  They both work the same way: when a
> breakpoint is requested for symbol, this breakpoint is only realized if
> symbol is neither latent, nor delayed.  If any of these flags are
> cleared, breakpoint enablement is retried.
> 
> These two flags differ in who owns them.  Latent is owned and cleared by
> ltrace core, and used for tracking -l.  Delayed is owned by a backend,
> and cleared when backend calls proc_activate_delayed_symbol.  I hope
> this can be used in lieu of the current hack with a special PLT type.
> I didn't really test whether delayed flag works, but it's so similar to
> latent flag, that hopefully I didn't make any bugs.
> 
> The code is on the branch pmachata/dash-l.  Comments welcome, I intend
> to merge this sometime next week.

Hi,

The patches look good. I've tried to replace the TOPLT_GOTONLY approach
used by MIPS with delayed syms and it works nicely.

Here's how it looks (note that it's rebased on top of my latest
MIPS fixes posted today):

commit 6719ae92c150344a0561cd1f97e5c2241954d9dd
Author: Edgar E. Iglesias <edgar at axis.com>
Date:   Fri Sep 28 14:02:02 2012 +0200

    mipsel: Replace LS_TOPLT_GOTONLY with delayed syms
    
    Signed-off-by: Edgar E. Iglesias <edgar at axis.com>

diff --git a/library.h b/library.h
index cea9156..eb986ea 100644
--- a/library.h
+++ b/library.h
@@ -31,7 +31,6 @@ struct library;
 
 enum toplt {
 	LS_TOPLT_NONE = 0,	/* PLT not used for this symbol. */
-	LS_TOPLT_GOTONLY,	/* Has a GOT entry but no PLT.  */
 	LS_TOPLT_EXEC,		/* PLT for this symbol is executable. */
 };
 
diff --git a/proc.c b/proc.c
index 222e6b4..3dab1e2 100644
--- a/proc.c
+++ b/proc.c
@@ -635,25 +635,12 @@ breakpoint_for_symbol(struct library_symbol *libsym, struct Process *proc)
 	arch_addr_t bp_addr;
 	assert(proc->leader == proc);
 
-	bp_addr = sym2addr(proc, libsym);
-
-	/* For external function pointers, MIPS brings in stub-less funcs
-	 * that point to zero at startup. These symbols get resolved by
-	 * the dynamic linker and are ready to use at arch_dynlink_done().
-	 *
-	 * Allow the backend to add these into the process representation
-	 * but don't put breakpoints at this point. Let the backend fix that
-	 * up later.
-	 *
-	 * XXX This should be changed to delayed symbols.  */
-	if (bp_addr == 0 && libsym->plt_type == LS_TOPLT_GOTONLY) {
-		/* Don't add breakpoints yet.  */
-		return CBS_CONT;
-	}
 	/* Don't enable latent or delayed symbols.  */
 	if (libsym->latent || libsym->delayed)
 		return 0;
 
+	bp_addr = sym2addr(proc, libsym);
+
 	/* If there is an artificial breakpoint on the same address,
 	 * its libsym will be NULL, and we can smuggle our libsym
 	 * there.  That artificial breakpoint is there presumably for
diff --git a/sysdeps/linux-gnu/mipsel/arch.h b/sysdeps/linux-gnu/mipsel/arch.h
index 5f5a7dd..e9a8962 100644
--- a/sysdeps/linux-gnu/mipsel/arch.h
+++ b/sysdeps/linux-gnu/mipsel/arch.h
@@ -56,6 +56,9 @@ struct arch_library_symbol_data {
 	enum mips_plt_type type;
 	GElf_Addr resolved_addr;
 	GElf_Addr stub_addr;
+
+	/* Set for FUNCs that have GOT entries but not PLT entries.  */
+	int gotonly : 1;
 };
 
 #endif /* LTRACE_MIPS_ARCH_H */
diff --git a/sysdeps/linux-gnu/mipsel/plt.c b/sysdeps/linux-gnu/mipsel/plt.c
index 7513c28..16db7e5 100644
--- a/sysdeps/linux-gnu/mipsel/plt.c
+++ b/sysdeps/linux-gnu/mipsel/plt.c
@@ -70,7 +70,7 @@ void *
 sym2addr(Process *proc, struct library_symbol *sym) {
     long ret;
 
-    if (sym->plt_type == LS_TOPLT_NONE) {
+    if (!sym->arch.gotonly && sym->plt_type == LS_TOPLT_NONE) {
         return sym->enter_addr;
     }
 
@@ -225,10 +225,9 @@ static enum callback_status
 cb_enable_breakpoint_sym(struct library_symbol *libsym, void *data)
 {
 	struct Process *proc = data;
-	struct breakpoint *bp;
 	arch_addr_t bp_addr;
 
-	if (libsym->plt_type != LS_TOPLT_GOTONLY)
+	if (!libsym->arch.gotonly)
 		return CBS_CONT;
 
 	/* Update state.  */
@@ -243,28 +242,11 @@ cb_enable_breakpoint_sym(struct library_symbol *libsym, void *data)
 
 	libsym->arch.type = MIPS_PLT_RESOLVED;
 
-	/* Add breakpoint.  */
-	bp = malloc(sizeof *bp);
-	if (bp == NULL
-	    || breakpoint_init(bp, proc, bp_addr, libsym) < 0) {
-		goto fail;
-	}
-
-	if (proc_add_breakpoint(proc, bp) < 0) {
-		breakpoint_destroy(bp);
-		goto fail;
-	}
-
-	if (breakpoint_turn_on(bp, proc) < 0) {
-		proc_remove_breakpoint(proc, bp);
-		breakpoint_destroy(bp);
-		goto fail;
+	/* Now, activate the symbol which will add a breakpoint.  */
+	if (proc_activate_delayed_symbol(proc, libsym) < 0) {
+		fprintf(stderr, "Failed to activate delayed sym %s\n",
+			libsym->name);
 	}
-
-	return CBS_CONT;
-fail:
-	free(bp);
-	fprintf(stderr, "Failed to add breakpoint for %s\n", libsym->name);
 	return CBS_CONT;
 }
 
@@ -317,8 +299,13 @@ arch_elf_add_plt_entry(struct Process *proc, struct ltelf *lte,
 
 	if (bp_addr == 0) {
 		/* Function pointers without PLT entries.  */
-		libsym->plt_type = LS_TOPLT_GOTONLY;
+		libsym->plt_type = LS_TOPLT_NONE;
+		libsym->arch.gotonly = 1;
 		libsym->arch.type = MIPS_PLT_UNRESOLVED;
+
+		/* Delay breakpoint activation until the symbol gets
+		 * resolved.  */
+		libsym->delayed = 1;
 	}
 
 	*ret = libsym;
@@ -333,6 +320,7 @@ fail:
 int
 arch_library_symbol_init(struct library_symbol *libsym)
 {
+	libsym->arch.gotonly = 0;
 	libsym->arch.type = MIPS_PLT_UNRESOLVED;
 	if (libsym->plt_type == LS_TOPLT_NONE) {
 		libsym->arch.type = MIPS_PLT_RESOLVED;



More information about the Ltrace-devel mailing list