Simon Marchi [Fri, 9 Apr 2021 17:14:44 +0000 (13:14 -0400)]
lttng add-trigger: parse condition / action name as option arguments
Change the `add-trigger` command's --condition and --action options to
take the condition name and action name as option arguments. Currently,
when we see the --condition option, we go into the parse_condition
function and expect the first argument to be the condition name (same
idea for the action). Since the condition/action name is mandatory
after --condition/--action, it is simpler to say that --condition and
--action require an argument.
This will let the user do:
--condition=on-event
in addition to the current syntax:
--condition on-event
The code is a bit cleaner as a result.
Change-Id: Ic7ea97c2657e5de6ae18563694ad22bbffbb2aa8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 14 Apr 2021 18:49:54 +0000 (14:49 -0400)]
Add .editorconfig file
Many editors support reading this file to automatically configure indent
/ whitespace settings. The author of this patch would be extremely
grateful if the maintainer of the project would consider merging the
patch in the master branch of the project's repository.
Change-Id: I84ae62fe3e028d0d40421943b3a53207854efed7
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Francis Deslauriers [Thu, 15 Apr 2021 19:46:18 +0000 (15:46 -0400)]
Allow users to create trigger with their uid
Currently, the `add-trigger` prints the following if a user uses the
`--user-id` with its own uid:
Error: Failed to set trigger's user identity.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie0a642b3faf2259a892def3cb19a899e61cb4705
Francis Deslauriers [Wed, 14 Apr 2021 21:25:27 +0000 (17:25 -0400)]
lttng: list-triggers: implement listing of SDT userspace-probe
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Id57c265dcdb3b324ffbb274ca97ead9364275cf0
Francis Deslauriers [Thu, 15 Apr 2021 13:16:38 +0000 (09:16 -0400)]
Tests: cleanup: rework trigger listing tests
- Rename `list_triggers()` -> `list_triggers_matches_ok()` to make it
obvious it's testing something,
- Add `diag` to all test cases,
- Launch only one lttng-sessiond for all test cases to reduce test
runtime, and
- remove-trigger all triggers at the end of every test cases.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Icbbd08f521565ef1cae901bb192cf6f8f1cdb7ea
Francis Deslauriers [Wed, 14 Apr 2021 17:09:15 +0000 (13:09 -0400)]
Tests: event-notifier: multi UID error accounting support
The new test case adds one root trigger and one regular user trigger as
well as apps from both of these users.
The test then confirm that the error accounting of the root trigger
reports what is happening on the regular user's app as well as its own
app.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iaec231863de98b14891602bdae010449989de6f9
Francis Deslauriers [Mon, 19 Apr 2021 18:58:31 +0000 (14:58 -0400)]
Fix: tests: quote variable in case it's not set
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib0751699eb9deb02f47739062c99dbb8535c3163
Francis Deslauriers [Wed, 21 Apr 2021 20:04:25 +0000 (16:04 -0400)]
ust: error accounting: don't print error messages if app is dead
UST applications may exit during the registering process. It's not an
error and should be expected.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I90f91252849ba362e73dad6c30b9cfdc0185abe4
Francis Deslauriers [Mon, 12 Apr 2021 15:39:08 +0000 (11:39 -0400)]
sessiond: split event notifier error counter CLI options
With this commit, users can specify the size of event notifier error
counting buffers for each domain independently using the following new
lttng-sessiond options:
--event-notifier-error-buffer-size-kernel=
--event-notifier-error-buffer-size-userspace=
The index allocation is now also per-domain meaning that index
allocation in the kernel domain doesn't affect available indices in the
userspace domain and reversely.
Small changes:
- Add manual page description of the new options,
- Rename `struct error_account_entry` to
`struct ust_error_accounting_entry`.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4758d32c00cb432be377fd67eaffa11b193bad74
Jérémie Galarneau [Wed, 21 Apr 2021 19:39:09 +0000 (15:39 -0400)]
Clean-up: sessiond: error-query: remove unneeded goto
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie0d02a5151b359ffb1e9ce487811d8235c4a3d9d
Jérémie Galarneau [Wed, 21 Apr 2021 19:36:04 +0000 (15:36 -0400)]
Fix: sessiond: action-executor: uninitialized session_name used in logging
CID
1452636 (#1 of 1): Uninitialized pointer read (UNINIT)
8. uninit_use_in_call: Using uninitialized value session_name when
calling fprintf
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I497c613e8cb1342948b726cc5c196e46d61d83dd
Jérémie Galarneau [Wed, 21 Apr 2021 19:31:38 +0000 (15:31 -0400)]
Clean-up: lttng: add-trigger: silence coverity warning
CID
1452634 (#1 of 1): Resource leak (RESOURCE_LEAK)
23. leaked_storage: Variable error going out of scope leaks the storage
it points to.
The code path shown by Coverity does not appear reachable, but this
fixes the warning nonetheless.
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9682616b123de108c5717cee2c17349212f224d5
Jérémie Galarneau [Wed, 21 Apr 2021 19:28:31 +0000 (15:28 -0400)]
Fix: error-query: leak of trigger on malformed error-query comm buffer
CID
1452633 (#1 of 1): Resource leak (RESOURCE_LEAK)
10. leaked_storage: Variable trigger going out of scope leaks the
storage it points to
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic3ab0c41b6667fb875992e545fcba046edc071fc
Jérémie Galarneau [Wed, 21 Apr 2021 18:39:52 +0000 (14:39 -0400)]
Fix: error-query: leak of trigger on allocation error
1452630 Resource leak
The system resource will not be reclaimed and reused, reducing the
future availability of the resource.
In lttng_error_query_action_create: Leak of memory or pointers to system
resources (CWE-404)
CID
1452630 (#1 of 1): Resource leak (RESOURCE_LEAK)
21. leaked_storage: Variable trigger_copy going out of scope leaks the
storage it points to.
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I4dd99390f1fd5ad957ab6500971dd2ed67a1c722
Jérémie Galarneau [Wed, 21 Apr 2021 18:34:46 +0000 (14:34 -0400)]
Clean-up: tests: notification-client: unused assignment
1452629 Unused value
An assigned value that is never used may represent unnecessary
computation, an incorrect algorithm, or possibly the need for cleanup or
refactoring.
In main: A value assigned to a variable is never used. (CWE-563)
CID
1452629 (#1 of 1): Unused value (UNUSED_VALUE)assigned_value:
Assigning value -1 to ret here, but that stored value is overwritten
before it can be used
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0d80a244a7b2b3d6aed57d58ca5435230d664f99
Jérémie Galarneau [Wed, 21 Apr 2021 18:30:02 +0000 (14:30 -0400)]
Fix: relayd: leak of config_path on duplicate --config option
1452207 Resource leak
The system resource will not be reclaimed and reused, reducing the
future availability of the resource.
In lttng_notification_create_from_payload: Leak of memory or pointers to
system resources (CWE-404)
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib4f6b388f4ff24eedcbff64570bd9fa10a445304
Jérémie Galarneau [Wed, 21 Apr 2021 18:22:08 +0000 (14:22 -0400)]
Fix: lttng-ctl: trigger leak on failure to deserialize evaluation
1452207 Resource leak
The system resource will not be reclaimed and reused, reducing the
future availability of the resource.
In lttng_notification_create_from_payload: Leak of memory or pointers to
system resources (CWE-404)
Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I5a66af837d46ca4b8e1909012cd28dc8ea990bad
Francis Deslauriers [Tue, 20 Apr 2021 13:52:00 +0000 (09:52 -0400)]
Fix: tests: discarded notifications: missing `list-triggers` call
I get the following test failure when running the
test_notification_notifier_discarded_count test case.
# Kernel event notifer error counter
ok 1 - Add trigger my_trigger
ok 2 - No discarded tracer notification message
ok 3 - Trigger 'errors: none' notification message
ok 4 - Tracer notification discarded line printed
ok 5 - Discarded tracer notification number non-zero as expected
ok 6 - Remove trigger my_trigger
ok 7 - No `on-event` kernel notifier enabled as expected
ok 8 - Add trigger my_trigger
ok 9 - No discarded tracer notification message
not ok 10 - Trigger 'errors: none' notification message
# Failed test 'Trigger 'errors: none' notification message'
# in tests/regression/tools/notification/test_notification_notifier_discarded_count:test_kernel_notifier_discarded_count() at line 108.
ok 11 - Remove trigger my_trigger
This happens because we test the `list-triggers` output on outdated
content. We need to call `list-triggers` right before testing the output
of it.
In this commit, I also changed the number of iteration to 2000 as it's
sufficient to generate discarded notifications.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I66d54f9178f9f37ff2fd899278e77fe50e455c10
Jérémie Galarneau [Tue, 20 Apr 2021 05:01:47 +0000 (01:01 -0400)]
Tests: fix: notifier discarded count: typo results in failure
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I94b8c60764e1a0d79b25fd252c1a344d807fa61f
Francis Deslauriers [Wed, 14 Apr 2021 21:30:13 +0000 (17:30 -0400)]
Cleanup: tests: test_userspace_probe: remove extra whitespaces
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6e890a6c1927feb4a09f46b200294cb2faf8a580
Francis Deslauriers [Wed, 20 Jan 2021 21:18:09 +0000 (16:18 -0500)]
Cleanup: add `lttng_ht_type_str()`
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1f7d9ae029f11a454cbda6b05eb22457d6b0be74
Francis Deslauriers [Thu, 15 Apr 2021 01:19:41 +0000 (21:19 -0400)]
Fix: lttng-ctl: appending to dynamic buffer invalidates its data member
Issue
=====
The following commands fail:
lttng add-trigger --id T0 --condition on-event -u some-event --action snapshot-session ze-session3 --path /some/path
lttng remove-trigger T0
Error: Attempt to create buffer view from another view with invalid length (length > space left after offset in source): source size = 0, offset in source = 0, length = 25
Error: Invalid trigger received as part of command payload
Valgrind complains in the following way:
==706109==
==706109== Invalid write of size 4
==706109== at 0x489FED7: lttng_unregister_trigger (lttng-ctl.c:3281)
==706109== by 0x43C175: cmd_remove_trigger (remove_trigger.c:171)
==706109== by 0x43F56B: handle_command (lttng.c:237)
==706109== by 0x43E9B1: parse_args (lttng.c:421)
==706109== by 0x43E158: main (lttng.c:470)
==706109== Address 0x73d8d20 is 4,688 bytes inside a block of size 16,384 free'd
==706109== at 0x483DFAF: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==706109== by 0x48C1478: lttng_dynamic_buffer_set_capacity (dynamic-buffer.c:166)
==706109== by 0x48C138C: lttng_dynamic_buffer_append (dynamic-buffer.c:55)
==706109== by 0x48E3325: lttng_snapshot_output_serialize (snapshot.c:120)
==706109== by 0x48B46C3: lttng_action_snapshot_session_serialize (snapshot-session.c:173)
==706109== by 0x48B1FB2: lttng_action_serialize (action.c:130)
==706109== by 0x48B2DFE: lttng_action_group_serialize (group.c:165)
==706109== by 0x48B1FB2: lttng_action_serialize (action.c:130)
==706109== by 0x48ECE66: lttng_trigger_serialize (trigger.c:372)
==706109== by 0x489FEA0: lttng_unregister_trigger (lttng-ctl.c:3275)
==706109== by 0x43C175: cmd_remove_trigger (remove_trigger.c:171)
==706109== by 0x43F56B: handle_command (lttng.c:237)
==706109== Block was alloc'd at
==706109== at 0x483B723: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==706109== by 0x483E017: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==706109== by 0x48C1478: lttng_dynamic_buffer_set_capacity (dynamic-buffer.c:166)
==706109== by 0x48C138C: lttng_dynamic_buffer_append (dynamic-buffer.c:55)
==706109== by 0x489FE66: lttng_unregister_trigger (lttng-ctl.c:3263)
==706109== by 0x43C175: cmd_remove_trigger (remove_trigger.c:171)
==706109== by 0x43F56B: handle_command (lttng.c:237)
==706109== by 0x43E9B1: parse_args (lttng.c:421)
==706109== by 0x43E158: main (lttng.c:470)
`lttng_unregister_trigger` samples the address of the lsm header in the
message payload. However, it does so before calling
`lttng_trigger_serialize()` which may increase the underlying buffer's
size (and cause a realloc()).
Most of the time the message buffer is large enough _or_ its realloc
yields the same address which hid the problem. However, I stumbled on a
case (a trigger which snapshots to a location) where the realloc ends-up
returning a completely different address, causing invalid data to be
sent to the session daemon.
Solution
========
Sample the lsm header address after the serialization of the trigger.
Note
====
An identical fix was done for the `lttng_register_trigger` function in:
commit
b22f4f54e95ae13edda1d4d5efd1e4845a6319c4
Author: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Thu Feb 18 18:13:19 2021 -0500
Fix: lttng-ctl: appending to dynamic buffer invalidates its data member
I reuse the bug explanation for this commit message.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic50c96dcada9e0595b0fab1d2f357c183b53e1de
Francis Deslauriers [Wed, 14 Apr 2021 14:50:28 +0000 (10:50 -0400)]
Fix: ust-app: error counter index never set
Issue
=====
The `struct ust_app_event_notifier_rule::error_counter_index` field is
currently never set so all error counting is done in bucket index 0.
Fix
===
Set the error counter index.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I951095dee5586111699b0ea0e3bc26c0fb4a535d
Francis Deslauriers [Wed, 14 Apr 2021 14:12:04 +0000 (10:12 -0400)]
Fix: lttng: -Wshadow error in cmd_snapshot
The optarg variable name shadows a variable in an external dependency.
clang returns the following error on my machine:
commands/snapshot.c:627:16: error: declaration shadows a variable in the global scope [-Werror,-Wshadow]
const char *optarg = poptGetOptArg(pc);
^
/usr/include/x86_64-linux-gnu/bits/getopt_core.h:36:14: note: previous declaration is here
extern char *optarg;
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I85dd7b0761ebc2c023d84ba869c0551f91f38a17
Francis Deslauriers [Wed, 14 Apr 2021 19:14:45 +0000 (15:14 -0400)]
Fix: python: -Wshadow warning
Fixes warning when building Python Bindings with SWIG 3.0.12.
lttng_wrap.c: In function ‘SWIG_Python_NewShadowInstance’:
lttng_wrap.c:2483:65: warning: declaration of ‘swig_this’ shadows a global declaration [-Wshadow]
SWIG_Python_NewShadowInstance(SwigPyClientData *data, PyObject *swig_this)
^~~~~~~~~
lttng_wrap.c:2224:18: note: shadowed declaration is here
static PyObject *swig_this = NULL;
^~~~~~~~~
lttng_wrap.c: In function ‘SWIG_Python_SetSwigThis’:
lttng_wrap.c:2558:51: warning: declaration of ‘swig_this’ shadows a global declaration [-Wshadow]
SWIG_Python_SetSwigThis(PyObject *inst, PyObject *swig_this)
^~~~~~~~~
lttng_wrap.c:2224:18: note: shadowed declaration is here
static PyObject *swig_this = NULL;
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Icde5f124e914a158eb190f95e3c012b7f800d7d1
Jérémie Galarneau [Mon, 19 Apr 2021 15:07:12 +0000 (11:07 -0400)]
Fix: kernel consumerd: use of uninitialized variable
‘ret_add_channel’ may be used uninitialized in this function [-Wmaybe-uninitialized]
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1db71aef9a9585d8737347f8375e688994f56c5f
Jérémie Galarneau [Mon, 19 Apr 2021 15:06:23 +0000 (11:06 -0400)]
Clean-up: sessiond: cmd: abort() on invalid query target
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia02fb651a18d927c3b04e20bb55aa588d071b87c
Jérémie Galarneau [Mon, 19 Apr 2021 15:00:23 +0000 (11:00 -0400)]
Clean-up: sessiond: cmd: fix bogus -Wmaybe-uninitialized
Old gcc versions report:
cmd.c:4751: ‘query_target_action’ may be used uninitialized in this function [-Wmaybe-uninitialized]
For this to be reachable, a query target would have to change suddenly
during the execution of the function.
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Id90ca37e232385f73d54558f2a00d01cbf773a92
Jérémie Galarneau [Thu, 15 Apr 2021 17:21:14 +0000 (13:21 -0400)]
Remove error count property of lttng_condition_on_event
Errors related to dropped tracer messages are not reported through
the error query interface.
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib5f8bf841f37d98b582209abb44ab8a26c8fbbc6
Jérémie Galarneau [Thu, 15 Apr 2021 17:14:37 +0000 (13:14 -0400)]
lttng: list-triggers: retrieve trigger errors using error queries
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I1a80125122e2568491d940c598d8445410bf58d9
Jérémie Galarneau [Thu, 15 Apr 2021 17:11:09 +0000 (13:11 -0400)]
sessiond: implement EXECUTE_ERROR_QUERY command
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I3404e85e18b0b4e5d51b620c3a082736d3190066
Jérémie Galarneau [Thu, 15 Apr 2021 17:09:35 +0000 (13:09 -0400)]
lttng-ctl: Add error query interface
Change-Id: Ibb0ba800330976aad7b7c501f43a0d23a0dba027
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Jonathan Rajotte [Thu, 15 Apr 2021 21:35:20 +0000 (17:35 -0400)]
add-trigger: rename --fire-* to --rate-policy=*:value
With the move to firing policy to the action and the renaming to rate
policy, a rename of the CLI options is necessary.
We introduce the following format:
--rate-policy=<type>:<value>
Where type is either: once-after or every.
Value is an unsigned long long value.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6df264c536293e686033d0baa8fe870c75c28b8d
Jonathan Rajotte [Thu, 15 Apr 2021 03:18:25 +0000 (23:18 -0400)]
Rename firing policy to rate policy
This is necessary since that now that the policy is moved to the actions
it is a bit weird to use the term "firing".
CLI change is done in a later patch.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Id6940a49cd0640a8fa7145dab5b805fcfa515f40
Jonathan Rajotte [Tue, 13 Apr 2021 18:49:52 +0000 (14:49 -0400)]
action executor: use an execution context to validate enqueued action before execution
Problem
======
Let's walk over the following scenario:
Trigger my_trigger:
condition: ust event a
action: start session S
At time T, session S exists.
At T + 1, the event A is hit.
At T + 2, the tracer event notification is received and the work item is
queued. Here session S have an id of 1.
At T + 3, the session S is destroyed and a new session S is created,
with a resulting id of 200.
At T +4, the work item is popped from the queue and begin execution and
will start session S with an id of 200 instead of the session S id 1
that was present at the queuing phase.
We effectively want to assign an execution context to an action at the
queuing phase and validate that the execution context is still valid
upon actual execution.
Solution
=====
Instead of iterating over the trigger's actions, we prepare an array of
work sub-item (action_work_subitem) that effectively is a flattened and
augmented array of the trigger's action. For now only the session id is
an action level execution context.
Each action handler is responsible of validating the execution context
before performing the action.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I72fbe8d52d77d1d696fa7d99b662b77e9dad02d5
Jonathan Rajotte [Mon, 12 Apr 2021 20:38:16 +0000 (16:38 -0400)]
trigger/action executor: move trigger registration state change to client thread
Set the unregistered state of the sessiond side trigger object in the
client thread since we want to minimize the possibility of the
notification thread being stalled due to a long execution of an action
in the action executor thread.
Stalling the client thread is, comparatively, less harmful.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7de875996b430e49ec50896f8b26a75af7993c74
Francis Deslauriers [Mon, 8 Mar 2021 22:19:51 +0000 (17:19 -0500)]
notification-thread: add `GET_TRIGGER` command
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ifc94ea894999b9da796aa4fda0d929d237e12d3e
Jonathan Rajotte [Mon, 12 Apr 2021 19:41:36 +0000 (15:41 -0400)]
action executor: execute action only if the associated trigger is registered
It was decided that a trigger removal should have immediate effect and
not result in a waiting on action queued to be executed.
There was two major way to do this:
1) On unregistrer trigger, lock the action executor queue and remove
all work-item related to the trigger ebing unregistered.
2) Mark the trigger as "unregistered" and validate its state when a work
item is ready for execution. If the trigger is marked as "unregistered",
simply consume and skip the work item.
To isolate the un-registration and the execution of a work item
depending on that trigger, we use the trigger lock to have exclusivity
during the execution for that trigger.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9f42a323e0ec0dc51339f39f07536ec8060dc02a
Jonathan Rajotte [Mon, 12 Apr 2021 18:45:24 +0000 (14:45 -0400)]
trigger: keep state of if a trigger is currently registered
Since a trigger can be referenced even when is was "unregistered" in
other part of lttng-sessiond, namely the action executor queue, we must
keep track of the registration state.
This will allows us to easily skip any actions to be executed
if the associated trigger is "unregistered" at the moment of
execution. This is implemented in a following patch.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I06c9d437fce975c9c8393e8d1be3e66a24618af3
Jérémie Galarneau [Sat, 17 Apr 2021 22:47:16 +0000 (18:47 -0400)]
Tests: fix: leak of notification-client arguments
Valgrind reports the following leaks:
==334435== HEAP SUMMARY:
==334435== in use at exit: 8,568 bytes in 24 blocks
==334435== total heap usage: 846 allocs, 822 frees, 1,425,964 bytes allocated
==334435==
==334435== 9 bytes in 1 blocks are definitely lost in loss record 1 of 11
==334435== at 0x483E77F: malloc (vg_replace_malloc.c:307)
==334435== by 0x4BCC5BE: strdup (in /usr/lib/libc-2.33.so)
==334435== by 0x10A48B: main (notification-client.c:123)
==334435==
==334435== 13 bytes in 1 blocks are definitely lost in loss record 2 of 11
==334435== at 0x483E77F: malloc (vg_replace_malloc.c:307)
==334435== by 0x4BCC5BE: strdup (in /usr/lib/libc-2.33.so)
==334435== by 0x10A473: main (notification-client.c:120)
==334435==
==334435== 27 bytes in 1 blocks are definitely lost in loss record 3 of 11
==334435== at 0x483E77F: malloc (vg_replace_malloc.c:307)
==334435== by 0x4BCC5BE: strdup (in /usr/lib/libc-2.33.so)
==334435== by 0x10A444: main (notification-client.c:114)
Free the strdup()'ed' arguments.
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I21523e847141aef50858d3018f37f0125dee0e2c
Jonathan Rajotte [Tue, 30 Mar 2021 02:00:37 +0000 (22:00 -0400)]
tests: trigger action firing policy
Implements the regression tests for the trigger action firing polices.
For now the focus is put on the notify action behavior. In the future,
the ability to fetch the number of time an action was enqueued for
execution and the number of type it was executed should greatly
facilitate this work. For now, use the notify action for minimal
regression testing.
gen-ust-events is modified to allow a caller to specify that an `end`
event should be emitted at the end of the program. This is used to
guaranteed that all previous event tracer notifications should be
received when the `end` event notification is received. Note that
ordering is guaranteed for delivery of tracer notification and
notification for a given process.
notificiation-client is modified to allow reception of N notifications
and to validate that it received the count it expects.
notificiation-client is modified to allow reception of N notifications
and to exit only when an `end` event notification is received. This
allow us validate the firing policy.
The tests are pretty straightforward. We register 2 triggers, one with
an event rule condition on `tp:tptest` and the other one with a event
rule condition on `tp:end`. From there we perform a phase of event hits
were we expect no notification except for the `tp:end` then we force a
single event hit were we expect a notification for `tp:tptest`. For the
fire every N policy, we rinse and repeat to validate the behaviour. For
the once after N, we simply do a third phase were we generate N events
and expect no notification.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia3c88f38dcca0389165e1416b06768338b2889b6
Jonathan Rajotte [Tue, 30 Mar 2021 01:43:12 +0000 (21:43 -0400)]
Move firing policy from lttng_trigger to lttng_action
After some reflection on the future of the importance of the trigger
feature and the centralization we wish to carry around it, it is
required that the notion of firing policy be moved from the trigger
object to each action object of a trigger.
This is necessary since we plan on introducing tracer side actions, such
as increment value of map. Controlling the firing policy on the tracer
side is not an easy thing to do since for UST tracing a lot a
synchronizations must take place and also we must define the behaviour
when multiple apps are present. Hence, we need a way to ensure that we
are not painting ourself in a corner. The middle ground that was chosen
was to move the notion of firing policy to the action object level. This
allows us to scope the concept to the action and decide for each type if
firing policy can be supported and, as needed, define the behaviour per
action type.
Essentially this patch perform the complete transition. It removes the
notion of firing policy at the trigger level and exposes the firing
policy of each action type if applicable.
CLI
======
For the `add-trigger` command the change essentially boils down to
moving the `--fire-every` and `--fire-once-after` from a top-level
parsing to the parsing of each actions. Yes, for now all actions
supports the `--fire-*` options but it will not be the case in the
future. A side effect of this is that a user could decide to have
different firing policy for each actions, but this also mean that if a
user want to apply the same firing policy across actions, the user needs
to specify it for each actions. This could be solved in the future as
the trigger feature matures and that common ground are found in the
behaviour or implementation of the actions (tracer side action, async
action, sync actions etc.) such that "syntactic sugar" options emerge.
As for the `list-trigger`, we move the firing policy printing to each
actions.
Tests have been updated to reflect the changes.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib29d4319922096c0c4b3f00782f3bbefb17e2f40
Jonathan Rajotte [Wed, 7 Apr 2021 13:19:18 +0000 (09:19 -0400)]
action-executor: consider action firing policy on action execution
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I83f73e905e87a401187e243ed3beafa41964434a
Jonathan Rajotte [Thu, 8 Apr 2021 19:48:39 +0000 (15:48 -0400)]
test: unit: snapshot session action
Note that the introduced test are mostly toward the firing policy
parameter.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7685610f963576e50747a1801c4e0cd754e241b8
Jonathan Rajotte [Thu, 8 Apr 2021 17:51:35 +0000 (13:51 -0400)]
Implement firing policy for the snapshot session action
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ia849bf101cbdf09977adc0781809139a3c769ab2
Jonathan Rajotte [Thu, 8 Apr 2021 13:35:05 +0000 (09:35 -0400)]
tests: units: action: stop session
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib8b230857180966085d682a29e54f6baa525258f
Jonathan Rajotte [Thu, 8 Apr 2021 13:34:43 +0000 (09:34 -0400)]
Implement firing policy for stop session action
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iaed5d8c3cc59b2284692ed56f1b4a8ba69736fd6
Jonathan Rajotte [Thu, 8 Apr 2021 13:14:22 +0000 (09:14 -0400)]
tests: unit: action: start_session
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie224592426d286b6abd4c7b627853bef71862bb8
Jonathan Rajotte [Thu, 8 Apr 2021 13:14:06 +0000 (09:14 -0400)]
Implement firing policy for the start session action
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ica842dbd9980e8dbf73e38cbb1136115394c001c
Jonathan Rajotte [Thu, 8 Apr 2021 01:17:13 +0000 (21:17 -0400)]
tests: unit: action: rotate_session
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib412196dea5cb9617b4c56e8eb93698bcfaa40d7
Jonathan Rajotte [Thu, 8 Apr 2021 01:02:31 +0000 (21:02 -0400)]
Implement firing policy for the rotate session action
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I0e2b50d6653cca7a33abc6ee68bed237950476a9
Jonathan Rajotte [Fri, 9 Apr 2021 14:59:09 +0000 (10:59 -0400)]
tests: unit: action: notify
Note: the added tests are mostly toward testing of the firing policy
parameter.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I2aec17175ac2fed4cf21a15f998b24dfd353e77b
Jonathan Rajotte [Fri, 9 Apr 2021 01:40:12 +0000 (21:40 -0400)]
Implement firing policy for the notify action
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I40b25033868fd4ec9cb6764f3c47ef3e006bec3e
Jonathan Rajotte [Fri, 9 Apr 2021 14:58:34 +0000 (10:58 -0400)]
Move test_action to its own file
Further unit testing on action objects will be conducted in the
test_action.c file.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib17aed9eb02592d5c13bc2875d4203b81ab04cf3
Jonathan Rajotte [Tue, 6 Apr 2021 15:09:09 +0000 (11:09 -0400)]
tests: unit: add firing policy tests
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ib92bd40b2c93f4e90ec430f2ac70ef2a5ce8f891
Jonathan Rajotte [Fri, 2 Apr 2021 01:20:23 +0000 (21:20 -0400)]
Introduce firing policy object
Change-Id: I436ce58d083cd70178a51d0d9ebfddc814f2cd15
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Jonathan Rajotte [Tue, 30 Mar 2021 01:38:54 +0000 (21:38 -0400)]
Fix: test: start-stop trigger: test execution is invalid
Observed issue
==============
The test expects the notification client to be a background process but
`&` is not used.
The notification client is expected to sync with its launcher via the
sync file but the invocation of mktemp already creates the file.
Solution
=======
Use `&` and `mktemp -u`.
While there a little cleanup in notification_client for error code path
was done.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iba80413d6ca36989cd967895d3330860cb4ef614
Jonathan Rajotte [Mon, 15 Mar 2021 21:52:24 +0000 (17:52 -0400)]
Fix: sessiond: session destroy hang in per-uid when context cannot be added
Observed issue
==============
The system_test CI jobs hang on the perf test suite during the destroy
command steps of the ust perf raw subtest.
Cause
=====
The system_test are running inside a kvm as root. It turns out that the
PMU (UNHALTED_REFERENCE_CYCLES) the test suite is trying to add is
unavailable on the qemu host.
ustctl_add_context return -1024 since it fails to add the context.
This leads us down the error path for the callstack leading to the
ustctl_add_context call.
1) `ust_app_channel_create` returns `ret` != 0;
2) `find_or_create_ust_app_channel` returns `ret != 0`;
3) `ust_app_synchronize` based on the `ret` value goes directly to the
end of the function to an error path without passing on the
`create_ust_app_metadata` function and clean-up structure related to
the app.
Note that being in per-uid mode, data and metadata
channel/streams/buffer allocation is done on the fly for the first app
during `ust_app_synchronize` and its callee. For the current problematic
scenario, only the data channels have been allocated on the consumer for
the uid at that point. The metadata for that uid is not yet created.
Now that we know more of what is going on during an ""add context""
let's take a look at the actual hang.
The client never complete the destroy command since the consumerd
indicates that the trace chunk for the session is not closed. The trace
chunk still exists despite the fact that a close chunk command has been
issued. This is the case since its refcount never reaches zero and thus
the release does not complete.
In a normal execution without the use of contexts, the release of the
trace hunk (refcount == 0) occurs during the final rotation on destroy.
Upon further comparison between a working execution and a non-working
execution, in a non-working execution the `cmd_rotate_session` does not
issue the rotation for the data channels since the loop detects that no
metadata is present. Which, as we discussed earlier, can happen if we
fail to add the context to the app channel.
[1]
```
cds_list_for_each_entry(reg, &usess->buffer_reg_uid_list, lnode) {
struct buffer_reg_channel *reg_chan;
struct consumer_socket *socket;
if (!reg->registry->reg.ust->metadata_key) {
/* Skip since no metadata is present */
continue;
}
....
/* Rotate the data channels. */
cds_lfht_for_each_entry(reg->registry->channels->ht, &iter.iter,
reg_chan, node.node) {
ret = consumer_rotate_channel(socket,
reg_chan->consumer_key,
usess->uid, usess->gid,
usess->consumer,
/* is_metadata_channel */ false);
if (ret < 0) {
cmd_ret = LTTNG_ERR_ROTATION_FAIL_CONSUMER;
goto error;
}
}
....
}
```
Solution
========
Move the metadata check after the data channel rotation since it is
possible to have data channels but no metadata channel, although it is a
corner case.
Note that per-pid mode and kernel are not affected by the current bug
since a complete teardown of all objects is done. This only affect
per-uid due to the "on the fly" allocation nature of it since we need to
share the channel/stream/buffers across apps.
Known drawbacks
=========
None.
References
==========
[1] https://github.com/lttng/lttng-tools/blob/
3d1384a4add389c38f8554130e8dec2e2d06009d/src/bin/lttng-sessiond/ust-app.c#L7057
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ie8fd1167170706ec9abc5f31f3b33a7306e92cd9
Jonathan Rajotte [Wed, 13 Jan 2021 15:09:06 +0000 (10:09 -0500)]
Fix: backward relayd: path contains a leading "ust" folder
Observed issue
==============
test_output_path_relayd[lttng-tools-2.13-lttng-tools-2.10-uid] from
the lttng-ivc fails on path validation for the resulting trace.
Here lttng-sessiond is 2.13 and lttng-relayd is 2.10
Traces are generated and the following hierarchy is found in the
lttng-relayd trace folder.
lttng_home
└── lttng-traces
* └── ust
└── joraj-alpa
├── auto-
20210113-165054
│ └── ust
│ └── uid
Note the extra "ust" (*) in the hierarchy.
The tests expects:
lttng_home
└── lttng-traces
└── joraj-alpa
├── auto-
20210113-165054
│ └── ust
│ └── uid
Cause
=====
Introduced by:
5da88b0f58d7f838068037ea449ddfb25d3e85ad [1]
relayd_add_stream now suffixes the domain_name to the pathname.
This is only necessary for cases where the corresponding
lttng-relayd version is greater than 2.10. In other cases, modification
of pathname is not necessary.
Solution
========
Perform domain suffixing only for relayd > 2.10.
Known drawbacks
=========
None.
References
==========
[1] https://github.com/lttng/lttng-tools/commit/
5da88b0f58d7f838068037ea449ddfb25d3e85ad
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Iaca4667074aaf7f4f20b50ddb4778fbc02156b5a
Christophe Bedard [Fri, 28 Aug 2020 15:00:30 +0000 (11:00 -0400)]
Fix: lttng_destroy_session_no_wait: return 0 on success
lttng_destroy_session_no_wait() is supposed to behave like
lttng_destroy_session():
> Return 0 on success else a negative LTTNg error code.
However, it returns LTTNG_OK on success. Make it return 0 instead.
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: If51f6a2cc3ca77237f7cbac806c5206a807dadf5
Jonathan Rajotte [Wed, 3 Mar 2021 20:45:11 +0000 (15:45 -0500)]
Fix: notification: client with uid != trigger uid assert on gid check
The gid was removed from the trigger object since gid is not an exposed
control measure anywhere in lttng except for the "tracing" group and
lttng-sessiond to client communication.
Also based on this:
/*
* As for privilieged users, they can register triggers against the objects of
* other users. They can then subscribe to the notifications associated to their
* triggers. Privilieged users _can't_ subscribe to the notifications of
* triggers owned by other users; they must create their own triggers.
* /
This is why the current check is not bypassed if the notification client
have a UID == 0 (root).
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I690ea355b0b1ab0bfe82b7db47275876afe622f2
Jonathan Rajotte [Thu, 4 Mar 2021 21:10:18 +0000 (16:10 -0500)]
event-rule: Normalize pattern for syscall and tracepoint
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: If5bae2cd5df76db3f9bd626f95fcace114fcc420
Simon Marchi [Fri, 2 Apr 2021 19:52:46 +0000 (15:52 -0400)]
config: accept "0" and "1" as XML boolean values
These are valid values when validating a "boolean" using a schema (XSD).
Somebody could therefore write a file with
<started>0</started>
instead of
<started>false</started>
The file would validate against session.xsd, but we would fail after
that:
Warning: Invalid boolean value encoutered (0).
Error: Invalid session configuration
Reference: https://www.w3.org/TR/xmlschema11-2/#boolean
Change-Id: Icea85b13233ec01aa09ef3b578906f4c0e482088
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Fri, 2 Apr 2021 19:52:27 +0000 (15:52 -0400)]
config: fix typo in error message
Change-Id: Ie02ccd1e79aecb87fbb9c308ffba09353d50b8f3
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Francis Deslauriers [Fri, 19 Mar 2021 20:11:40 +0000 (16:11 -0400)]
Cleanup: reuse `lttng_action_type_string()` to print action names
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I86be1201466a11510ded9da34311650a8aadd1e2
Mathieu Desnoyers [Mon, 25 May 2020 20:19:18 +0000 (16:19 -0400)]
Tests: array expressions without contant index are invalid
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I121387b63eb85851be59416c758b7ca12f823f45
Mathieu Desnoyers [Mon, 25 May 2020 20:15:40 +0000 (16:15 -0400)]
Fix: validate that array expression contains constant
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ic7ea2192b3bf0f5c32f5db8fce622f73b182906f
Jonathan Rajotte [Wed, 2 Dec 2020 22:01:35 +0000 (17:01 -0500)]
Fix: test: base-path tests are not run
Observed issue
==============
The base-path tests are never run during `make check`.
Cause
=====
Albeit
2a1668643ca94195d5c3889d0337e19165805a42 [1] introduces the tests,
the test file is not added to the test list.
Solution
========
Add the test file to the test list.
One of the substest is failing.
not ok 20 - Snapshot recorded
# Failed test 'Snapshot recorded'
# in .//../../../utils/utils.sh:lttng_snapshot_record() at line 1468.
ok 21 - Destroy session ust_app_snapshot_base_path
not ok 22 - Validate trace for event tp:tptest
This is caused by the removal of `trace_path=$2` in
c28fcefd993b7539716bb5cd9557a08a217ec463 [2]. The removal is most
probably a merge error or simply a mistake on my end. This commit revert
that change.
Also add load-stream-extra-path.lttng in EXTRA_DIST.
Known drawbacks
=========
None
References
==========
[1] https://github.com/lttng/lttng-tools/commit/
2a1668643ca94195d5c3889d0337e19165805a42
[2] https://github.com/lttng/lttng-tools/commit/
2a1668643ca94195d5c3889d0337e19165805a42
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I9a8040141c29e391363b17954a20155abdb3cc04
Jonathan Rajotte [Wed, 17 Feb 2021 14:19:50 +0000 (09:19 -0500)]
Fix: tests: handling of subprocesses on bail out
Observed issue
==============
Background processes of the test_notification_ust tests are present on CI
worker after the jobs is finished.
Cause
=====
The usage of BAIL_OUT during the notap version of stop lttng-sessiond
would end up preventing the normal cleanup of background processes since
we exit directly when issuing the bail out.
Solution
========
Introduce LTTNG_BAIL_OUT.
LTTNG_BAIL_OUT ensure that we call the cleanup path that is normally
used when testing is interrupted.
Add the flag `is_cleanup` to all relevant `*stop_opts` functions.
And introduce the `*_cleanup` functions for relayd, sessiond, consumerd.
While at it, a small rework on how we kill the subprocesses of the
non-iteractive shell was done. This is useful because when using the
test runner the group id of the running process performing the kill is
not valid and result in simply not propagating the SIGTERM signal.
We now use "set -m" to enable monitor mode ensuring that all background
jobs is its own process group id, facilitating the usage of kill with
negative value to ensure that each background jobs subprocesses receives
the SIGTERM signal.
Known drawbacks
=========
We introduce a new lttng specific bail out directive instead of using
the BAIL_OUT from tap.sh
We could override the BAIL_OUT function based on [1].
[1] https://mharrison.org/post/bashfunctionoverride/
References
==========
https://linux.die.net/man/1/bash#:~:text=Monitor%20mode
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I42ce8659df3e1d9078cc1a46a11a33a2df9a145e
Simon Marchi [Fri, 9 Apr 2021 16:04:13 +0000 (12:04 -0400)]
Fix: sessiond: fix memory leak in receive_lttng_trigger
After adding a trigger, ASan reports this when exiting the sessiond:
Direct leak of 128 byte(s) in 1 object(s) allocated from:
#0 0x7ffff767783a in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:164
#1 0x55555579a415 in lttng_dynamic_buffer_set_capacity /home/simark/src/lttng-tools/src/common/dynamic-buffer.c:166
#2 0x55555579a1df in lttng_dynamic_buffer_set_size /home/simark/src/lttng-tools/src/common/dynamic-buffer.c:118
#3 0x5555556d3cc1 in receive_lttng_trigger /home/simark/src/lttng-tools/src/bin/lttng-sessiond/client.c:712
#4 0x5555556dba46 in process_client_msg /home/simark/src/lttng-tools/src/bin/lttng-sessiond/client.c:2154
#5 0x5555556deef7 in thread_manage_clients /home/simark/src/lttng-tools/src/bin/lttng-sessiond/client.c:2601
#6 0x5555556c8a83 in launch_thread /home/simark/src/lttng-tools/src/bin/lttng-sessiond/thread.c:66
#7 0x7ffff714c298 in start_thread (/usr/lib/libpthread.so.0+0x9298)
It seems like we don't free the payload in receive_lttng_trigger, fix
that.
Change-Id: Ie9bc3bad24fb55b98c8232c0cd63483a3e94bfb0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Fri, 9 Apr 2021 15:19:17 +0000 (11:19 -0400)]
Fix: filter: memory leak in filter_parser_ctx
When running
$ lttng add-trigger --condition on-event -u ust_tests_demo2:loop --capture intfield --action notify
I get the leaks pasted below. It seems like filter_parser_ctx_free
doesn't free everything in filter_parser_ctx. Add what's missing.
Re-order the frees so that they are in the same order as the members of
the struct, just because it's easier to follow and make sure we didn't
forget anything.
=================================================================
==
1073803==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 128 byte(s) in 1 object(s) allocated from:
#0 0x7ffff767783a in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:164
#1 0x5555556833be in bytecode_reserve /home/simark/src/lttng-tools/src/common/bytecode/bytecode.c:59
#2 0x55555568360f in bytecode_push /home/simark/src/lttng-tools/src/common/bytecode/bytecode.c:79
#3 0x5555556a3d61 in filter_visitor_bytecode_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:667
#4 0x55555569c9b1 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:394
#5 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#6 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#7 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#8 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#9 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#10 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#11 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#12 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x7ffff767783a in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:164
#1 0x5555556833be in bytecode_reserve /home/simark/src/lttng-tools/src/common/bytecode/bytecode.c:59
#2 0x55555568360f in bytecode_push /home/simark/src/lttng-tools/src/common/bytecode/bytecode.c:79
#3 0x5555556a1b94 in visit_node_load_expression_legacy /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:198
#4 0x5555556a1d18 in visit_node_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:231
#5 0x5555556a2540 in visit_node_load /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:399
#6 0x5555556a3a8b in recursive_visit_gen_bytecode /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:622
#7 0x5555556a12fa in visit_node_root /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:53
#8 0x5555556a3a76 in recursive_visit_gen_bytecode /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:620
#9 0x5555556a3c55 in filter_visitor_bytecode_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-bytecode.c:661
#10 0x55555569c9b1 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:394
#11 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#12 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#13 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#14 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#15 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#16 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#17 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#18 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7ffff7677639 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5555556a3dd2 in make_op_root /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:35
#2 0x5555556a73a5 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:874
#3 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#4 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#5 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#6 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#7 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#8 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#9 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#10 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#11 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#12 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Indirect leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7ffff7677639 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5555556a4f1d in make_op_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:280
#2 0x5555556a696f in make_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:637
#3 0x5555556a73df in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:882
#4 0x5555556a7382 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:870
#5 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#6 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#7 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#8 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#9 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#10 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#11 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#12 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#13 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#14 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7ffff7677639 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5555556a484d in create_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:201
#2 0x5555556a5040 in make_op_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:287
#3 0x5555556a696f in make_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:637
#4 0x5555556a73df in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:882
#5 0x5555556a7382 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:870
#6 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#7 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#8 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#9 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#10 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#11 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#12 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#13 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#14 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#15 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7ffff7677639 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5555556a4e64 in create_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:262
#2 0x5555556a5040 in make_op_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:287
#3 0x5555556a696f in make_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:637
#4 0x5555556a73df in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:882
#5 0x5555556a7382 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:870
#6 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#7 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#8 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#9 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#10 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#11 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#12 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#13 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#14 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#15 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7ffff7677639 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5555556a4bbc in create_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:233
#2 0x5555556a5040 in make_op_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:287
#3 0x5555556a696f in make_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:637
#4 0x5555556a73df in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:882
#5 0x5555556a7382 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:870
#6 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#7 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#8 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#9 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#10 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#11 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#12 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#13 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#14 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#15 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Indirect leak of 9 byte(s) in 1 object(s) allocated from:
#0 0x7ffff761fa69 in __interceptor_strdup /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:452
#1 0x5555556a4c41 in create_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:238
#2 0x5555556a5040 in make_op_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:287
#3 0x5555556a696f in make_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:637
#4 0x5555556a73df in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:882
#5 0x5555556a7382 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:870
#6 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#7 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#8 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#9 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#10 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#11 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#12 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#13 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#14 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#15 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Indirect leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x7ffff7677639 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5555556a4829 in create_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:196
#2 0x5555556a5040 in make_op_load_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:287
#3 0x5555556a696f in make_expression /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:637
#4 0x5555556a73df in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:882
#5 0x5555556a7382 in generate_ir_recursive /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:870
#6 0x5555556a74d6 in filter_visitor_ir_generate /home/simark/src/lttng-tools/src/common/filter/filter-visitor-generate-ir.c:903
#7 0x55555569c859 in filter_parser_ctx_create_from_filter_expression /home/simark/src/lttng-tools/src/common/filter/filter-parser.y:353
#8 0x55555560542e in parse_event_rule /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:704
#9 0x555555607429 in handle_condition_event /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1088
#10 0x555555608760 in parse_condition /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1326
#11 0x55555560bca0 in cmd_add_trigger /home/simark/src/lttng-tools/src/bin/lttng/commands/add_trigger.c:1925
#12 0x555555616b55 in handle_command /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:237
#13 0x555555617516 in parse_args /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:421
#14 0x555555617812 in main /home/simark/src/lttng-tools/src/bin/lttng/lttng.c:470
#15 0x7ffff700bb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
SUMMARY: AddressSanitizer: 409 byte(s) leaked in 9 allocation(s).
Change-Id: I04f9eb5ab7b18ae4ffdf7a49842768a6fdae5dbc
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:42:42 +0000 (10:42 -0400)]
configure: enable -Wshadow diagnostic
The codebase should compile cleanly with this flag, enable it by
default.
Change-Id: Ia9581845aec079347616fccd554d7199d012c2f1
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 19:14:44 +0000 (15:14 -0400)]
Clean-up: tests: fix -Wshadow error in run_active_set_combination
... by renaming one of the pipe variables.
Change-Id: I550c4c4c8c71171f435bdc1fb6afffdd68d1147b
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 19:10:13 +0000 (15:10 -0400)]
Clean-up: lttng-session: fix -Wshadow error in unregister_all_triggers
We can safely re-use the variable at function scope.
Change-Id: I0c357c328325853842d599b2d79dc2076c48365e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 19:08:05 +0000 (15:08 -0400)]
Clean-up: lttng-sessiond: fix -Wshadow error in check_session_rotation_pending
We can safely re-use the variable at function scope.
Change-Id: I2f9440c676a5458a063212d7143b0b69635fec87
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 19:06:34 +0000 (15:06 -0400)]
Fix: sessiond: fix -Wshadow error in save.c
I think this actually fixes a bug. Because of the second ret variable,
the return value from init_ust_event_from_agent_event or save_ust_event
would not be forwarded correctly, in case of error.
Change-Id: I6aad9e36f31ae5abf8ef487680af7c79853ffa3b
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 19:05:53 +0000 (15:05 -0400)]
Clean-up: lttng-relayd: prepend `the_` to global variable names tcp_keep_alive.c
To avoid clashing with function parameter names.
Change-Id: Ib8be1640d74101b21ee5921c2b966ed8b90cd9e1
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 19:02:58 +0000 (15:02 -0400)]
Clean-up: lttng-crash: prepend `the_` to input_path global variable
... to avoid clashing with a function parameter name.
Change-Id: I96ca04576b4cc18adff7e5e19483d4fb74a7c669
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:59:43 +0000 (14:59 -0400)]
Clean-up: lttng: fix -Wshadow error in run_command_string
We can safely re-use the `ret` variable.
Change-Id: Iba98715bf0c2810df7510eeb6a00b17ed56c9f58
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:57:53 +0000 (14:57 -0400)]
Clean-up: lttng: prepend `the_` to global variables in load.c
This avoids name clashes between the global variable `session_name` and
function parameters of the same name. Change all global variables of the
file while at it.
Change-Id: I79fe01e22b5797f156d47dbf425baa8f1fc4a6b0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:56:14 +0000 (14:56 -0400)]
Clean-up: lttng: fix -Wshadow error in cmd_snapshot
... by renaming a local variable.
Change-Id: If0aeb5adc71a0cc72efa60582df5447e12ad74d0
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:55:41 +0000 (14:55 -0400)]
Clean-up: lttng: fix -Wshadow error in lttng_session_descriptor
... by moving variable declaractions to narrower scopes.
Change-Id: I2ae3586596f543706b8e2ee5c7d4ce52e0658a9e
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:54:08 +0000 (14:54 -0400)]
Clean-up: lttng: prepend `the_` to global variables in list.c
This avoids name clashes between the global variable `handle` and
function parameters of the same name. Change all global variables of the
file while at it.
Change-Id: Ice1a4f98270f439f5e5001014d9b3d6f2d8a5803
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:53:04 +0000 (14:53 -0400)]
Clean-up: liblttng-ctl: fix two trivial -Wshadow errors
Fix two similar cases by declaring a variable in a narrower scope.
Change-Id: I6e36f4cf07086ccb5e58a175aff04671cc64323d
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:51:12 +0000 (14:51 -0400)]
Clean-up: liblttng-ctl: fix -Wshadow error in lttng_enable_event_with_exclusions
The local variable `handle` shadows the parameter, rename the local
variable.
Change-Id: Ic1f3b8ae9fd6ac356eeee691c0328d6e8e1d0216
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:50:28 +0000 (14:50 -0400)]
Clean-up: common: fix -Wshadow error in lttng_trace_chunk_set_as_owner
We can safely re-use the reference_acquired variable at function scope.
Change-Id: I4e45c4995d07188d48f5aa36280859ebb3ed2233
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:44:06 +0000 (14:44 -0400)]
Clean-up: common: fix -Wshadow errors in event-field-value.c
We get this when building with -Wshadow:
CC event-field-value.lo
In file included from /home/simark/src/lttng-tools/src/common/error.h:19,
from /home/simark/src/lttng-tools/src/common/event-field-value.c:17:
/home/simark/src/lttng-tools/src/common/event-field-value.c: In function ‘lttng_event_field_value_unsigned_int_get_value’:
/home/simark/src/lttng-tools/src/common/macros.h:55:42: error: declaration of ‘__ptr’ shadows a previous local [-Werror=shadow]
55 | const typeof(((type *)NULL)->member) * __ptr = (ptr); \
| ^~~~~
/home/simark/src/lttng-tools/src/common/event-field-value.c:390:10: note: in expansion of macro ‘container_of’
390 | *val = container_of(
| ^~~~~~~~~~~~
/home/simark/src/lttng-tools/src/common/event-field-value.c:391:5: note: in expansion of macro ‘container_of’
391 | container_of(field_val,
| ^~~~~~~~~~~~
/home/simark/src/lttng-tools/src/common/macros.h:55:42: note: shadowed declaration is here
55 | const typeof(((type *)NULL)->member) * __ptr = (ptr); \
| ^~~~~
/home/simark/src/lttng-tools/src/common/event-field-value.c:390:10: note: in expansion of macro ‘container_of’
390 | *val = container_of(
| ^~~~~~~~~~~~
This is because of the nested use of container_of, causing two temporary
__ptr variables to be declared in the same scope. Fix it by assigning
results of container_of to temporary variables. I think the temporary
variables make it more readable anyway, showing what's going on.
Change-Id: I7b66bc40227a4c76b7f5416a911dcdc696f4efc8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:41:48 +0000 (14:41 -0400)]
Clean-up: common: fix -Wshadow error in lttng_daemonize
There are two ret variables in the function, not of the same type. Fix
it by moving one to a narrower scope.
Change-Id: If09dc67e95f3d01f9796555fa005638bcb172279
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:40:13 +0000 (14:40 -0400)]
Clean-up: consumer: prepend `the_` to global variable
This avoids name clashes between the global variable and local variables
/ function parameters. This is a step towards enabling -Wshadow.
This also helps readability, in my opinion, as it helps quickly spot
that some code is using a global variable.
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I20eb47b91e3500bf1aebee17fdb995940cae7a33
Simon Marchi [Wed, 31 Mar 2021 18:39:24 +0000 (14:39 -0400)]
Clean-up: consumer: fix -Wshadow error in lttng_consumer_trace_chunk_exists
We can safely re-use the ret variable.
Change-Id: Icac6922acac8ca590c8f6b37df3fa07c90fd94dd
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:38:26 +0000 (14:38 -0400)]
Clean-up: consumer: fix -Wshadow error in lttng_consumer_rotate_channel
We can safely re-use the chunk_status declared in the broader scope.
Change-Id: I0dc501168b1c35522a2ce4805d4514854162cefc
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 18:37:15 +0000 (14:37 -0400)]
Clean-up: config: fix -Wshadow error in config_load_session
Fix:
CC libconfig_la-session-config.lo
/home/simark/src/lttng-tools/src/common/config/session-config.c: In function ‘config_load_session’:
/home/simark/src/lttng-tools/src/common/config/session-config.c:4020:9: error: declaration of ‘path’ shadows a parameter [-Werror=shadow]
4020 | char path[PATH_MAX];
| ^~~~
/home/simark/src/lttng-tools/src/common/config/session-config.c:3999:37: note: shadowed declaration is here
3999 | int config_load_session(const char *path, const char *session_name,
| ~~~~~~~~~~~~^~~~
Change-Id: Id7d5f6fcf0e88f43dee420dfb174d9bf01aa8fbd
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 16:08:41 +0000 (12:08 -0400)]
Clean-up: ust-consumer: fix -Wshadow issues in lttng_ustconsumer_recv_cmd
Same rationale and idea as previous patch, but for UST.
To differentiante the top-level `channel` variable, which owns a
channel, and the non-owning `channel` variables used when looking up
existing channels, the latter are renamed `found_channel`.
Change-Id: I6bf94be8d4368b5716756f4e12bae17c3e0d7cee
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 15:10:30 +0000 (11:10 -0400)]
Clean-up: kernel-consumer: fix -Wshadow issues in lttng_kconsumer_recv_cmd
There is a "ret" variable declared at the top and used throughout, but
there are also some other "ret" variables declared in narrower scopes.
Most of the time, the return value is not communicated from the "case"s
to the return value of the function. On failure, they goto an error
label which overrides ret to -1. There is one exception in "case
LTTNG_CONSUMER_ADD_STREAM", which goes directly to label end, not sure
if that's on purpose or not.
So what I did is: declare variables in narrower scopes when possible,
name them according to their intended use (e.g. ret_send, ret_recv). In
the end, there is no variable simply named "ret". The variable holding
the function's return value is "ret_func".
Change-Id: Ic02067c23757bd98d4b2d3d0f2b77384b4b0d743
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:48:13 +0000 (10:48 -0400)]
Clean-up: kernel-ctl: rename local variables in LTTNG_IOCTL_{,NO_}CHECK
Same rationale as previous patch.
Change-Id: Ie1782adad99616b3705c015360fa31d7e9273718
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:46:20 +0000 (10:46 -0400)]
Clean-up: common: rename local variables in PERROR
If PERROR is used in a function where a `buf` or `tmp` variable already
exists, we get this when enabling -Wshadow:
CC poll.lo
In file included from /home/simark/src/lttng-tools/src/common/compat/poll.c:15:
/home/simark/src/lttng-tools/src/common/compat/poll.c: In function ‘compat_epoll_set_max_size’:
/home/simark/src/lttng-tools/src/common/error.h:247:9: error: declaration of ‘buf’ shadows a previous local [-Werror=shadow]
247 | char *buf; \
| ^~~
/home/simark/src/lttng-tools/src/common/compat/poll.c:335:3: note: in expansion of macro ‘PERROR’
335 | PERROR("read set max size");
| ^~~~~~
/home/simark/src/lttng-tools/src/common/compat/poll.c:313:7: note: shadowed declaration is here
313 | char buf[64];
| ^~~
Avoid that by giving PERROR's variables names that are unlikely to be
used elsewhere.
Change-Id: I167491fd3b232e6cfd86d13c0003ad4facb40c58
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:45:01 +0000 (10:45 -0400)]
Clean-up: filter: fix variable shadowing in visit_node_load_expression
It seems safe to just re-use the `ret` variable at the function-level
scope.
Change-Id: I7826edf2b8bb41f9c202af75a8b5e7dff9cbbcc8
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:43:46 +0000 (10:43 -0400)]
Clean-up: filter: rename variable in filter-grammar-test.c
... to avoid variable shadowing.
Change-Id: If5781b7e2f41ab5b1df361b9b247714b17751a32
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:40:44 +0000 (10:40 -0400)]
Clean-up: sessiond: remove some declarations from lttng-sessiond.h
As far as I can see, they aren't used anymore, there is no corresponding
definition.
Change-Id: Ie72e643e04da033bd590525de617201303f6dd23
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Simon Marchi [Wed, 31 Mar 2021 14:36:33 +0000 (10:36 -0400)]
Clean-up: sessiond: prepend `the_` to global variable names
This avoids name clashes between global variables and local variables or
function parameters (notification_thread_handle, for example). This is a
step towards enabling -Wshadow.
This also helps readability, in my opinion, as it helps quickly spot
that some code is using a global variable.
Change-Id: Ib0e35ad7efcc54fa88e1900cab3388b98a06b8d9
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
This page took 0.078022 seconds and 5 git commands to generate.