lttng-ctl: manage memory automatically in kernel tracer status check Use a unique_ptr to manage the dynamically allocated payload returned by lttng_ctl_ask_sessiond. Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Change-Id: I685fc03c1da7ff7903503ed82636d27f98f9895e
sessiond: lttng: Add command to check kernel tracer status Issue observed -------------- When `lttng enable-channel --kernel` fails, little feedback is available to user to help them to understand the cause. Eg. ``` Error: Channel asdf: Kernel tracer not available (session auto-20231123-092621) ``` Solution -------- The semantic status of the kernel tracer is tracked and persisted in the session daemon (through `init_kernel_tracer` and `cleanup_tracer_tracer`. A new client command `lttng_kernel_tracer_status` is added to request the current value of the `kernel_tracer_status`. The `lttng` client uses this command after enabling a kernel-domain channel fails to provide the user with a more specific cause of the failure. Eg. ``` Error: Channel asdf: Kernel tracer not available (session auto-20231123-092621) Missing one or more required kernel modules Consult lttng-sessiond logs for more information ``` The kernel tracer status is tracked with an enum defined in `include/lttng/kernel.h` to avoid passing potentially different errno values or locale-dependant strings between the LTTng client and session daemon. Loading modules and checking signatures can fail with a number of different errno values. For example: C.f. https://gitlab.com/linux-kernel/stable/-/blob/master/kernel/module/signing.c#L70 * `EKEYREJECTED` * Any other error code C.f. https://gitlab.com/linux-kernel/stable/-/blob/master/Documentation/security/keys/core.rst * `EKEYREVOKED` * `EKEYEXPIRED` * `ENOKEY` * Others, such as `ENOMEM` Known drawbacks --------------- None. Signed-off-by: Kienan Stewart <kstewart@efficios.com> Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Change-Id: I2ae4b188f0110a472200c2511439b9e3e600527d
Fix clang-tidy cppcoreguidelines-pro-type-const-cast warning clang-tidy reports: cppcoreguidelines-pro-type-const-cast; do not use const_cast The const_cast adds a const qualifier so this warning seems a bit strict. Regardless, we can dodge the whole question by passing the exclusion_list as `const char * const *`, which is closer to the original intention of the API anyhow. For more information on the safety of these types of casts, see: https://isocpp.org/wiki/faq/const-correctness#constptrptr-conversion Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Change-Id: Ia3129b7d1ed4e450f3f2d63920d2fd67c66a6d73
Fix: sessiond: incorrect use of exclusions array leads to crash Issue observed -------------- When using the CLI to list the configuration of a session that has an event rule which makes use of multiple exclusions, the session daemon crashes with the following stack trace: (gdb) bt #0 0x00007fa9ed401445 in ?? () from /usr/lib/libc.so.6 #1 0x0000560cd5fc5199 in lttng_strnlen (str=0x615f6f6c6c6568 <error: Cannot access memory at address 0x615f6f6c6c6568>, max=256) at ../../src/common/compat/string.h:19 #2 0x0000560cd5fc6b39 in lttng_event_serialize (event=0x7fa9cc01d8b0, exclusion_count=2, exclusion_list=0x7fa9cc011794, filter_expression=0x0, bytecode_len=0, bytecode=0x0, payload=0x7fa9d3ffda88) at event.c:767 #3 0x0000560cd5f380b5 in list_lttng_ust_global_events (nb_events=<synthetic pointer>, reply_payload=0x7fa9d3ffda88, ust_global=<optimized out>, channel_name=<optimized out>) at cmd.c:472 #4 cmd_list_events (domain=<optimized out>, session=<optimized out>, channel_name=<optimized out>, reply_payload=0x7fa9d3ffda88) at cmd.c:3860 #5 0x0000560cd5f6d76a in process_client_msg (cmd_ctx=0x7fa9d3ffa710, sock=0x7fa9d3ffa5b0, sock_error=0x7fa9d3ffa5b4) at client.c:1890 #6 0x0000560cd5f6f876 in thread_manage_clients (data=0x560cd7879490) at client.c:2629 #7 0x0000560cd5f65a54 in launch_thread (data=0x560cd7879500) at thread.c:66 #8 0x00007fa9ed32d44b in ?? () from /usr/lib/libc.so.6 #9 0x00007fa9ed3b0e40 in ?? () from /usr/lib/libc.so.6 Cause ----- lttng_event_serialize expects a `char **` list of exclusion names, as provided by the other callsite in liblttng-ctl. However, the callsite in list_lttng_ust_global_events passes pointer to the exclusions as stored in lttng_event_exclusion. lttng_event_exclusion contains an array of fixed-length strings (with a stride of 256 bytes) which isn't an expected layout for lttng_event_serialize. Solution -------- A temporary array of pointers is constructed before invoking lttng_event_serialize to construct a list of exclusions with the layout that lttng_event_serialize expects. The array itself is reused for all events, limiting the number of allocations. Note ---- None. Change-Id: I266a1cc9e9f18e0476177a0047b1d8f468110575 Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Clean-up: run format-cpp on the tree The original re-format commit missed a number of files that were caught by format-cpp. Hopefully this is the last large reformat commit for a while. Change-Id: I493ee6d9fe6187e0bd087c68ed346af69c929c1e Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Build fix: POD static_assert check fails on lttng_event_exclusions A build error occurs when building using g++ 6.4.0 on PPC32: In file included from ../../src/common/buffer-view.hpp:11:0, from event.cpp:9: ../../src/common/macros.hpp: In instantiation of 'AllocatedType* zmalloc(size_t) [with AllocatedType = lttng_event_exclusion; size_t = unsigned int]': event.cpp:270:77: required from here ../../src/common/macros.hpp:102:2: error: static assertion failed: type can be malloc'ed static_assert(can_malloc<AllocatedType>::value, "type can be malloc'ed"); ^~~~~~~~~~~~~ A bug affecting gcc [6.1, 7.4] causes flexible array members to generate a destructor for compound types. In turn, this makes any type that contains a flexible array a non-POD object which is a problem under some use-case (e.g., being allocated using C-style memory management facilities). Explicitly specifying a length of zero works around this bug, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70932 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71147 The same bug affects re2 and they have fixed it in a similar way: https://github.com/google/re2/blob/9049cd28d7496e05e7b7beaec89291d8bc6a31ee/re2/dfa.cc#L123 Change-Id: I730cdeb86bb39cdbfdc5165f854ab5906aeb2192 Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
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>
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
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
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>
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
Build fix: missing initializer for member 'payload' gcc 5.4.0 complains that: channel.cpp:584:2: warning: missing initializer for member 'lttng_notification_channel_message::payload' [-Wmissing-field-initializers] The structure's members are initialized one by one. Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Change-Id: Ia5a5f37f6fe6977169771e4a298d1ce73ab74ea4
common: replace container_of with a C++ safe implementation As more code moves to a more idiomatic C++ style, structures like typically end up becoming classes that use different access controls, virtual functions, etc. This, in turn, makes them adopt a non standard layout and causes GCC and clang to emit the following warning when container_of is used: error: 'offsetof' within non-standard-layout type 'foo' is conditionally-supported [-Werror=invalid-offsetof] This new implementation of container_of makes use of a pointer to a data member to find the parent's address. The use of ptr_to_member against the null dummy_parent makes me uneasy as it seems equivalent to performing arithmetic on a null pointer, which I understand is undefined behavior (C++11 Standard 5.7.5). However, Boost.Instrusive uses an approach that seems roughly equivalent to lttng::utils::container_of() [1]. It seems like a reasonable compromise that works on all mainstream compilers. [1] https://github.com/boostorg/intrusive/blob/3c5c8cec3f0356a028a4b56ba6cac2256340dab1/include/boost/intrusive/detail/parent_from_member.hpp#L92 Change-Id: Ia6287e1648bce85dfe6de936f17ec5df46ea648d Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Fix: liblttng-ctl: leak of payload on field listing LeakSanitizer reports the following leak: ==974957==ERROR: LeakSanitizer: detected memory leaks Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7fdb86fcd1b2 in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:164 #1 0x7fdb86d7c296 in lttng_dynamic_buffer_set_capacity(lttng_dynamic_buffer*, unsigned long) /home/jgalar/EfficiOS/src/lttng-tools/src/common/dynamic-buffer.cpp:159 #2 0x7fdb86d7c060 in lttng_dynamic_buffer_set_size(lttng_dynamic_buffer*, unsigned long) /home/jgalar/EfficiOS/src/lttng-tools/src/common/dynamic-buffer.cpp:112 #3 0x7fdb86d2589a in recv_payload_sessiond /home/jgalar/EfficiOS/src/lttng-tools/src/lib/lttng-ctl/lttng-ctl.cpp:230 #4 0x7fdb86d26fa5 in lttng_ctl_ask_sessiond_payload(lttng_payload_view*, lttng_payload*) /home/jgalar/EfficiOS/src/lttng-tools/src/lib/lttng-ctl/lttng-ctl.cpp:662 #5 0x7fdb86d2cd8d in lttng_list_tracepoint_fields /home/jgalar/EfficiOS/src/lttng-tools/src/lib/lttng-ctl/lttng-ctl.cpp:1767 #6 0x56481623cb4c in list_ust_event_fields commands/list.cpp:850 #7 0x5648162448d9 in cmd_list(int, char const**) commands/list.cpp:2394 #8 0x56481628fb3e in handle_command /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng/lttng.cpp:238 #9 0x564816290601 in parse_args /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng/lttng.cpp:427 #10 0x564816290908 in main /home/jgalar/EfficiOS/src/lttng-tools/src/bin/lttng/lttng.cpp:476 #11 0x7fdb8661730f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f) SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s). The session daemon's reply is indeed never released in lttng_list_tracepoint_fields. Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Change-Id: Idd244b52a69f3b74e5c131c1c36c6ee6d76f4285
Fix: notification: assert on len > 0 for dropped notification message Observed issue ============== Using the notification client from doc/examples/trigger-condition-event-matches/notification-client.cpp, an assert is hit when the notification subsystem is under load. #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007f69eab58859 in __GI_abort () at abort.c:79 #2 0x00007f69eab58729 in __assert_fail_base (fmt=0x7f69eacee588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7f69eae1d5dd "len > 0", file=0x7f69eae1d5cb "unix.cpp", line=179, function=<optimized out>) at assert.c:92 #3 0x00007f69eab6a006 in __GI___assert_fail (assertion=0x7f69eae1d5dd "len > 0", file=0x7f69eae1d5cb "unix.cpp", line=179, function=0x7f69eae1d598 "ssize_t lttcomm_recv_unix_sock(int, void*, size_t)") at assert.c:101 #4 0x00007f69eadd5fe6 in lttcomm_recv_unix_sock (sock=3, buf=0x55da9ecd5f89, len=0) at unix.cpp:179 #5 0x00007f69ead7df3f in receive_message (channel=0x55da9ecd6ee0) at channel.cpp:64 #6 0x00007f69ead7e478 in lttng_notification_channel_get_next_notification (channel=0x55da9ecd6ee0, _notification=0x7ffdefed2570) at channel.cpp:279 #7 0x000055da9e0e742f in main (argc=2, argv=0x7ffdefed2698) at notification-client.cpp:506 (gdb) frame #5 0x00007f69ead7df3f in receive_message (channel=0x55da9ecd6ee0) at channel.cpp:64 64 ret = lttcomm_recv_unix_sock(channel->socket, (gdb) print msg $2 = {type = 5 '\005', size = 0, fds = 0, payload = 0x7ffdefed24a8 ""} The msg type 5 is `LTTNG_NOTIFICATION_CHANNEL_MESSAGE_TYPE_NOTIFICATION_DROPPED` Cause ===== The msg portion of a `LTTNG_NOTIFICATION_CHANNEL_MESSAGE_TYPE_NOTIFICATION_DROPPED` is indeed zero. There is no extra payload. Solution ======== When the msg size is zero, skip the 'payload' reception phase. Known drawbacks ========= None. Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com> Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Change-Id: Ibabb922d0e410c9902414a5eabbe04738861d772
Add type-checked versions of allocation and deallocations functions A common mistake when porting things from C to C++ is to use malloc for types that have a on-trivial constructor (and to not call the constructor explicitly either). For example: struct foo { std::vector<int> field; }; foo *f = (foo *) zmalloc(sizeof(*f)); This allocates a `foo` without calling the constructor, leaving it in an invalid state. Same idea when free-ing with free something that has a non-trivial destructor. To avoid this, I suggest adding templated allocation functions that we will use throughout, that verify if the given type is safe to malloc (and generate a compilation failure if not). The existing code barely needs changes. For example: - (foo *) zmalloc(sizeof(*f)) + zmalloc<foo>() For simplicity I propose that as soon as a type is non-POD (std::is_pod<T>::value is false), we prevent using malloc/free on it. It would be ok in theory to allocate such a type with malloc and free with free, but call the constructor (using placement-new) and destructor explicitly, but I don't see why we would want to do that. It might also be technically more correct to use a combination of std::is_trivially_constructible and std::is_trivially_destructible (std::is_pod being not fine-grained enough), but using std::is_pod just keeps things simpler. This patch introduces the following templated allocation functions: 1. zmalloc<T>() 2. zmalloc<T>(size) 3. malloc<T>() 4. malloc<T>(size) 5. calloc<T>(nmemb) 1. Allocate one T, zero-initialized 2. Allocate a buffer of size `size`, zero-initialized, this is used when the caller calculates the size to allocate, like when using flexible array members 3. Same as 1, but without the zero-initialization 4. Same as 2, but without the zero-initialization 5. Allocate an array of `nmemb` elements of type T, zero-initialized For the de-allocation side, add templated `free` function declaration that uses std::enable_if (SFINAE) to declare a deleted prototype if the type T isn't safe to free (causing a compilation error). There are a lot of places where we pass pointers to void to free. These can't be checked, as we don't know what type of object the pointer really points to. We could forbid that and fix all callers to pass a typed pointer, but that seems a bit too much to chew for the moment. So for now, simply accept that freeing pointers to void won't be checked. It's a best effort. As an example, if I add an explicit constructor to type ctf_trace (in src/bin/lttng-relayd/ctf-trace.h), I get the following errors with clang. For the allocation: /home/simark/src/lttng-tools/src/common/macros.h:57:2: error: static_assert failed due to requirement 'std::is_pod<ctf_trace>::value' "type is POD" static_assert (std::is_pod<T>::value, "type is POD"); ^ ~~~~~~~~~~~~~~~~~~~~~ /home/simark/src/lttng-tools/src/bin/lttng-relayd/ctf-trace.cpp:84:10: note: in instantiation of function template specialization 'zmalloc<ctf_trace>' requested here trace = zmalloc<ctf_trace>(); ^ For the de-allocation: /home/simark/src/lttng-tools/src/bin/lttng-relayd/ctf-trace.cpp:29:2: error: call to deleted function 'free' free(trace); ^~~~ /home/simark/src/lttng-tools/src/common/macros.h:125:6: note: candidate function [with T = ctf_trace, $1 = void] has been explicitly deleted void free(T *p) = delete; ^ /usr/include/stdlib.h:565:13: note: candidate function extern void free (void *__ptr) __THROW; ^ Change-Id: I246a9113d08fa36b81a49137f4e80a5e808de913 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Rename C++ header files to .hpp Rename all C++ header files (include/**/*-internal.h, src/**/*.h except argpar and msgpack, some headers in tests) to have the .hpp extension. Doing so highlights that we include some C++ header files in some test files still compiled as C. This is ok for now, as the files they include don't actually contain C++ code incompatible with C yet, but they could eventually. This is something we can fix later. Change-Id: I8bf326b6b2946a3e26704f3ef3ac5831bbe9bc26 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>