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)


Thanks Roland. I've tried your fix and it works for most cases.

However, I just clone a clean branch and merged my changes on top, and
found that files in the rtld module still needs to have IN_LIB defined.
If I don't do that, then it would not compile because for example
dl-lookup.c calls lll_futex_wake() (via. THREAD_GSCOPE_RESET_FLAG). So
there really are files that eventually use the LIBC_PROBE macro outside
of the pthread module & other modules that have IN_LIB properly defined.

I looked at your changes and found that it adds ",,," to the definition
to IN_LIB, so that when it is used it would bomb out. However, as those
places really use the LIBC_PROBE() macro, I don't think there is a way
to work around it than to define IN_LIB. With your fix, things are a lot
better but there are still 2 Makefile changes required:

1) elf/Makefile

- add a parallel case (-DIN_LIB=rtld) when "-DNOT_IN_libc=1" is defined.

2) elf/rtld-Rules

- similarly, add a parallel case (-DIN_LIB=rtld) when "-DNOT_IN_libc=1"
is defined.

And I believe they are correct fixes, as the existing code already has
the macro "-DIS_IN_rtld=1" in those Makefiles.

Secondly, testing against the systemtap disabled case, I found that if a
probe is used in an assembly file (which was what I changed last week
instead of using LIBC_PROBE_ASM), then the definitions in stap-probe.h
would cause it to blow up because we are inserting C code do {}
while(0).

So my fix is to define the DUMMY_PROBEs to "nothing" when it 
USE_STAP_PROBE is not defined & we are in ASSEMBLER mode:

===================================================================

diff --git a/include/stap-probe.h b/include/stap-probe.h
index 9ed6206..1915ce6 100644
--- a/include/stap-probe.h
+++ b/include/stap-probe.h
@@ -68,31 +68,39 @@
 /* This silliness lets us evaluate all the arguments for each arity
    of probe.  My kingdom for a real macro system.  */

-# define DUMMY_PROBE0()                        do {} while (0)
-# define DUMMY_PROBE1(a1)              do {} while ((void) (a1), 0)
-# define DUMMY_PROBE2(a1, a2)          do {} while ((void) (a1), \
+# ifdef __ASSEMBLER__
+#  define DUMMY_PROBE0()                 /* Nothing.  */
+#  define DUMMY_PROBE1(a1)               /* Nothing.  */
+#  define DUMMY_PROBE2(a1, a2)           /* Nothing.  */
+#  define DUMMY_PROBE3(a1, a2, a3)       /* Nothing.  */
+
+# else
+
+#  define DUMMY_PROBE0()                       do {} while (0)
+#  define DUMMY_PROBE1(a1)             do {} while ((void) (a1), 0)
+#  define DUMMY_PROBE2(a1, a2)         do {} while ((void) (a1), \
                                                     (void) (a2), 0)
-# define DUMMY_PROBE3(a1, a2, a3)      do {} while ((void) (a1), \
+#  define DUMMY_PROBE3(a1, a2, a3)     do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), 0)
-# define DUMMY_PROBE4(a1, a2, a3, a4)  do {} while ((void) (a1), \
+#  define DUMMY_PROBE4(a1, a2, a3, a4) do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
                                                     (void) (a4), 0)
-# define DUMMY_PROBE5(a1, a2, a3, a4, a5)                        \
+#  define DUMMY_PROBE5(a1, a2, a3, a4, a5)                       \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
                                                     (void) (a4), \
                                                     (void) (a5), 0)
-# define DUMMY_PROBE6(a1, a2, a3, a4, a5, a6)                    \
+#  define DUMMY_PROBE6(a1, a2, a3, a4, a5, a6)                   \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
                                                     (void) (a4), \
                                                     (void) (a5), \
                                                     (void) (a6), 0)
-# define DUMMY_PROBE7(a1, a2, a3, a4, a5, a6, a7)                \
+#  define DUMMY_PROBE7(a1, a2, a3, a4, a5, a6, a7)               \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -100,7 +108,7 @@
                                                     (void) (a5), \
                                                     (void) (a6), \
                                                     (void) (a7), 0)
-# define DUMMY_PROBE8(a1, a2, a3, a4, a5, a6, a7, a8)            \
+#  define DUMMY_PROBE8(a1, a2, a3, a4, a5, a6, a7, a8)           \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -109,7 +117,7 @@
                                                     (void) (a6), \
                                                     (void) (a7), \
                                                     (void) (a8), 0)
-# define DUMMY_PROBE9(a1, a2, a3, a4, a5, a6, a7, a8, a9)        \
+#  define DUMMY_PROBE9(a1, a2, a3, a4, a5, a6, a7, a8, a9)       \
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -119,7 +127,7 @@
                                                     (void) (a7), \
                                                     (void) (a8), \
                                                     (void) (a9), 0)
-# define DUMMY_PROBE10(a1, a2, a3, a4, a5, a6, a7, a8, a9, a10)
\
+#  define DUMMY_PROBE10(a1, a2, a3, a4, a5, a6, a7, a8, a9, a10)
\
                                        do {} while ((void) (a1), \
                                                     (void) (a2), \
                                                     (void) (a3), \
@@ -130,6 +138,7 @@
                                                     (void) (a8), \
                                                     (void) (a9), \
                                                     (void) (a10), 0)
+#endif /* __ASSEMBLER__ */

 #endif /* USE_STAP_PROBE.  */

===================================================================

Please let me know if you are OK with those changes, and if so I will
send the pthread patches to the list tonight.

Rayson




On Fri, 2011-02-04 at 11:03 -0800, Roland McGrath wrote:
> > > 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.
> 
> I've fixed that problem differently in the branch.  It should now bomb
> out--as we want it to--if any macros containing probes are actually used in
> files that don't define IN_LIB, but won't just because of #include chains.
> 
> 
> Thanks,
> Roland



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