* [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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ messages in thread
* 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ 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; 24+ 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] 24+ messages in thread
* Re: [RFC PATCH 2/2] Add -ust to the name of UST threads of the application
[not found] ` <57502F60.10709@gydle.com>
@ 2016-06-03 14:14 ` Raphaël Beamonte
0 siblings, 0 replies; 24+ messages in thread
From: Raphaël Beamonte @ 2016-06-03 14:14 UTC (permalink / raw)
To: Sebastien Boisvert; +Cc: lttng-dev
2016-06-02 9:06 GMT-04:00 Sebastien Boisvert <sboisvert@gydle.com>:
>
>
>> 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 at gmail.com <https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev>>
>> ---
>> 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
>>
>
> Hi Raphaël,
>
> Since the only difference between the Linux hunk and the FreeBSD hunk is *one* line, you
> should simplify this patch.
>
> Z1$ diff -u linux-hunk freebsd-hunk
> --- linux-hunk 2016-06-02 09:02:14.790865459 -0400
> +++ freebsd-hunk 2016-06-02 09:02:38.863865110 -0400
> @@ -12,6 +12,6 @@
> +
> + sprintf(name, "%s%s", name, "-ust");
> +
> -+ pthread_setname_np(pthread_self(), name);
> ++ pthread_set_name_np(pthread_self(), name);
> +}
> +
> \ No newline at end of file
>
Hi Sebastien,
Right. Should have used a define! Will do.
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] 24+ messages in thread
* Re: [RFC PATCH 2/2] Add -ust to the name of UST threads of the application
[not found] <57502DD9.5030700@gydle.com>
@ 2016-06-02 13:06 ` Sebastien Boisvert
[not found] ` <57502F60.10709@gydle.com>
1 sibling, 0 replies; 24+ messages in thread
From: Sebastien Boisvert @ 2016-06-02 13:06 UTC (permalink / raw)
To: Raphaël Beamonte; +Cc: lttng-dev
> 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 at gmail.com <https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev>>
> ---
> 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
>
Hi Raphaël,
Since the only difference between the Linux hunk and the FreeBSD hunk is *one* line, you
should simplify this patch.
Z1$ diff -u linux-hunk freebsd-hunk
--- linux-hunk 2016-06-02 09:02:14.790865459 -0400
+++ freebsd-hunk 2016-06-02 09:02:38.863865110 -0400
@@ -12,6 +12,6 @@
+
+ sprintf(name, "%s%s", name, "-ust");
+
-+ pthread_setname_np(pthread_self(), name);
++ pthread_set_name_np(pthread_self(), name);
+}
+
\ No newline at end of file
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-06-05 19:19 UTC | newest]
Thread overview: 24+ 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
[not found] <57502DD9.5030700@gydle.com>
2016-06-02 13:06 ` [RFC PATCH 2/2] " Sebastien Boisvert
[not found] ` <57502F60.10709@gydle.com>
2016-06-03 14:14 ` Raphaël Beamonte
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.