[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