[Ltrace-devel] Some patches for ltrace

Andrey Zonov zont at FreeBSD.org
Tue Aug 28 16:01:04 UTC 2012


On 8/27/12 5:35 PM, Petr Machata wrote:
> Andrey Zonov <zont at FreeBSD.org> writes:
> 
>> On 8/27/12 1:30 PM, Petr Machata wrote:
>>> Andrey Zonov <zont at FreeBSD.org> writes:
>>>
>>>> I'm now porting ltrace on FreeBSD
> 
> One more nit.  I don't like that the directory is called FreeBSD.  Why
> not call it "freebsd"?  That's what the triplet name is.
> 

The project name is FreeBSD, not freebsd, but I will rename directory if
you have objections.

>>>
>>> Thank you.  Next time around, would you please be so kind as to post
>>> each of those as a patch, like what git-format-patch does?  This would
>>> allow me to review in e-mail, and eventually apply them.
>>
>> Sure, but I thought that the merge from my tree is easier way to apply
>> them.
> 
> I'd need to add a remote and then cherry-pick, which is annoying.
> Sending patches to mailing list is important for review anyway.
> 

Attached.

>>> You commit messages start with dashes.  Our don't.  The first line of a
>>> commit message acts similarly to Subject: in e-mail messages.
>>
>> AFAIK, you can change commit message as you want.
> 
> I guess I could if you posted them as patches ;) I could anyway, but
> then I'd need to amend stuff after each cherry pick, which is yet more
> work for me.  In any case, I hope you will want to merge your FreeBSD
> work to official ltrace repository eventually, and it is desirable that
> it fits the style of the rest of the repo.
> 

It would be great to add FreeBSD support in official tree, but I think
it should be done after fixing thread support.

>>> Apart from the particular commits that you point out.  Most of the
>>> FreeBSD code is very similar to Linux code.  This is guaranteed to
>>> bitrot.  Either FreeBSD will see fixes that would be useful for Linux,
>>> or the other way around.  It seems like the job control parts should be
>>> fairly similar among these two OS's, and it should be possible to reuse
>>> this and only call back to OS hooks for the low-level work.
>>
>> The thread implementation is strongly different in FreeBSD, that is the
>> main problem of the port.
> 
> Yeah, I see that proc.c has changed a fair deal.  If more changes are
> comming to trace.c, than sharing might indeed be impractical.  Is that
> the case?
> 

Yes.

>>> Also note that I intended to merge pmachata/abi branch for some time
>>> now.  I don't think it collides with your work, but it does touch a lot
>>> of ltrace core, and might introduce bugs that you will see on FreeBSD as
>>> well.  I'll try to get around to the merge really soon now.
>>>
>>
>> Where can I find this branch to try it?
> 
> Apologies, it's actually called pmachata/revamp.  It used to be called
> pmachata/abi before it turned out that deeper changes are necessary for
> the intended functionality.  I'll need to rebase it on top of current
> master, which has seen a few changes in the mean time, and then merge
> it.  Hopefully I'll get around it sometime this week.
> 

OK.

>>>> a2d199eda1f0e6dd5e3dc38fdef9383dca602993
>>>
>>> I don't understand the problem.  When does it break and how?  Leaking
>>> memory is certainly less bad than crashing, but we would prefer fixing
>>> the underlying bug.
>>
>> This is a using memory after free(3).  I found it, because "FreeBSD
>> current" uses memory allocator that trashes memory after free(3) and
>> ltrace got SIGSEGV.
>>
>> You can easily reproduce that problem with valgrind under Linux.  Test
>> program is attached.
>>
>> $ valgrind ./ltrace -f ./run 1 /bin/sleep 1
> 
> OK.  I'll look into this and see if I can come up with a reasonable
> solution.
> 

I have an idea to use reference counters instead of doing deep copy, but
I had no time to implement this.

>>>> ebd3e4c7e68065f1829ca84d7830c583efc12cff
>>>
>>> Why is this needed?
>>
>> Also using after free(3).
> 
> Interesting.  I'm talking about "Save callstack on exec", are you as
> well?
> 

I can't remember the test case, but under FreeBSD this fixes tracing
exec() and under Linux it does nothing bad.

-- 
Andrey Zonov
-------------- next part --------------
commit 1fa87f78e9d9cf989fc8c465402ed4ab235b63c9
Author: Andrey Zonov <zont at FreeBSD.org>
Date:   Thu Aug 16 18:12:39 2012 +0400

    Prevent using of uninitialized value

diff --git a/library.c b/library.c
index 92fccea..cdbb641 100644
--- a/library.c
+++ b/library.c
@@ -207,6 +207,7 @@ static void
 private_library_init(struct library *lib, enum library_type type)
 {
 	lib->next = NULL;
+	lib->key = NULL;
 	lib->soname = NULL;
 	lib->own_soname = 0;
 	lib->pathname = NULL;
-------------- next part --------------
commit 9cde54c8b96e88afda37ead3c8cec1ff32117912
Author: Andrey Zonov <zont at FreeBSD.org>
Date:   Sun Aug 5 00:11:46 2012 +0400

    Fix typo

diff --git a/configure.ac b/configure.ac
index 1ec7242..a17e155 100644
--- a/configure.ac
+++ b/configure.ac
@@ -12,7 +12,7 @@ AC_CANONICAL_HOST
 
 case "${host_os}" in
     linux-gnu*)	HOST_OS="linux-gnu" ;;
-    *)		AC_MSG_ERROR([unkown host-os ${host_osx}]) ;;
+    *)		AC_MSG_ERROR([unkown host-os ${host_os}]) ;;
 esac
 AC_SUBST(HOST_OS)
 
-------------- next part --------------
commit 81b3039f045c790350695a6207c4b8f017c7a059
Author: Andrey Zonov <zont at FreeBSD.org>
Date:   Mon Aug 13 22:28:21 2012 +0400

    Save callstack on exec(), it eventually resets in breakpoints_init()

diff --git a/proc.c b/proc.c
index 3e6d5cb..cec905b 100644
--- a/proc.c
+++ b/proc.c
@@ -180,6 +180,9 @@ process_destroy(struct Process *proc)
 int
 process_exec(struct Process *proc)
 {
+	int callstack_depth;
+
+	callstack_depth = proc->callstack_depth;
 	/* Call exec first, before we destroy the main state.  */
 	if (arch_process_exec(proc) < 0)
 		return -1;
@@ -191,6 +194,7 @@ process_exec(struct Process *proc)
 		process_bare_destroy(proc, 1);
 		return -1;
 	}
+	proc->callstack_depth = callstack_depth;
 	return 0;
 }
 
-------------- next part --------------
commit a95185630321992a7416847f362ed4e8eaf2f298
Author: Andrey Zonov <zont at FreeBSD.org>
Date:   Sun Aug 5 00:19:51 2012 +0400

    Hide linux specific headers

diff --git a/debug.h b/debug.h
index 52e8e96..54710a4 100644
--- a/debug.h
+++ b/debug.h
@@ -1,7 +1,9 @@
 #ifndef _DEBUG_H
 #define _DEBUG_H
 
+#ifdef	__linux__
 #include <features.h>
+#endif
 
 /* debug levels:
  */
diff --git a/ltrace-elf.c b/ltrace-elf.c
index ab1d505..14826c0 100644
--- a/ltrace-elf.c
+++ b/ltrace-elf.c
@@ -1,7 +1,9 @@
 #include "config.h"
 
 #include <assert.h>
+#ifdef	__linux__
 #include <endian.h>
+#endif
 #include <errno.h>
 #include <fcntl.h>
 #include <gelf.h>
-------------- next part --------------
commit b82e0b82ecc83b458df6386ac4ef2bd844934150
Author: Andrey Zonov <zont at FreeBSD.org>
Date:   Sun Aug 5 00:16:55 2012 +0400

    Add missed header for struct timeval

diff --git a/proc.h b/proc.h
index e04c29c..dbf34e7 100644
--- a/proc.h
+++ b/proc.h
@@ -25,6 +25,8 @@
 
 #include "config.h"
 
+#include <sys/time.h>
+
 #if defined(HAVE_LIBUNWIND)
 # include <libunwind.h>
 #endif /* defined(HAVE_LIBUNWIND) */
-------------- next part --------------
commit cd04e5da9075512ec144c7b6e851443ab66d5ab5
Author: Andrey Zonov <zont at FreeBSD.org>
Date:   Sun Aug 5 00:10:12 2012 +0400

    Change shebang to /bin/sh

diff --git a/autogen.sh b/autogen.sh
index 3938818..b35491b 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 
 set -e
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 535 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/ltrace-devel/attachments/20120828/73c7fda6/attachment.pgp>


More information about the Ltrace-devel mailing list