* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <80ee3399800d43b5d3f51aa095ebca160d4f20b9.1464966729.git.raphael.beamonte@gmail.com> @ 2016-06-03 15:47 ` Sebastien Boisvert [not found] ` <5751A690.9000501@gydle.com> 1 sibling, 0 replies; 22+ messages in thread From: Sebastien Boisvert @ 2016-06-03 15:47 UTC (permalink / raw) To: Raphaël Beamonte, lttng-dev, mathieu.desnoyers Hi Raphaël, I think you have a buffer overflow, see below. On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: > Add the required functions to change the thread name of the UST > threads and add the -ust string at its end. This will help to > identify LTTng-UST processes when analyzing the trace of a process. > > Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> > --- > configure.ac | 5 +++++ > liblttng-ust/Makefile.am | 1 + > liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- > liblttng-ust/lttng-ust-comm.c | 6 ++++++ > 4 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 5692553..de462ff 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test "x$have_libdl" = "xyes"]) > AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) > > AC_CHECK_LIB([pthread], [pthread_create]) > +AC_CHECK_LIB([pthread], [pthread_setname_np], > + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np is available.]), > + AC_CHECK_LIB([pthread], [pthread_set_name_np], > + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if pthread_set_name_np is available.]), > + AC_MSG_RESULT([pthread setname/set_name not found.]))) > > # Check for dlfcn.h > AC_CHECK_HEADER([dlfcn.h]) > diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am > index f1801cf..05929be 100644 > --- a/liblttng-ust/Makefile.am > +++ b/liblttng-ust/Makefile.am > @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ > error.h > liblttng_ust_tracepoint_la_LIBADD = \ > -lurcu-bp \ > + -lpthread \ > $(top_builddir)/snprintf/libustsnprintf.la > liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info $(LTTNG_UST_LIBRARY_VERSION) > liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" -fno-strict-aliasing > diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h > index 43b2223..4fee919 100644 > --- a/liblttng-ust/compat.h > +++ b/liblttng-ust/compat.h > @@ -3,6 +3,7 @@ > > /* > * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -23,7 +24,6 @@ > * lttng_ust_getprocname. > */ > #ifdef __linux__ > - > #include <sys/prctl.h> > > #define LTTNG_UST_PROCNAME_LEN 17 > @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) > (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); > } > > +/* > + * if pthread_setname_np is available. > + */ > +#ifdef HAVE_PTHREAD_SETNAME_NP > +#define PTHREAD_SETNAME_NP pthread_setname_np > +#endif > + > #elif defined(__FreeBSD__) > #include <stdlib.h> > #include <string.h> > @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) > strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); > } > > +/* > + * if pthread_set_name_np is available. > + */ > +#ifdef HAVE_PTHREAD_SET_NAME_NP > +#define PTHREAD_SETNAME_NP pthread_set_name_np > +#endif > + > +#endif > + > +/* > + * if a pthread setname/set_name function is available, declare > + * the setustprocname() function that will add '-ust' to the end > + * of the current process name, while truncating it if needed. > + */ > +#ifdef PTHREAD_SETNAME_NP > +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() > + > +#include <pthread.h> > + > +static inline > +void lttng_ust_setustprocname() > +{ > + char name[LTTNG_UST_PROCNAME_LEN]; Let's say that LTTNG_UST_PROCNAME_LEN is 10. > + int limit = LTTNG_UST_PROCNAME_LEN - 4; Then, limit is 10 - 4 = 6. > + int len = 0; This can be moved on the strlen(name) line. > + > + lttng_ust_getprocname(name); Let's suppose that after the call to lttng_ust_getprocname(), name contains "012345678\0", > + > + len = strlen(name); The variable "len" will contain the integer 9. > + if (len >= limit) { > + len = limit; > + } > + After that, len is >= limit (9 >= 6), so len becomes 6. > + sprintf(name + len, "%s", "-ust"); name is a pointer to "012345678\0" (9 bytes + '\0') name + len is name + 6, so it points to "678\0" sprintf will essentially copy "-ust\0" to "678\0". So, if my calculations are not wrong, you have a buffer overflow which will corrupt the stack memory of the current stack frame. > + > + PTHREAD_SETNAME_NP(pthread_self(), name); > +} > +#else > +#define LTTNG_UST_SETUSTPROCNAME > #endif > > #include <errno.h> > diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c > index e00a22c..4de68c7 100644 > --- a/liblttng-ust/lttng-ust-comm.c > +++ b/liblttng-ust/lttng-ust-comm.c > @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) > int sock, ret, prev_connect_failed = 0, has_waited = 0; > long timeout; > > + /* > + * If available, add '-ust' to the end of this thread's > + * process name > + */ > + LTTNG_UST_SETUSTPROCNAME; Is this common practice ? Just by looking at this line, it is unclear that it is a function call. I would prefer LTTNG_UST_SETUSTPROCNAME() or lttng_ust_setustprocname() > + > /* Restart trying to connect to the session daemon */ > restart: > if (prev_connect_failed) { > _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <5751A690.9000501@gydle.com>]
* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <5751A690.9000501@gydle.com> @ 2016-06-03 15:54 ` Mathieu Desnoyers [not found] ` <1992157209.27136.1464969273436.JavaMail.zimbra@efficios.com> 1 sibling, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 15:54 UTC (permalink / raw) To: Sebastien Boisvert; +Cc: Raphaël Beamonte, lttng-dev ----- On Jun 3, 2016, at 5:47 PM, Sebastien Boisvert sboisvert@gydle.com wrote: > Hi Raphaël, > > I think you have a buffer overflow, see below. > > On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: >> Add the required functions to change the thread name of the UST >> threads and add the -ust string at its end. This will help to >> identify LTTng-UST processes when analyzing the trace of a process. >> >> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> >> --- >> configure.ac | 5 +++++ >> liblttng-ust/Makefile.am | 1 + >> liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- >> liblttng-ust/lttng-ust-comm.c | 6 ++++++ >> 4 files changed, 59 insertions(+), 1 deletion(-) >> >> diff --git a/configure.ac b/configure.ac >> index 5692553..de462ff 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test >> "x$have_libdl" = "xyes"]) >> AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) >> >> AC_CHECK_LIB([pthread], [pthread_create]) >> +AC_CHECK_LIB([pthread], [pthread_setname_np], >> + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np >> is available.]), >> + AC_CHECK_LIB([pthread], [pthread_set_name_np], >> + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if >> pthread_set_name_np is available.]), >> + AC_MSG_RESULT([pthread setname/set_name not found.]))) >> >> # Check for dlfcn.h >> AC_CHECK_HEADER([dlfcn.h]) >> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >> index f1801cf..05929be 100644 >> --- a/liblttng-ust/Makefile.am >> +++ b/liblttng-ust/Makefile.am >> @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ >> error.h >> liblttng_ust_tracepoint_la_LIBADD = \ >> -lurcu-bp \ >> + -lpthread \ >> $(top_builddir)/snprintf/libustsnprintf.la >> liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info >> $(LTTNG_UST_LIBRARY_VERSION) >> liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" >> -fno-strict-aliasing >> diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h >> index 43b2223..4fee919 100644 >> --- a/liblttng-ust/compat.h >> +++ b/liblttng-ust/compat.h >> @@ -3,6 +3,7 @@ >> >> /* >> * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> >> * >> * This library is free software; you can redistribute it and/or >> * modify it under the terms of the GNU Lesser General Public >> @@ -23,7 +24,6 @@ >> * lttng_ust_getprocname. >> */ >> #ifdef __linux__ >> - >> #include <sys/prctl.h> >> >> #define LTTNG_UST_PROCNAME_LEN 17 >> @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) >> (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); >> } >> >> +/* >> + * if pthread_setname_np is available. >> + */ >> +#ifdef HAVE_PTHREAD_SETNAME_NP >> +#define PTHREAD_SETNAME_NP pthread_setname_np >> +#endif >> + >> #elif defined(__FreeBSD__) >> #include <stdlib.h> >> #include <string.h> >> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) >> strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); >> } >> >> +/* >> + * if pthread_set_name_np is available. >> + */ >> +#ifdef HAVE_PTHREAD_SET_NAME_NP >> +#define PTHREAD_SETNAME_NP pthread_set_name_np >> +#endif >> + >> +#endif >> + >> +/* >> + * if a pthread setname/set_name function is available, declare >> + * the setustprocname() function that will add '-ust' to the end >> + * of the current process name, while truncating it if needed. >> + */ >> +#ifdef PTHREAD_SETNAME_NP >> +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() >> + >> +#include <pthread.h> >> + >> +static inline >> +void lttng_ust_setustprocname() >> +{ >> + char name[LTTNG_UST_PROCNAME_LEN]; > > Let's say that LTTNG_UST_PROCNAME_LEN is 10. > >> + int limit = LTTNG_UST_PROCNAME_LEN - 4; > > Then, limit is 10 - 4 = 6. > >> + int len = 0; > > This can be moved on the strlen(name) line. > >> + >> + lttng_ust_getprocname(name); > > Let's suppose that after the call to lttng_ust_getprocname(), name contains > "012345678\0", > >> + >> + len = strlen(name); > > The variable "len" will contain the integer 9. > >> + if (len >= limit) { >> + len = limit; >> + } >> + > > After that, len is >= limit (9 >= 6), so len becomes 6. > >> + sprintf(name + len, "%s", "-ust"); > > name is a pointer to "012345678\0" (9 bytes + '\0') > > name + len is name + 6, so it points to "678\0" > > sprintf will essentially copy "-ust\0" to "678\0". > > So, if my calculations are not wrong, you have a buffer overflow which will > corrupt the stack memory of the > current stack frame. > > >> + >> + PTHREAD_SETNAME_NP(pthread_self(), name); >> +} >> +#else >> +#define LTTNG_UST_SETUSTPROCNAME >> #endif >> >> #include <errno.h> >> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >> index e00a22c..4de68c7 100644 >> --- a/liblttng-ust/lttng-ust-comm.c >> +++ b/liblttng-ust/lttng-ust-comm.c >> @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) >> int sock, ret, prev_connect_failed = 0, has_waited = 0; >> long timeout; >> >> + /* >> + * If available, add '-ust' to the end of this thread's >> + * process name >> + */ >> + LTTNG_UST_SETUSTPROCNAME; > > Is this common practice ? Just by looking at this line, it is unclear that it is > a function call. > > I would prefer > > LTTNG_UST_SETUSTPROCNAME() > > or > > lttng_ust_setustprocname() The latter is preferred by our coding style, indeed. Thanks, Mathieu > > > >> + >> /* Restart trying to connect to the session daemon */ >> restart: >> if (prev_connect_failed) { -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1992157209.27136.1464969273436.JavaMail.zimbra@efficios.com>]
* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <1992157209.27136.1464969273436.JavaMail.zimbra@efficios.com> @ 2016-06-03 15:55 ` Mathieu Desnoyers [not found] ` <1936873452.27140.1464969327630.JavaMail.zimbra@efficios.com> 1 sibling, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 15:55 UTC (permalink / raw) To: Sebastien Boisvert; +Cc: Raphaël Beamonte, lttng-dev ----- On Jun 3, 2016, at 5:54 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > ----- On Jun 3, 2016, at 5:47 PM, Sebastien Boisvert sboisvert@gydle.com wrote: > >> Hi Raphaël, >> >> I think you have a buffer overflow, see below. >> >> On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: >>> Add the required functions to change the thread name of the UST >>> threads and add the -ust string at its end. This will help to >>> identify LTTng-UST processes when analyzing the trace of a process. >>> >>> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> >>> --- >>> configure.ac | 5 +++++ >>> liblttng-ust/Makefile.am | 1 + >>> liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- >>> liblttng-ust/lttng-ust-comm.c | 6 ++++++ >>> 4 files changed, 59 insertions(+), 1 deletion(-) >>> >>> diff --git a/configure.ac b/configure.ac >>> index 5692553..de462ff 100644 >>> --- a/configure.ac >>> +++ b/configure.ac >>> @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test >>> "x$have_libdl" = "xyes"]) >>> AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) >>> >>> AC_CHECK_LIB([pthread], [pthread_create]) >>> +AC_CHECK_LIB([pthread], [pthread_setname_np], >>> + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np >>> is available.]), >>> + AC_CHECK_LIB([pthread], [pthread_set_name_np], >>> + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if >>> pthread_set_name_np is available.]), >>> + AC_MSG_RESULT([pthread setname/set_name not found.]))) >>> >>> # Check for dlfcn.h >>> AC_CHECK_HEADER([dlfcn.h]) >>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >>> index f1801cf..05929be 100644 >>> --- a/liblttng-ust/Makefile.am >>> +++ b/liblttng-ust/Makefile.am >>> @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ >>> error.h >>> liblttng_ust_tracepoint_la_LIBADD = \ >>> -lurcu-bp \ >>> + -lpthread \ >>> $(top_builddir)/snprintf/libustsnprintf.la >>> liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info >>> $(LTTNG_UST_LIBRARY_VERSION) >>> liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" >>> -fno-strict-aliasing >>> diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h >>> index 43b2223..4fee919 100644 >>> --- a/liblttng-ust/compat.h >>> +++ b/liblttng-ust/compat.h >>> @@ -3,6 +3,7 @@ >>> >>> /* >>> * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >>> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> >>> * >>> * This library is free software; you can redistribute it and/or >>> * modify it under the terms of the GNU Lesser General Public >>> @@ -23,7 +24,6 @@ >>> * lttng_ust_getprocname. >>> */ >>> #ifdef __linux__ >>> - >>> #include <sys/prctl.h> >>> >>> #define LTTNG_UST_PROCNAME_LEN 17 >>> @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) >>> (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); >>> } >>> >>> +/* >>> + * if pthread_setname_np is available. >>> + */ >>> +#ifdef HAVE_PTHREAD_SETNAME_NP >>> +#define PTHREAD_SETNAME_NP pthread_setname_np >>> +#endif >>> + >>> #elif defined(__FreeBSD__) >>> #include <stdlib.h> >>> #include <string.h> >>> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) >>> strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); >>> } >>> >>> +/* >>> + * if pthread_set_name_np is available. >>> + */ >>> +#ifdef HAVE_PTHREAD_SET_NAME_NP >>> +#define PTHREAD_SETNAME_NP pthread_set_name_np >>> +#endif >>> + >>> +#endif >>> + >>> +/* >>> + * if a pthread setname/set_name function is available, declare >>> + * the setustprocname() function that will add '-ust' to the end >>> + * of the current process name, while truncating it if needed. >>> + */ >>> +#ifdef PTHREAD_SETNAME_NP >>> +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() >>> + >>> +#include <pthread.h> >>> + >>> +static inline >>> +void lttng_ust_setustprocname() >>> +{ >>> + char name[LTTNG_UST_PROCNAME_LEN]; >> >> Let's say that LTTNG_UST_PROCNAME_LEN is 10. >> >>> + int limit = LTTNG_UST_PROCNAME_LEN - 4; >> >> Then, limit is 10 - 4 = 6. >> >>> + int len = 0; >> >> This can be moved on the strlen(name) line. >> >>> + >>> + lttng_ust_getprocname(name); >> >> Let's suppose that after the call to lttng_ust_getprocname(), name contains >> "012345678\0", >> >>> + >>> + len = strlen(name); >> >> The variable "len" will contain the integer 9. >> >>> + if (len >= limit) { >>> + len = limit; >>> + } >>> + >> >> After that, len is >= limit (9 >= 6), so len becomes 6. >> >>> + sprintf(name + len, "%s", "-ust"); >> >> name is a pointer to "012345678\0" (9 bytes + '\0') >> >> name + len is name + 6, so it points to "678\0" >> >> sprintf will essentially copy "-ust\0" to "678\0". >> >> So, if my calculations are not wrong, you have a buffer overflow which will >> corrupt the stack memory of the >> current stack frame. >> >> >>> + >>> + PTHREAD_SETNAME_NP(pthread_self(), name); >>> +} >>> +#else >>> +#define LTTNG_UST_SETUSTPROCNAME >>> #endif >>> >>> #include <errno.h> >>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >>> index e00a22c..4de68c7 100644 >>> --- a/liblttng-ust/lttng-ust-comm.c >>> +++ b/liblttng-ust/lttng-ust-comm.c >>> @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) >>> int sock, ret, prev_connect_failed = 0, has_waited = 0; >>> long timeout; >>> >>> + /* >>> + * If available, add '-ust' to the end of this thread's >>> + * process name >>> + */ >>> + LTTNG_UST_SETUSTPROCNAME; >> >> Is this common practice ? Just by looking at this line, it is unclear that it is >> a function call. >> >> I would prefer >> >> LTTNG_UST_SETUSTPROCNAME() >> >> or >> >> lttng_ust_setustprocname() > > The latter is preferred by our coding style, indeed. > And there is no reason to make it a macro. It should be a static inline whenever possible. Thanks, Mathieu > Thanks, > > Mathieu > >> >> >> >>> + >>> /* Restart trying to connect to the session daemon */ >>> restart: >>> if (prev_connect_failed) { > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1936873452.27140.1464969327630.JavaMail.zimbra@efficios.com>]
* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <1936873452.27140.1464969327630.JavaMail.zimbra@efficios.com> @ 2016-06-03 16:12 ` Raphaël Beamonte [not found] ` <CAE_Gge2Av=iucY8abMCHpU8PjgptdCLhvyoWk7nwfazt6ei3VQ@mail.gmail.com> 1 sibling, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-03 16:12 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: lttng-dev 2016-06-03 11:55 GMT-04:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>: > ----- On Jun 3, 2016, at 5:54 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > >> ----- On Jun 3, 2016, at 5:47 PM, Sebastien Boisvert sboisvert@gydle.com wrote: >> >>> Hi Raphaël, >>> >>> I think you have a buffer overflow, see below. >>> >>> On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: >>>> Add the required functions to change the thread name of the UST >>>> threads and add the -ust string at its end. This will help to >>>> identify LTTng-UST processes when analyzing the trace of a process. >>>> >>>> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> >>>> --- >>>> configure.ac | 5 +++++ >>>> liblttng-ust/Makefile.am | 1 + >>>> liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- >>>> liblttng-ust/lttng-ust-comm.c | 6 ++++++ >>>> 4 files changed, 59 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/configure.ac b/configure.ac >>>> index 5692553..de462ff 100644 >>>> --- a/configure.ac >>>> +++ b/configure.ac >>>> @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test >>>> "x$have_libdl" = "xyes"]) >>>> AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) >>>> >>>> AC_CHECK_LIB([pthread], [pthread_create]) >>>> +AC_CHECK_LIB([pthread], [pthread_setname_np], >>>> + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np >>>> is available.]), >>>> + AC_CHECK_LIB([pthread], [pthread_set_name_np], >>>> + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if >>>> pthread_set_name_np is available.]), >>>> + AC_MSG_RESULT([pthread setname/set_name not found.]))) >>>> >>>> # Check for dlfcn.h >>>> AC_CHECK_HEADER([dlfcn.h]) >>>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >>>> index f1801cf..05929be 100644 >>>> --- a/liblttng-ust/Makefile.am >>>> +++ b/liblttng-ust/Makefile.am >>>> @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ >>>> error.h >>>> liblttng_ust_tracepoint_la_LIBADD = \ >>>> -lurcu-bp \ >>>> + -lpthread \ >>>> $(top_builddir)/snprintf/libustsnprintf.la >>>> liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info >>>> $(LTTNG_UST_LIBRARY_VERSION) >>>> liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" >>>> -fno-strict-aliasing >>>> diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h >>>> index 43b2223..4fee919 100644 >>>> --- a/liblttng-ust/compat.h >>>> +++ b/liblttng-ust/compat.h >>>> @@ -3,6 +3,7 @@ >>>> >>>> /* >>>> * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >>>> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> >>>> * >>>> * This library is free software; you can redistribute it and/or >>>> * modify it under the terms of the GNU Lesser General Public >>>> @@ -23,7 +24,6 @@ >>>> * lttng_ust_getprocname. >>>> */ >>>> #ifdef __linux__ >>>> - >>>> #include <sys/prctl.h> >>>> >>>> #define LTTNG_UST_PROCNAME_LEN 17 >>>> @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) >>>> (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); >>>> } >>>> >>>> +/* >>>> + * if pthread_setname_np is available. >>>> + */ >>>> +#ifdef HAVE_PTHREAD_SETNAME_NP >>>> +#define PTHREAD_SETNAME_NP pthread_setname_np >>>> +#endif >>>> + >>>> #elif defined(__FreeBSD__) >>>> #include <stdlib.h> >>>> #include <string.h> >>>> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) >>>> strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); >>>> } >>>> >>>> +/* >>>> + * if pthread_set_name_np is available. >>>> + */ >>>> +#ifdef HAVE_PTHREAD_SET_NAME_NP >>>> +#define PTHREAD_SETNAME_NP pthread_set_name_np >>>> +#endif >>>> + >>>> +#endif >>>> + >>>> +/* >>>> + * if a pthread setname/set_name function is available, declare >>>> + * the setustprocname() function that will add '-ust' to the end >>>> + * of the current process name, while truncating it if needed. >>>> + */ >>>> +#ifdef PTHREAD_SETNAME_NP >>>> +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() >>>> + >>>> +#include <pthread.h> >>>> + >>>> +static inline >>>> +void lttng_ust_setustprocname() >>>> +{ >>>> + char name[LTTNG_UST_PROCNAME_LEN]; >>> >>> Let's say that LTTNG_UST_PROCNAME_LEN is 10. >>> >>>> + int limit = LTTNG_UST_PROCNAME_LEN - 4; >>> >>> Then, limit is 10 - 4 = 6. >>> >>>> + int len = 0; >>> >>> This can be moved on the strlen(name) line. >>> >>>> + >>>> + lttng_ust_getprocname(name); >>> >>> Let's suppose that after the call to lttng_ust_getprocname(), name contains >>> "012345678\0", >>> >>>> + >>>> + len = strlen(name); >>> >>> The variable "len" will contain the integer 9. >>> >>>> + if (len >= limit) { >>>> + len = limit; >>>> + } >>>> + >>> >>> After that, len is >= limit (9 >= 6), so len becomes 6. >>> >>>> + sprintf(name + len, "%s", "-ust"); >>> >>> name is a pointer to "012345678\0" (9 bytes + '\0') >>> >>> name + len is name + 6, so it points to "678\0" >>> >>> sprintf will essentially copy "-ust\0" to "678\0". >>> >>> So, if my calculations are not wrong, you have a buffer overflow which will >>> corrupt the stack memory of the >>> current stack frame. You're right, will fix that. I'm also questionning the interest of limit being a variable here. Should I replace both occurrences directly by "LTTNG_UST_PROCNAME_LEN - 5" or create a "#define LTTNG_UST_PROCNAME_LIMIT (LTTNG_UST_PROCNAME_LEN - 5)" macro ? Mathieu, what do you think would be the best to do here ? The condition will also be replaced from ">=" to ">", as the "=" part is not useful anymore in that version of the patch (it was in v1, because of the operations that were done). >>> >>> >>>> + >>>> + PTHREAD_SETNAME_NP(pthread_self(), name); >>>> +} >>>> +#else >>>> +#define LTTNG_UST_SETUSTPROCNAME >>>> #endif >>>> >>>> #include <errno.h> >>>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >>>> index e00a22c..4de68c7 100644 >>>> --- a/liblttng-ust/lttng-ust-comm.c >>>> +++ b/liblttng-ust/lttng-ust-comm.c >>>> @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) >>>> int sock, ret, prev_connect_failed = 0, has_waited = 0; >>>> long timeout; >>>> >>>> + /* >>>> + * If available, add '-ust' to the end of this thread's >>>> + * process name >>>> + */ >>>> + LTTNG_UST_SETUSTPROCNAME; >>> >>> Is this common practice ? Just by looking at this line, it is unclear that it is >>> a function call. >>> >>> I would prefer >>> >>> LTTNG_UST_SETUSTPROCNAME() >>> >>> or >>> >>> lttng_ust_setustprocname() >> >> The latter is preferred by our coding style, indeed. >> > > And there is no reason to make it a macro. It should be > a static inline whenever possible. I made it a macro as I was not sure about the best path to take, and the macro seemed to be the cleaner approach! However, would it be better like that: #ifdef PTHREAD_SETNAME_NP #include <pthread.h> #endif static inline void lttng_ust_setustprocname() { #ifdef PTHREAD_SETNAME_NP ... PTHREAD_SETNAME_NP(pthread_self(), name); #endif } Or like that ? #ifdef PTHREAD_SETNAME_NP #include <pthread.h> static inline void lttng_ust_setustprocname() { ... PTHREAD_SETNAME_NP(pthread_self(), name); } #else static inline void lttng_ust_setustprocname() {} #endif In both those cases, the macro disappears, and is replaced by a call to lttng_ust_setustprocname(). I'm not sure however which is preferable according to the coding style (In one case, two #ifdef are required, in the other, we declare the function in each branch of the ifdef/else). Thoughts ? I'll send a v3 of the patch according to your preference in both matters. Thanks a lot! Raphaël _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAE_Gge2Av=iucY8abMCHpU8PjgptdCLhvyoWk7nwfazt6ei3VQ@mail.gmail.com>]
* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <CAE_Gge2Av=iucY8abMCHpU8PjgptdCLhvyoWk7nwfazt6ei3VQ@mail.gmail.com> @ 2016-06-03 16:19 ` Mathieu Desnoyers 2016-06-03 16:24 ` Sebastien Boisvert [not found] ` <5751AF3F.4000405@gydle.com> 2 siblings, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 16:19 UTC (permalink / raw) To: Raphaël Beamonte; +Cc: lttng-dev ----- On Jun 3, 2016, at 6:12 PM, Raphaël Beamonte raphael.beamonte@gmail.com wrote: > 2016-06-03 11:55 GMT-04:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>: >> ----- On Jun 3, 2016, at 5:54 PM, Mathieu Desnoyers >> mathieu.desnoyers@efficios.com wrote: >> >>> ----- On Jun 3, 2016, at 5:47 PM, Sebastien Boisvert sboisvert@gydle.com wrote: >>> >>>> Hi Raphaël, >>>> >>>> I think you have a buffer overflow, see below. >>>> >>>> On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: >>>>> Add the required functions to change the thread name of the UST >>>>> threads and add the -ust string at its end. This will help to >>>>> identify LTTng-UST processes when analyzing the trace of a process. >>>>> >>>>> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> >>>>> --- >>>>> configure.ac | 5 +++++ >>>>> liblttng-ust/Makefile.am | 1 + >>>>> liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- >>>>> liblttng-ust/lttng-ust-comm.c | 6 ++++++ >>>>> 4 files changed, 59 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/configure.ac b/configure.ac >>>>> index 5692553..de462ff 100644 >>>>> --- a/configure.ac >>>>> +++ b/configure.ac >>>>> @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test >>>>> "x$have_libdl" = "xyes"]) >>>>> AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) >>>>> >>>>> AC_CHECK_LIB([pthread], [pthread_create]) >>>>> +AC_CHECK_LIB([pthread], [pthread_setname_np], >>>>> + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np >>>>> is available.]), >>>>> + AC_CHECK_LIB([pthread], [pthread_set_name_np], >>>>> + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if >>>>> pthread_set_name_np is available.]), >>>>> + AC_MSG_RESULT([pthread setname/set_name not found.]))) >>>>> >>>>> # Check for dlfcn.h >>>>> AC_CHECK_HEADER([dlfcn.h]) >>>>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >>>>> index f1801cf..05929be 100644 >>>>> --- a/liblttng-ust/Makefile.am >>>>> +++ b/liblttng-ust/Makefile.am >>>>> @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ >>>>> error.h >>>>> liblttng_ust_tracepoint_la_LIBADD = \ >>>>> -lurcu-bp \ >>>>> + -lpthread \ >>>>> $(top_builddir)/snprintf/libustsnprintf.la >>>>> liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info >>>>> $(LTTNG_UST_LIBRARY_VERSION) >>>>> liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" >>>>> -fno-strict-aliasing >>>>> diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h >>>>> index 43b2223..4fee919 100644 >>>>> --- a/liblttng-ust/compat.h >>>>> +++ b/liblttng-ust/compat.h >>>>> @@ -3,6 +3,7 @@ >>>>> >>>>> /* >>>>> * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >>>>> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> >>>>> * >>>>> * This library is free software; you can redistribute it and/or >>>>> * modify it under the terms of the GNU Lesser General Public >>>>> @@ -23,7 +24,6 @@ >>>>> * lttng_ust_getprocname. >>>>> */ >>>>> #ifdef __linux__ >>>>> - >>>>> #include <sys/prctl.h> >>>>> >>>>> #define LTTNG_UST_PROCNAME_LEN 17 >>>>> @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) >>>>> (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); >>>>> } >>>>> >>>>> +/* >>>>> + * if pthread_setname_np is available. >>>>> + */ >>>>> +#ifdef HAVE_PTHREAD_SETNAME_NP >>>>> +#define PTHREAD_SETNAME_NP pthread_setname_np >>>>> +#endif >>>>> + >>>>> #elif defined(__FreeBSD__) >>>>> #include <stdlib.h> >>>>> #include <string.h> >>>>> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) >>>>> strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); >>>>> } >>>>> >>>>> +/* >>>>> + * if pthread_set_name_np is available. >>>>> + */ >>>>> +#ifdef HAVE_PTHREAD_SET_NAME_NP >>>>> +#define PTHREAD_SETNAME_NP pthread_set_name_np >>>>> +#endif >>>>> + >>>>> +#endif >>>>> + >>>>> +/* >>>>> + * if a pthread setname/set_name function is available, declare >>>>> + * the setustprocname() function that will add '-ust' to the end >>>>> + * of the current process name, while truncating it if needed. >>>>> + */ >>>>> +#ifdef PTHREAD_SETNAME_NP >>>>> +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() >>>>> + >>>>> +#include <pthread.h> >>>>> + >>>>> +static inline >>>>> +void lttng_ust_setustprocname() >>>>> +{ >>>>> + char name[LTTNG_UST_PROCNAME_LEN]; >>>> >>>> Let's say that LTTNG_UST_PROCNAME_LEN is 10. >>>> >>>>> + int limit = LTTNG_UST_PROCNAME_LEN - 4; >>>> >>>> Then, limit is 10 - 4 = 6. >>>> >>>>> + int len = 0; >>>> >>>> This can be moved on the strlen(name) line. >>>> >>>>> + >>>>> + lttng_ust_getprocname(name); >>>> >>>> Let's suppose that after the call to lttng_ust_getprocname(), name contains >>>> "012345678\0", >>>> >>>>> + >>>>> + len = strlen(name); >>>> >>>> The variable "len" will contain the integer 9. >>>> >>>>> + if (len >= limit) { >>>>> + len = limit; >>>>> + } >>>>> + >>>> >>>> After that, len is >= limit (9 >= 6), so len becomes 6. >>>> >>>>> + sprintf(name + len, "%s", "-ust"); >>>> >>>> name is a pointer to "012345678\0" (9 bytes + '\0') >>>> >>>> name + len is name + 6, so it points to "678\0" >>>> >>>> sprintf will essentially copy "-ust\0" to "678\0". >>>> >>>> So, if my calculations are not wrong, you have a buffer overflow which will >>>> corrupt the stack memory of the >>>> current stack frame. > > You're right, will fix that. > I'm also questionning the interest of limit being a variable here. > Should I replace both occurrences directly by "LTTNG_UST_PROCNAME_LEN > - 5" or create a "#define LTTNG_UST_PROCNAME_LIMIT > (LTTNG_UST_PROCNAME_LEN - 5)" macro ? Mathieu, what do you think would > be the best to do here ? Hardcoded constants are always a bad idea. #define MYSTRING "blah" LTTNG_UST_PROCNAME_LEN - strlen(MYSTRING) would be ok though. > The condition will also be replaced from ">=" to ">", as the "=" part > is not useful anymore in that version of the patch (it was in v1, > because of the operations that were done). > >>>> >>>> >>>>> + >>>>> + PTHREAD_SETNAME_NP(pthread_self(), name); >>>>> +} >>>>> +#else >>>>> +#define LTTNG_UST_SETUSTPROCNAME >>>>> #endif >>>>> >>>>> #include <errno.h> >>>>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >>>>> index e00a22c..4de68c7 100644 >>>>> --- a/liblttng-ust/lttng-ust-comm.c >>>>> +++ b/liblttng-ust/lttng-ust-comm.c >>>>> @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) >>>>> int sock, ret, prev_connect_failed = 0, has_waited = 0; >>>>> long timeout; >>>>> >>>>> + /* >>>>> + * If available, add '-ust' to the end of this thread's >>>>> + * process name >>>>> + */ >>>>> + LTTNG_UST_SETUSTPROCNAME; >>>> >>>> Is this common practice ? Just by looking at this line, it is unclear that it is >>>> a function call. >>>> >>>> I would prefer >>>> >>>> LTTNG_UST_SETUSTPROCNAME() >>>> >>>> or >>>> >>>> lttng_ust_setustprocname() >>> >>> The latter is preferred by our coding style, indeed. >>> >> >> And there is no reason to make it a macro. It should be >> a static inline whenever possible. > > I made it a macro as I was not sure about the best path to take, and > the macro seemed to be the cleaner approach! > However, would it be better like that: > > #ifdef PTHREAD_SETNAME_NP > #include <pthread.h> > #endif > > static inline > void lttng_ust_setustprocname() > { > #ifdef PTHREAD_SETNAME_NP > ... > > PTHREAD_SETNAME_NP(pthread_self(), name); > #endif > } > > Or like that ? > > #ifdef PTHREAD_SETNAME_NP > #include <pthread.h> > > static inline > void lttng_ust_setustprocname() > { > ... > > PTHREAD_SETNAME_NP(pthread_self(), name); > } > #else > static inline > void lttng_ust_setustprocname() > {} Change for static inline void lttng_ust_setustprocname(void) { } > #endif > Don't forget that (void) in C differs from (). () is like a variadic argument function. The latter form is prefered by our coding style. No #ifdef within functions unless it's really impossible to do otherwise. Thanks, Mathieu > > In both those cases, the macro disappears, and is replaced by a call > to lttng_ust_setustprocname(). > I'm not sure however which is preferable according to the coding style > (In one case, two #ifdef are required, in the other, we declare the > function in each branch of the ifdef/else). > > Thoughts ? I'll send a v3 of the patch according to your preference in > both matters. > > Thanks a lot! > Raphaël -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <CAE_Gge2Av=iucY8abMCHpU8PjgptdCLhvyoWk7nwfazt6ei3VQ@mail.gmail.com> 2016-06-03 16:19 ` Mathieu Desnoyers @ 2016-06-03 16:24 ` Sebastien Boisvert [not found] ` <5751AF3F.4000405@gydle.com> 2 siblings, 0 replies; 22+ messages in thread From: Sebastien Boisvert @ 2016-06-03 16:24 UTC (permalink / raw) To: Raphaël Beamonte, Mathieu Desnoyers; +Cc: lttng-dev Hi Raphaël, I had another thought. If the process name is "my-super-process", I think it only makes sense to append the "-ust" suffix if there is sufficient space. "my-super-process-ust" is fine, but I feel that "my-super-pro-ust" is meaningless. On 06/03/2016 12:12 PM, Raphaël Beamonte wrote: > 2016-06-03 11:55 GMT-04:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>: >> ----- On Jun 3, 2016, at 5:54 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: >> >>> ----- On Jun 3, 2016, at 5:47 PM, Sebastien Boisvert sboisvert@gydle.com wrote: >>> >>>> Hi Raphaël, >>>> >>>> I think you have a buffer overflow, see below. >>>> >>>> On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: >>>>> Add the required functions to change the thread name of the UST >>>>> threads and add the -ust string at its end. This will help to >>>>> identify LTTng-UST processes when analyzing the trace of a process. >>>>> >>>>> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> >>>>> --- >>>>> configure.ac | 5 +++++ >>>>> liblttng-ust/Makefile.am | 1 + >>>>> liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- >>>>> liblttng-ust/lttng-ust-comm.c | 6 ++++++ >>>>> 4 files changed, 59 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/configure.ac b/configure.ac >>>>> index 5692553..de462ff 100644 >>>>> --- a/configure.ac >>>>> +++ b/configure.ac >>>>> @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test >>>>> "x$have_libdl" = "xyes"]) >>>>> AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) >>>>> >>>>> AC_CHECK_LIB([pthread], [pthread_create]) >>>>> +AC_CHECK_LIB([pthread], [pthread_setname_np], >>>>> + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np >>>>> is available.]), >>>>> + AC_CHECK_LIB([pthread], [pthread_set_name_np], >>>>> + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if >>>>> pthread_set_name_np is available.]), >>>>> + AC_MSG_RESULT([pthread setname/set_name not found.]))) >>>>> >>>>> # Check for dlfcn.h >>>>> AC_CHECK_HEADER([dlfcn.h]) >>>>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >>>>> index f1801cf..05929be 100644 >>>>> --- a/liblttng-ust/Makefile.am >>>>> +++ b/liblttng-ust/Makefile.am >>>>> @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ >>>>> error.h >>>>> liblttng_ust_tracepoint_la_LIBADD = \ >>>>> -lurcu-bp \ >>>>> + -lpthread \ >>>>> $(top_builddir)/snprintf/libustsnprintf.la >>>>> liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info >>>>> $(LTTNG_UST_LIBRARY_VERSION) >>>>> liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" >>>>> -fno-strict-aliasing >>>>> diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h >>>>> index 43b2223..4fee919 100644 >>>>> --- a/liblttng-ust/compat.h >>>>> +++ b/liblttng-ust/compat.h >>>>> @@ -3,6 +3,7 @@ >>>>> >>>>> /* >>>>> * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >>>>> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> >>>>> * >>>>> * This library is free software; you can redistribute it and/or >>>>> * modify it under the terms of the GNU Lesser General Public >>>>> @@ -23,7 +24,6 @@ >>>>> * lttng_ust_getprocname. >>>>> */ >>>>> #ifdef __linux__ >>>>> - >>>>> #include <sys/prctl.h> >>>>> >>>>> #define LTTNG_UST_PROCNAME_LEN 17 >>>>> @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) >>>>> (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); >>>>> } >>>>> >>>>> +/* >>>>> + * if pthread_setname_np is available. >>>>> + */ >>>>> +#ifdef HAVE_PTHREAD_SETNAME_NP >>>>> +#define PTHREAD_SETNAME_NP pthread_setname_np >>>>> +#endif >>>>> + >>>>> #elif defined(__FreeBSD__) >>>>> #include <stdlib.h> >>>>> #include <string.h> >>>>> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) >>>>> strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); >>>>> } >>>>> >>>>> +/* >>>>> + * if pthread_set_name_np is available. >>>>> + */ >>>>> +#ifdef HAVE_PTHREAD_SET_NAME_NP >>>>> +#define PTHREAD_SETNAME_NP pthread_set_name_np >>>>> +#endif >>>>> + >>>>> +#endif >>>>> + >>>>> +/* >>>>> + * if a pthread setname/set_name function is available, declare >>>>> + * the setustprocname() function that will add '-ust' to the end >>>>> + * of the current process name, while truncating it if needed. >>>>> + */ >>>>> +#ifdef PTHREAD_SETNAME_NP >>>>> +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() >>>>> + >>>>> +#include <pthread.h> >>>>> + >>>>> +static inline >>>>> +void lttng_ust_setustprocname() >>>>> +{ >>>>> + char name[LTTNG_UST_PROCNAME_LEN]; >>>> >>>> Let's say that LTTNG_UST_PROCNAME_LEN is 10. >>>> >>>>> + int limit = LTTNG_UST_PROCNAME_LEN - 4; >>>> >>>> Then, limit is 10 - 4 = 6. >>>> >>>>> + int len = 0; >>>> >>>> This can be moved on the strlen(name) line. >>>> >>>>> + >>>>> + lttng_ust_getprocname(name); >>>> >>>> Let's suppose that after the call to lttng_ust_getprocname(), name contains >>>> "012345678\0", >>>> >>>>> + >>>>> + len = strlen(name); >>>> >>>> The variable "len" will contain the integer 9. >>>> >>>>> + if (len >= limit) { >>>>> + len = limit; >>>>> + } >>>>> + >>>> >>>> After that, len is >= limit (9 >= 6), so len becomes 6. >>>> >>>>> + sprintf(name + len, "%s", "-ust"); >>>> >>>> name is a pointer to "012345678\0" (9 bytes + '\0') >>>> >>>> name + len is name + 6, so it points to "678\0" >>>> >>>> sprintf will essentially copy "-ust\0" to "678\0". >>>> >>>> So, if my calculations are not wrong, you have a buffer overflow which will >>>> corrupt the stack memory of the >>>> current stack frame. > > You're right, will fix that. > I'm also questionning the interest of limit being a variable here. > Should I replace both occurrences directly by "LTTNG_UST_PROCNAME_LEN > - 5" or create a "#define LTTNG_UST_PROCNAME_LIMIT > (LTTNG_UST_PROCNAME_LEN - 5)" macro ? Mathieu, what do you think would > be the best to do here ? > The condition will also be replaced from ">=" to ">", as the "=" part > is not useful anymore in that version of the patch (it was in v1, > because of the operations that were done). > >>>> >>>> >>>>> + >>>>> + PTHREAD_SETNAME_NP(pthread_self(), name); >>>>> +} >>>>> +#else >>>>> +#define LTTNG_UST_SETUSTPROCNAME >>>>> #endif >>>>> >>>>> #include <errno.h> >>>>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >>>>> index e00a22c..4de68c7 100644 >>>>> --- a/liblttng-ust/lttng-ust-comm.c >>>>> +++ b/liblttng-ust/lttng-ust-comm.c >>>>> @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) >>>>> int sock, ret, prev_connect_failed = 0, has_waited = 0; >>>>> long timeout; >>>>> >>>>> + /* >>>>> + * If available, add '-ust' to the end of this thread's >>>>> + * process name >>>>> + */ >>>>> + LTTNG_UST_SETUSTPROCNAME; >>>> >>>> Is this common practice ? Just by looking at this line, it is unclear that it is >>>> a function call. >>>> >>>> I would prefer >>>> >>>> LTTNG_UST_SETUSTPROCNAME() >>>> >>>> or >>>> >>>> lttng_ust_setustprocname() >>> >>> The latter is preferred by our coding style, indeed. >>> >> >> And there is no reason to make it a macro. It should be >> a static inline whenever possible. > > I made it a macro as I was not sure about the best path to take, and > the macro seemed to be the cleaner approach! > However, would it be better like that: > > #ifdef PTHREAD_SETNAME_NP > #include <pthread.h> > #endif > > static inline > void lttng_ust_setustprocname() > { > #ifdef PTHREAD_SETNAME_NP > ... > > PTHREAD_SETNAME_NP(pthread_self(), name); > #endif > } > > Or like that ? > > #ifdef PTHREAD_SETNAME_NP > #include <pthread.h> > > static inline > void lttng_ust_setustprocname() > { > ... > > PTHREAD_SETNAME_NP(pthread_self(), name); > } > #else > static inline > void lttng_ust_setustprocname() > {} > #endif > > > In both those cases, the macro disappears, and is replaced by a call > to lttng_ust_setustprocname(). > I'm not sure however which is preferable according to the coding style > (In one case, two #ifdef are required, in the other, we declare the > function in each branch of the ifdef/else). > > Thoughts ? I'll send a v3 of the patch according to your preference in > both matters. > > Thanks a lot! > Raphaël > _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <5751AF3F.4000405@gydle.com>]
* Re: [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <5751AF3F.4000405@gydle.com> @ 2016-06-03 16:31 ` Mathieu Desnoyers 0 siblings, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 16:31 UTC (permalink / raw) To: Sebastien Boisvert; +Cc: Raphaël Beamonte, lttng-dev ----- On Jun 3, 2016, at 6:24 PM, Sebastien Boisvert sboisvert@gydle.com wrote: > Hi Raphaël, > > I had another thought. > > If the process name is "my-super-process", I think it only makes sense to append > the > "-ust" suffix if there is sufficient space. "my-super-process-ust" is fine, but > I feel > that "my-super-pro-ust" is meaningless. I see having the -ust overwriting the end of the process name as a feature that would be useful for users to distinguish between ust threads and non-ust threads in cases where the binary name is long. Thanks, Mathieu > > > On 06/03/2016 12:12 PM, Raphaël Beamonte wrote: >> 2016-06-03 11:55 GMT-04:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>: >>> ----- On Jun 3, 2016, at 5:54 PM, Mathieu Desnoyers >>> mathieu.desnoyers@efficios.com wrote: >>> >>>> ----- On Jun 3, 2016, at 5:47 PM, Sebastien Boisvert sboisvert@gydle.com wrote: >>>> >>>>> Hi Raphaël, >>>>> >>>>> I think you have a buffer overflow, see below. >>>>> >>>>> On 06/03/2016 11:17 AM, Raphaël Beamonte wrote: >>>>>> Add the required functions to change the thread name of the UST >>>>>> threads and add the -ust string at its end. This will help to >>>>>> identify LTTng-UST processes when analyzing the trace of a process. >>>>>> >>>>>> Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> >>>>>> --- >>>>>> configure.ac | 5 +++++ >>>>>> liblttng-ust/Makefile.am | 1 + >>>>>> liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- >>>>>> liblttng-ust/lttng-ust-comm.c | 6 ++++++ >>>>>> 4 files changed, 59 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/configure.ac b/configure.ac >>>>>> index 5692553..de462ff 100644 >>>>>> --- a/configure.ac >>>>>> +++ b/configure.ac >>>>>> @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test >>>>>> "x$have_libdl" = "xyes"]) >>>>>> AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) >>>>>> >>>>>> AC_CHECK_LIB([pthread], [pthread_create]) >>>>>> +AC_CHECK_LIB([pthread], [pthread_setname_np], >>>>>> + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np >>>>>> is available.]), >>>>>> + AC_CHECK_LIB([pthread], [pthread_set_name_np], >>>>>> + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if >>>>>> pthread_set_name_np is available.]), >>>>>> + AC_MSG_RESULT([pthread setname/set_name not found.]))) >>>>>> >>>>>> # Check for dlfcn.h >>>>>> AC_CHECK_HEADER([dlfcn.h]) >>>>>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >>>>>> index f1801cf..05929be 100644 >>>>>> --- a/liblttng-ust/Makefile.am >>>>>> +++ b/liblttng-ust/Makefile.am >>>>>> @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ >>>>>> error.h >>>>>> liblttng_ust_tracepoint_la_LIBADD = \ >>>>>> -lurcu-bp \ >>>>>> + -lpthread \ >>>>>> $(top_builddir)/snprintf/libustsnprintf.la >>>>>> liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info >>>>>> $(LTTNG_UST_LIBRARY_VERSION) >>>>>> liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" >>>>>> -fno-strict-aliasing >>>>>> diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h >>>>>> index 43b2223..4fee919 100644 >>>>>> --- a/liblttng-ust/compat.h >>>>>> +++ b/liblttng-ust/compat.h >>>>>> @@ -3,6 +3,7 @@ >>>>>> >>>>>> /* >>>>>> * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >>>>>> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> >>>>>> * >>>>>> * This library is free software; you can redistribute it and/or >>>>>> * modify it under the terms of the GNU Lesser General Public >>>>>> @@ -23,7 +24,6 @@ >>>>>> * lttng_ust_getprocname. >>>>>> */ >>>>>> #ifdef __linux__ >>>>>> - >>>>>> #include <sys/prctl.h> >>>>>> >>>>>> #define LTTNG_UST_PROCNAME_LEN 17 >>>>>> @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) >>>>>> (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); >>>>>> } >>>>>> >>>>>> +/* >>>>>> + * if pthread_setname_np is available. >>>>>> + */ >>>>>> +#ifdef HAVE_PTHREAD_SETNAME_NP >>>>>> +#define PTHREAD_SETNAME_NP pthread_setname_np >>>>>> +#endif >>>>>> + >>>>>> #elif defined(__FreeBSD__) >>>>>> #include <stdlib.h> >>>>>> #include <string.h> >>>>>> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) >>>>>> strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); >>>>>> } >>>>>> >>>>>> +/* >>>>>> + * if pthread_set_name_np is available. >>>>>> + */ >>>>>> +#ifdef HAVE_PTHREAD_SET_NAME_NP >>>>>> +#define PTHREAD_SETNAME_NP pthread_set_name_np >>>>>> +#endif >>>>>> + >>>>>> +#endif >>>>>> + >>>>>> +/* >>>>>> + * if a pthread setname/set_name function is available, declare >>>>>> + * the setustprocname() function that will add '-ust' to the end >>>>>> + * of the current process name, while truncating it if needed. >>>>>> + */ >>>>>> +#ifdef PTHREAD_SETNAME_NP >>>>>> +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() >>>>>> + >>>>>> +#include <pthread.h> >>>>>> + >>>>>> +static inline >>>>>> +void lttng_ust_setustprocname() >>>>>> +{ >>>>>> + char name[LTTNG_UST_PROCNAME_LEN]; >>>>> >>>>> Let's say that LTTNG_UST_PROCNAME_LEN is 10. >>>>> >>>>>> + int limit = LTTNG_UST_PROCNAME_LEN - 4; >>>>> >>>>> Then, limit is 10 - 4 = 6. >>>>> >>>>>> + int len = 0; >>>>> >>>>> This can be moved on the strlen(name) line. >>>>> >>>>>> + >>>>>> + lttng_ust_getprocname(name); >>>>> >>>>> Let's suppose that after the call to lttng_ust_getprocname(), name contains >>>>> "012345678\0", >>>>> >>>>>> + >>>>>> + len = strlen(name); >>>>> >>>>> The variable "len" will contain the integer 9. >>>>> >>>>>> + if (len >= limit) { >>>>>> + len = limit; >>>>>> + } >>>>>> + >>>>> >>>>> After that, len is >= limit (9 >= 6), so len becomes 6. >>>>> >>>>>> + sprintf(name + len, "%s", "-ust"); >>>>> >>>>> name is a pointer to "012345678\0" (9 bytes + '\0') >>>>> >>>>> name + len is name + 6, so it points to "678\0" >>>>> >>>>> sprintf will essentially copy "-ust\0" to "678\0". >>>>> >>>>> So, if my calculations are not wrong, you have a buffer overflow which will >>>>> corrupt the stack memory of the >>>>> current stack frame. >> >> You're right, will fix that. >> I'm also questionning the interest of limit being a variable here. >> Should I replace both occurrences directly by "LTTNG_UST_PROCNAME_LEN >> - 5" or create a "#define LTTNG_UST_PROCNAME_LIMIT >> (LTTNG_UST_PROCNAME_LEN - 5)" macro ? Mathieu, what do you think would >> be the best to do here ? >> The condition will also be replaced from ">=" to ">", as the "=" part >> is not useful anymore in that version of the patch (it was in v1, >> because of the operations that were done). >> >>>>> >>>>> >>>>>> + >>>>>> + PTHREAD_SETNAME_NP(pthread_self(), name); >>>>>> +} >>>>>> +#else >>>>>> +#define LTTNG_UST_SETUSTPROCNAME >>>>>> #endif >>>>>> >>>>>> #include <errno.h> >>>>>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >>>>>> index e00a22c..4de68c7 100644 >>>>>> --- a/liblttng-ust/lttng-ust-comm.c >>>>>> +++ b/liblttng-ust/lttng-ust-comm.c >>>>>> @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) >>>>>> int sock, ret, prev_connect_failed = 0, has_waited = 0; >>>>>> long timeout; >>>>>> >>>>>> + /* >>>>>> + * If available, add '-ust' to the end of this thread's >>>>>> + * process name >>>>>> + */ >>>>>> + LTTNG_UST_SETUSTPROCNAME; >>>>> >>>>> Is this common practice ? Just by looking at this line, it is unclear that it is >>>>> a function call. >>>>> >>>>> I would prefer >>>>> >>>>> LTTNG_UST_SETUSTPROCNAME() >>>>> >>>>> or >>>>> >>>>> lttng_ust_setustprocname() >>>> >>>> The latter is preferred by our coding style, indeed. >>>> >>> >>> And there is no reason to make it a macro. It should be >>> a static inline whenever possible. >> >> I made it a macro as I was not sure about the best path to take, and >> the macro seemed to be the cleaner approach! >> However, would it be better like that: >> >> #ifdef PTHREAD_SETNAME_NP >> #include <pthread.h> >> #endif >> >> static inline >> void lttng_ust_setustprocname() >> { >> #ifdef PTHREAD_SETNAME_NP >> ... >> >> PTHREAD_SETNAME_NP(pthread_self(), name); >> #endif >> } >> >> Or like that ? >> >> #ifdef PTHREAD_SETNAME_NP >> #include <pthread.h> >> >> static inline >> void lttng_ust_setustprocname() >> { >> ... >> >> PTHREAD_SETNAME_NP(pthread_self(), name); >> } >> #else >> static inline >> void lttng_ust_setustprocname() >> {} >> #endif >> >> >> In both those cases, the macro disappears, and is replaced by a call >> to lttng_ust_setustprocname(). >> I'm not sure however which is preferable according to the coding style >> (In one case, two #ifdef are required, in the other, we declare the >> function in each branch of the ifdef/else). >> >> Thoughts ? I'll send a v3 of the patch according to your preference in >> both matters. >> >> Thanks a lot! >> Raphaël -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <cover.1461269886.git.raphael.beamonte@gmail.com>]
* [RFC PATCH 1/2] Add -ust to the name of UST threads of the application [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> @ 2016-04-21 20:50 ` Raphaël Beamonte 2016-04-21 20:50 ` [RFC PATCH 2/2] " Raphaël Beamonte ` (6 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-04-21 20:50 UTC (permalink / raw) To: lttng-dev; +Cc: Raphaël Beamonte Add the required functions to change the thread name of the UST threads and add the -ust string at its end. This will help to identify LTTng-UST processes when analyzing the trace of a process. Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> --- liblttng-ust/compat.h | 22 ++++++++++++++++++++++ liblttng-ust/lttng-ust-comm.c | 2 ++ 2 files changed, 24 insertions(+) diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h index 43b2223..8d5fc77 100644 --- a/liblttng-ust/compat.h +++ b/liblttng-ust/compat.h @@ -34,6 +34,23 @@ void lttng_ust_getprocname(char *name) (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); } +static inline +void lttng_ust_setustprocname() +{ + char name[LTTNG_UST_PROCNAME_LEN]; + int limit = LTTNG_UST_PROCNAME_LEN - 4; + + lttng_ust_getprocname(name); + + if (strlen(name) >= limit) { + name[limit] = 0; + } + + sprintf(name, "%s%s", name, "-ust"); + + (void) prctl(PR_SET_NAME, name, 0, 0, 0); +} + #elif defined(__FreeBSD__) #include <stdlib.h> #include <string.h> @@ -59,6 +76,11 @@ void lttng_ust_getprocname(char *name) strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); } +static inline +void lttng_ust_setustprocname() +{ +} + #endif #include <errno.h> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index e00a22c..a85cb21 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -1295,6 +1295,8 @@ void *ust_listener_thread(void *arg) int sock, ret, prev_connect_failed = 0, has_waited = 0; long timeout; + lttng_ust_setustprocname(); + /* Restart trying to connect to the session daemon */ restart: if (prev_connect_failed) { -- 2.1.4 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [RFC PATCH 2/2] Add -ust to the name of UST threads of the application [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> 2016-04-21 20:50 ` [RFC PATCH 1/2] " Raphaël Beamonte @ 2016-04-21 20:50 ` Raphaël Beamonte 2016-06-01 15:04 ` [RFC PATCH 0/2] Identify UST threads by changing thread names Raphaël Beamonte ` (5 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-04-21 20:50 UTC (permalink / raw) To: lttng-dev; +Cc: Raphaël Beamonte Add the required functions to change the thread name of the UST threads and add the -ust string at its end. This will help to identify LTTng-UST processes when analyzing the trace of a process. Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> --- liblttng-ust/Makefile.am | 1 + liblttng-ust/compat.h | 35 +++++++++++++++++++++++++++++++++++ liblttng-ust/lttng-ust-comm.c | 2 ++ 3 files changed, 38 insertions(+) diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am index 876e9b5..8d78d63 100644 --- a/liblttng-ust/Makefile.am +++ b/liblttng-ust/Makefile.am @@ -13,6 +13,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ error.h liblttng_ust_tracepoint_la_LIBADD = \ -lurcu-bp \ + -lpthread \ $(top_builddir)/snprintf/libustsnprintf.la liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info $(LTTNG_UST_LIBRARY_VERSION) liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" -fno-strict-aliasing diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h index 43b2223..161b957 100644 --- a/liblttng-ust/compat.h +++ b/liblttng-ust/compat.h @@ -24,6 +24,7 @@ */ #ifdef __linux__ +#include <pthread.h> #include <sys/prctl.h> #define LTTNG_UST_PROCNAME_LEN 17 @@ -34,6 +35,23 @@ void lttng_ust_getprocname(char *name) (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); } +static inline +void lttng_ust_setustprocname() +{ + char name[LTTNG_UST_PROCNAME_LEN]; + int limit = LTTNG_UST_PROCNAME_LEN - 4; + + lttng_ust_getprocname(name); + + if (strlen(name) >= limit) { + name[limit] = 0; + } + + sprintf(name, "%s%s", name, "-ust"); + + pthread_setname_np(pthread_self(), name); +} + #elif defined(__FreeBSD__) #include <stdlib.h> #include <string.h> @@ -59,6 +77,23 @@ void lttng_ust_getprocname(char *name) strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); } +static inline +void lttng_ust_setustprocname() +{ + char name[LTTNG_UST_PROCNAME_LEN]; + int limit = LTTNG_UST_PROCNAME_LEN - 4; + + lttng_ust_getprocname(name); + + if (strlen(name) >= limit) { + name[limit] = 0; + } + + sprintf(name, "%s%s", name, "-ust"); + + pthread_set_name_np(pthread_self(), name); +} + #endif #include <errno.h> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index e00a22c..a85cb21 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -1295,6 +1295,8 @@ void *ust_listener_thread(void *arg) int sock, ret, prev_connect_failed = 0, has_waited = 0; long timeout; + lttng_ust_setustprocname(); + /* Restart trying to connect to the session daemon */ restart: if (prev_connect_failed) { -- 2.1.4 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> 2016-04-21 20:50 ` [RFC PATCH 1/2] " Raphaël Beamonte 2016-04-21 20:50 ` [RFC PATCH 2/2] " Raphaël Beamonte @ 2016-06-01 15:04 ` Raphaël Beamonte [not found] ` <CAE_Gge2jacOMKQaPdp_dXf3NP+yJTS+jWgJQ-SUnv1-B6WgcGw@mail.gmail.com> ` (4 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-01 15:04 UTC (permalink / raw) To: lttng-dev Hi, I did not get any feedback about this, any thoughts ? Thanks, Raphaël 2016-04-21 16:50 GMT-04:00 Raphaël Beamonte <raphael.beamonte@gmail.com>: > Hi, > > This is an RFC patch for comments and ideas about how this should be > done. In recent work I've been doing, I found out that some UST- > instrumented userspace process was preempting itself. Looking further, > it was not the same TID, but the same process name. These processes > were LTTng-UST listener threads, but were not identified as so. > Adding a simple symbol, such as "-ust" or "-lttng" at the end of the > process name would help to identify such process directly in the > generated trace. > > You'll find below two different implementations with their pros > and cons, for which the "-ust" symbol was choosen: > > > 1/ An implementation using prctl > It works properly on Linux, but I didn't find a way to make > it work under FreeBSD, hence the empty function in FreeBSD > case (keeping the situation as it is now). > > Raphaël Beamonte (1): > Add -ust to the name of UST threads of the application > > liblttng-ust/compat.h | 22 ++++++++++++++++++++++ > liblttng-ust/lttng-ust-comm.c | 2 ++ > 2 files changed, 24 insertions(+) > > > > 2/ An implementation using pthread > It works on both Linux and FreeBSD, but requires to add a > link to the pthread library in liblttng-ust Makefile.am. > Also, this approach relies on the presence of the > pthread_setname_np function, which was included in glibc > 2.12, meaning it will not work with older versions (but > this should not be a problem?) > > Raphaël Beamonte (1): > Add -ust to the name of UST threads of the application > > liblttng-ust/Makefile.am | 1 + > liblttng-ust/compat.h | 35 +++++++++++++++++++++++++++++++++++ > liblttng-ust/lttng-ust-comm.c | 2 ++ > 3 files changed, 38 insertions(+) > > > > I'll wait for your comments and ideas on that subject. > > Thanks, > Raphaël > > > -- > 2.1.4 > _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAE_Gge2jacOMKQaPdp_dXf3NP+yJTS+jWgJQ-SUnv1-B6WgcGw@mail.gmail.com>]
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <CAE_Gge2jacOMKQaPdp_dXf3NP+yJTS+jWgJQ-SUnv1-B6WgcGw@mail.gmail.com> @ 2016-06-03 6:10 ` Jérémie Galarneau [not found] ` <CA+jJMxvmHRYYtHWYjumXrS1BrBCih3WJnRA5yGFBXTw7jL3HiA@mail.gmail.com> 1 sibling, 0 replies; 22+ messages in thread From: Jérémie Galarneau @ 2016-06-03 6:10 UTC (permalink / raw) To: Raphaël Beamonte; +Cc: lttng-dev Hi Raphaël, Adding Mathieu in CC. Please make sure you CC the maintainers when submitting a patch. Regards, Jérémie On Wed, Jun 1, 2016 at 11:04 AM, Raphaël Beamonte <raphael.beamonte@gmail.com> wrote: > Hi, > > I did not get any feedback about this, any thoughts ? > > Thanks, > Raphaël > > 2016-04-21 16:50 GMT-04:00 Raphaël Beamonte <raphael.beamonte@gmail.com>: >> Hi, >> >> This is an RFC patch for comments and ideas about how this should be >> done. In recent work I've been doing, I found out that some UST- >> instrumented userspace process was preempting itself. Looking further, >> it was not the same TID, but the same process name. These processes >> were LTTng-UST listener threads, but were not identified as so. >> Adding a simple symbol, such as "-ust" or "-lttng" at the end of the >> process name would help to identify such process directly in the >> generated trace. >> >> You'll find below two different implementations with their pros >> and cons, for which the "-ust" symbol was choosen: >> >> >> 1/ An implementation using prctl >> It works properly on Linux, but I didn't find a way to make >> it work under FreeBSD, hence the empty function in FreeBSD >> case (keeping the situation as it is now). >> >> Raphaël Beamonte (1): >> Add -ust to the name of UST threads of the application >> >> liblttng-ust/compat.h | 22 ++++++++++++++++++++++ >> liblttng-ust/lttng-ust-comm.c | 2 ++ >> 2 files changed, 24 insertions(+) >> >> >> >> 2/ An implementation using pthread >> It works on both Linux and FreeBSD, but requires to add a >> link to the pthread library in liblttng-ust Makefile.am. >> Also, this approach relies on the presence of the >> pthread_setname_np function, which was included in glibc >> 2.12, meaning it will not work with older versions (but >> this should not be a problem?) >> >> Raphaël Beamonte (1): >> Add -ust to the name of UST threads of the application >> >> liblttng-ust/Makefile.am | 1 + >> liblttng-ust/compat.h | 35 +++++++++++++++++++++++++++++++++++ >> liblttng-ust/lttng-ust-comm.c | 2 ++ >> 3 files changed, 38 insertions(+) >> >> >> >> I'll wait for your comments and ideas on that subject. >> >> Thanks, >> Raphaël >> >> >> -- >> 2.1.4 >> > _______________________________________________ > lttng-dev mailing list > lttng-dev@lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CA+jJMxvmHRYYtHWYjumXrS1BrBCih3WJnRA5yGFBXTw7jL3HiA@mail.gmail.com>]
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <CA+jJMxvmHRYYtHWYjumXrS1BrBCih3WJnRA5yGFBXTw7jL3HiA@mail.gmail.com> @ 2016-06-03 6:11 ` Jérémie Galarneau [not found] ` <CA+jJMxs9yNOeeqtYBfGTz1ADOqqiVO=XZd-7qm48BS8dzVo-iw@mail.gmail.com> 1 sibling, 0 replies; 22+ messages in thread From: Jérémie Galarneau @ 2016-06-03 6:11 UTC (permalink / raw) To: Raphaël Beamonte; +Cc: lttng-dev And, of course, I forget to add Mathieu in CC... Coffee time! Jérémie On Fri, Jun 3, 2016 at 2:10 AM, Jérémie Galarneau <jeremie.galarneau@efficios.com> wrote: > Hi Raphaël, > > Adding Mathieu in CC. Please make sure you CC the maintainers when > submitting a patch. > > Regards, > Jérémie > > On Wed, Jun 1, 2016 at 11:04 AM, Raphaël Beamonte > <raphael.beamonte@gmail.com> wrote: >> Hi, >> >> I did not get any feedback about this, any thoughts ? >> >> Thanks, >> Raphaël >> >> 2016-04-21 16:50 GMT-04:00 Raphaël Beamonte <raphael.beamonte@gmail.com>: >>> Hi, >>> >>> This is an RFC patch for comments and ideas about how this should be >>> done. In recent work I've been doing, I found out that some UST- >>> instrumented userspace process was preempting itself. Looking further, >>> it was not the same TID, but the same process name. These processes >>> were LTTng-UST listener threads, but were not identified as so. >>> Adding a simple symbol, such as "-ust" or "-lttng" at the end of the >>> process name would help to identify such process directly in the >>> generated trace. >>> >>> You'll find below two different implementations with their pros >>> and cons, for which the "-ust" symbol was choosen: >>> >>> >>> 1/ An implementation using prctl >>> It works properly on Linux, but I didn't find a way to make >>> it work under FreeBSD, hence the empty function in FreeBSD >>> case (keeping the situation as it is now). >>> >>> Raphaël Beamonte (1): >>> Add -ust to the name of UST threads of the application >>> >>> liblttng-ust/compat.h | 22 ++++++++++++++++++++++ >>> liblttng-ust/lttng-ust-comm.c | 2 ++ >>> 2 files changed, 24 insertions(+) >>> >>> >>> >>> 2/ An implementation using pthread >>> It works on both Linux and FreeBSD, but requires to add a >>> link to the pthread library in liblttng-ust Makefile.am. >>> Also, this approach relies on the presence of the >>> pthread_setname_np function, which was included in glibc >>> 2.12, meaning it will not work with older versions (but >>> this should not be a problem?) >>> >>> Raphaël Beamonte (1): >>> Add -ust to the name of UST threads of the application >>> >>> liblttng-ust/Makefile.am | 1 + >>> liblttng-ust/compat.h | 35 +++++++++++++++++++++++++++++++++++ >>> liblttng-ust/lttng-ust-comm.c | 2 ++ >>> 3 files changed, 38 insertions(+) >>> >>> >>> >>> I'll wait for your comments and ideas on that subject. >>> >>> Thanks, >>> Raphaël >>> >>> >>> -- >>> 2.1.4 >>> >> _______________________________________________ >> lttng-dev mailing list >> lttng-dev@lists.lttng.org >> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > > > > -- > Jérémie Galarneau > EfficiOS Inc. > http://www.efficios.com -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CA+jJMxs9yNOeeqtYBfGTz1ADOqqiVO=XZd-7qm48BS8dzVo-iw@mail.gmail.com>]
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <CA+jJMxs9yNOeeqtYBfGTz1ADOqqiVO=XZd-7qm48BS8dzVo-iw@mail.gmail.com> @ 2016-06-03 6:18 ` Mathieu Desnoyers [not found] ` <1192105907.26502.1464934699922.JavaMail.zimbra@efficios.com> 1 sibling, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 6:18 UTC (permalink / raw) To: Jeremie Galarneau; +Cc: Raphaël Beamonte, lttng-dev ----- On Jun 3, 2016, at 8:11 AM, Jeremie Galarneau jeremie.galarneau@efficios.com wrote: > And, of course, I forget to add Mathieu in CC... Coffee time! > > Jérémie > > On Fri, Jun 3, 2016 at 2:10 AM, Jérémie Galarneau > <jeremie.galarneau@efficios.com> wrote: >> Hi Raphaël, >> >> Adding Mathieu in CC. Please make sure you CC the maintainers when >> submitting a patch. >> >> Regards, >> Jérémie >> >> On Wed, Jun 1, 2016 at 11:04 AM, Raphaël Beamonte >> <raphael.beamonte@gmail.com> wrote: >>> Hi, >>> >>> I did not get any feedback about this, any thoughts ? >>> >>> Thanks, >>> Raphaël >>> >>> 2016-04-21 16:50 GMT-04:00 Raphaël Beamonte <raphael.beamonte@gmail.com>: >>>> Hi, >>>> >>>> This is an RFC patch for comments and ideas about how this should be >>>> done. In recent work I've been doing, I found out that some UST- >>>> instrumented userspace process was preempting itself. Looking further, >>>> it was not the same TID, but the same process name. These processes >>>> were LTTng-UST listener threads, but were not identified as so. >>>> Adding a simple symbol, such as "-ust" or "-lttng" at the end of the >>>> process name would help to identify such process directly in the >>>> generated trace. >>>> >>>> You'll find below two different implementations with their pros >>>> and cons, for which the "-ust" symbol was choosen: >>>> >>>> >>>> 1/ An implementation using prctl >>>> It works properly on Linux, but I didn't find a way to make >>>> it work under FreeBSD, hence the empty function in FreeBSD >>>> case (keeping the situation as it is now). >>>> >>>> Raphaël Beamonte (1): >>>> Add -ust to the name of UST threads of the application >>>> >>>> liblttng-ust/compat.h | 22 ++++++++++++++++++++++ >>>> liblttng-ust/lttng-ust-comm.c | 2 ++ >>>> 2 files changed, 24 insertions(+) >>>> >>>> >>>> >>>> 2/ An implementation using pthread >>>> It works on both Linux and FreeBSD, but requires to add a >>>> link to the pthread library in liblttng-ust Makefile.am. >>>> Also, this approach relies on the presence of the >>>> pthread_setname_np function, which was included in glibc >>>> 2.12, meaning it will not work with older versions (but >>>> this should not be a problem?) lttng-ust already uses libc for mutexes and other. So I don't see why you have to _add_ any explicit dependency there. About the glibc version, we should use autotools to detect the features supported by the system libc, and pass a config define to the source code, so we can use the glibc pthread_setname_np if available, else we keep the current behavior. Thoughts ? Thanks, Mathieu >>>> >>>> Raphaël Beamonte (1): >>>> Add -ust to the name of UST threads of the application >>>> >>>> liblttng-ust/Makefile.am | 1 + >>>> liblttng-ust/compat.h | 35 +++++++++++++++++++++++++++++++++++ >>>> liblttng-ust/lttng-ust-comm.c | 2 ++ >>>> 3 files changed, 38 insertions(+) >>>> >>>> >>>> >>>> I'll wait for your comments and ideas on that subject. >>>> >>>> Thanks, >>>> Raphaël >>>> >>>> >>>> -- >>>> 2.1.4 >>>> >>> _______________________________________________ >>> lttng-dev mailing list >>> lttng-dev@lists.lttng.org >>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev >> >> >> >> -- >> Jérémie Galarneau >> EfficiOS Inc. >> http://www.efficios.com > > > > -- > Jérémie Galarneau > EfficiOS Inc. > http://www.efficios.com -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1192105907.26502.1464934699922.JavaMail.zimbra@efficios.com>]
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <1192105907.26502.1464934699922.JavaMail.zimbra@efficios.com> @ 2016-06-03 6:24 ` Mathieu Desnoyers [not found] ` <293416701.26545.1464935065348.JavaMail.zimbra@efficios.com> 1 sibling, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 6:24 UTC (permalink / raw) To: Jeremie Galarneau; +Cc: Raphaël Beamonte, lttng-dev ----- On Jun 3, 2016, at 8:18 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > ----- On Jun 3, 2016, at 8:11 AM, Jeremie Galarneau > jeremie.galarneau@efficios.com wrote: > >> And, of course, I forget to add Mathieu in CC... Coffee time! >> >> Jérémie >> >> On Fri, Jun 3, 2016 at 2:10 AM, Jérémie Galarneau >> <jeremie.galarneau@efficios.com> wrote: >>> Hi Raphaël, >>> >>> Adding Mathieu in CC. Please make sure you CC the maintainers when >>> submitting a patch. >>> >>> Regards, >>> Jérémie >>> >>> On Wed, Jun 1, 2016 at 11:04 AM, Raphaël Beamonte >>> <raphael.beamonte@gmail.com> wrote: >>>> Hi, >>>> >>>> I did not get any feedback about this, any thoughts ? >>>> >>>> Thanks, >>>> Raphaël >>>> >>>> 2016-04-21 16:50 GMT-04:00 Raphaël Beamonte <raphael.beamonte@gmail.com>: >>>>> Hi, >>>>> >>>>> This is an RFC patch for comments and ideas about how this should be >>>>> done. In recent work I've been doing, I found out that some UST- >>>>> instrumented userspace process was preempting itself. Looking further, >>>>> it was not the same TID, but the same process name. These processes >>>>> were LTTng-UST listener threads, but were not identified as so. >>>>> Adding a simple symbol, such as "-ust" or "-lttng" at the end of the >>>>> process name would help to identify such process directly in the >>>>> generated trace. >>>>> >>>>> You'll find below two different implementations with their pros >>>>> and cons, for which the "-ust" symbol was choosen: >>>>> >>>>> >>>>> 1/ An implementation using prctl >>>>> It works properly on Linux, but I didn't find a way to make >>>>> it work under FreeBSD, hence the empty function in FreeBSD >>>>> case (keeping the situation as it is now). >>>>> >>>>> Raphaël Beamonte (1): >>>>> Add -ust to the name of UST threads of the application >>>>> >>>>> liblttng-ust/compat.h | 22 ++++++++++++++++++++++ >>>>> liblttng-ust/lttng-ust-comm.c | 2 ++ >>>>> 2 files changed, 24 insertions(+) >>>>> >>>>> >>>>> >>>>> 2/ An implementation using pthread >>>>> It works on both Linux and FreeBSD, but requires to add a >>>>> link to the pthread library in liblttng-ust Makefile.am. >>>>> Also, this approach relies on the presence of the >>>>> pthread_setname_np function, which was included in glibc >>>>> 2.12, meaning it will not work with older versions (but >>>>> this should not be a problem?) > > lttng-ust already uses libc for mutexes and other. So I don't see why > you have to _add_ any explicit dependency there. and we already explicitly use pthread (pthread_create). Thanks, Mathieu > > About the glibc version, we should use autotools to detect the > features supported by the system libc, and pass a config define > to the source code, so we can use the glibc pthread_setname_np > if available, else we keep the current behavior. > > Thoughts ? > > Thanks, > > Mathieu > >>>>> >>>>> Raphaël Beamonte (1): >>>>> Add -ust to the name of UST threads of the application >>>>> >>>>> liblttng-ust/Makefile.am | 1 + >>>>> liblttng-ust/compat.h | 35 +++++++++++++++++++++++++++++++++++ >>>>> liblttng-ust/lttng-ust-comm.c | 2 ++ >>>>> 3 files changed, 38 insertions(+) >>>>> >>>>> >>>>> >>>>> I'll wait for your comments and ideas on that subject. >>>>> >>>>> Thanks, >>>>> Raphaël >>>>> >>>>> >>>>> -- >>>>> 2.1.4 >>>>> >>>> _______________________________________________ >>>> lttng-dev mailing list >>>> lttng-dev@lists.lttng.org >>>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev >>> >>> >>> >>> -- >>> Jérémie Galarneau >>> EfficiOS Inc. >>> http://www.efficios.com >> >> >> >> -- >> Jérémie Galarneau >> EfficiOS Inc. >> http://www.efficios.com > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com > _______________________________________________ > lttng-dev mailing list > lttng-dev@lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <293416701.26545.1464935065348.JavaMail.zimbra@efficios.com>]
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <293416701.26545.1464935065348.JavaMail.zimbra@efficios.com> @ 2016-06-03 14:12 ` Raphaël Beamonte [not found] ` <CAE_Gge2032JkDubp=vpaqNzoQre7MfvQC4UGm-wXPmux-Fwh6g@mail.gmail.com> 1 sibling, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-03 14:12 UTC (permalink / raw) To: Mathieu Desnoyers; +Cc: lttng-dev Hi! Thanks for all of your comments, I appreciate it! Thanks Jeremie for adding Mathieu in Cc, forgot to do it... Will remember next time! :) 2016-06-02 9:22 GMT-04:00 Sebastien Boisvert <sboisvert@gydle.com>: > Do you know if the name of these listener threads can be set in their constructor, directly > when they are brought into this world ? pthread_create does not allow to specify a name when creating the new thread, as far as I know! That's why the 'lttng_ust_setustprocname()' function is called first thing in the ust_listener_thread function. > Perhaps the availability of pthread_setname_np and/or pthread_set_name_np (you have both names > in your patch 2) should be ascertained in another venue, such as the configure.ac -- where it belongs. True! 2016-06-03 2:24 GMT-04:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>: > ----- On Jun 3, 2016, at 8:18 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: >> lttng-ust already uses libc for mutexes and other. So I don't see why >> you have to _add_ any explicit dependency there. > > and we already explicitly use pthread (pthread_create). You're right, pthread is already used. However, when compiling the code without adding the pthread dependency, I get the following warning: In file included from lttng-tracer.h:30:0, from lttng-ust-abi.c:50: compat.h: In function ‘lttng_ust_setustprocname’: compat.h:52:2: warning: implicit declaration of function ‘pthread_setname_np’ [-Wimplicit-function-declaration] pthread_setname_np(pthread_self(), name); ^ CC lttng-context-procname.lo In file included from lttng-context-procname.c:28:0: compat.h: In function ‘lttng_ust_setustprocname’: compat.h:52:2: warning: implicit declaration of function ‘pthread_setname_np’ [-Wimplicit-function-declaration] pthread_setname_np(pthread_self(), name); ^ It seems to be because pthread is not specified as dependency for liblttng-ust-tracepoint in liblttng-ust/Makefile.am. However, perhaps that pthread dependency should be added elsewhere? >> About the glibc version, we should use autotools to detect the >> features supported by the system libc, and pass a config define >> to the source code, so we can use the glibc pthread_setname_np >> if available, else we keep the current behavior. That's a good idea, will do! I'll update and rebase the patch following your comments. If you have any other insight about the pthread dependency, I'm all ears! Thanks, Raphaël _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAE_Gge2032JkDubp=vpaqNzoQre7MfvQC4UGm-wXPmux-Fwh6g@mail.gmail.com>]
* Re: [RFC PATCH 0/2] Identify UST threads by changing thread names [not found] ` <CAE_Gge2032JkDubp=vpaqNzoQre7MfvQC4UGm-wXPmux-Fwh6g@mail.gmail.com> @ 2016-06-03 15:38 ` Mathieu Desnoyers 0 siblings, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-03 15:38 UTC (permalink / raw) To: Raphaël Beamonte; +Cc: lttng-dev ----- On Jun 3, 2016, at 4:12 PM, Raphaël Beamonte raphael.beamonte@gmail.com wrote: > Hi! > > Thanks for all of your comments, I appreciate it! > Thanks Jeremie for adding Mathieu in Cc, forgot to do it... Will > remember next time! :) > > 2016-06-02 9:22 GMT-04:00 Sebastien Boisvert <sboisvert@gydle.com>: >> Do you know if the name of these listener threads can be set in their >> constructor, directly >> when they are brought into this world ? > > pthread_create does not allow to specify a name when creating the new > thread, as far as I know! That's why the 'lttng_ust_setustprocname()' > function is called first thing in the ust_listener_thread function. > >> Perhaps the availability of pthread_setname_np and/or pthread_set_name_np (you >> have both names >> in your patch 2) should be ascertained in another venue, such as the >> configure.ac -- where it belongs. > > True! > > 2016-06-03 2:24 GMT-04:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>: >> ----- On Jun 3, 2016, at 8:18 AM, Mathieu Desnoyers >> mathieu.desnoyers@efficios.com wrote: >>> lttng-ust already uses libc for mutexes and other. So I don't see why >>> you have to _add_ any explicit dependency there. >> >> and we already explicitly use pthread (pthread_create). > > You're right, pthread is already used. However, when compiling the > code without adding the pthread dependency, I get the following > warning: > > In file included from lttng-tracer.h:30:0, > from lttng-ust-abi.c:50: > compat.h: In function ‘lttng_ust_setustprocname’: > compat.h:52:2: warning: implicit declaration of function > ‘pthread_setname_np’ [-Wimplicit-function-declaration] > pthread_setname_np(pthread_self(), name); > ^ > CC lttng-context-procname.lo > In file included from lttng-context-procname.c:28:0: > compat.h: In function ‘lttng_ust_setustprocname’: > compat.h:52:2: warning: implicit declaration of function > ‘pthread_setname_np’ [-Wimplicit-function-declaration] > pthread_setname_np(pthread_self(), name); > ^ > > It seems to be because pthread is not specified as dependency for > liblttng-ust-tracepoint in liblttng-ust/Makefile.am. However, perhaps > that pthread dependency should be added elsewhere? Seems fair. Thanks, Mathieu > >>> About the glibc version, we should use autotools to detect the >>> features supported by the system libc, and pass a config define >>> to the source code, so we can use the glibc pthread_setname_np >>> if available, else we keep the current behavior. > > That's a good idea, will do! > > I'll update and rebase the patch following your comments. > If you have any other insight about the pthread dependency, I'm all ears! > > Thanks, > Raphaël -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
* [lttng-ust RFC PATCH v2 0/1] Identify UST threads by changing thread names [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> ` (3 preceding siblings ...) [not found] ` <CAE_Gge2jacOMKQaPdp_dXf3NP+yJTS+jWgJQ-SUnv1-B6WgcGw@mail.gmail.com> @ 2016-06-03 15:16 ` Raphaël Beamonte 2016-06-03 15:17 ` [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application Raphaël Beamonte ` (2 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-03 15:16 UTC (permalink / raw) To: lttng-dev, mathieu.desnoyers; +Cc: Raphaël Beamonte Hi, This is the update of the RFC patch following your comments. This patch updates the pthread_setname_np/pthread_set_name_np version of the RFC patch, and includes the following changes: - Check in configure.ac whether the pthread_setname_np or pthread_set_name_np functions exists. If it's not the case, current behavior is preserved. - Update compat.h to define a macro for the pthread_set*name_np function to use in order to reduce repeated code. - Update compat.h and lttng-ust-comm.c to use a macro to call the lttng_ust_setustprocname function in order for that macro to be empty when the required pthread_setn*ame_np is not available. - Update compat.h to fix sprintf usage with the same variable to read from and write to. - The patch has been rebased on current lttng-ust's master branch. This patch still contains the link to the pthread library in the liblttng-ust/Makefile.am file, which still seems required to avoid compilation warnings about implicit function declaration. Thoughts and comments? Thanks! Raphaël Raphaël Beamonte (1): Add -ust to the name of UST threads of the application configure.ac | 5 +++++ liblttng-ust/Makefile.am | 1 + liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- liblttng-ust/lttng-ust-comm.c | 6 ++++++ 4 files changed, 59 insertions(+), 1 deletion(-) -- 2.8.1 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
* [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> ` (4 preceding siblings ...) 2016-06-03 15:16 ` [lttng-ust RFC PATCH v2 0/1] " Raphaël Beamonte @ 2016-06-03 15:17 ` Raphaël Beamonte 2016-06-03 16:45 ` [lttng-ust PATCH v3 0/1] Identify UST threads by changing thread names Raphaël Beamonte 2016-06-03 16:45 ` [lttng-ust PATCH v3 1/1] Add -ust to the name of UST threads of the application Raphaël Beamonte 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-03 15:17 UTC (permalink / raw) To: lttng-dev, mathieu.desnoyers; +Cc: Raphaël Beamonte Add the required functions to change the thread name of the UST threads and add the -ust string at its end. This will help to identify LTTng-UST processes when analyzing the trace of a process. Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> --- configure.ac | 5 +++++ liblttng-ust/Makefile.am | 1 + liblttng-ust/compat.h | 48 ++++++++++++++++++++++++++++++++++++++++++- liblttng-ust/lttng-ust-comm.c | 6 ++++++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 5692553..de462ff 100644 --- a/configure.ac +++ b/configure.ac @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test "x$have_libdl" = "xyes"]) AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) AC_CHECK_LIB([pthread], [pthread_create]) +AC_CHECK_LIB([pthread], [pthread_setname_np], + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np is available.]), + AC_CHECK_LIB([pthread], [pthread_set_name_np], + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if pthread_set_name_np is available.]), + AC_MSG_RESULT([pthread setname/set_name not found.]))) # Check for dlfcn.h AC_CHECK_HEADER([dlfcn.h]) diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am index f1801cf..05929be 100644 --- a/liblttng-ust/Makefile.am +++ b/liblttng-ust/Makefile.am @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ error.h liblttng_ust_tracepoint_la_LIBADD = \ -lurcu-bp \ + -lpthread \ $(top_builddir)/snprintf/libustsnprintf.la liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info $(LTTNG_UST_LIBRARY_VERSION) liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" -fno-strict-aliasing diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h index 43b2223..4fee919 100644 --- a/liblttng-ust/compat.h +++ b/liblttng-ust/compat.h @@ -3,6 +3,7 @@ /* * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -23,7 +24,6 @@ * lttng_ust_getprocname. */ #ifdef __linux__ - #include <sys/prctl.h> #define LTTNG_UST_PROCNAME_LEN 17 @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); } +/* + * if pthread_setname_np is available. + */ +#ifdef HAVE_PTHREAD_SETNAME_NP +#define PTHREAD_SETNAME_NP pthread_setname_np +#endif + #elif defined(__FreeBSD__) #include <stdlib.h> #include <string.h> @@ -59,6 +66,45 @@ void lttng_ust_getprocname(char *name) strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); } +/* + * if pthread_set_name_np is available. + */ +#ifdef HAVE_PTHREAD_SET_NAME_NP +#define PTHREAD_SETNAME_NP pthread_set_name_np +#endif + +#endif + +/* + * if a pthread setname/set_name function is available, declare + * the setustprocname() function that will add '-ust' to the end + * of the current process name, while truncating it if needed. + */ +#ifdef PTHREAD_SETNAME_NP +#define LTTNG_UST_SETUSTPROCNAME lttng_ust_setustprocname() + +#include <pthread.h> + +static inline +void lttng_ust_setustprocname() +{ + char name[LTTNG_UST_PROCNAME_LEN]; + int limit = LTTNG_UST_PROCNAME_LEN - 4; + int len = 0; + + lttng_ust_getprocname(name); + + len = strlen(name); + if (len >= limit) { + len = limit; + } + + sprintf(name + len, "%s", "-ust"); + + PTHREAD_SETNAME_NP(pthread_self(), name); +} +#else +#define LTTNG_UST_SETUSTPROCNAME #endif #include <errno.h> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index e00a22c..4de68c7 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) int sock, ret, prev_connect_failed = 0, has_waited = 0; long timeout; + /* + * If available, add '-ust' to the end of this thread's + * process name + */ + LTTNG_UST_SETUSTPROCNAME; + /* Restart trying to connect to the session daemon */ restart: if (prev_connect_failed) { -- 2.8.1 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [lttng-ust PATCH v3 0/1] Identify UST threads by changing thread names [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> ` (5 preceding siblings ...) 2016-06-03 15:17 ` [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application Raphaël Beamonte @ 2016-06-03 16:45 ` Raphaël Beamonte 2016-06-03 16:45 ` [lttng-ust PATCH v3 1/1] Add -ust to the name of UST threads of the application Raphaël Beamonte 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-03 16:45 UTC (permalink / raw) To: lttng-dev, mathieu.desnoyers; +Cc: Raphaël Beamonte Hi, This is the update of the RFC patch following your comments. This patch updates the pthread_setname_np/pthread_set_name_np version of the RFC patch, and includes the following changes: - Add a LTTNG_UST_PROCNAME_SUFFIX macro in compat.h that contains the string to be added at the end of the process name - Fix the limit value (was 4 instead of 5, now uses directly strlen on the LTTNG_UST_PROCNAME_SUFFIX macro value) - Removes the macro used to call the lttng_ust_setustprocname function, an empty lttng_ust_setustprocname is called instead when the set*name pthread functions are not available - Replaced 'lttng_ust_setustprocname()' per 'lttng_ust_setustprocname(void)' in the function declaration. This should address all the comments you had! Any other thoughts or comments? Thanks a lot! Raphaël Raphaël Beamonte (1): Add -ust to the name of UST threads of the application configure.ac | 5 +++++ liblttng-ust/Makefile.am | 1 + liblttng-ust/compat.h | 51 ++++++++++++++++++++++++++++++++++++++++++- liblttng-ust/lttng-ust-comm.c | 6 +++++ 4 files changed, 62 insertions(+), 1 deletion(-) -- 2.8.1 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
* [lttng-ust PATCH v3 1/1] Add -ust to the name of UST threads of the application [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> ` (6 preceding siblings ...) 2016-06-03 16:45 ` [lttng-ust PATCH v3 0/1] Identify UST threads by changing thread names Raphaël Beamonte @ 2016-06-03 16:45 ` Raphaël Beamonte 7 siblings, 0 replies; 22+ messages in thread From: Raphaël Beamonte @ 2016-06-03 16:45 UTC (permalink / raw) To: lttng-dev, mathieu.desnoyers; +Cc: Raphaël Beamonte Add the required functions to change the thread name of the UST threads and add the -ust string at its end. This will help to identify LTTng-UST processes when analyzing the trace of a process. Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> --- configure.ac | 5 +++++ liblttng-ust/Makefile.am | 1 + liblttng-ust/compat.h | 51 ++++++++++++++++++++++++++++++++++++++++++- liblttng-ust/lttng-ust-comm.c | 6 +++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 5692553..de462ff 100644 --- a/configure.ac +++ b/configure.ac @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test "x$have_libdl" = "xyes"]) AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) AC_CHECK_LIB([pthread], [pthread_create]) +AC_CHECK_LIB([pthread], [pthread_setname_np], + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np is available.]), + AC_CHECK_LIB([pthread], [pthread_set_name_np], + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if pthread_set_name_np is available.]), + AC_MSG_RESULT([pthread setname/set_name not found.]))) # Check for dlfcn.h AC_CHECK_HEADER([dlfcn.h]) diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am index f1801cf..05929be 100644 --- a/liblttng-ust/Makefile.am +++ b/liblttng-ust/Makefile.am @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ error.h liblttng_ust_tracepoint_la_LIBADD = \ -lurcu-bp \ + -lpthread \ $(top_builddir)/snprintf/libustsnprintf.la liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info $(LTTNG_UST_LIBRARY_VERSION) liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" -fno-strict-aliasing diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h index 43b2223..68d4d50 100644 --- a/liblttng-ust/compat.h +++ b/liblttng-ust/compat.h @@ -3,6 +3,7 @@ /* * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -23,7 +24,6 @@ * lttng_ust_getprocname. */ #ifdef __linux__ - #include <sys/prctl.h> #define LTTNG_UST_PROCNAME_LEN 17 @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); } +/* + * if pthread_setname_np is available. + */ +#ifdef HAVE_PTHREAD_SETNAME_NP +#define PTHREAD_SETNAME_NP pthread_setname_np +#endif + #elif defined(__FreeBSD__) #include <stdlib.h> #include <string.h> @@ -59,6 +66,48 @@ void lttng_ust_getprocname(char *name) strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); } +/* + * if pthread_set_name_np is available. + */ +#ifdef HAVE_PTHREAD_SET_NAME_NP +#define PTHREAD_SETNAME_NP pthread_set_name_np +#endif + +#endif + +/* + * if a pthread setname/set_name function is available, declare + * the setustprocname() function that will add '-ust' to the end + * of the current process name, while truncating it if needed. + */ +#ifdef PTHREAD_SETNAME_NP +#define LTTNG_UST_PROCNAME_SUFFIX "-ust" + +#include <pthread.h> + +static inline +void lttng_ust_setustprocname(void) +{ + char name[LTTNG_UST_PROCNAME_LEN]; + int limit = LTTNG_UST_PROCNAME_LEN - strlen(LTTNG_UST_PROCNAME_SUFFIX); + int len; + + lttng_ust_getprocname(name); + + len = strlen(name); + if (len > limit) { + len = limit; + } + + sprintf(name + len, LTTNG_UST_PROCNAME_SUFFIX); + + PTHREAD_SETNAME_NP(pthread_self(), name); +} +#else +static inline +void lttng_ust_setustprocname(void) +{ +} #endif #include <errno.h> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index e00a22c..c32e8ae 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) int sock, ret, prev_connect_failed = 0, has_waited = 0; long timeout; + /* + * If available, add '-ust' to the end of this thread's + * process name + */ + lttng_ust_setustprocname(); + /* Restart trying to connect to the session daemon */ restart: if (prev_connect_failed) { -- 2.8.1 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <cover.1464971957.git.raphael.beamonte@gmail.com>]
[parent not found: <1d3230f091658e56c8ccf12279c3afd89b4062c7.1464971957.git.raphael.beamonte@gmail.com>]
* Re: [lttng-ust PATCH v3 1/1] Add -ust to the name of UST threads of the application [not found] ` <1d3230f091658e56c8ccf12279c3afd89b4062c7.1464971957.git.raphael.beamonte@gmail.com> @ 2016-06-03 16:51 ` Sebastien Boisvert 2016-06-05 19:19 ` Mathieu Desnoyers 1 sibling, 0 replies; 22+ messages in thread From: Sebastien Boisvert @ 2016-06-03 16:51 UTC (permalink / raw) To: Raphaël Beamonte, lttng-dev, mathieu.desnoyers +1 On 06/03/2016 12:45 PM, Raphaël Beamonte wrote: > Add the required functions to change the thread name of the UST > threads and add the -ust string at its end. This will help to > identify LTTng-UST processes when analyzing the trace of a process. > > Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> > --- > configure.ac | 5 +++++ > liblttng-ust/Makefile.am | 1 + > liblttng-ust/compat.h | 51 ++++++++++++++++++++++++++++++++++++++++++- > liblttng-ust/lttng-ust-comm.c | 6 +++++ > 4 files changed, 62 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 5692553..de462ff 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test "x$have_libdl" = "xyes"]) > AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) > > AC_CHECK_LIB([pthread], [pthread_create]) > +AC_CHECK_LIB([pthread], [pthread_setname_np], > + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np is available.]), > + AC_CHECK_LIB([pthread], [pthread_set_name_np], > + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if pthread_set_name_np is available.]), > + AC_MSG_RESULT([pthread setname/set_name not found.]))) > > # Check for dlfcn.h > AC_CHECK_HEADER([dlfcn.h]) > diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am > index f1801cf..05929be 100644 > --- a/liblttng-ust/Makefile.am > +++ b/liblttng-ust/Makefile.am > @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ > error.h > liblttng_ust_tracepoint_la_LIBADD = \ > -lurcu-bp \ > + -lpthread \ > $(top_builddir)/snprintf/libustsnprintf.la > liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info $(LTTNG_UST_LIBRARY_VERSION) > liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" -fno-strict-aliasing > diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h > index 43b2223..68d4d50 100644 > --- a/liblttng-ust/compat.h > +++ b/liblttng-ust/compat.h > @@ -3,6 +3,7 @@ > > /* > * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -23,7 +24,6 @@ > * lttng_ust_getprocname. > */ > #ifdef __linux__ > - > #include <sys/prctl.h> > > #define LTTNG_UST_PROCNAME_LEN 17 > @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) > (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); > } > > +/* > + * if pthread_setname_np is available. > + */ > +#ifdef HAVE_PTHREAD_SETNAME_NP > +#define PTHREAD_SETNAME_NP pthread_setname_np > +#endif > + > #elif defined(__FreeBSD__) > #include <stdlib.h> > #include <string.h> > @@ -59,6 +66,48 @@ void lttng_ust_getprocname(char *name) > strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); > } > > +/* > + * if pthread_set_name_np is available. > + */ > +#ifdef HAVE_PTHREAD_SET_NAME_NP > +#define PTHREAD_SETNAME_NP pthread_set_name_np > +#endif > + > +#endif > + > +/* > + * if a pthread setname/set_name function is available, declare > + * the setustprocname() function that will add '-ust' to the end > + * of the current process name, while truncating it if needed. > + */ > +#ifdef PTHREAD_SETNAME_NP > +#define LTTNG_UST_PROCNAME_SUFFIX "-ust" > + > +#include <pthread.h> > + > +static inline > +void lttng_ust_setustprocname(void) > +{ > + char name[LTTNG_UST_PROCNAME_LEN]; > + int limit = LTTNG_UST_PROCNAME_LEN - strlen(LTTNG_UST_PROCNAME_SUFFIX); > + int len; > + > + lttng_ust_getprocname(name); > + > + len = strlen(name); > + if (len > limit) { > + len = limit; > + } > + > + sprintf(name + len, LTTNG_UST_PROCNAME_SUFFIX); > + > + PTHREAD_SETNAME_NP(pthread_self(), name); > +} > +#else > +static inline > +void lttng_ust_setustprocname(void) > +{ > +} > #endif > > #include <errno.h> > diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c > index e00a22c..c32e8ae 100644 > --- a/liblttng-ust/lttng-ust-comm.c > +++ b/liblttng-ust/lttng-ust-comm.c > @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) > int sock, ret, prev_connect_failed = 0, has_waited = 0; > long timeout; > > + /* > + * If available, add '-ust' to the end of this thread's > + * process name > + */ > + lttng_ust_setustprocname(); > + > /* Restart trying to connect to the session daemon */ > restart: > if (prev_connect_failed) { > _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [lttng-ust PATCH v3 1/1] Add -ust to the name of UST threads of the application [not found] ` <1d3230f091658e56c8ccf12279c3afd89b4062c7.1464971957.git.raphael.beamonte@gmail.com> 2016-06-03 16:51 ` Sebastien Boisvert @ 2016-06-05 19:19 ` Mathieu Desnoyers 1 sibling, 0 replies; 22+ messages in thread From: Mathieu Desnoyers @ 2016-06-05 19:19 UTC (permalink / raw) To: Raphaël Beamonte; +Cc: lttng-dev ----- On Jun 3, 2016, at 12:45 PM, Raphaël Beamonte raphael.beamonte@gmail.com wrote: > Add the required functions to change the thread name of the UST > threads and add the -ust string at its end. This will help to > identify LTTng-UST processes when analyzing the trace of a process. > > Signed-off-by: Raphaël Beamonte <raphael.beamonte@gmail.com> > --- > configure.ac | 5 +++++ > liblttng-ust/Makefile.am | 1 + > liblttng-ust/compat.h | 51 ++++++++++++++++++++++++++++++++++++++++++- > liblttng-ust/lttng-ust-comm.c | 6 +++++ > 4 files changed, 62 insertions(+), 1 deletion(-) > > diff --git a/configure.ac b/configure.ac > index 5692553..de462ff 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -135,6 +135,11 @@ AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBDL], [test > "x$have_libdl" = "xyes"]) > AM_CONDITIONAL([LTTNG_UST_BUILD_WITH_LIBC_DL], [test "x$have_libc_dl" = "xyes"]) > > AC_CHECK_LIB([pthread], [pthread_create]) > +AC_CHECK_LIB([pthread], [pthread_setname_np], > + AC_DEFINE([HAVE_PTHREAD_SETNAME_NP], [1], [Define to 1 if pthread_setname_np > is available.]), > + AC_CHECK_LIB([pthread], [pthread_set_name_np], > + AC_DEFINE([HAVE_PTHREAD_SET_NAME_NP], [1], [Define to 1 if > pthread_set_name_np is available.]), > + AC_MSG_RESULT([pthread setname/set_name not found.]))) > > # Check for dlfcn.h > AC_CHECK_HEADER([dlfcn.h]) > diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am > index f1801cf..05929be 100644 > --- a/liblttng-ust/Makefile.am > +++ b/liblttng-ust/Makefile.am > @@ -14,6 +14,7 @@ liblttng_ust_tracepoint_la_SOURCES = \ > error.h > liblttng_ust_tracepoint_la_LIBADD = \ > -lurcu-bp \ > + -lpthread \ > $(top_builddir)/snprintf/libustsnprintf.la > liblttng_ust_tracepoint_la_LDFLAGS = -no-undefined -version-info > $(LTTNG_UST_LIBRARY_VERSION) > liblttng_ust_tracepoint_la_CFLAGS = -DUST_COMPONENT="liblttng_ust_tracepoint" > -fno-strict-aliasing > diff --git a/liblttng-ust/compat.h b/liblttng-ust/compat.h > index 43b2223..68d4d50 100644 > --- a/liblttng-ust/compat.h > +++ b/liblttng-ust/compat.h > @@ -3,6 +3,7 @@ > > /* > * Copyright (C) 2011 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > + * Copyright (C) 2016 Raphaël Beamonte <raphael.beamonte@gmail.com> > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -23,7 +24,6 @@ > * lttng_ust_getprocname. > */ > #ifdef __linux__ > - > #include <sys/prctl.h> > > #define LTTNG_UST_PROCNAME_LEN 17 > @@ -34,6 +34,13 @@ void lttng_ust_getprocname(char *name) > (void) prctl(PR_GET_NAME, (unsigned long) name, 0, 0, 0); > } > > +/* > + * if pthread_setname_np is available. > + */ > +#ifdef HAVE_PTHREAD_SETNAME_NP > +#define PTHREAD_SETNAME_NP pthread_setname_np replace by: static inline int lttng_pthread_setname_np(pthread_t thread, const char *name) { return pthread_setname_np(thread, name); } > +#endif > + > #elif defined(__FreeBSD__) > #include <stdlib.h> > #include <string.h> > @@ -59,6 +66,48 @@ void lttng_ust_getprocname(char *name) > strncpy(name, bsd_name, LTTNG_UST_PROCNAME_LEN - 1); > } > > +/* > + * if pthread_set_name_np is available. > + */ > +#ifdef HAVE_PTHREAD_SET_NAME_NP > +#define PTHREAD_SETNAME_NP pthread_set_name_np replace by: static inline int lttng_pthread_setname_np(pthread_t thread, const char *name) { return pthread_set_name_np(thread, name); } > +#endif > + > +#endif > + > +/* > + * if a pthread setname/set_name function is available, declare if -> If > + * the setustprocname() function that will add '-ust' to the end > + * of the current process name, while truncating it if needed. > + */ > +#ifdef PTHREAD_SETNAME_NP #if defined(HAVE_PTHREAD_SETNAME_NP) || defined(HAVE_PTHREAD_SETNAME_NP) > +#define LTTNG_UST_PROCNAME_SUFFIX "-ust" > + > +#include <pthread.h> > + > +static inline > +void lttng_ust_setustprocname(void) should probably return int (error). > +{ > + char name[LTTNG_UST_PROCNAME_LEN]; > + int limit = LTTNG_UST_PROCNAME_LEN - strlen(LTTNG_UST_PROCNAME_SUFFIX); > + int len; > + > + lttng_ust_getprocname(name); > + > + len = strlen(name); > + if (len > limit) { > + len = limit; > + } > + > + sprintf(name + len, LTTNG_UST_PROCNAME_SUFFIX); missing sprintf error handling. > + > + PTHREAD_SETNAME_NP(pthread_self(), name); replace by lttng_pthread_setname_np(), and add error handling. > +} > +#else > +static inline > +void lttng_ust_setustprocname(void) should return int. > +{ > +} > #endif > > #include <errno.h> > diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c > index e00a22c..c32e8ae 100644 > --- a/liblttng-ust/lttng-ust-comm.c > +++ b/liblttng-ust/lttng-ust-comm.c > @@ -1295,6 +1295,12 @@ void *ust_listener_thread(void *arg) > int sock, ret, prev_connect_failed = 0, has_waited = 0; > long timeout; > > + /* > + * If available, add '-ust' to the end of this thread's > + * process name > + */ > + lttng_ust_setustprocname(); Add error handling. Thanks, Mathieu > + > /* Restart trying to connect to the session daemon */ > restart: > if (prev_connect_failed) { > -- > 2.8.1 > > _______________________________________________ > lttng-dev mailing list > lttng-dev@lists.lttng.org > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-06-05 19:19 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1464966729.git.raphael.beamonte@gmail.com> [not found] ` <80ee3399800d43b5d3f51aa095ebca160d4f20b9.1464966729.git.raphael.beamonte@gmail.com> 2016-06-03 15:47 ` [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application Sebastien Boisvert [not found] ` <5751A690.9000501@gydle.com> 2016-06-03 15:54 ` Mathieu Desnoyers [not found] ` <1992157209.27136.1464969273436.JavaMail.zimbra@efficios.com> 2016-06-03 15:55 ` Mathieu Desnoyers [not found] ` <1936873452.27140.1464969327630.JavaMail.zimbra@efficios.com> 2016-06-03 16:12 ` Raphaël Beamonte [not found] ` <CAE_Gge2Av=iucY8abMCHpU8PjgptdCLhvyoWk7nwfazt6ei3VQ@mail.gmail.com> 2016-06-03 16:19 ` Mathieu Desnoyers 2016-06-03 16:24 ` Sebastien Boisvert [not found] ` <5751AF3F.4000405@gydle.com> 2016-06-03 16:31 ` Mathieu Desnoyers [not found] ` <cover.1461269886.git.raphael.beamonte@gmail.com> 2016-04-21 20:50 ` [RFC PATCH 1/2] " Raphaël Beamonte 2016-04-21 20:50 ` [RFC PATCH 2/2] " Raphaël Beamonte 2016-06-01 15:04 ` [RFC PATCH 0/2] Identify UST threads by changing thread names Raphaël Beamonte [not found] ` <CAE_Gge2jacOMKQaPdp_dXf3NP+yJTS+jWgJQ-SUnv1-B6WgcGw@mail.gmail.com> 2016-06-03 6:10 ` Jérémie Galarneau [not found] ` <CA+jJMxvmHRYYtHWYjumXrS1BrBCih3WJnRA5yGFBXTw7jL3HiA@mail.gmail.com> 2016-06-03 6:11 ` Jérémie Galarneau [not found] ` <CA+jJMxs9yNOeeqtYBfGTz1ADOqqiVO=XZd-7qm48BS8dzVo-iw@mail.gmail.com> 2016-06-03 6:18 ` Mathieu Desnoyers [not found] ` <1192105907.26502.1464934699922.JavaMail.zimbra@efficios.com> 2016-06-03 6:24 ` Mathieu Desnoyers [not found] ` <293416701.26545.1464935065348.JavaMail.zimbra@efficios.com> 2016-06-03 14:12 ` Raphaël Beamonte [not found] ` <CAE_Gge2032JkDubp=vpaqNzoQre7MfvQC4UGm-wXPmux-Fwh6g@mail.gmail.com> 2016-06-03 15:38 ` Mathieu Desnoyers 2016-06-03 15:16 ` [lttng-ust RFC PATCH v2 0/1] " Raphaël Beamonte 2016-06-03 15:17 ` [lttng-ust RFC PATCH v2 1/1] Add -ust to the name of UST threads of the application Raphaël Beamonte 2016-06-03 16:45 ` [lttng-ust PATCH v3 0/1] Identify UST threads by changing thread names Raphaël Beamonte 2016-06-03 16:45 ` [lttng-ust PATCH v3 1/1] Add -ust to the name of UST threads of the application Raphaël Beamonte [not found] <cover.1464971957.git.raphael.beamonte@gmail.com> [not found] ` <1d3230f091658e56c8ccf12279c3afd89b4062c7.1464971957.git.raphael.beamonte@gmail.com> 2016-06-03 16:51 ` Sebastien Boisvert 2016-06-05 19:19 ` Mathieu Desnoyers
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.