On Thu, Nov 08, 2012 at 03:53:26PM +0100, Sedat Dilek wrote:
> On Fri, Nov 2, 2012 at 11:58 PM, Petr Machata <[email protected]> wrote:
> > "Edgar E. Iglesias" <[email protected]> writes:
> >
> >> On Fri, Nov 02, 2012 at 06:07:50PM +0100, Sedat Dilek wrote:
> >>> This happens with latest ltrace-git...
> >>>
> >>> ./ltrace: proc.c: 755: breakpoint_for_symbol: Assertion `bp->libsym ==
> >>> ((void *)0)' failed.
> >>>
> >> I've seen this bug, and I'm actually inclined to think that the assert
> >> should go away and that we instead should somehow just not use that
> >> break point. But not sure, maybe Petr has more input on this.
> >
> > If you assign more than one symbol to a breakpoint, and that breakpoint
> > hits, there's no way to tell which of the symbols actually hit. So
> > there's no point storing more than one symbol to a breakpoint. We could
> > still drop the assert and just ignore the symbol, as you write.
> >
> > What might make sense is having several breakpoint handlers per
> > breakpoint, so that ltrace allows several service breakpoints per
> > address. Software singlestepping in particular should work well even if
> > it steps over another breakpoint, service or not. (As should normal
> > singlestepping, and I'm not positive that it does, but that's a
> > different can of worms.)
> >
> >> The issue I've seen is with weak symbols that are made to point to
> >> other symbols. I think, I saw it with strlen IIRC. In our glibc,
> >> strlen points resolves to the same addr as __strlen but it's got
> >> it's own PLT entry and symbol.
> >
> > Hmm, how about having two PLT slots, each for a different symbol, but
> > those symbols are aliases in the target library. Ltrace filters
> > aliases, but only for purposes of -x. For -e there's no way to tell
> > that they end up being aliases. And normally you don't care anyway, but
> > on MIPS breakpoints are put not to PLT slots, but to destination
> > addresses. That's how you end up needing two breakpoints on the same
> > address. Could it be the cause?
> >
> > If yes, then the reproducer is as simple as -e func -x func.
> >
> >> I've got a local patch for it, but I unfortunately I haven't had much
> >> time lately to look at it further. My plan was to create a test case
> >> that hopefully can trig the issue on x86 aswell. Then we can work out
> >> a solution from there.
> >
> > I guess on x86, one could trigger the same by IFUNC symbols, but those
> > will need to be supported at all in the first place.
> >
>
> Hi Edgar,
>
> Any news or even patch(es) around for this issue?
>
> I re-tested with ltrace-0.7.0-git1584cfc again to see if any new
> changes did or did not break build on MIPSEL (own toolchain built with
> Freetz build-system).
> Building is fine, but the reported issue still remains.
>
> I can only help with more testing these days...
Hi Sedak,
This was the latest patch in my tree when I had to stop. I need to go
through this before something can be submitted. But if you want to try it
out, that would help.
Thanks,
Edgar
diff --git a/libltrace.c b/libltrace.c
index f4bb2c8..9a6056d 100644
--- a/libltrace.c
+++ b/libltrace.c
@@ -103,6 +103,7 @@ ltrace_init(int argc, char **argv) {
struct opt_p_t *opt_p_tmp;
atexit(normal_exit);
+ signal(SIGPIPE, signal_exit);
signal(SIGINT, signal_exit); /* Detach processes when interrupted */
signal(SIGTERM, signal_exit); /* ... or killed */
diff --git a/proc.c b/proc.c
index 3dab1e2..08cd3c2 100644
--- a/proc.c
+++ b/proc.c
@@ -656,8 +659,13 @@ breakpoint_for_symbol(struct library_symbol *libsym,
struct Process *proc)
struct breakpoint *bp = dict_find_entry(proc->breakpoints,
bp_addr);
if (bp != NULL) {
- assert(bp->libsym == NULL);
- bp->libsym = libsym;
+ fprintf(stderr, "%s: bp_addr=%p bp->libsym=%p libsym=%p %s
%s\n",
+ __func__, bp_addr, bp->libsym, libsym,
+ bp->libsym->name, libsym->name);
+ fflush(NULL);
+// assert(bp->libsym == NULL || bp->libsym == libsym);
+// bp->libsym = libsym;
+ libsym->delayed = 1;
return 0;
}
diff --git a/sysdeps/linux-gnu/mipsel/plt.c b/sysdeps/linux-gnu/mipsel/plt.c
index 7799dfa..7bb00e0 100644
--- a/sysdeps/linux-gnu/mipsel/plt.c
+++ b/sysdeps/linux-gnu/mipsel/plt.c
@@ -251,6 +251,13 @@ void arch_symbol_ret(struct Process *proc, struct
library_symbol *libsym)
return;
}
+ /* For multiple symbols that resolve to the same address, we need
+ * to avoid adding multiple breakpoints. */
+ if (dict_find_entry(proc->leader->breakpoints, resolved_addr) != NULL) {
+ fprintf(stderr, "%s: skip %p %s\n", __func__, resolved_addr,
libsym->name);
+ return;
+ }
+
bp = malloc(sizeof (*bp));
if (bp == NULL) {
fprintf(stderr, "Failed to allocate bp for %s\n",
@@ -361,8 +368,23 @@ arch_elf_add_plt_entry(struct Process *proc, struct ltelf
*lte,
/* Delay breakpoint activation until the symbol gets
* resolved. */
libsym->delayed = 1;
- } else if (mips_elf_is_cpic(lte->ehdr.e_flags)) {
- libsym->arch.pltalways = 1;
+ } else {
+ if (mips_elf_is_cpic(lte->ehdr.e_flags)) {
+ libsym->arch.pltalways = 1;
+ }
+
+ /* For multiple symbols that resolve to the same address,
+ * we need to avoid adding multiple breakpoints. */
+ if (dict_find_entry(proc->leader->breakpoints,
+ bp_addr) != NULL) {
+ fprintf(stderr, "%s: dup %p %s\n", __func__, bp_addr,
libsym->name);
+ libsym->delayed = 1;
+ }
+ }
+
+ if (strstr(name, "strdup") != NULL) {
+ fprintf(stderr, "%s: %s %p idx=%d\n",
+ __func__, name, bp_addr, ndx);
}
*ret = libsym;
_______________________________________________
Ltrace-devel mailing list
[email protected]
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/ltrace-devel
