On Tue, 5 Dec 2017 13:59:17 +0300, "Dmitry V. Levin" <ldv at altlinux.org> wrote:
> On Tue, Dec 05, 2017 at 07:01:03PM +0900, Masatake YAMATO wrote:
>> This patch is for demangling c++ symbols appeared in strack traces
>> with using cplus_demangle in GNU libiberty library,
>>
>> I'm not sure this is acceptable or not because this change introdues a depedency
>> to a GPLv3+ library to strace.
>
> There has to be a configure option, e.g. --with-libibery[=yes|no|check].
Thank you very much for the quick response. I understand this is a "go sign";
users should make a choice.
I will work on this item after the kvm related patches are merged.
... Could you look at My v4 patches about kvm ioctl?
Your reviewing is extremely fast, so I think you don't noticed
them.
Masatake YAMATO
>> This is an example of output of demangled stack trace:
>>
>> fstat(5, {st_mode=S_IFREG|0664, st_size=0, ...}) = 0
>> > /usr/lib64/libc-2.25.so(__fxstat64+0x12) [0xffd62]
>> > /usr/lib64/libc-2.25.so(_IO_file_doallocate+0x5f) [0x717ff]
>> > /usr/lib64/libc-2.25.so(_IO_doallocbuf+0x79) [0x81699]
>> > /usr/lib64/libc-2.25.so(_IO_file_overflow+0x198) [0x807b8]
>> > /usr/lib64/libc-2.25.so(_IO_file_xsputn+0xbd) [0x7ed5d]
>> > /usr/lib64/libc-2.25.so(fwrite_unlocked+0x60) [0x7d800]
>> > /usr/lib64/libleveldb.so.1.18(leveldb::EnvWrapper::StartThread+0x3b6) [0x48656]
>> > /usr/lib64/libleveldb.so.1.18(leveldb::log::Writer::EmitPhysicalRecord+0x89) [0x28bc9]
>> > /usr/lib64/libleveldb.so.1.18(leveldb::log::Writer::AddRecord+0x9e) [0x28d9e]
>> > /usr/lib64/libleveldb.so.1.18(leveldb::DBImpl::Write+0x208) [0x1ce18]
>> > /usr/lib64/libleveldb.so.1.18(leveldb::DB::Put+0x59) [0x192b9]
>> > /usr/lib64/libleveldb.so.1.18(leveldb::DBImpl::Put+0x1d) [0x1931d]
>
> Yes, this looks nicer.
>
>> > /home/yamato/var/leveldb/doc/a.out(main+0x120) [0x1107]
>> > /usr/lib64/libc-2.25.so(__libc_start_main+0xea) [0x2088a]
>> > /home/yamato/var/leveldb/doc/a.out(_start+0x2a) [0xf3a]
>> ---
>> Makefile.am | 2 +-
>> configure.ac | 10 ++++++++++
>> unwind.c | 18 ++++++++++++++++++
>> 3 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 60376056..e69bd1fd 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -332,7 +332,7 @@ if USE_LIBUNWIND
>> strace_SOURCES += unwind.c
>> strace_CPPFLAGS += $(libunwind_CPPFLAGS)
>> strace_LDFLAGS += $(libunwind_LDFLAGS)
>> -strace_LDADD += $(libunwind_LIBS)
>> +strace_LDADD += $(libunwind_LIBS) $(libiberty_LIBS)
>> endif
>>
>> @CODE_COVERAGE_RULES@
>> diff --git a/configure.ac b/configure.ac
>> index fa451d84..b7ec9240 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -877,6 +877,16 @@ fi
>> AM_CONDITIONAL([USE_LIBUNWIND], [test "x$use_libunwind" = xyes])
>> AC_MSG_RESULT([$use_libunwind])
>>
>> +libiberty_LIBS=
>> +if test "x$use_libunwind" = xyes; then
>> + AC_CHECK_HEADERS([demangle.h],
>> + [AC_CHECK_LIB([iberty], [cplus_demangle],
>> + [libiberty_LIBS=-liberty
>> + AC_DEFINE([USE_DEMANGLE], 1, [Do demangling symbols when tracking stack])
>> + ])])
>> + AC_SUBST(libiberty_LIBS)
>> +fi
>> +
>> if test "$arch" = mips && test "$no_create" != yes; then
>> mkdir -p linux/mips
>> if $srcdir/linux/mips/genstub.sh linux/mips; then
>> diff --git a/unwind.c b/unwind.c
>> index 031795cc..bb357317 100644
>> --- a/unwind.c
>> +++ b/unwind.c
>> @@ -29,6 +29,10 @@
>> #include <limits.h>
>> #include <libunwind-ptrace.h>
>>
>> +#ifdef USE_DEMANGLE
>> +#include <demangle.h>
>> +#endif
>> +
>> #ifdef _LARGEFILE64_SOURCE
>> # ifdef HAVE_FOPEN64
>> # define fopen_for_input fopen64
>> @@ -325,11 +329,25 @@ print_stack_frame(struct tcb *tcp,
>> &function_offset);
>> true_offset = ip - cur_mmap_cache->start_addr +
>> cur_mmap_cache->mmap_offset;
>> +
>> +#ifdef USE_DEMANGLE
>> + char *demangled_name = cplus_demangle (*symbol_name, 0);
>> +#endif
>> +
>> call_action(data,
>> cur_mmap_cache->binary_filename,
>> +#ifdef USE_DEMANGLE
>> + demangled_name? demangled_name: *symbol_name,
>> +#else
>> *symbol_name,
>> +#endif
>> function_offset,
>> true_offset);
>> +#ifdef USE_DEMANGLE
>> + if (demangled_name)
>> + free (demangled_name);
>
> Just free(demangled_name), no extra check for non-NULL is needed.
>
>
> --
> ldv