[Ltrace-devel] -l reintroduced

Edgar E. Iglesias edgar.iglesias at gmail.com
Mon Oct 1 10:43:34 UTC 2012


On Mon, Oct 01, 2012 at 10:59:33AM +0200, Petr Machata wrote:
> "Edgar E. Iglesias" <edgar.iglesias at gmail.com> writes:
> > The patches look good. I've tried to replace the TOPLT_GOTONLY approach
> > used by MIPS with delayed syms and it works nicely.
> 
> Great, thanks.  I'll merge after I get your MIPS optimizations in.
> 
> The patch looks fine content-wise, but what you sent seems like output
> of git log -p.  I don't think there is a straightforward way to apply
> these (but feel free to educate me).  It seems like passing
> --pretty=email would work, or, obviously, using git format-patch
> instead.

Yes, my bad. It was meant as an inlined RFC in response to the thread and
I didn't think too much about the format. But lets try the following
(rebased ontop of the mips patches from today and your -l patchset).

Can you apply with git am --scissors?

-- >8 --

Subject: [PATCH] mipsel: Replace LS_TOPLT_GOTONLY with delayed syms

Signed-off-by: Edgar E. Iglesias <edgar at axis.com>
---
 library.h                       |    1 -
 proc.c                          |   17 ++---------------
 sysdeps/linux-gnu/mipsel/arch.h |    3 +++
 sysdeps/linux-gnu/mipsel/plt.c  |   38 +++++++++++++-------------------------
 4 files changed, 18 insertions(+), 41 deletions(-)

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..6684867 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 causing a breakpoint to be added.  */
+	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;
-- 
1.7.8.6




More information about the Ltrace-devel mailing list