This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Adding systemtap probe points in pthread library (slightly revised again)


On Thu, 2011-01-27 at 23:20 -0800, Roland McGrath wrote:
> > The problem I had at the beginning was related to some modules not
> > defining IN_LIB, so the following code was complaining:
> 
> I see.  No such code should be actually using these macros.  But it may
> wind up indirectly including lowlevellock.h for some reason.  Is that
> what you saw?  Please cite the particular compilation errors you saw.

That's exactly the issue in the first place - lowlevellock.h is
indirectly included in many places. Without the Makefile changes, IN_LIB
is not defined for the non-library modules.

The compilation error message is:

gcc iconv_charmap.c -c -std=gnu99 -fgnu89-inline -O -Wall -Winline
-Wwrite-strings -fmerge-all-constants -g -Wstrict-prototypes
-I../locale/programs   -I/home/rayson/tmp922st/systemtap/includes
-I../include -I/home/rayson/tmp929glibc/glibc/bld/iconv
-I/home/rayson/tmp929glibc/glibc/bld -I../sysdeps/x86_64/elf
-I../nptl/sysdeps/unix/sysv/linux/x86_64
-I../sysdeps/unix/sysv/linux/x86_64
-I../sysdeps/unix/sysv/linux/wordsize-64
-I../nptl/sysdeps/unix/sysv/linux -I../nptl/sysdeps/pthread
-I../sysdeps/pthread -I../sysdeps/unix/sysv/linux -I../sysdeps/gnu
-I../sysdeps/unix/common -I../sysdeps/unix/mman -I../sysdeps/unix/inet
-I../nptl/sysdeps/unix/sysv -I../sysdeps/unix/sysv
-I../sysdeps/unix/x86_64 -I../nptl/sysdeps/unix -I../sysdeps/unix
-I../sysdeps/posix -I../sysdeps/x86_64/fpu -I../sysdeps/x86_64/multiarch
-I../nptl/sysdeps/x86_64 -I../sysdeps/x86_64 -I../sysdeps/wordsize-64
-I../sysdeps/ieee754/ldbl-96 -I../sysdeps/ieee754/dbl-64/wordsize-64
-I../sysdeps/ieee754/dbl-64 -I../sysdeps/ieee754/flt-32
-I../sysdeps/ieee754 -I../sysdeps/generic/elf -I../sysdeps/generic
-I../nptl  -I.. -I../libio -I.  -D_LIBC_REENTRANT
-include ../include/libc-symbols.h      -DNOT_IN_libc
-o /home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o -MD -MP
-MF /home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o.dt
-MT /home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o
In file included
from ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h:23,
                 from ../nptl/descr.h:30,
                 from ../nptl/sysdeps/x86_64/tls.h:98,
                 from ../include/tls.h:6,
                 from ../include/errno.h:22,
                 from iconv_charmap.c:21:
../include/stap-probe.h:39:4: error: #error "missing -DIN_LIB=... -- not
extra-lib.mk?"
make[2]: *** [/home/rayson/tmp929glibc/glibc/bld/iconv/iconv_charmap.o]
Error 1
make[2]: Leaving directory `/home/rayson/tmp929glibc/glibc/iconv'
make[1]: *** [iconv/others] Error 2
make[1]: Leaving directory `/home/rayson/tmp929glibc/glibc'
make: *** [all] Error 2

I tried to find a better solution but I think it is deep in the include
chain, so I think we will need to either fix it/workaround it from the
root stap-probe.h, or define IN_LIB to satify the #define requirement.


> Sure, but I really don't think any of that code ought to be using the
> macros where you have probes used.

Except those that indirectly include lowlevellock.h .


> 
> If the header is used but not the macros that use the probe macros, then
> the safe thing is to define IN_LIB to something that ensures a syntax
> error.  Then there won't be the #error failures just from having the header
> file, but if any uses of those ever macros leaks into non-library code,
> then compilation will fail loudly so we can see what is going on.  We don't
> want to quietly insert probes with stupid names into places where we don't
> intend to have probes.
> 
> > A better fix but involves touch a lot of Makefiles this one -- note that
> > I defined IN_LIB for non libraries code to "nonlib":
> 
> No, this is a bad change to make.

It's actually 6 Makefile changes, so may not sound as what I described
in my previous email.


> 
> > +condattr_destroy - probe for pthread_condattr_destroy
> 
> Who cares about that?  It makes no sense to have this probe an no other
> probes about pthread_*attr_* calls.  I see no reason anyone would ever
> wants probes on these calls.  They are ancillary machinery to making
> pthread_*_init calls, nothing interesting themselves.
> 
> > @@ -556,6 +556,8 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
> >    /* Pass the descriptor to the caller.  */
> >    *newthread = (pthread_t) pd;
> >  
> > +  LIBC_PROBE (pthread_create, 2, start_routine, arg);
> 
> Why doesn't this get all four arguments?
> 
> >  __pthread_cond_timedwait:
> >  .LSTARTCODE:
> >  	cfi_startproc
> > +
> > +        LIBC_PROBE_ASM (cond_timedwait, %rdi)
> 
> Why doesn't this get all the arguments?
> 
> >  __pthread_cond_wait:
> > +
> > +        LIBC_PROBE_ASM (cond_wait, %rdi)
> 
> Why doesn't this get all the arguments?

All those are fixed -- I tried to use STAP_PROBE_ASM &
STAP_PROBE_ASM_TEMPLATE in those places and having some issues, and I
just changed it to much simpler LIBC_PROBE().

Rayson


> 
> 
> Thanks,
> Roland



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]