lttng-tools.git
20 months agoFix: lttng-disable-event: erroneous error code used in logging
Jérémie Galarneau [Fri, 3 Feb 2023 19:31:39 +0000 (14:31 -0500)] 
Fix: lttng-disable-event: erroneous error code used in logging

66cefebdc introduced `disable_ret` to hold the integral result of
lttng_disable_event_ext() instead of mixing enums and integers.

In doing so, one use site was left unmodified which resulted in the
wrong variable being used to log an error when the command failed.

This was spotted by clang:
  commands/disable_events.cpp:252:21: warning: result of comparison of
  constant -83 with expression of type 'enum   cmd_error_code' is always false [-Wtautological-constant-out-of-range-compare]
                                      command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME ?

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I282ebee0140f7044847aab6c43ba6bd0c185fc46

20 months agoAdd a .git-blame-ignore-revs file
Jérémie Galarneau [Fri, 3 Feb 2023 19:26:24 +0000 (14:26 -0500)] 
Add a .git-blame-ignore-revs file

The git-blame command supports the `--ignore-revs-file` option
since git 2.23. This option allows a file containing a list of revisions
that git-blame must ignore to be specified.

It can be used as such:
  git blame --ignore-revs-file=.git-blame-ignore-revs some-file.cpp

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I39692f9eda999703e3e3bacb6e02f6e714eeb8a8

20 months agoBuild fix: consumer: unused-but-set-variable warning
Jérémie Galarneau [Fri, 3 Feb 2023 19:05:05 +0000 (14:05 -0500)] 
Build fix: consumer: unused-but-set-variable warning

clang 15.0.7 produces the following warning, preventing the build of the
project with -Werror:

  consumer/consumer.cpp:2519:15: error: variable 'num_hup' set but not used [-Werror,-Wunused-but-set-variable]
          int num_rdy, num_hup, high_prio, ret, i, err = -1;
                       ^

num_hup is indeed not used, so remove its declaration and its
assignations.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iccd8aa7716602d6babd1ed5aa23d06268385480c

20 months agoBuild fix: filter parser fails to build with -Werror
Jérémie Galarneau [Fri, 3 Feb 2023 18:59:23 +0000 (13:59 -0500)] 
Build fix: filter parser fails to build with -Werror

Using bison 3.8.2 and clang 15.0.7, the project fails to build with
-Werror as building the generated parser results in the following
warning:

  CXX      filter/libfilter_la-filter-parser.lo
  filter/filter-parser.cpp:1552:9: error: variable 'yynerrs' set but not used [-Werror,-Wunused-but-set-variable]
      int yynerrs = 0;
          ^

A pragma directive is inserted to silence the warning and allow the
build to go through.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4c46a5df91bdeca6ad4e2a8ff7f2a98ee94c4f1d

21 months agotests: Execute perl using /usr/bin/env
Olivier Dion [Wed, 1 Feb 2023 18:28:50 +0000 (13:28 -0500)] 
tests: Execute perl using /usr/bin/env

Perl is not necessary installed under /bin.  Thus, use /usr/bin/env to
get the corretion location from PATH.

Change-Id: Ie5f037bcfa69c344b63cf04ec7374dc28218d1c7
Signed-off-by: Olivier Dion <odion@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
21 months agoReplace explicit rcu_read_lock/unlock with lttng::urcu::read_lock_guard
Jérémie Galarneau [Sat, 28 Jan 2023 09:54:09 +0000 (04:54 -0500)] 
Replace explicit rcu_read_lock/unlock with lttng::urcu::read_lock_guard

The explicit use of rcu_read_lock/unlock() has been a frequent source of
bugs in the code base as the locks can often end-up imbalanced when
error paths are taken (through goto's).

Since lttng::urcu::read_lock_guard was implemented in a previous commit,
replace all usages of the rcu_read_lock/unlock() API with an RAII
lock_guard wrapper.

Essentially, lttng::urcu::read_lock_guard acquires the RCU reader lock
on construction, and releases it when it goes out of scope. This
automates what is accomplished in the various error paths by jumping to
error handling labels.

For more info, see: https://en.cppreference.com/w/cpp/thread/lock_guard

No user-visible change of behaviour is intended by this patch. The scope
of some critical sections has been reduced when possible and when it
didn't matter from a correctness standpoint. The RCU reader lock would
often be held longer than necessary to simplify the error paths.

Explicit scopes are used to limit the lifetime of the reader lock guards
when used in long functions or when it is only intended to protect the
iteration over cds_lfht instances. In those cases, the following pattern
is used:

```cpp
void foo()
{
  [...]
  {
    lttng::urcu::read_lock_guard read_guard;

    cds_lfht_for_each(...) {
      [...]
  }
  [...]
}
```

This explicitly bounds the critical section and also serves as a hint as
to why the read guard is being used. It is fairly verbose, but I think
it's a good compromise until we add an idiomatic urcu wrapper that
automates this stuff too.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie3ef8ddf0f1ab813971522176115688939696370

21 months agoClean-up: replace uses of `int enabled` with boolean flags
Jérémie Galarneau [Sat, 28 Jan 2023 18:18:32 +0000 (13:18 -0500)] 
Clean-up: replace uses of `int enabled` with boolean flags

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: If9cf1b6e2db67d461479a43d8ed3c3e07c4133ba

21 months agoClean-up: replace uses of `int found` as bool by `bool found`
Jérémie Galarneau [Sat, 28 Jan 2023 18:10:39 +0000 (13:10 -0500)] 
Clean-up: replace uses of `int found` as bool by `bool found`

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I95c4cefdc0836203d3bf8e6b9c273cf5f1cbf3fc

21 months agoClean-up: ust-consumerd: remove stub of removed function
Jérémie Galarneau [Wed, 1 Feb 2023 19:22:32 +0000 (14:22 -0500)] 
Clean-up: ust-consumerd: remove stub of removed function

lttng_ustconsumer_allocate_channel() no longer exists. However,
it is still declared and a stub is present when building without
liblttng-ust. Remove both as they are no longer needed.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I33f357bf642822a4825b9d3ceba098f1421c0132

21 months agoBuild fix: missing cstdint include in time.hpp on macOS
Michael Jeanson [Wed, 18 Jan 2023 20:15:14 +0000 (15:15 -0500)] 
Build fix: missing cstdint include in time.hpp on macOS

The order of inclusions was changed and the build fails on macOS since
time.hpp doesn't include cstdint for its use of uint64_t.

Change-Id: Id992160bcc3320f093453c2e66b90cc0f55db1c7
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
21 months agoclang-tidy: disable analysis for swig-generated code
Jérémie Galarneau [Tue, 17 Jan 2023 17:18:25 +0000 (12:18 -0500)] 
clang-tidy: disable analysis for swig-generated code

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Idd951400c20ee942b6c9a8fba197d106a3f3d02b

21 months agoclang-tidy: add a subset of cppcoreguidelines and other style checks
Jérémie Galarneau [Tue, 17 Jan 2023 03:00:38 +0000 (22:00 -0500)] 
clang-tidy: add a subset of cppcoreguidelines and other style checks

Enforce some of the coding style rules using clang-tidy. The changes to
the code concern cppcoreguidelines-special-member-functions.

Change-Id: I47ec220505a625814b18b42ae9a070b8bf413337
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
21 months agoFix: ini parser: truncation of value name
Jérémie Galarneau [Mon, 16 Jan 2023 19:19:20 +0000 (14:19 -0500)] 
Fix: ini parser: truncation of value name

clang 14 reports the following:

  ini-config/ini.cpp:88:16: warning: 'char* strncpy(char*, const char*, size_t)' output may be truncated copying 49 bytes from a string of length 199 [-Wstringop-truncation]
     88 |         strncpy(dest, src, size - 1);
        |         ~~~~~~~^~~~~~~~~~~~~~~~~~~~~

Indeed, a silent truncation of `name` occurs whenever it is longer than
prev_name (49 characters, excluding the terminator).

Report an error when this condition occurs.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I973bd27185e0130d8e4a452525d9277de45ba200

21 months agoBuild fix: warning: clang warns that uuid_scan may be uninitialized
Jérémie Galarneau [Mon, 16 Jan 2023 19:12:41 +0000 (14:12 -0500)] 
Build fix: warning: clang warns that uuid_scan may be uninitialized

clang warns that uuid_scan may be uninitialized. While this can't
happen given the function's code flow, silence the warning by
initializing uuid_scan.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I143131663cde02234c47ec16c74ee9176e49c1cc

21 months agoClean-up: ensure all template parameter names end with Type
Jérémie Galarneau [Mon, 16 Jan 2023 19:12:28 +0000 (14:12 -0500)] 
Clean-up: ensure all template parameter names end with Type

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1d3f307b173fe5fd8796cdaa89a8405217e251db

21 months agoclang-tidy: add readability-simplify-boolean-expr
Jérémie Galarneau [Sat, 14 Jan 2023 04:56:05 +0000 (23:56 -0500)] 
clang-tidy: add readability-simplify-boolean-expr

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2dc131c79b9014fe4d20a58ec83d69bd6986bb85

21 months agoClean-up: sessiond: remove useless LTTNG_ASSERT
Jérémie Galarneau [Sat, 14 Jan 2023 04:47:35 +0000 (23:47 -0500)] 
Clean-up: sessiond: remove useless LTTNG_ASSERT

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iaa232da2f2a6d50b47b2144befee4c66d45caf91

21 months agoclang-tidy: add most bugprone warnings
Jérémie Galarneau [Sat, 14 Jan 2023 00:34:42 +0000 (19:34 -0500)] 
clang-tidy: add most bugprone warnings

Most of the changes proposed by clang-tidy are explicit checks for the
result of strcmp() and adding parentheses for all macro parameters.

Change-Id: I6ce7384b6d96035454d5456ac920becbf2882e65
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
21 months agoclang-tidy: add Chrome-inspired checks
Jérémie Galarneau [Fri, 13 Jan 2023 22:14:04 +0000 (17:14 -0500)] 
clang-tidy: add Chrome-inspired checks

Add the checks used by the Chrome project. Most of these changes
were produced and applied by clang-tidy.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4058dafbdef00e81ac04f202fdfe377384a99e6b

21 months agoAdd a basic .clang-tidy file and fix typedef warnings
Jérémie Galarneau [Fri, 13 Jan 2023 17:24:13 +0000 (12:24 -0500)] 
Add a basic .clang-tidy file and fix typedef warnings

This initial .clang-tidy file only enforces the use of 'using' instead
of 'typedef'.

Note that a second .clang-tidy is added to the filter sub-folder to
disable checks against generated code.

The entire tree can be checked by running `run-clang-tidy` after
generating a compilation command database. I personally use `bear`
to generate `compile_commands.json`.

$ ./configure
$ bear -- make -j$(nproc)
$ run-clang-tidy -fix

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I074b6f2724e1af9a2df311e07a2fcdacb689bcf5

21 months agoRun clang-format on the whole tree
Jérémie Galarneau [Fri, 13 Jan 2023 17:02:15 +0000 (12:02 -0500)] 
Run clang-format on the whole tree

Generated by running:

find -not \( -path "./src/vendor*" -prune \) -iname "*.h" -o -iname "*.hpp" -o -iname "*.c" -o -iname "*.cpp" -exec clang-format -i {} \;

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9557e7d84e305187475ef88124857cf2f438bbb1

21 months agoBuild fix: missing stdio.h include in signal-helper.hpp
Jérémie Galarneau [Thu, 15 Dec 2022 22:39:09 +0000 (17:39 -0500)] 
Build fix: missing stdio.h include in signal-helper.hpp

A follow-up commit changes the order of inclusions and fails to build
since signal-helper.hpp doesn't include stdio.h for its use of perror().

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Id402346fcd727f23d382697d06d3886a915aa014

21 months agoBuild fix: missing unistd.h include in utils.h
Jérémie Galarneau [Thu, 15 Dec 2022 22:34:25 +0000 (17:34 -0500)] 
Build fix: missing unistd.h include in utils.h

A follow-up commit changes the order of inclusions and fails to
build since utils.h doesn't include unistd.h for its use
of `useconds_t`.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I34a03311bb4f0eb43be9c004a057b10f3168f117

21 months agoBuild fix: missing consumer.hpp include in lttng-consumerd.hpp
Jérémie Galarneau [Thu, 15 Dec 2022 22:24:19 +0000 (17:24 -0500)] 
Build fix: missing consumer.hpp include in lttng-consumerd.hpp

A follow-up commit changes the order of inclusions and fails to build
since lttng-consumerd.hpp doesn't include consumer.hpp for its use of
`enum lttng_consumer_type`.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I8eb6e9ab0e97624c3cae85057c15703873534c2c

21 months agoBuild fix: missing poll wrapper include in utils-poll.cpp
Jérémie Galarneau [Thu, 15 Dec 2022 22:22:15 +0000 (17:22 -0500)] 
Build fix: missing poll wrapper include in utils-poll.cpp

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7d751007af88416e4281be5af3c4a252eb8bfcde

21 months agoBuild fix: missing cstdlib include in filter-ir.hpp
Jérémie Galarneau [Thu, 15 Dec 2022 22:15:15 +0000 (17:15 -0500)] 
Build fix: missing cstdlib include in filter-ir.hpp

A follow-up commit changes the order of inclusions and fails to build
since filter-ir.hpp doesn't include cstdlib for its use of abort().

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7d96d183745c9c834d272cbbbbb4d9a17e1f281e

21 months agoBuild fix: missing cstdint include in futex.hpp
Jérémie Galarneau [Thu, 15 Dec 2022 22:12:31 +0000 (17:12 -0500)] 
Build fix: missing cstdint include in futex.hpp

A follow-up commit changes the order of inclusions and fails to build
since futex.hpp doesn't include cstdint for its use of int32_t.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia4a4be7902c024250717b3349055b9c8c3e44f8b

21 months agoUpdate .clang-format to match existing style
Jérémie Galarneau [Tue, 8 Nov 2022 20:55:58 +0000 (15:55 -0500)] 
Update .clang-format to match existing style

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I082446af8267dbd6c8b7eca3a35388722b804d8d

21 months agoUpdate CodingStyle to account for C++ migration
Jérémie Galarneau [Tue, 8 Nov 2022 20:55:25 +0000 (15:55 -0500)] 
Update CodingStyle to account for C++ migration

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ifd8a849e3a730ec709ef06633f127a8d1a8449d1

21 months agoClean-up: default_pipe_size_getter: coding style adjustments
Jérémie Galarneau [Fri, 13 Jan 2023 16:58:51 +0000 (11:58 -0500)] 
Clean-up: default_pipe_size_getter: coding style adjustments

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I24bbe1bbfa0608c1952ab1c86743320e77ea7c3c

21 months agoport: tests: F_GETPIPE_SZ is linux specific
Michael Jeanson [Fri, 16 Jul 2021 18:48:48 +0000 (14:48 -0400)] 
port: tests: F_GETPIPE_SZ is linux specific

Change-Id: Ifa6631cc72b8998e1331905ff851cd2477fab148
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
21 months agoClean-up: remove unused _trace_abi field from the CTF2 trace class visitor
Jérémie Galarneau [Fri, 13 Jan 2023 16:41:32 +0000 (11:41 -0500)] 
Clean-up: remove unused _trace_abi field from the CTF2 trace class visitor

This unused field results in a build failure when using clang++ 14.0.6
with Werror.

  ./ctf2-trace-class-visitor.hpp:39:37: error: private field '_trace_abi' is not used [-Werror,-Wunused-private-field]
          const lttng::sessiond::trace::abi& _trace_abi;

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib85ad5c9c5affce123ee6300b8549f884a55df31

21 months agoClean-up: sessiond: rename public accessors
Jérémie Galarneau [Thu, 12 Jan 2023 22:09:05 +0000 (17:09 -0500)] 
Clean-up: sessiond: rename public accessors

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Icf87c518d632a176c7786a48b3921e0638545d9c

21 months agosessiond: rename transfer_* method to move_*
Jérémie Galarneau [Thu, 12 Jan 2023 21:40:32 +0000 (16:40 -0500)] 
sessiond: rename transfer_* method to move_*

Other similar methods use the 'move' terminology to express a transfer
of ownership.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ide2bc0f49ab8b40dd6af01b45c79e058f9c6b14a

21 months agosessiond: ust: conditionally enable the underscore prefix variant quirk
Jérémie Galarneau [Mon, 28 Nov 2022 23:02:44 +0000 (18:02 -0500)] 
sessiond: ust: conditionally enable the underscore prefix variant quirk

Application contexts are expressed as variants. LTTng-UST announces
those by registering an enumeration named `..._tag`. It then registers a
variant as part of the event context that contains the various possible
types.

Unfortunately, the names used in the enumeration and variant don't
match: the enumeration names are all prefixed with an underscore while
the variant type tag fields aren't.

While the CTF 1.8.3 specification mentions that
underscores *should* (not *must*) be removed by CTF readers. Babeltrace
1.x (and possibly others) expect a perfect match between the names used
by tags and variants.

When the UNDERSCORE_PREFIXED_VARIANT_TAG_MAPPINGS quirk is enabled,
the variant's fields are modified to match the mappings of its tag.

From ABI version >= 10.x, the variant fields and tag mapping names
correctly match, making this work-around unnecessary.

However, since the variants produced by LTTng-UST contain TSDL-unsafe
names, a variant/selector sanitization pass is performed before
serializing a trace class hierarchy to TSDL.

The variant_tsdl_keyword_sanitizer visitor is used to visit field before
it is handed-over to the actual TSDL-producing visitor.

As it visits fields, the variant_tsdl_keyword_sanitizer populates a
"type_overrider" with TSDL-safe replacements for any variant or
enumeration that uses TSDL-unsafe identifiers (reserved keywords).

The type_overrider, in turn, is used by the rest of the TSDL
serialization visitor (tsdl_field_visitor) to swap any TSDL-unsafe types
with their sanitized version.

The tsdl_field_visitor owns the type_overrider and only briefly shares
it with the variant_tsdl_keyword_sanitizer which takes a reference to
it.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib61eafc452338a99a02b9829cbd049cb6fa48ead
Depends-on: lttng-ust: Ia7e78096a9c31cd4c0574d599c961067d8f03791

21 months agoClean-up: coverity warns of uncaught exception during logging
Jérémie Galarneau [Fri, 6 Jan 2023 17:01:53 +0000 (12:01 -0500)] 
Clean-up: coverity warns of uncaught exception during logging

Coverity reports:

  1502349 Uncaught exception If the exception is ever thrown, the
  program will crash.

  In lttng::​file_descriptor::​~file_descriptor(): A C++ exception is
  thrown but never caught (CWE-248)

and

  1502348 Uncaught exception If the exception is ever thrown, the
  program will crash.

  In main: A C++ exception is thrown but never caught (CWE-248)

Both have the same cause: libfmt should not be used in "final" catch
blocks before returning to non exception-safe code in order to contain
all exceptions.

As we add custom formaters, it could be interesting to revisit this and
provide a noexcept wrapper for fmt::format. For the moment not much is
lost (beyond format string type safety) from using the existing logging
macros directly.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia70505517678ae182f6479feeb264c9402aa1381

22 months agoFix: sessiond: instance uuid is not sufficiently unique
Jérémie Galarneau [Tue, 3 Jan 2023 23:41:23 +0000 (18:41 -0500)] 
Fix: sessiond: instance uuid is not sufficiently unique

Observed issue
==============

Tracing a cluster of machines -- all launched simultaneously -- to the
same relay daemon occasionally produces corrupted traces.

The size of packets received (as seen from the relay daemon logs) and
that of those present in the on-disk stream occasionally didn't match.

The traces were observed to all relate to the same trace UUID, but
present packet begin timestamps that were not monotonic for a given
stream.

This causes both Babeltrace 1.x and 2.x to fail to open the traces with
different error messages related to clocks.

Cause
=====

On start, the session daemon generates a UUID to uniquely identify the
sessiond instance. Since the UUID generation utils use time() to seed
the random number generator, two session daemons launched within the
same second can end up with the same instance UUID.

Since the relay daemon relies on this UUID to uniquely identify a
session daemon accross its various connections, identifier clashes can
cause streams from the same `uid` or `pid` to become scrambled resulting
in corrupted traces.

Solution
========

The UUID utils now initializes its random seed using the getrandom() API
in non-blocking mode. If that fails -- most likely because the random
pool is depleted or the syscall is not available on the platform -- it
falls back to using a hash of two time readings (with nanosecond
precision), of the hostname, and the PID.

Known drawbacks
===============

This fix implements many fallbacks, each with their own caveats and we
don't have full test coverage for all of those for the moment.

This article presents the different drawbacks of using /dev/urandom vs
getrandom().

https://lwn.net/Articles/884875/

As for the pseudo-random time and configuration based fallback, it is
meant as a last resort for platforms or configurations where both
getrandom() (old kernels or non-Linux platforms) and /dev/urandom (e.g.
locked-down container) are not be available. I haven't done a formal
analysis of the entropy of this home-grown method. The practical
use-case we want to enable is launching multiple virtual machines (or
containers) at roughly the same time and ensure that they don't end up
using the same sessiond UUID.

In that respect, having a different host name and minute timing changes
seem enough to prevent a UUID clash.

Using the PID as part of the hash also helps when launching multiple
session daemons simultaneously for different users.

Change-Id: I064753b9ff0f5bf2279be0bd0cfbfd2b0dd19bfc
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agoLog uts information on launch of the session and relay daemon
Jérémie Galarneau [Tue, 3 Jan 2023 19:11:21 +0000 (14:11 -0500)] 
Log uts information on launch of the session and relay daemon

To make debugging easier, log uts information on launch of the daemons.

Produces an output of the following form when the daemons are launched
in verbose mode:
  DBG1 - 14:30:14.997217006 [Main]: System information: (in log_system_information() at logging-utils.cpp:23)
  DBG1 - 14:30:14.997221139 [Main]:  sysname: `Linux` (in log_system_information() at logging-utils.cpp:24)
  DBG1 - 14:30:14.997227199 [Main]:  nodename: `carbonara` (in log_system_information() at logging-utils.cpp:25)
  DBG1 - 14:30:14.997231284 [Main]:  release: `6.1.1-arch1-1` (in log_system_information() at logging-utils.cpp:26)
  DBG1 - 14:30:14.997235261 [Main]:  version: `#1 SMP PREEMPT_DYNAMIC Wed, 21 Dec 2022 22:27:55 +0000` (in log_system_information() at logging-utils.cpp:27)
  DBG1 - 14:30:14.997240214 [Main]:  machine: `x86_64` (in log_system_information() at logging-utils.cpp:28)

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I399fe8a88da8480e617cc33dd6b1dc2723c300c7

22 months agoBuild fix: g++ < 7.1 mishandles namespaces of specializations
Jérémie Galarneau [Thu, 5 Jan 2023 19:46:31 +0000 (14:46 -0500)] 
Build fix: g++ < 7.1 mishandles namespaces of specializations

Due to a bug in g++ < 7.1, these specializations must be explicitly
enclosed in the namespaces rather than using the usual
`namespace::namespace::function` notation.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480.

We already use this work-around in a number of places.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie0e6a3c4dcfb4bc1581c2d273d73ea3e65b2ccd6

22 months agoFix: lttng: poptGetArg doesn't provide string ownership
Michael Jeanson [Mon, 31 Oct 2022 19:52:46 +0000 (15:52 -0400)] 
Fix: lttng: poptGetArg doesn't provide string ownership

The string returned by poptGetArg() is 'const' because it is owned by
the popt librairy and is free'd by it when poptFreeContext() is called.
Copy those strings when we need to alter them to maintain proper
ownership.

The latest release of popt (v1.19) introduced a breaking
change (changing the ownership of left-over command line arguments) that
can cause double free()-s.

This is ultimately due to this upstream commit in popt 1.19:
https://github.com/rpm-software-management/popt/commit/7182e4618ad5a0186145fc2aa4a98c2229afdfa8

which is derived from a package patch:
https://src.fedoraproject.org/rpms/babeltrace/c/d48452beff87b145c038f070e7182358db04336c?branch=rawhide

Change-Id: Id2535d1534c0e47cc0747968d6dd60a587f0b810
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agoCleanup: lttng: remove unused 'session_name' variable
Michael Jeanson [Thu, 3 Nov 2022 16:02:49 +0000 (12:02 -0400)] 
Cleanup: lttng: remove unused 'session_name' variable

Change-Id: I46ef649b68d1d9dd22378d34e7d5900b26d827ce
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agoFix: tap.h conflicts with Clang C++ headers
Michael Jeanson [Thu, 27 Oct 2022 21:06:22 +0000 (17:06 -0400)] 
Fix: tap.h conflicts with Clang C++ headers

Always include tap.h last since it defines a 'fail' macro that clashes
with some version of the clang++ system headers.

Change-Id: I20938697045d00b467b12ca1e6dae159c634720a
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agoFix: relayd: missing space in trace creation logging statement
Jérémie Galarneau [Fri, 2 Dec 2022 22:11:00 +0000 (17:11 -0500)] 
Fix: relayd: missing space in trace creation logging statement

A missing space results in hard to read logging statements when a
ctf_trace is created:

[...] Created ctf_trace 15of session "hello" [...]

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3881ddfc17556f2300f2140939c45d49e3b18d2b

22 months agoTests: add basic ust context tests for $app, vpid, vuid, vgid
Jérémie Galarneau [Thu, 17 Nov 2022 16:44:09 +0000 (11:44 -0500)] 
Tests: add basic ust context tests for $app, vpid, vuid, vgid

Context tracing has very little testing coverage beyond the namespace
tests. This test is initially introduced to help troubleshoot an issue
with application contexts.

However, with the scaffolding now in place, it's trivial to exercise
some other context types.

This also adds a basic framework to write tests in Python.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I85be842fab252d8b853392d3f742b1461a69f0fe

22 months agoFix: sessiond: work-around mismatching variant type tag field and selector names
Jérémie Galarneau [Fri, 25 Nov 2022 21:18:42 +0000 (16:18 -0500)] 
Fix: sessiond: work-around mismatching variant type tag field and selector names

Observed issue
--------------

Applications fail to register to the session daemon when an application
context is used. The following error is printed by the session daemon:

Error: Failed to handle application context: Invalid variant choice: `none` does not match any mapping in `$app.mayo:ketchup_tag` enumeration [operator()() ust-field-convert.cpp:606]

Cause
-----

Application contexts are expressed as variants. LTTng-UST announces
those by registering an enumeration named `..._tag`. It then registers a
variant as part of the event context that contains the various possible
types.

Unfortunately, the names used in the enumeration and variant don't
match: the enumeration names are all prefixed with an underscore while
the variant type tag fields aren't.

In preparation for adding the support for the CTF 2 format, the session
daemon's handling of user space field class declarations was decoupled
from the generation of TSDL. Trace layouts are now expressed in an
internal intermediate representation that is used to generate TSDL or
CTF 2 independently.

As part of the conversion from liblttng-ust-ctl's communication
structures to the internal IR (see ust-field-convert.cpp), a certain
number of validations steps are taken. One of them ensures that variant
fields match a mapping within its '_tag' enumeration. This step fails
and produces the error message above.

While the CTF 1.8.3 specification mentions that
underscores *should* (not *must*) be removed by CTF readers. Babeltrace
1.x (and possibly others) expect a perfect match between the names used
by tags and variants.

The previous implementation of the TSDL producer always prepended '_' to
identifiers in order to side-step the problem of escaping TSDL keywords
and ensuring identifiers started with an alphabetic character.

This is not the case for enumeration mappings. Hence, I presume that the
underscores were added on the LTTng-UST side to coherce the session
daemon into producing a trace that Babeltrace 1.x would accept.

For reference, using LTTng 2.13 to trace the `$app:hey:you` application
context results in the following declaration in the TSDL metadata:

stream {
id = 0;
event.header := struct event_header_large;
packet.context := struct packet_context;
event.context := struct {
enum : integer { size = 8; align = 8; signed = 1; encoding = none; base = 10; } {
"_none" = 0,
"_int8" = 1,
"_int16" = 2,
"_int32" = 3,
"_int64" = 4,
"_uint8" = 5,
"_uint16" = 6,
"_uint32" = 7,
"_uint64" = 8,
"_float" = 9,
"_double" = 10,
"_string" = 11,
} __app_hey_you_tag;
variant <__app_hey_you_tag> {
struct {} _none;
integer { size = 8; align = 8; signed = 1; encoding = none; base = 10; } _int8;
integer { size = 16; align = 8; signed = 1; encoding = none; base = 10; } _int16;
integer { size = 32; align = 8; signed = 1; encoding = none; base = 10; } _int32;
integer { size = 64; align = 8; signed = 1; encoding = none; base = 10; } _int64;
integer { size = 8; align = 8; signed = 0; encoding = none; base = 10; } _uint8;
integer { size = 16; align = 8; signed = 0; encoding = none; base = 10; } _uint16;
integer { size = 32; align = 8; signed = 0; encoding = none; base = 10; } _uint32;
integer { size = 64; align = 8; signed = 0; encoding = none; base = 10; } _uint64;
floating_point { exp_dig = 8; mant_dig = 24; align = 8; } _float;
floating_point { exp_dig = 11; mant_dig = 53; align = 8; } _double;
string _string;
} __app_hey_you;
};
};

Solution
--------

During the creation of variant field types, the check for matching
variant/tag names is made more permissive if the registering tracer has
an affected ABI major version, which is always true for the moment.

In that mode, the variant fields are renamed to match the mappings in
the tag field when they differ by a leading underscore. For a reader,
this results in the same apparent trace where mappings and variant
fields are prefixed with an underscore.

A change to LTTng-UST removing the underscores from the enumeration
mapping names has been submitted. That fix bumps the
LTTNG_UST_ABI_MAJOR_VERSION to 10. Once/if that fix is accepted, the
check will return to its original strict-ness.

This change ensures that CTF 2 traces don't carry the leading underscore
baggage when tracing applications built against LTTng-UST 2.14+.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0e3ad6668baefaea011a878746e041bd17f6373c

22 months agoCleanup: tsdl: fix misleading indentation
Jérémie Galarneau [Fri, 25 Nov 2022 20:59:05 +0000 (15:59 -0500)] 
Cleanup: tsdl: fix misleading indentation

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iedb85ce7692c19e373dbb9a80b1d85adc6c4f2bd

22 months agocommon: session load: use session descriptor for session creation
Jonathan Rajotte [Tue, 26 Apr 2022 18:40:21 +0000 (14:40 -0400)] 
common: session load: use session descriptor for session creation

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: If6bc9ddf9c5cf252f8838f5793870d0ffdc4dfc8

22 months agolttng-ctl: use the parsed URI instead of raw string for override
Jonathan Rajotte [Tue, 26 Apr 2022 18:33:52 +0000 (14:33 -0400)] 
lttng-ctl: use the parsed URI instead of raw string for override

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0439c1ecdc935011036e3772e59f0caba017de34

22 months agosessiond: add a CTF 2-generating trace class visitor
Jérémie Galarneau [Mon, 4 Jul 2022 19:19:00 +0000 (15:19 -0400)] 
sessiond: add a CTF 2-generating trace class visitor

The enumeration_type class is modified to guarantee that a mapping is
present. Implicit (or auto) mappings provided by the tracer are
converted to an explicit mapping.

This results in a slightly more verbose TSDL output which can be fixed
later.

Change-Id: I4372ad528384c1b7d9321dd5183525e3bd60ded4
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agosessiond: add variant selector intervals
Jérémie Galarneau [Tue, 26 Jul 2022 21:06:30 +0000 (17:06 -0400)] 
sessiond: add variant selector intervals

In CTF 2, variant options are chosen using a set of ranges. This differs
from CTF 1.8 (and the current API) which assumed that a variant had
options that perfectly matched the selector enumeration's mappings.

Add integer mappings to variants and also carry a choice name that
becomes the name used by the TSDL producer.

Change-Id: Ib31cf9cc211dad36759bd73dc722c49d634c666b
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agosessiond: validate existence of field reference received from LTTng-UST
Jérémie Galarneau [Tue, 26 Jul 2022 22:34:44 +0000 (18:34 -0400)] 
sessiond: validate existence of field reference received from LTTng-UST

Some field descriptions received from traced applications refer to
other fields (a sequence's length field, a variant's selector field,
etc.)

Validate that those references are valid.

Change-Id: I73f5b464b007810beb228128d5bc3243e3b05157
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
22 months agosessiond: express field references as locations instead of names
Jérémie Galarneau [Tue, 26 Jul 2022 19:17:20 +0000 (15:17 -0400)] 
sessiond: express field references as locations instead of names

CTF2 requires type references (a sequence's length, a variant's
selector) to be expressed as absolute field locations.

The `field` API currently expresses these references as names directly
since those are provided by the tracer.

This change introduces the notion of a field location and adapts
the tsdl visitor to serialize those as expected.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I56f603062d6748051adf3fa31bc08422c47c144d

22 months agosessiond: return raw pointer to packet header
Jérémie Galarneau [Thu, 21 Jul 2022 19:41:43 +0000 (15:41 -0400)] 
sessiond: return raw pointer to packet header

There is no benefit to transfering the ownership of a new packet header
everytime it is serialized. Packet headers don't change after the
creation of a session; prefer allocating it once and provide a
raw (const) pointer to the session's instance.

While LTTng always produces one, a trace doesn't absolutely need a
packet header so a pointer is preferred over a reference.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I66b8f9ad74e332edcc8273c076137a7fff0d4d5e

22 months agosessiond: tsdl: remove useless std::move
Jérémie Galarneau [Wed, 13 Jul 2022 21:14:01 +0000 (17:14 -0400)] 
sessiond: tsdl: remove useless std::move

append_metadata_fragment accepts the string by reference and it is
passed as such all the way; there is no need for an ownership transfer.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I49baab00c333b97f0dde49bad34fee43e7e40a2b

22 months agosessiond: generate packet header, packet context and event header dynamically
Jérémie Galarneau [Thu, 7 Jul 2022 16:20:33 +0000 (12:20 -0400)] 
sessiond: generate packet header, packet context and event header dynamically

Instead of hardcoding event header and packet header/context layouts,
their layout is expressed using the lttng::sessiond::trace::field/type
API and serialized to TSDL using the visitor.

This reduces the duplication of code in the CTF2 visitor.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9a4255889e36e6861a59536ce70b1e708da06ac5

22 months agoFix: sessiond: tsdl: don't prepend underscore for stream_id
Jérémie Galarneau [Mon, 11 Jul 2022 19:48:49 +0000 (15:48 -0400)] 
Fix: sessiond: tsdl: don't prepend underscore for stream_id

Although the CTF v1.8 specification recommends ignoring any leading
underscore, Some readers, such as Babeltrace 1.x, expect special
identifiers without a prepended underscore.

This causes problems in a follow-up patch since packet header, context,
and event headers become dynamically generated.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0aeb24ab2e1c0ee1ea70d2fb9bb0eabbb2e9e035

22 months agosessiond: field: add field roles and blob types
Jérémie Galarneau [Mon, 4 Jul 2022 19:00:25 +0000 (15:00 -0400)] 
sessiond: field: add field roles and blob types

Extend the field/field type to include the blob field types that
are used to express the trace class uuid in CTF2.

Roles defined by the CTF2 specification are also added to allow a trace
class to define the layout of some scopes (e.g. packet header and
context) instead of hard-coding the serialized version of the layout
description.

The TSDL trace class visitor takes the new types into account by
converting them into arrays of "bytes" (uint8_t).

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic9d976416ed765b157e951bf9b8b299abbe20e5b

22 months agoAdd nlohmann/json 3.10.5 to vendor libraries
Jérémie Galarneau [Mon, 13 Jun 2022 18:43:22 +0000 (14:43 -0400)] 
Add nlohmann/json 3.10.5 to vendor libraries

JSON is released under the MIT license. The MIT license text is already
under LICENCES.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1aab2c0af7e71248a8ce8b10240daf191d057e09

23 months agoFix: event field value: assertion fails on empty string
Mathieu Desnoyers [Mon, 26 Sep 2022 17:36:38 +0000 (13:36 -0400)] 
Fix: event field value: assertion fails on empty string

When converting msgpack objects to their event_field_value equivalent,
the following assertion fails: LTTNG_ASSERT(val);

 #4  0x00007f1f65349486 in __assert_fail () from /usr/lib/libc.so.6
 #5  0x00007f1f65584da7 in lttng_event_field_value_string_create_with_size (val=0x0, size=0) at event-field-value.cpp:186
 #6  0x00007f1f65576a1a in event_field_value_from_obj (obj=0x557f597ccdb8, field_val=0x7ffcc9675dd0)
     at conditions/event-rule-matches.cpp:1120
 #7  0x00007f1f65577176 in event_field_value_from_capture_payload (condition=0x557f597c8520,
     capture_payload=0x557f597c825b "\221\240", capture_payload_size=2) at conditions/event-rule-matches.cpp:1340
 #8  0x00007f1f655772ea in lttng_evaluation_event_rule_matches_create (condition=0x557f597c8520,
     capture_payload=0x557f597c825b "\221\240", capture_payload_size=2, decode_capture_payload=true)
     at conditions/event-rule-matches.cpp:1398
 #9  0x00007f1f655765fc in lttng_evaluation_event_rule_matches_create_from_payload (condition=0x557f597c8520,
     view=0x7ffcc9675ff0, _evaluation=0x7ffcc9676080) at conditions/event-rule-matches.cpp:990
 #10 0x00007f1f6557f273 in lttng_evaluation_create_from_payload (condition=0x557f597c8520, src_view=0x7ffcc9676100,
     evaluation=0x7ffcc9676080) at evaluation.cpp:120
 #11 0x00007f1f6559ba36 in lttng_notification_create_from_payload (src_view=0x7ffcc9676190, notification=0x7ffcc9676180)
     at notification.cpp:123
 #12 0x00007f1f65552577 in create_notification_from_current_message (channel=0x557f597c8ee0) at channel.cpp:124
 #13 0x00007f1f6555298c in lttng_notification_channel_get_next_notification (channel=0x557f597c8ee0, _notification=0x7ffcc9676280)
     at channel.cpp:292

The msgpack API represents string as p-style while the implementation of
event_field_value relies on null-terminated strings. When an empty
string is captured by a tracer, it is decoded as a msgpack_object with
`str = {size = 0, ptr = 0x0}`.

lttng_event_field_value_string_create_with_size does not require a
null-terminated string since it also receives the length. Hence, this
fix causes lttng_event_field_value_string_create_with_size to accept
null strings when their length is zero. A copy of an empty string is
made to accomodate the null-termination convention used by the rest of
that API.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7c3a839dbbeeb95a1b3bf6ddc3205a2f6b4538e3

2 years agoport: BSD cut doesn't support '\0' as a delimiter
Michael Jeanson [Tue, 20 Oct 2020 21:08:31 +0000 (17:08 -0400)] 
port: BSD cut doesn't support '\0' as a delimiter

Use a combination of 'tr' and 'head' instead which works with both the
GNU and BSD version of coreutils.

Change-Id: I5628863ed41030864ec08f45a3d4153c32fc5496
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoCleanup: remove ignored flags from poll events bitmasks
Michael Jeanson [Wed, 11 Nov 2020 15:38:51 +0000 (10:38 -0500)] 
Cleanup: remove ignored flags from poll events bitmasks

The POLLHUP and POLLERR flags are only valid in 'revents', they are
implicitly enabled regardless of the fact they were set in 'events' or not.

As such remove those flags from all poll events to reduce possible
confusion as to which flags can be returned by poll.

Change-Id: Id22c78c38257d96dfc47e1337795f13c70dd5f91
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoport: add support for BSD mktemp
Michael Jeanson [Fri, 16 Jul 2021 21:52:35 +0000 (17:52 -0400)] 
port: add support for BSD mktemp

Use '-t' which is portable instead of the GNU specific '--tmpdir'.

Change-Id: I430af6b96c27c2766a2cc4b5574af8563297d717
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoFix: C++ syntax of FreeBSD compat code
Michael Jeanson [Thu, 27 Oct 2022 21:05:23 +0000 (17:05 -0400)] 
Fix: C++ syntax of FreeBSD compat code

Change-Id: I4c463bb965c34692718dadd1731f2a1fab7a1248
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoTests: fix randstring() with BSD `tr`
Michael Jeanson [Fri, 13 Nov 2020 23:19:31 +0000 (18:19 -0500)] 
Tests: fix randstring() with BSD `tr`

On FreeBSD, `tr` will error out with an "Illegal byte sequence"
error when it is provided with an invalid multi-byte character.
This happens regularly when its input is random.

Forcing `tr` to use the 'C' locale works around this problem as
that locale only allows single-byte characters.

Change-Id: I8d8e84fb7f356205dd45479538e5bc0bff3c1668
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoFix: tests: CFLAGS to CXXFLAGS when converting to C++
Michael Jeanson [Thu, 27 Oct 2022 20:57:18 +0000 (16:57 -0400)] 
Fix: tests: CFLAGS to CXXFLAGS when converting to C++

The test was converted to C++ but the custom flags used to build it were
left in the CFLAGS variable which doesn't apply to C++ code. This begs
the question of the usefulness of these flags, except for POPT_CFLAGS
which lead to this finding.

Change-Id: I87412ff0730b960b191d8725a13407c67f7b431d
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoport: tests: uprobe is Linux specific
Michael Jeanson [Tue, 20 Oct 2020 18:42:32 +0000 (14:42 -0400)] 
port: tests: uprobe is Linux specific

Change-Id: I6db00a9d01f7534874051eef7477db7dead3a670
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoport: build kernel tests only on linux
Michael Jeanson [Fri, 16 Jul 2021 18:54:37 +0000 (14:54 -0400)] 
port: build kernel tests only on linux

Change-Id: I81e4b2cf161d25aa939f312bb6a728cf9420a9d4
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoport: extensions are enable by AC_USE_SYSTEM_EXTENSIONS
Michael Jeanson [Fri, 13 Nov 2020 20:10:39 +0000 (15:10 -0500)] 
port: extensions are enable by AC_USE_SYSTEM_EXTENSIONS

Available extensions are enabled at configure, make the poll wrapper use
them.

Change-Id: I5a4f99f0b498152d779a2bca3e53775013fdba60
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoport: shutdown(2) can return ENOTCONN on FreeBSD
Michael Jeanson [Mon, 9 Nov 2020 16:40:37 +0000 (11:40 -0500)] 
port: shutdown(2) can return ENOTCONN on FreeBSD

On FreeBSD shutdown(2) will return ENOTCONN when called on a
disconnected socket which is not the case on Linux. It will however
still run the shutdown code on the socket and wakeup threads waiting on
the socket, so we can safely ignore this specific error.

For more details see this bug report:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=227259

Change-Id: I0046382ab996060f940dd1283d18447792cf1fd3
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoStandardize quit pipes behavior
Michael Jeanson [Tue, 10 Nov 2020 21:33:36 +0000 (16:33 -0500)] 
Standardize quit pipes behavior

Standardize the behavior of the quit pipes to trigger on any poll events.

Change-Id: I0beeefcbd1a55b2aa308eb28b617487ffdeb737e
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoBuild fix: rpath of test libraries results in non-reproducible build
Jérémie Galarneau [Mon, 24 Oct 2022 14:53:40 +0000 (10:53 -0400)] 
Build fix: rpath of test libraries results in non-reproducible build

Yocto has trouble making LTTng-tools builds reproducibile since
abs_builddir is used as the rpath of the test libraries under
tests/regression/ust/ust-dl/.

From their commit message:
  Specifing abs_builddir as an RPATH is plain wrong when cross
  compiling. Sadly, removing the rpath makes libtool/automake do weird
  things and breaks the build as shared libs are no longer generated.

  We already try and delete the RPATH at do_install with chrpath however
  that does leave the path in the string table so it doesn't help us
  with reproducibility.

Their fix consists in hardcoding /usr/lib as the rpath of those
libraries. As mentionned, the rpath doesn't matter; it's used to
workaround libtool's behaviour.

This fix uses `$libdir`, which takes the user's specified prefix into
account to generate an rpath that is consistent with the one inserted in
the other artifacts.

Note that the change in the notification tests is a bit more involved
since we have to bypass libtool to instrument a test application with
uprobes.

Fixes #1361

Change-Id: I7739956f8bc8571ef90802c3b37a4e1f21352385
Reported-by: Alexander Kanavin <alex.kanavin@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoCleanup: tests: fix typos in sdt test app comments
Jérémie Galarneau [Wed, 26 Oct 2022 19:09:58 +0000 (15:09 -0400)] 
Cleanup: tests: fix typos in sdt test app comments

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I88e2daf8e7175f5ac784ae373833f8e6f17808bf

2 years agoCleanup: tests: notification: fix typo kprobe/uprobe
Jérémie Galarneau [Tue, 25 Oct 2022 20:09:26 +0000 (16:09 -0400)] 
Cleanup: tests: notification: fix typo kprobe/uprobe

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib57af09d331e559d25a62cc66c72aad567283a6e

2 years agoCleanup: tests: run metadata-regeneration during `make check`
Jonathan Rajotte [Tue, 8 Mar 2022 15:13:59 +0000 (10:13 -0500)] 
Cleanup: tests: run metadata-regeneration during `make check`

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia7e300e00576d28013068b3314d537893fa3b869

2 years agoCleanup: tests: remove redundant tests against `make check`
Jonathan Rajotte [Tue, 8 Mar 2022 14:59:05 +0000 (09:59 -0500)] 
Cleanup: tests: remove redundant tests against `make check`

These tests are already managed by the `make check` based test suite.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I72c382d22a17566ac3beaa95950efabb77405ed3

2 years agoCleanup: sessiond: rename the sessiond main thread quit pipe
Michael Jeanson [Fri, 13 Nov 2020 17:58:17 +0000 (12:58 -0500)] 
Cleanup: sessiond: rename the sessiond main thread quit pipe

Rename the sessiond main thread 'thread_quit_pipe' as the
'main_quit_pipe' to make it easier to distinguish it from the other
threads quit pipes.

While we're there, adjust some debug statements and internal function
names to distinguish between notifying the quit pipe and actually
stopping the threads.

Change-Id: I9caf81f6d03805658e2470a026b15dde03f2a88b
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoAdd some warnings useful in C++
Simon Marchi [Thu, 11 Nov 2021 13:38:16 +0000 (08:38 -0500)] 
Add some warnings useful in C++

Change-Id: Ie8f0ee45ac2f4a4883119a70869a7e4507e7bd0d
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoFix: never use 'no' in a command variable
Michael Jeanson [Tue, 18 Oct 2022 15:10:46 +0000 (11:10 -0400)] 
Fix: never use 'no' in a command variable

Command variable may be used in the Makefiles and tests, when not found,
set them to an empty string instead of trying to execute the command
'no'.

Change-Id: I5429bb96599e3b7f166e52545269cd99eed758ae
Reported-by: Heng Guo <heng.guo@windriver.com>
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoTests: improve trace match reporting
Michael Jeanson [Thu, 3 Feb 2022 16:19:28 +0000 (16:19 +0000)] 
Tests: improve trace match reporting

Add information to the various trace match function tests output to help
with diagnose test failures.

Change-Id: Ibd2943fe3d17829e216ae5aebaff48a2ad5ca875
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agosessiond-comm: prefix lttcomm_sessiond_command entries
Jérémie Galarneau [Thu, 13 Oct 2022 16:29:28 +0000 (17:29 +0100)] 
sessiond-comm: prefix lttcomm_sessiond_command entries

Add LTTCOMM_SESSIOND_COMMAND as the prefix of sessiond client commands
to honor the usual namespacing convention.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie0f9c5282d442e27bcdbbf390ab23d6076c413de

2 years agoFix: sessiond: abort called on undefined client command
Jérémie Galarneau [Thu, 13 Oct 2022 13:28:59 +0000 (14:28 +0100)] 
Fix: sessiond: abort called on undefined client command

Issue observed
--------------

When running in verbose mode, the session daemon calls abort()
when it receives an unknown client command:

 #1  0x00007f66ffd69958 in raise () from /usr/lib/libc.so.6
 #2  0x00007f66ffd5353d in abort () from /usr/lib/libc.so.6
 #3  0x000055a671a6f6bd in lttcomm_sessiond_command_str (cmd=1633771873)
     at ../../../src/common/sessiond-comm/sessiond-comm.hpp:199
 #4  0x000055a671a73897 in process_client_msg (cmd_ctx=0x7f66f5ff6d10,
     sock=0x7f66f5ff6c34, sock_error=0x7f66f5ff6c38) at client.cpp:1006
 #5  0x000055a671a777fc in thread_manage_clients (data=0x55a673956100)
     at client.cpp:2622
 #6  0x000055a671a6d290 in launch_thread (data=0x55a673956170) at thread.cpp:68

Cause
-----

process_client_msg() logs the client command on entry. While it
previously logged the numerical value, it now provides the string-ified
version of the command id (since 19f912db8).

The lttcomm_sessiond_command_str() function aborts when it encounters an
unknown command id. This is reasonable (in so far that it is how we
handle these situations, typically). However, the validity of the
command must be checked beforehand as it comes from an
external (untrusted) source.

Solution
--------

Add lttcomm_sessiond_command_is_valid and tombstone command IDs to
lttcomm_sessiond_command to ensure only valid command ids are passed
to lttcomm_sessiond_command_str when logging.

Drawbacks
---------

None

Reported-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibd36f1e69da984c7f27b55ec68e5e3fe06d7ac91

2 years agoREADME: Update the Userspace RCU requirements
Alistair Francis [Fri, 7 Oct 2022 00:39:17 +0000 (10:39 +1000)] 
README: Update the Userspace RCU requirements

Commit cc22de985fbd "Bump URCU dependency to 0.14" increase the
Userspace RCU requirements but didn't update the README. Let's ensure
the README has the correct information.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1557bb7d8457eb2a7c0909d9aaee1c69c440aa08

2 years agoFix: sessiond: uninitialized bytes sent to lttng-ust
Jonathan Rajotte [Mon, 6 Jun 2022 19:48:16 +0000 (15:48 -0400)] 
Fix: sessiond: uninitialized bytes sent to lttng-ust

Valgrind reports:

==3421594== Thread 9 UST registratio:
==3421594== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
==3421594==    at 0x4DCA12D: __libc_sendmsg (sendmsg.c:28)
==3421594==    by 0x4DCA12D: sendmsg (sendmsg.c:25)
==3421594==    by 0x4B6BDE2: ustcomm_send_unix_sock (ustcomm.c:323)
==3421594==    by 0x4B31549: lttng_ust_ctl_send_counter_data_to_ust (ustctl.c:3167)
==3421594==    by 0x18E8D8: send_counter_data_to_ust(ust_app*, lttng_ust_abi_object_data*) (event-notifier-error-accounting.cpp:535)
==3421594==    by 0x18EDE4: event_notifier_error_accounting_register_app(ust_app*) (event-notifier-error-accounting.cpp:647)
==3421594==    by 0x1AA796: ust_app_setup_event_notifier_group(ust_app*) (ust-app.cpp:4252)
==3421594==    by 0x184AC1: thread_dispatch_ust_registration(void*) (dispatch.cpp:420)
==3421594==    by 0x178192: launch_thread(void*) (thread.cpp:68)
==3421594==    by 0x4DBE608: start_thread (pthread_create.c:477)
==3421594==    by 0x4EF8162: clone (clone.S:95)

==3421594==  Address 0x6f3e2fc is 28 bytes inside a block of size 192 alloc'd
==3421594==    at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==3421594==    by 0x4B2BC3F: zmalloc (macros.h:23)
==3421594==    by 0x4B2BC3F: lttng_ust_ctl_duplicate_ust_object_data (ustctl.c:1332)
==3421594==    by 0x18ED4F: event_notifier_error_accounting_register_app(ust_app*) (event-notifier-error-accounting.cpp:638)
==3421594==    by 0x1AA796: ust_app_setup_event_notifier_group(ust_app*) (ust-app.cpp:4252)
==3421594==    by 0x184AC1: thread_dispatch_ust_registration(void*) (dispatch.cpp:420)
==3421594==    by 0x178192: launch_thread(void*) (thread.cpp:68)
==3421594==    by 0x4DBE608: start_thread (pthread_create.c:477)
==3421594==    by 0x4EF8162: clone (clone.S:95)

==3421594==  Uninitialised value was created by a stack allocation
==3421594==    at 0x18DCF3: ust_error_accounting_entry_create(ust_app const*) (event-notifier-error-accounting.cpp:377)

The underflow/overflow indices are left uninitialized and are ultimately
sent.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6dfff2e39a8fda0e74dd874ab34be66845629069

2 years agoClean-up: consumerd: reduce duplication of stream output close code
Jérémie Galarneau [Thu, 23 Jun 2022 18:28:01 +0000 (14:28 -0400)] 
Clean-up: consumerd: reduce duplication of stream output close code

The kernel space consumer implements its own version of a stream_close
operation where it could use the common consumer code. This change
separates the tear down of the buffers (munmap, in the kernel case) from
the closing of the output stream in consumer_stream_close().

This change allows the kernel snapshot code to re-use the common
close function instead of rolling its own `finalize_snapshot_stream`.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I48e3193ceb3c15ddd8c6fcecd37ab60b793f7e66

2 years agoFix: consumer: snapshot: assertion on subsequent snapshot
Jonathan Rajotte [Wed, 1 Jun 2022 19:20:55 +0000 (15:20 -0400)] 
Fix: consumer: snapshot: assertion on subsequent snapshot

Observed issue
==============

While a snapshot is being taken, the containing folder can disappear
unexpectedly. This can lead to the following errors, which are expected
and mostly handled fine:

PERROR - 14:47:32.002564464 [2922498/2922507]: Failed to open file relative to trace chunk file_path = "channel0_0", flags = 577, mode = 432: No such file or directory (in _lttng_trace_chunk_open_fs_handle_locked() at trace-chunk.cpp:1411)
Error: Failed to open stream file "channel0_0"
Error: Snapshot channel failed

The problem happens on the subsequent snapshot for the session:

 #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
 #1  0x00007fbbdadb3859 in __GI_abort () at abort.c:79
 #2  0x00007fbbdadb3729 in __assert_fail_base (fmt=0x7fbbdaf49588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x55c4212cfbb5 "!stream->trace_chunk", file=0x55c4212cf820 "kernel-co
 #3  0x00007fbbdadc5006 in __GI___assert_fail (assertion=0x55c4212cfbb5 "!stream->trace_chunk", file=0x55c4212cf820 "kernel-consumer/kernel-consumer.cpp", line=188, function=0x55c4212cfb00 "
 #4  0x000055c421268cc6 in lttng_kconsumer_snapshot_channel (channel=0x7fbbc4000b60, key=1, path=0x7fbbd37f8fd4 "", relayd_id=18446744073709551615, nb_packets_per_stream=0) at kernel-consume
 #5  0x000055c42126b39d in lttng_kconsumer_recv_cmd (ctx=0x55c421b80a90, sock=31, consumer_sockpoll=0x7fbbd37fd280) at kernel-consumer/kernel-consumer.cpp:986
 #6  0x000055c4212546d1 in lttng_consumer_recv_cmd (ctx=0x55c421b80a90, sock=31, consumer_sockpoll=0x7fbbd37fd280) at consumer/consumer.cpp:2090
 #7  0x000055c421259963 in consumer_thread_sessiond_poll (data=0x55c421b80a90) at consumer/consumer.cpp:3281
 #8  0x00007fbbdaf8b609 in start_thread (arg=<optimized out>) at pthread_create.c:477
 #9  0x00007fbbdaeb0163 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

How to reproduce:

 1. Setting a breakpoint on snapshot_channel() inside
    src/common/ust-consumer/ust-consumer.cpp
 2. When the breakpoint hits, remove the the complete lttng directory
    containing the session data.
 3. Continue the lttng_consumerd process from gdb.
 4. In that case you see a negative return value -1 from
    consumer_stream_create_output_files() inside snapshot_channel().
 5. Take another snapshot and lttng_consumerd crashes because
    of the `assert(!stream->trace_chunk)` in snapshot_channel().

    This last action does not require any breakpoint intervention.

Cause
=====

During the snapshot, the stream is assigned the channel current chunk.
It is expected that the stream does not have a chunk at this point.

The error handling is faulty here, the stream chunk must be
invalidated/reset on error to allow its reuse later on.

The problem exists for both consumer domains (user/kernel).

Solution
========

For the ust consumer, we can directly use the `error_close_stream`
label.

For the kernel consumer, the code path is slightly different since it
does not uses `consumer_stream_close`. Note that `consumer_stream_close`
cannot be used as is for the kernel consumer. The current implementation
partially resembles `consumer_stream_close` at the end of the iteration.
It is extracted to its own function for easier reuse from the new
`error_finalize_stream` label.

Known drawbacks
=========

None.

Fixes: #1352
Signed-off-by: Marcel Hamer <marcel.hamer@windriver.com>
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9fc81917b19aa436ed8e8679672648f2d5baf41a

2 years agoFix: waiter: futex wait: handle spurious futex wakeups
Mathieu Desnoyers [Thu, 23 Jun 2022 20:27:41 +0000 (16:27 -0400)] 
Fix: waiter: futex wait: handle spurious futex wakeups

Observed issue
==============

The waiter lttng_waiter_wait() implements a futex wait/wakeup
scheme similar to the liburcu workqueue code, which has an issue with
spurious wakeups.

A spurious wakeup on lttng_waiter_wait can cause
lttng_waiter_wait to reach label skip_futex_wait with a
waiter->state state of WAITER_WAITING, which is unexpected. It would
cause busy-waiting on WAITER_TEARDOWN state to start early. The
wait-teardown stage is done with WAIT_ATTEMPTS active attempts,
following by attempts spaced by 10ms sleeps. I do not expect that these
spurious wakeups will cause user-observable effects other than being
slightly less efficient that it should be.

This issue will cause spurious unexpected high CPU use, but will not
lead to data corruption.

Cause
=====

From futex(5):

       FUTEX_WAIT
              Returns 0 if the caller was woken up.  Note that a  wake-up  can
              also  be caused by common futex usage patterns in unrelated code
              that happened to have previously used the  futex  word's  memory
              location  (e.g., typical futex-based implementations of Pthreads
              mutexes can cause this under some conditions).  Therefore, call‐
              ers should always conservatively assume that a return value of 0
              can mean a spurious wake-up, and  use  the  futex  word's  value
              (i.e.,  the user-space synchronization scheme) to decide whether
              to continue to block or not.

Solution
========

We therefore need to validate whether the value differs from
WAITER_WAITING in user-space after the call to FUTEX_WAIT returns 0.

Known drawbacks
===============

None.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ida9905d1f0b5d9543c8b85ecbd7d748a6f7c1c97

2 years agoFix: futex wait: handle spurious futex wakeups
Mathieu Desnoyers [Thu, 23 Jun 2022 20:15:51 +0000 (16:15 -0400)] 
Fix: futex wait: handle spurious futex wakeups

Observed issue
==============

The futex futex_nto1_wait() implements a futex wait/wakeup scheme
identical to the liburcu workqueue code, which has an issue with
spurious wakeups.

A spurious wakeup on futex_nto1_wait can cause futex_nto1_wait to return
with a futex state of -1, which is unexpected.

futex_nto1_wait is used by the relayd live dispatcher thread, by the
relayd main dispatcher thread, as well as by the sessiond dispatcher
thread.

Given that following a futex_nto1_wait returning due to a spurious
wakeup futex_nto1_prepare will set the futex value to -1, things go
back to normal for the following futex_nto1_wait calls.

Therefore, the only impact of this issue is to spuriously use slightly
more CPU time than strictly required.

The effect is even shorter-lasting that in the liburcu counterparts
because futex_nto1_prepare explicitly sets the futex state to -1 rather
than use an atomic decrement, which immediately sets to state back to
a consistent state.

Cause
=====

From futex(5):

       FUTEX_WAIT
              Returns 0 if the caller was woken up.  Note that a  wake-up  can
              also  be caused by common futex usage patterns in unrelated code
              that happened to have previously used the  futex  word's  memory
              location  (e.g., typical futex-based implementations of Pthreads
              mutexes can cause this under some conditions).  Therefore, call‐
              ers should always conservatively assume that a return value of 0
              can mean a spurious wake-up, and  use  the  futex  word's  value
              (i.e.,  the user-space synchronization scheme) to decide whether
              to continue to block or not.

Solution
========

We therefore need to validate whether the value differs from -1 in
user-space after the call to FUTEX_WAIT returns 0.

Known drawbacks
===============

None.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I8a1b6aaf77b6a2f941fd4f89b61bed71cf17906b

2 years agoFix: ust metadata: resample clock on regenerate metadata
Jonathan Rajotte [Fri, 8 Jul 2022 21:19:32 +0000 (17:19 -0400)] 
Fix: ust metadata: resample clock on regenerate metadata

Observed issue
==============
The system test jobs complain that the clock regeneration is broken
since the move to the tsdl visitor approach for metadata generation.

 # Test UST local with metadata regeneration
 # destructive//../../src/bin/lttng/lttng create regen -o /tmp/tmp.metadata_regen_after_data_change.Vzb0vL
 ok 23 - Create session regen in -o /tmp/tmp.metadata_regen_after_data_change.Vzb0vL
 # destructive//../../src/bin/lttng/lttng enable-event tp:tptest -s regen -u
 ok 24 - Enable ust event tp:tptest for session regen
 # destructive//../../src/bin/lttng/lttng start regen
 ok 25 - Start tracing for session regen
 # destructive//../../src/bin/lttng/lttng stop regen
 ok 26 - Stop lttng tracing for session regen
 ok 27 - Validate trace at date 1970-02-02
 # destructive//../../src/bin/lttng/lttng start regen
 ok 28 - Start tracing for session regen
 # destructive//../../src/bin/lttng/lttng regenerate metadata -s regen
 ok 29 - Metadata regenerate regen
 # destructive//../../src/bin/lttng/lttng stop regen
 ok 30 - Stop lttng tracing for session regen
 # destructive//../../src/bin/lttng/lttng destroy regen
 ok 31 - Destroy session regen
 not ok 32 - The trace is not at the expected date
 #   Failed test 'The trace is not at the expected date'
 #   in destructive//../utils/tap/tap.sh:fail() at line 159.

Cause
=====

Previously the clock was sampled on each call to `ust_metadata_session_statedump`
, currently the clock is only sampled on creation of the
`lttng::sessiond::ust::registry_session::registry_session` object.

Solution
========

On `lsu::registry_session::regenerate_metadata`, sample the clock and
replace the registry_session _clock object.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Change-Id: I6f671e4c01f71e1574824236cef03915a9c79b36

2 years agoFix: utils: unhandled close return value
Jérémie Galarneau [Mon, 22 Aug 2022 15:25:21 +0000 (11:25 -0400)] 
Fix: utils: unhandled close return value

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3f932ac53e6b39c86babe726d5ec8e9ba999e561

2 years agoFix: agent port file is o+w when launching as root
Jonathan Rajotte [Thu, 21 Jul 2022 13:30:27 +0000 (09:30 -0400)] 
Fix: agent port file is o+w when launching as root

Observed issue
==============

When starting as root, the following permissions are observed:

[-rw-rw-rw-]  agent.port
[-rw-r--r--]  lttng-sessiond.pid

When starting as user:

[-rw-rw----]  agent.port
[-rw-rw-r--]  lttng-sessiond.pid

Note that despite being created by the same function,
`utils_create_pid_file`, the permissions are not the same.

Cause
=====

`get_wait_shm` manipulates the umask and does not restore it, thus
influencing the outcome of following file creations that don't enforce
specific permissions (using chmod).

Also `fopen` defaults to mode `0666 & ~umask`, thus resulting in
unnecessarily lax permissions when the session daemon is started as a
non-privileged user (umask = 0002, most of the time).

Solution
========

Mimic other call sites of umask(), modify then revert the umask.

Open the pid and agent port files as 0644 letting the umask to do its
job as necessary for those files.

Remove unnecessary umask() usage when chmod is directly used.

Known drawbacks
===============

Use of umask in a multi-threaded process is not recommended. Still our
current usage is limited and mostly happens during the initialization
phase. The usage of umask() is required for the `wait_shm` since on
FreeBSD it is not possible to chmod an shm file descriptor. The default
umask would interfere here.

Discussion
==========

The usage in run-as is valid even when in no-clone mode (valgrind) since
it is the sole user of umask() following the initialization phase. When
spawned as a separate process the clearing of umask is totally valid
even if it is not ideal since we are ignoring any umask set by the user.

It seems like the current usage is the lesser evil here.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie224d254714fff05f4bced471ebfa8f19eede26a

2 years agoFix: tests: don't assume sequential cpuids
Michael Jeanson [Tue, 9 Aug 2022 15:38:16 +0000 (15:38 +0000)] 
Fix: tests: don't assume sequential cpuids

On Linux CPU ids aren't sequential if a CPU is offlined or unplugged.
Get the list of currently available CPU ids from sysfs and pick a random
one, if sysfs is not available use the previous behavior.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibdb63c7d036389104ac2f629827a6dce59e06983

2 years agoFix: sessiond: null pointer dereference on initial evaluation of session
Jérémie Galarneau [Fri, 8 Jul 2022 15:45:01 +0000 (11:45 -0400)] 
Fix: sessiond: null pointer dereference on initial evaluation of session

Coverity reports:
1490492 Dereference after null check
Either the check against null is unnecessary, or there may be a null
pointer dereference.

In evaluate_session_condition(lttng_condition const *, session_info const *, session_state_sample const *, lttng_evaluation **): Pointer is checked against null but then dereferenced anyway (CWE-476)

This function is used to evaluate the initial state of a session and its
transitions against a given condition.

In the case of an initial evaluation, the wrong state sample is used
which results in a null dereference.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia465e26d2bf0dae725504915fa62332ecf8c7784

2 years agoTests: size-based rotation: implement a trace size cutoff protection
Jérémie Galarneau [Tue, 5 Jul 2022 20:43:26 +0000 (16:43 -0400)] 
Tests: size-based rotation: implement a trace size cutoff protection

Stop waiting for rotations when the trace exceeds a certain size cutoff.
This prevents those tests from filling a hard drive when they fail.

However, this check is racy since it is possible for an arbitrary number
of apps to run before the session daemon gets a chance to perform the
scheduled rotations.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I465462e6f1d5c17ada2b3aceb68662d8663254eb

2 years agoFix: sessiond: handle empty scheduled rotations
Jérémie Galarneau [Wed, 6 Jul 2022 16:09:41 +0000 (12:09 -0400)] 
Fix: sessiond: handle empty scheduled rotations

A number of error codes were added to cmd_rotate_session since the
implementation of size-based rotations. The rotation thread doesn't
expect LTTNG_ERR_ROTATION_MULTIPLE_AFTER_STOP and
LTTNG_ERR_ROTATION_AFTER_STOP_CLEAR which are not fatal failures.

These rotations would simply result in an empty trace archive and are,
therefore, not produced. In both cases, it is safe to wait for the next
size cycle.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib90526b586de17c0b14d16970c862d9f981ed464

2 years agoClean-up: sessiond: remove left-over code
Jérémie Galarneau [Tue, 5 Jul 2022 22:31:15 +0000 (18:31 -0400)] 
Clean-up: sessiond: remove left-over code

ust_metadata_channel_statedump no longer exists and _metadata_dumped
is unused.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2fa3f3bcb8d1c5a779ab1cfa0375174b479ed21f

2 years agoTests: rotation: add a kernel size-based rotation test
Jérémie Galarneau [Wed, 22 Jun 2022 19:05:55 +0000 (15:05 -0400)] 
Tests: rotation: add a kernel size-based rotation test

Change-Id: I035814dcbe5e74227f907bae300eacffd132d80f
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
2 years agoTests: rotation: add a per-pid size-based rotation test
Jérémie Galarneau [Wed, 22 Jun 2022 17:52:43 +0000 (13:52 -0400)] 
Tests: rotation: add a per-pid size-based rotation test

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I8f6875567d52dd7d28000ff34fff47f992cdab3f

2 years agoTests: add a local size-based rotation test in per-uid buffering mode
Jérémie Galarneau [Tue, 21 Jun 2022 04:42:34 +0000 (00:42 -0400)] 
Tests: add a local size-based rotation test in per-uid buffering mode

This test runs an application up until we observe four archived traces.
Unfortunately, we can't validate their size since they are approximative.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iecdbf3ee33ed02745a99e7af22c0645b4375413e

2 years agoFix: sessiond: report client list allocation failure as a fatal error
Jérémie Galarneau [Thu, 30 Jun 2022 16:03:23 +0000 (12:03 -0400)] 
Fix: sessiond: report client list allocation failure as a fatal error

Report the failure to allocate a notification client list as a fatal
notification thread error.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I8f2654020d0d890cc9275d445fdeccde940d2ae0

2 years agoFix: leak of channel-bound trigger list element
Jérémie Galarneau [Wed, 29 Jun 2022 20:37:54 +0000 (16:37 -0400)] 
Fix: leak of channel-bound trigger list element

The list element is never free'd when a channel-bound trigger
is unregistered.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I32c4ee4512c460363e3384c2e3aac9feff4343b1

This page took 0.053778 seconds and 4 git commands to generate.