[lttng-dev] [PATCH lttng-tools 0/5] Support probes with the same name but different event payload

2018-02-07 Thread Francis Deslauriers 
This patch set allows for multiple probes with the same name to be
hooked on the same callsite. Right now, the Session Daemon only
considers the name and signature of the events to determine if two
events are identical. This could lead to trace corruptions when two
probes would have the same name and signature but really different
binary trace layouts.

We now compare probes by doing a deep compare of every field.
If two events are _exactly_ the same then the same event ID will be
used, if they are different a new event ID is created.

When used with this UST patch set[1], it is possible to dlclose probe
provider and callsite libraries during tracing.

This patch set also includes regression tests for both the deep
comparaison and the newly added dlclose capability.

[1]: https://github.com/frdeso/lttng-ust/tree/dlclose-support

Francis Deslauriers (5):
  Fix: probes should not be compared by their names and callsite
signatures
  Fix: should pass the reg_enum_lookup pointer directly
  Tests: allow the use of regular expressions to match events
  Tests: add function to validate the number of an event name in
metadata
  Tests: add duplicated providers tests

 configure.ac|   1 +
 src/bin/lttng-sessiond/Makefile.am  |   3 +-
 src/bin/lttng-sessiond/ust-field-utils.c| 261 +++
 src/bin/lttng-sessiond/ust-field-utils.h|  29 +++
 src/bin/lttng-sessiond/ust-registry.c   |  44 +++-
 tests/fast_regression   |   1 +
 tests/regression/ust/multi-lib/Makefile.am  | 114 +++
 tests/regression/ust/multi-lib/README   |  21 ++
 tests/regression/ust/multi-lib/callsites.c  |  34 +++
 tests/regression/ust/multi-lib/callsites.h  |  21 ++
 tests/regression/ust/multi-lib/multi-lib-test.c | 251 +++
 tests/regression/ust/multi-lib/probes.c |  18 ++
 tests/regression/ust/multi-lib/probes.h | 195 ++
 tests/regression/ust/multi-lib/test_multi_lib   | 262 
 tests/unit/Makefile.am  |   1 +
 tests/utils/utils.sh|  27 ++-
 16 files changed, 1270 insertions(+), 13 deletions(-)
 create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
 create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
 create mode 100644 tests/regression/ust/multi-lib/Makefile.am
 create mode 100644 tests/regression/ust/multi-lib/README
 create mode 100644 tests/regression/ust/multi-lib/callsites.c
 create mode 100644 tests/regression/ust/multi-lib/callsites.h
 create mode 100644 tests/regression/ust/multi-lib/multi-lib-test.c
 create mode 100644 tests/regression/ust/multi-lib/probes.c
 create mode 100644 tests/regression/ust/multi-lib/probes.h
 create mode 100755 tests/regression/ust/multi-lib/test_multi_lib

-- 
2.7.4

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[lttng-dev] [PATCH lttng-tools 4/5] Tests: add function to validate the number of an event name in metadata

2018-02-07 Thread Francis Deslauriers 
Signed-off-by: Francis Deslauriers 
---
 tests/utils/utils.sh | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
index 9bf1fcc..60df376 100644
--- a/tests/utils/utils.sh
+++ b/tests/utils/utils.sh
@@ -1343,6 +1343,29 @@ function add_context_kernel_fail()
add_context_lttng 1 -k "$@"
 }
 
+function validate_metadata_event ()
+{
+   local event_name=$1
+   local nr_event_id=$2
+   local trace_path=$3
+
+   local metadata_file=$(find $trace_path | grep metadata)
+   local metadata_path=$(dirname $metadata_file)
+
+   which $BABELTRACE_BIN >/dev/null
+   skip $? -ne 0 "Babeltrace binary not found. Skipping trace matches"
+
+   local count=$($BABELTRACE_BIN --output-format=ctf-metadata 
$metadata_path | grep $event_name | wc -l)
+
+   if [ "$count" -ne "$nr_event_id" ]; then
+   fail "Metadata match with the metadata of $count event(s) named 
$event_name"
+   diag "$count matching event id found in metadata"
+   else
+   pass "Metadata match with the metadata of $count event(s) named 
$event_name"
+   fi
+
+}
+
 function trace_matches ()
 {
local event_name=$1
-- 
2.7.4

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[lttng-dev] [PATCH lttng-tools 5/5] Tests: add duplicated providers tests

2018-02-07 Thread Francis Deslauriers 
Signed-off-by: Francis Deslauriers 
---
 configure.ac|   1 +
 tests/fast_regression   |   1 +
 tests/regression/ust/multi-lib/Makefile.am  | 114 +++
 tests/regression/ust/multi-lib/README   |  21 ++
 tests/regression/ust/multi-lib/callsites.c  |  34 +++
 tests/regression/ust/multi-lib/callsites.h  |  21 ++
 tests/regression/ust/multi-lib/multi-lib-test.c | 251 +++
 tests/regression/ust/multi-lib/probes.c |  18 ++
 tests/regression/ust/multi-lib/probes.h | 195 ++
 tests/regression/ust/multi-lib/test_multi_lib   | 262 
 10 files changed, 918 insertions(+)
 create mode 100644 tests/regression/ust/multi-lib/Makefile.am
 create mode 100644 tests/regression/ust/multi-lib/README
 create mode 100644 tests/regression/ust/multi-lib/callsites.c
 create mode 100644 tests/regression/ust/multi-lib/callsites.h
 create mode 100644 tests/regression/ust/multi-lib/multi-lib-test.c
 create mode 100644 tests/regression/ust/multi-lib/probes.c
 create mode 100644 tests/regression/ust/multi-lib/probes.h
 create mode 100755 tests/regression/ust/multi-lib/test_multi_lib

diff --git a/configure.ac b/configure.ac
index b6ea39c..e22872c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1090,6 +1090,7 @@ AC_CONFIG_FILES([
tests/regression/ust/buffers-pid/Makefile
tests/regression/ust/periodical-metadata-flush/Makefile
tests/regression/ust/multi-session/Makefile
+   tests/regression/ust/multi-lib/Makefile
tests/regression/ust/overlap/Makefile
tests/regression/ust/overlap/demo/Makefile
tests/regression/ust/linking/Makefile
diff --git a/tests/fast_regression b/tests/fast_regression
index bbce068..f76b53d 100644
--- a/tests/fast_regression
+++ b/tests/fast_regression
@@ -21,6 +21,7 @@ regression/tools/regen-statedump/test_ust
 regression/ust/before-after/test_before_after
 regression/ust/buffers-pid/test_buffers_pid
 regression/ust/multi-session/test_multi_session
+regression/ust/multi-lib/test_multi_lib
 regression/ust/nprocesses/test_nprocesses
 regression/ust/overlap/test_overlap
 regression/ust/java-jul/test_java_jul
diff --git a/tests/regression/ust/multi-lib/Makefile.am 
b/tests/regression/ust/multi-lib/Makefile.am
new file mode 100644
index 000..f78ce7f
--- /dev/null
+++ b/tests/regression/ust/multi-lib/Makefile.am
@@ -0,0 +1,114 @@
+noinst_SCRIPTS = test_multi_lib
+noinst_PROGRAMS = exec-with-callsites exec-without-callsites
+
+exec_with_callsites_SOURCES = multi-lib-test.c callsites.c
+exec_with_callsites_LDFLAGS = -ldl -lpopt
+exec_with_callsites_CFLAGS = -DHAS_CALLSITES=1
+
+exec_without_callsites_SOURCES = multi-lib-test.c
+exec_without_callsites_LDFLAGS = -ldl -lpopt -llttng-ust
+exec_without_callsites_LDADD = probes.o
+exec_without_callsites_CFLAGS = -DHAS_CALLSITES=0
+
+PROBES_SRC=probes.c probes.h
+PROBES_LDF=-shared -module -llttng-ust -avoid-version -rpath 
$(abs_builddir)/.libs/
+PROBES_CF=-c -I$(srcdir)/
+
+probes.o: probes.c probes.h
+   $(CC) $(PROBES_CF) -o $@ $<
+
+noinst_LTLIBRARIES = libprobes_a.la libprobes_a_prime.la \
+   libprobes_b.la libprobes_c.la libprobes_c_prime.la \
+   libprobes_d.la libprobes_e.la libprobes_f.la \
+   libprobes_g.la libprobes_h.la libprobes_i.la \
+   libprobes_j.la libprobes_k.la libprobes_l.la \
+   libprobes_m.la libprobes_n.la libprobes_o.la \
+   libprobes_p.la
+
+noinst_LTLIBRARIES += libcallsites_1.la libcallsites_2.la
+
+CALLSITES_SRC=callsites.c callsites.h
+CALLSITES_LDF=-shared -module -llttng-ust -avoid-version -rpath 
$(abs_builddir)/.libs/
+CALLSITES_CF=-c -I.
+
+libprobes_a_la_SOURCES = $(PROBES_SRC)
+libprobes_a_la_LDFLAGS = $(PROBES_LDF)
+libprobes_a_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_A
+
+libprobes_a_prime_la_SOURCES = $(PROBES_SRC)
+libprobes_a_prime_la_LDFLAGS = $(PROBES_LDF)
+libprobes_a_prime_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_A
+
+libprobes_b_la_SOURCES = $(PROBES_SRC)
+libprobes_b_la_LDFLAGS = $(PROBES_LDF)
+libprobes_b_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_B
+
+libprobes_c_la_SOURCES = $(PROBES_SRC)
+libprobes_c_la_LDFLAGS = $(PROBES_LDF)
+libprobes_c_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_C
+
+libprobes_c_prime_la_SOURCES = $(PROBES_SRC)
+libprobes_c_prime_la_LDFLAGS = $(PROBES_LDF)
+libprobes_c_prime_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_C
+
+libprobes_d_la_SOURCES = $(PROBES_SRC)
+libprobes_d_la_LDFLAGS = $(PROBES_LDF)
+libprobes_d_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_D
+
+libprobes_e_la_SOURCES = $(PROBES_SRC)
+libprobes_e_la_LDFLAGS = $(PROBES_LDF)
+libprobes_e_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_E
+
+libprobes_f_la_SOURCES = $(PROBES_SRC)
+libprobes_f_la_LDFLAGS = $(PROBES_LDF)
+libprobes_f_la_CFLAGS = $(PROBES_CF) -DACTIVATE_PROBES_F
+

[lttng-dev] [PATCH lttng-tools 3/5] Tests: allow the use of regular expressions to match events

2018-02-07 Thread Francis Deslauriers 
Signed-off-by: Francis Deslauriers 
---
 tests/utils/utils.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
index e8dfcda..9bf1fcc 100644
--- a/tests/utils/utils.sh
+++ b/tests/utils/utils.sh
@@ -1457,7 +1457,7 @@ function validate_trace_exp()
which $BABELTRACE_BIN >/dev/null
skip $? -ne 0 "Babeltrace binary not found. Skipping trace validation"
 
-   traced=$($BABELTRACE_BIN $trace_path 2>/dev/null | grep ${event_exp} | 
wc -l)
+   traced=$($BABELTRACE_BIN $trace_path 2>/dev/null | grep 
--extended-regexp ${event_exp} | wc -l)
if [ "$traced" -ne 0 ]; then
pass "Validate trace for expression '${event_exp}', $traced 
events"
else
@@ -1476,7 +1476,7 @@ function validate_trace_only_exp()
which $BABELTRACE_BIN >/dev/null
skip $? -ne 0 "Babeltrace binary not found. Skipping trace matches"
 
-   local count=$($BABELTRACE_BIN $trace_path | grep ${event_exp} | wc -l)
+   local count=$($BABELTRACE_BIN $trace_path | grep --extended-regexp 
${event_exp} | wc -l)
local total=$($BABELTRACE_BIN $trace_path | wc -l)
 
if [ "$count" -ne 0 ] && [ "$total" -eq "$count" ]; then
-- 
2.7.4

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[lttng-dev] [PATCH lttng-tools 1/5] Fix: probes should not be compared by their names and callsite signatures

2018-02-07 Thread Francis Deslauriers 
Events with different payloads but identical name and signatures could
lead to corrupted trace as the Session Daemon would consider them
identical and give them the same event ID.

Events should be compared using the name, loglevel, fields and
model_emf_uri to ensure that their serialized layout is the same.

Signed-off-by: Francis Deslauriers 
---
 src/bin/lttng-sessiond/Makefile.am   |   3 +-
 src/bin/lttng-sessiond/ust-field-utils.c | 261 +++
 src/bin/lttng-sessiond/ust-field-utils.h |  29 
 src/bin/lttng-sessiond/ust-registry.c|  40 -
 tests/unit/Makefile.am   |   1 +
 5 files changed, 325 insertions(+), 9 deletions(-)
 create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
 create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h

diff --git a/src/bin/lttng-sessiond/Makefile.am 
b/src/bin/lttng-sessiond/Makefile.am
index 413fe75..6fc1809 100644
--- a/src/bin/lttng-sessiond/Makefile.am
+++ b/src/bin/lttng-sessiond/Makefile.am
@@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
 if HAVE_LIBLTTNG_UST_CTL
 lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
ust-consumer.c ust-consumer.h ust-thread.c \
-   ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
+   ust-metadata.c ust-clock.h agent-thread.c 
agent-thread.h \
+   ust-field-utils.h ust-field-utils.c
 endif
 
 # Add main.c at the end for compile order
diff --git a/src/bin/lttng-sessiond/ust-field-utils.c 
b/src/bin/lttng-sessiond/ust-field-utils.c
new file mode 100644
index 000..3c2da14
--- /dev/null
+++ b/src/bin/lttng-sessiond/ust-field-utils.c
@@ -0,0 +1,261 @@
+/*
+ * Copyright (C) 2018 - Francis Deslauriers 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License, version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include 
+#include 
+
+#include "ust-field-utils.h"
+
+/*
+ * The ustctl_field is made of a combinaison of C basic types
+ * ustctl_basic_type and _ustctl_basic_type.
+ *
+ * ustctl_basic_type contains an enumeration describing the abstract type.
+ * _ustctl_basic_type does _NOT_ contain an enumeration describing the
+ * abstract type.
+ *
+ * A layer is needed to use the same code for both structures.
+ * When dealing with _ustctl_basic_type, we need to use the abstract type of
+ * the ustctl_type struct.
+ */
+
+/*
+ * Compare two ustctl_integer_type fields.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
+   struct ustctl_integer_type *second)
+{
+   bool match = true;
+   match &= first->size == second->size;
+   match &= first->alignment == second->alignment;
+   match &= first->signedness == second->signedness;
+   match &= first->encoding == second->encoding;
+   match &= first->base == second->base;
+   match &= first->reverse_byte_order == second->reverse_byte_order;
+
+   return match;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type integer.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_integer_from_raw_basic_type(
+   union _ustctl_basic_type *first, union 
_ustctl_basic_type *second)
+{
+   return match_ustctl_field_integer(>integer, >integer);
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type enum.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_enum_from_raw_basic_type(
+   union _ustctl_basic_type *first, union 
_ustctl_basic_type *second)
+{
+   /*
+* Compare enumeration ID. Enumeration ID is provided to the 
application by
+* the session daemon before event registration.
+*/
+   if (first->enumeration.id != second->enumeration.id) {
+   goto no_match;
+   }
+
+   /*
+* Sanity check of the name and container type. Those were already 
checked
+* during enum registration.
+*/
+   if (strncmp(first->enumeration.name, second->enumeration.name,
+   LTTNG_UST_SYM_NAME_LEN)) {
+   goto no_match;
+   }
+   if (match_ustctl_field_integer(>enumeration.container_type,
+   >enumeration.container_type) ==

Re: [lttng-dev] [PATCH lttng-tools 1/5] Fix: probes should not be compared by their names and callsite signatures

2018-02-07 Thread Mathieu Desnoyers 
- On Feb 7, 2018, at 2:36 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

> Events with different payloads but identical name and signatures could
> lead to corrupted trace as the Session Daemon would consider them
> identical and give them the same event ID.
> 
> Events should be compared using the name, loglevel, fields and
> model_emf_uri to ensure that their serialized layout is the same.

model_emf_uri has no impact on the serialized layout, but does have an
impact on what ends up in the metadata description for the event. The
changelog should be adapated to clarify this.

> 
> Signed-off-by: Francis Deslauriers 
> ---
> src/bin/lttng-sessiond/Makefile.am   |   3 +-
> src/bin/lttng-sessiond/ust-field-utils.c | 261 +++
> src/bin/lttng-sessiond/ust-field-utils.h |  29 
> src/bin/lttng-sessiond/ust-registry.c|  40 -
> tests/unit/Makefile.am   |   1 +
> 5 files changed, 325 insertions(+), 9 deletions(-)
> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
> 
> diff --git a/src/bin/lttng-sessiond/Makefile.am
> b/src/bin/lttng-sessiond/Makefile.am
> index 413fe75..6fc1809 100644
> --- a/src/bin/lttng-sessiond/Makefile.am
> +++ b/src/bin/lttng-sessiond/Makefile.am
> @@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
> if HAVE_LIBLTTNG_UST_CTL
> lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
>   ust-consumer.c ust-consumer.h ust-thread.c \
> - ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
> + ust-metadata.c ust-clock.h agent-thread.c 
> agent-thread.h \
> + ust-field-utils.h ust-field-utils.c
> endif
> 
> # Add main.c at the end for compile order
> diff --git a/src/bin/lttng-sessiond/ust-field-utils.c
> b/src/bin/lttng-sessiond/ust-field-utils.c
> new file mode 100644
> index 000..3c2da14
> --- /dev/null
> +++ b/src/bin/lttng-sessiond/ust-field-utils.c
> @@ -0,0 +1,261 @@
> +/*
> + * Copyright (C) 2018 - Francis Deslauriers 
> 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License, version 2 only, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 51
> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include 
> +#include 
> +
> +#include "ust-field-utils.h"
> +
> +/*
> + * The ustctl_field is made of a combinaison of C basic types

combination

> + * ustctl_basic_type and _ustctl_basic_type.
> + *
> + * ustctl_basic_type contains an enumeration describing the abstract type.
> + * _ustctl_basic_type does _NOT_ contain an enumeration describing the
> + * abstract type.
> + *
> + * A layer is needed to use the same code for both structures.
> + * When dealing with _ustctl_basic_type, we need to use the abstract type of
> + * the ustctl_type struct.
> + */
> +
> +/*
> + * Compare two ustctl_integer_type fields.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
> + struct ustctl_integer_type *second)
> +{
> + bool match = true;

I don't like bitwise operators on C bool type. Is there anything that
guarantees us that bool always uses the same bit to represent "true" ?

Also, missing whiteline after the variable declaration.


> + match &= first->size == second->size;
> + match &= first->alignment == second->alignment;
> + match &= first->signedness == second->signedness;
> + match &= first->encoding == second->encoding;
> + match &= first->base == second->base;
> + match &= first->reverse_byte_order == second->reverse_byte_order;
> +
> + return match;


I'd prefer simply:

if (first->size != second->size)
   return false;
.
return true;

> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type integer.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_integer_from_raw_basic_type(
> + union _ustctl_basic_type *first, union 
> _ustctl_basic_type *second)
> +{
> + return match_ustctl_field_integer(>integer, >integer);
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type enum.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_enum_from_raw_basic_type(
> + union

Re: [lttng-dev] [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading

2018-02-07 Thread Mathieu Desnoyers 
- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

> dlopen() increments the refcount of the library thus preventing the
> refcount to reach zero in the case of dlclose;

The changelog and comment do not explain _why_ this is needed.

The scenario is:

- an application is _not_ linked against liblttng-ust.so
- the application dlopen() a tracepoint probe,
- the tracepoint probe .so is linked against liblttng-ust, and this is what 
ends up
  getting lttng-ust loaded,

Given that our goal is to allow dlclose() of tracepoint probes (new features), 
we don't
want the side effect of this dlclose() to also try to unload liblttng-ust. 
Because
liblttng-ust does _not_ support being dlclose'd, we need to "pin" it in memory 
by
grabbing an extra reference on the library, with a NODELETE RTLD flag.

Thanks,

Mathieu


> 
> Signed-off-by: Francis Deslauriers 
> ---
> configure.ac  |  1 +
> liblttng-ust/Makefile.am  |  2 ++
> liblttng-ust/lttng-ust-comm.c | 22 ++
> 3 files changed, 25 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index b0b4157..4fc6f9c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
> m4_define([UST_LIB_V_PATCH], [0])
> 
> AC_SUBST([LTTNG_UST_LIBRARY_VERSION],
> [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
> +AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
> # note: remember to update tracepoint.h dlopen() to match this version
> # number. TODO: eventually automate by exporting the major number.
> 
> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
> index 982be69..a7edfd5 100644
> --- a/liblttng-ust/Makefile.am
> +++ b/liblttng-ust/Makefile.am
> @@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
>   string-utils.c \
>   string-utils.h
> 
> +liblttng_ust_runtime_la_CFLAGS =
> -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
> +
> if HAVE_PERF_EVENT
> liblttng_ust_runtime_la_SOURCES += \
>   lttng-context-perf-counters.c \
> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
> index 511b9cf..ed912b8 100644
> --- a/liblttng-ust/lttng-ust-comm.c
> +++ b/liblttng-ust/lttng-ust-comm.c
> @@ -27,6 +27,7 @@
> #include 
> #include 
> #include 
> +#include 
> #include 
> #include 
> #include 
> @@ -59,6 +60,9 @@
> #include "../libringbuffer/getcpu.h"
> #include "getenv.h"
> 
> +/* Concatenate lttng ust shared library name with its major version number. 
> */
> +#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so."
> LTTNG_UST_LIBRARY_VERSION_MAJOR
> +
> /*
>  * Has lttng ust comm constructor been called ?
>  */
> @@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
>   pthread_attr_t thread_attr;
>   int timeout_mode;
>   int ret;
> + void *handle;
> 
>   if (uatomic_xchg(, 1) == 1)
>   return;
> @@ -1662,6 +1667,23 @@ void __attribute__((constructor)) lttng_ust_init(void)
>   lttng_ust_loaded = 1;
> 
>   /*
> +  * Manually load liblttng-ust.so to increment the dynamic loader's 
> internal
> +  * refcount for this library so it never becomes zero, thus never gets
> +  * unloaded from the address space of the process. Since we are already
> +  * running in the constructor of the LTTNG_UST_LIB_SO_NAME library, 
> calling
> +  * dlopen will simply increment the refcount and no additionnal work is
> +  * needed by the dynamic loader as the shared library is already loaded 
> in
> +  * the address space. As a safe guard, we use the RTLD_NODELETE flag to
> +  * prevent unloading of the UST library if its refcount becomes zero
> +  * (which should never happen). Do the return value check but discard 
> the
> +  * handle at the end of the function as it's not needed.
> +  */
> + handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
> + if (!handle) {
> + ERR("dlopen of liblttng-ust shared library (%s).", 
> LTTNG_UST_LIB_SO_NAME);
> + }
> +
> + /*
>* We want precise control over the order in which we construct
>* our sub-libraries vs starting to receive commands from
>* sessiond (otherwise leading to errors when trying to create
> --
> 2.7.4

-- 
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

Re: [lttng-dev] [PATCH lttng-tools 2/5] Fix: should pass the reg_enum_lookup pointer directly

2018-02-07 Thread Mathieu Desnoyers 
- On Feb 7, 2018, at 2:36 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

> As the ht_hash_enum and ht_match_enum functions are not changing the
> pointer there is no need to pass the address of the pointer.

The changelog seems too nice. The current situation is that the arguments
passed to the cds_lfht_lookup() function do not match the args expected by 
ht_has_enum
and ht_match_enum, and by chance we probably always end up comparing with 
garbage ?

The changelog should better describe the current problem.

Thanks,

Mathieu

> 
> Signed-off-by: Francis Deslauriers 
> ---
> src/bin/lttng-sessiond/ust-registry.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/ust-registry.c
> b/src/bin/lttng-sessiond/ust-registry.c
> index e336b9e..cd6fdac 100644
> --- a/src/bin/lttng-sessiond/ust-registry.c
> +++ b/src/bin/lttng-sessiond/ust-registry.c
> @@ -558,8 +558,8 @@ struct ust_registry_enum *
>   struct lttng_ht_iter iter;
> 
>   cds_lfht_lookup(session->enums->ht,
> - ht_hash_enum((void *) _enum_lookup, lttng_ht_seed),
> - ht_match_enum, _enum_lookup, );
> + ht_hash_enum((void *) reg_enum_lookup, lttng_ht_seed),
> + ht_match_enum, reg_enum_lookup, );
>   node = lttng_ht_iter_get_node_str();
>   if (!node) {
>   goto end;
> --
> 2.7.4
> 
> ___
> 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

Re: [lttng-dev] [PATCH lttng-ust 4/8] Add probe provider unregister function

2018-02-07 Thread Mathieu Desnoyers 

- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

> Signed-off-by: Francis Deslauriers 
> ---
> include/lttng/ust-events.h  |  1 +
> liblttng-ust/lttng-events.c | 89 +
> 2 files changed, 90 insertions(+)
> 
> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
> index caf7e63..019b0eb 100644
> --- a/include/lttng/ust-events.h
> +++ b/include/lttng/ust-events.h
> @@ -656,6 +656,7 @@ void synchronize_trace(void);
> 
> int lttng_probe_register(struct lttng_probe_desc *desc);
> void lttng_probe_unregister(struct lttng_probe_desc *desc);
> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
> int lttng_fix_pending_events(void);
> int lttng_probes_init(void);
> void lttng_probes_exit(void);
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index 7419f78..e8d4857 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -789,6 +789,95 @@ void lttng_create_event_if_missing(struct lttng_enabler
> *enabler)
> }
> 
> /*
> + * Iterate over all the UST sessions to unregister and destroy all probes 
> from
> + * the probe provider descriptor passed in arguments. Must me called with the

passed in arguments -> received as argument

> + * ust_lock held.
> + */
> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc
> *provider_desc)
> +{
> + int i;

move int i; to the last variable (shortest line).

> + struct cds_list_head *sessionsp;
> + struct lttng_session *session;
> + struct cds_hlist_head *head;
> + struct cds_hlist_node *node;
> + struct lttng_event *event;
> +
> + /* Get handle on list of sessions. */
> + sessionsp = _lttng_get_sessions();
> +
> + /*
> +  * Iterate over all events in the probe provider descriptions and 
> sessions
> +  * to queue the unregistration of the events.
> +  */
> + for (i = 0; i < provider_desc->nr_events; i++) {
> + const char *event_name;
> + uint32_t hash;
> + size_t name_len;
> + const struct lttng_event_desc *event_desc;

same here about var definition layout.

> +
> + event_desc = provider_desc->event_desc[i];
> + event_name = event_desc->name;
> + name_len = strlen(event_name);
> + hash = jhash(event_name, name_len, 0);
> +
> + /* Iterate over all session to find the current event 
> description. */
> + cds_list_for_each_entry(session, sessionsp, node) {

remove whiteline.

> +
> + /*
> +  * Get the list of events in the hashtable bucket and 
> iterate to
> +  * find the event matching this descriptor.
> +  */
> + head = >events_ht.table[hash & 
> (LTTNG_UST_EVENT_HT_SIZE - 1)];
> + cds_hlist_for_each_entry(event, node, head, hlist) {
> + if (event_desc == event->desc) {
> +
> + /* Queue the unregistration of this 
> event. */
> + _lttng_event_unregister(event);
> + break;
> + }
> + }
> + }
> + }
> +
> + /* Wait for grace period. */
> + synchronize_trace();
> + /* Prune the unregistration queue. */
> + __tracepoint_probe_prune_release_queue();
> +
> + /*
> +  * It is now safe to destroy the events and remove them from the event 
> list
> +  * and hashtables.
> +  */
> + for (i = 0; i < provider_desc->nr_events; i++) {
> + const char *event_name;
> + uint32_t hash;
> + size_t name_len;
> + const struct lttng_event_desc *event_desc;

same.

> +
> + event_desc = provider_desc->event_desc[i];
> + event_name = event_desc->name;
> + name_len = strlen(event_name);
> + hash = jhash(event_name, name_len, 0);
> +



> + /* Iterate over all sessions to find the current event 
> description. */
> + cds_list_for_each_entry(session, sessionsp, node) {

whiteline.

> +
> + /*
> +  * Get the list of events in the hashtable bucket and 
> iterate to
> +  * find the event matching this descriptor.
> +  */

beware for the iteration below: you may need to use a 
cds_list_for_each_entry_safe()
if you can remove the list entry while you iterate.

> + head = >events_ht.table[hash & 
> (LTTNG_UST_EVENT_HT_SIZE - 1)];
> + cds_hlist_for_each_entry(event, node, head, hlist) {
> + if (event_desc == event->desc) {
> + _lttng_event_destroy(event);

you have a

Re: [lttng-dev] [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable

2018-02-07 Thread Mathieu Desnoyers 
- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

Is it a fix or a preparation step for a new feature ?

Thanks,

Mathieu

> Signed-off-by: Francis Deslauriers 
> ---
> liblttng-ust/lttng-events.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index e8d4857..2b679b5 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -990,6 +990,7 @@ static
> void _lttng_enum_destroy(struct lttng_enum *_enum)
> {
>   cds_list_del(&_enum->node);
> + cds_hlist_del(&_enum->hlist);
>   free(_enum);
> }
> 
> --
> 2.7.4

-- 
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

Re: [lttng-dev] [PATCH lttng-ust 8/8] Support unloading of probe providers

2018-02-07 Thread Mathieu Desnoyers 
- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

> With this commit, it's now possible to dlclose() a library containing an
> actively used probe provider.
> 
> The destructor of such library will now iterate over all the sessions
> and over all probe definitions to unregister them from the respective
> callsites in the process.
> 
> Signed-off-by: Francis Deslauriers 
> ---
> liblttng-ust/lttng-events.c | 22 +-
> liblttng-ust/lttng-probes.c |  5 -
> liblttng-ust/tracepoint.c   | 43 ++-
> 3 files changed, 59 insertions(+), 11 deletions(-)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index 07385d7..698210c 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -793,7 +793,7 @@ void lttng_create_event_if_missing(struct lttng_enabler
> *enabler)
>  */
> void lttng_probe_provider_unregister_events(struct lttng_probe_desc
> *provider_desc)
> {
> - int i;
> + unsigned int i, j;

shorter lines below whenever possible.

>   struct cds_list_head *sessionsp;
>   struct lttng_session *session;
>   struct cds_hlist_head *head;
> @@ -867,7 +867,27 @@ void lttng_probe_provider_unregister_events(struct
> lttng_probe_desc *provider_de
>   head = >events_ht.table[hash & 
> (LTTNG_UST_EVENT_HT_SIZE - 1)];
>   cds_hlist_for_each_entry(event, node, head, hlist) {
>   if (event_desc == event->desc) {
> + /* Destroy enums of the current event. 
> */
> + for (j = 0; j < event->desc->nr_fields; 
> j++) {
> + const struct lttng_event_field 
> *field;
> + const struct lttng_enum_desc 
> *enum_desc;
> + struct lttng_enum *curr_enum;
> +
> + field = 
> &(event->desc->fields[j]);
> + if (field->type.atype != 
> atype_enum) {
> + continue;
> + }
> +
> + enum_desc = 
> field->type.u.basic.enumeration.desc;
> + curr_enum = 
> lttng_ust_enum_get_from_desc(session, enum_desc);
> + if (curr_enum) {
> + 
> _lttng_enum_destroy(curr_enum);
> + }
> + }
> +
> + /* Destroy event. */
>   _lttng_event_destroy(event);
> +
>   break;
>   }
>   }
> diff --git a/liblttng-ust/lttng-probes.c b/liblttng-ust/lttng-probes.c
> index a09497f..862b19e 100644
> --- a/liblttng-ust/lttng-probes.c
> +++ b/liblttng-ust/lttng-probes.c
> @@ -226,7 +226,10 @@ void lttng_probe_unregister(struct lttng_probe_desc 
> *desc)
>   cds_list_del(>head);
>   else
>   cds_list_del(>lazy_init_head);
> - DBG("just unregistered probe %s", desc->provider);
> +
> + lttng_probe_provider_unregister_events(desc);
> + DBG("just unregistered probes of provider %s", desc->provider);
> +
>   ust_unlock();
> }
> 
> diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c
> index 14b8231..8c630a6 100644
> --- a/liblttng-ust/tracepoint.c
> +++ b/liblttng-ust/tracepoint.c
> @@ -107,8 +107,8 @@ struct tracepoint_entry {
>   struct lttng_ust_tracepoint_probe *probes;
>   int refcount;   /* Number of times armed. 0 if disarmed. */
>   int callsite_refcount;  /* how many libs use this tracepoint */
> - const char *signature;
> - char name[0];
> + char *signature;
> + char *name;
> };
> 
> struct tp_probes {
> @@ -132,6 +132,7 @@ struct callsite_entry {
>   struct cds_hlist_node hlist;/* hash table node */
>   struct cds_list_head node;  /* lib list of callsites node */
>   struct lttng_ust_tracepoint *tp;
> + bool tp_entry_callsite_ref; /* Has a tp_entry took a ref on this 
> callsite*/

missing space after callsite.

Thanks,

Mathieu

> };
> 
> /* coverity[+alloc] */
> @@ -284,6 +285,8 @@ static struct tracepoint_entry *add_tracepoint(const char
> *name,
>   struct cds_hlist_node *node;
>   struct tracepoint_entry *e;
>   size_t name_len = strlen(name);
> + size_t sig_len = strlen(signature);
> + size_t sig_off, name_off;
>   uint32_t hash;
> 
>   if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) {
> @@ -298,19 +301,29 @@ static struct tracepoint_entry *add_tracepoint(const 
> char
> *name,

Re: [lttng-dev] [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable

2018-02-07 Thread Mathieu Desnoyers 


- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers 
francis.deslauri...@efficios.com wrote:

Is this really a fix ? Or is it a preparation step in order to be able to
remove events before the end of the session lifetime ?

Thanks,

Mathieu


> Signed-off-by: Francis Deslauriers 
> ---
> liblttng-ust/lttng-events.c | 4 
> 1 file changed, 4 insertions(+)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index f4a7ccc..7419f78 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
> {
>   struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
> 
> + /* Remove from event list. */
>   cds_list_del(>node);
> + /* Remove from event hash table. */
> + cds_hlist_del(>hlist);
> +
>   lttng_destroy_context(event->ctx);
>   lttng_free_event_filter_runtime(event);
>   /* Free event enabler refs */
> --
> 2.7.4

-- 
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

[lttng-dev] [RFC PATCH babeltrace-1.5] Propagate error from packet_seek in case of truncated packet

2018-02-07 Thread Jonathan Rajotte 
Report the error all the way up allowing users/scripts to perform error
detection and act on it.

Print to stderr the truncated packet information for easier
identification.

Introduce bt_packet_seek_error enum for specific error handling.

Use the ERANGE errno for error propagation inside bt_iter_next and
ctf_read_event.

Signed-off-by: Jonathan Rajotte 
---
 converter/babeltrace.c   | 12 --
 formats/ctf/ctf.c| 44 
 formats/lttng-live/lttng-live-comm.c |  6 ++---
 include/babeltrace/error.h   |  5 
 lib/iterator.c   |  4 
 5 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/converter/babeltrace.c b/converter/babeltrace.c
index f74384e..ef783ed 100644
--- a/converter/babeltrace.c
+++ b/converter/babeltrace.c
@@ -669,6 +669,7 @@ int convert_trace(struct bt_trace_descriptor *td_write,
struct bt_iter_pos *begin_pos = NULL, *end_pos = NULL;
struct bt_ctf_event *ctf_event;
int ret;
+   int error_holder = 0;
 
sout = container_of(td_write, struct ctf_text_stream_pos,
trace_descriptor);
@@ -695,11 +696,18 @@ int convert_trace(struct bt_trace_descriptor *td_write,
goto end;
}
ret = bt_iter_next(bt_ctf_get_iter(iter));
-   if (ret < 0) {
+   if (ret == -ERANGE) {
+   /*
+* Remember that a range (truncated packet)
+* error occurred and continue.
+*/
+   error_holder = 1;
+   continue;
+   } else if (ret < 0) {
goto end;
}
}
-   ret = 0;
+   ret = error_holder;
 
 end:
bt_ctf_iter_destroy(iter);
diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
index b74cddd..9acaa2a 100644
--- a/formats/ctf/ctf.c
+++ b/formats/ctf/ctf.c
@@ -476,6 +476,25 @@ void ctf_print_discarded_lost(FILE *fp, struct 
ctf_stream_definition *stream)
"buffers or with fewer events enabled.\n");
fflush(fp);
 }
+static
+void ctf_print_truncated_packet(FILE *fp, struct ctf_stream_definition *stream,
+   uint64_t packet_size, uint64_t remaining_file_size)
+{
+   fflush(stdout);
+   fprintf(fp, "[error] Packet size (%" PRIu64 " bits) is larger than 
remaining file size (%" PRIu64 " bits) ",
+   packet_size, remaining_file_size);
+   fprintf(fp, "in trace UUID ");
+   print_uuid(fp, stream->stream_class->trace->uuid);
+   if (stream->stream_class->trace->parent.path[0])
+   fprintf(fp, ", at path: \"%s\"",
+   stream->stream_class->trace->parent.path);
+
+   fprintf(fp, ", within stream id %" PRIu64, stream->stream_id);
+   if (stream->path[0])
+   fprintf(fp, ", at relative path: \"%s\"", stream->path);
+   fprintf(fp, ".\n");
+   fflush(fp);
+}
 
 static
 int ctf_read_event(struct bt_stream_pos *ppos, struct ctf_stream_definition 
*stream)
@@ -491,8 +510,12 @@ int ctf_read_event(struct bt_stream_pos *ppos, struct 
ctf_stream_definition *str
if (unlikely(pos->offset == EOF))
return EOF;
 
-   if (ctf_pos_get_event(pos))
+   ret = ctf_pos_get_event(pos);
+   if (ret == -BT_PACKET_SEEK_ERROR_TRUNCATED_PACKET) {
+   return -ERANGE;
+   } else if (ret) {
return EOF;
+   }
 
/* save the current position as a restore point */
pos->last_offset = pos->offset;
@@ -1059,7 +1082,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, 
size_t index, int whence)
case SEEK_SET:  /* Fall-through */
break;  /* OK */
default:
-   ret = -1;
+   ret = -BT_PACKET_SEEK_ERROR;
goto end;
}
 
@@ -1072,7 +1095,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, 
size_t index, int whence)
if (ret) {
fprintf(stderr, "[error] Unable to unmap old base: 
%s.\n",
strerror(errno));
-   ret = -1;
+   ret = -BT_PACKET_SEEK_ERROR;
goto end;
}
pos->base_mma = NULL;
@@ -1093,7 +1116,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, 
size_t index, int whence)
pos->cur_index = 0;
break;
default:
-   ret = -1;
+   ret = -BT_PACKET_SEEK_ERROR;
goto end;
}
pos->content_size = -1U;/* Unknown at this point */
@@ -1127,7 +1150,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, 
size_t index, int whence)