Fix: fd tracker: provide async-signal-safety for close wrapper
close(3) is part of the async-signal-safe functions. Therefore, it is
expected that the close wrapper provided by liblttng-ust-fd-tracker
behaves in a async-signal-safe way.
Use a similar strategy as ust_lock() does: disable signals when taking
and releasing the lock, and keep track of nesting with a TLS variable.
This ensures signals are restored to their original state when close(3)
ends up being invoked.
If fork() is performed while other threads are holding the fd tracker
lock, it will stay in locked state in the child process and eventually
cause a deadlock.
One way to solve this is to hold the fd tracker lock across fork(), in
the same way we do for the ust_lock. This ensures no other threads are
holding that lock in the parent, and therefore provides a consistent
lock state in the child.
Fix: wait for initial statedump before proceeding to the main program
In the case of short lived applications, the application may exit before
the initial statedump has completed.
Higher-level trace analysis features such as translating addresses to
symbols rely on statedump. That information is required for those
analyses to work on such short-lived applications.
Force the statedump to occur before handing the control to the
application.
Commit 973eac638e4fd introduces an uninitialised value that may prevent
shared memory from being allocated. The compiler didn't give any warning
because the pointer to the value is sent to a function that don't do anything
with it. We simply pass NULL to that function.
-Waddress-of-packed-member, enabled by default, warns about an
unaligned pointer value from the address of a packed member of a
struct or union.
The warning is triggered in some place in LTTng-UST in cases where we
pass a pointer to get a result. Rather than passing the pointer directly
from the struct member, we get the result into a local storage, then
write into in the struct.
Michael Jeanson [Mon, 3 Jun 2019 19:25:32 +0000 (15:25 -0400)]
Fix: namespace our gettid wrapper
Since glibc 2.30, a gettid wrapper was added that conflicts with our
static declaration. Namespace our wrapper so there is no conflict,
we'll add support for the glibc provided wrapper in a further commit.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fix: bitfield: shift undefined/implementation defined behaviors
bitfield.h uses the left shift operator with a left operand which
may be negative. The C99 standard states that shifting a negative
value is undefined.
When building with -Wshift-negative-value, we get this gcc warning:
In file included from /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:44:0,
from /home/smarchi/src/babeltrace/ctfser/ctfser.c:42:
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h: In function ‘bt_ctfser_write_unsigned_int’:
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:116:24: error: left shift of negative value [-Werror=shift-negative-value]
mask = ~((~(type) 0) << (__start % ts)); \
^
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:222:2: note: in expansion of macro ‘_bt_bitfield_write_le’
_bt_bitfield_write_le(ptr, type, _start, _length, _v)
^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:418:3: note: in expansion of macro ‘bt_bitfield_write_le’
bt_bitfield_write_le(mmap_align_addr(ctfser->base_mma) +
^~~~~~~~~~~~~~~~~~~~
This boils down to the fact that the expression ~((uint8_t)0) has type
"signed int", which is used as an operand of the left shift. This is due
to the integer promotion rules of C99 (6.3.3.1):
If an int can represent all values of the original type, the value is
converted to an int; otherwise, it is converted to an unsigned int.
These are called the integer promotions. All other types are unchanged
by the integer promotions.
We also need to cast the result explicitly into the left hand
side type to deal with:
warning: large integer implicitly truncated to unsigned type [-Woverflow]
The C99 standard states that a right shift has implementation-defined
behavior when shifting a signed negative value. Add a preprocessor check
that the compiler provides the expected behavior, else provide an
alternative implementation which guarantees the intended behavior.
A preprocessor check is also added to ensure that the compiler
representation for signed values is two's complement, which is expected
by this header.
Document that this header strictly respects the C99 standard, with
the exception of its use of __typeof__.
Fix: alignment of ring buffer shm space reservation
commit a9ff648cc "Implement file-backed ring buffer" changes the order
of backend fields with respect to the frontend per-subbuffer
commit_counters_hot and commit_counters_cold arrays, but does not change
that order when calculating the space needed in the initial pass.
This discrepancy can be an issue for field alignment calculation.
Let's analyse the situation. If the incorrect position of alignment
calculation leads to a larger space reserved than the actual
allocations, no ill effect will be perceived by the user. However,
if space calculation is less than the allocations, it will cause the
ring buffer (and thus channel) creation to fail.
The fields that are incorrectly misplaced in size calculation (in
officially released versions) are:
* struct commit_counters_hot is aligned on CAA_CACHE_LINE_SIZE,
* struct commit_counters_cold is aligned on CAA_CACHE_LINE_SIZE,
Those are placed after (should be before) the backend fields:
* struct lttng_ust_lib_ring_buffer_backend_pages_shmp aligned on the
natural alignment of ssize_t,
* alignment on page size,
* struct lttng_ust_lib_ring_buffer_backend_pages, aligned on the natural
alignment of ssize_t,
* struct lttng_ust_lib_ring_buffer_backend_subbuffer, aligned on natural
alignment of unsigned long,
* struct lttng_ust_lib_ring_buffer_backend_counts, aligned on natural
alignment of uint64_t.
The largest alignment is the alignment on page size in the backend
fields. If we have a channel configured within specific ranges of
sub-buffer count, we should reach commit counters array dimensions
which cause the page size alignment to be lower than it should be in
the space calculation, and therefore leads to a problematic scenario
where space allocation will fail, thus leading to channel creation
failures.
Fix: don't access packet header for stream_id and stream_instance_id getters
The stream ID and stream instance ID are invariant for a stream, so
there is no point reading them from the packet header currently owned by
the consumer (between get/put subbuf).
Actually, the consumer try to access the stream_id from the live timer
when sending a live beacon without getting the reader subbuffer first.
Doing so is racy against producers. In typical live scenarios
(non-overwrite channels), the producers will always write the same
stream id and stream instance id values at the same header offsets,
which will "work", except for the initial state of an empty buffer:
the value "0" will be returned (erroneously).
For the less frequently used scenario of a live session with "overwrite"
channels, this is handled by issuing a CHAN_WARN_ON, which disables
tracing for the channel, and prints warning to the consumerd console
when running consumerd with LTTNG_UST_DEBUG=1.
In the case where a ring buffer does not have any data ready, it makes
no sense to try to get a subbuffer for reading anyway, so the approach
was broken.
So return the stream id and stream instance id from the internal
data structures rather than reading it from the ring buffer.
Michael Jeanson [Wed, 20 Mar 2019 15:07:35 +0000 (11:07 -0400)]
compat: work around broken _SC_NPROCESSORS_CONF on MUSL libc
On MUSL libc the _SC_NPROCESSORS_CONF sysconf will report the number of
CPUs allocated to the task based on the affinity mask instead of the
total number of CPUs configured on the system.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Move wait_shm_mmap initialization to library constructor
Prevent us from deadlocking ourself if some glibc implementation
decide to hold the dl_load_* locks on fork operation.
This happens on Yocto Rocko and up when performing python tracing (import
lttngust). Why Yocto decided to patch glibc this way is a mystery
(ongoing effort) [1][2][3].
Anyhow, we can prevent this by moving the initialization of the
wait_shm_mmap to the library constructor since the dl_load_* locks are
nestable mutex.
Nothing in the git log for the wait_shm_mmap indicate a specific reason
to why it was done inside the listener thread. Doing it inside
wait_for_sessiond can help in some corner cases were /dev/shm
(or the shm path) files are unlinked. This is not much of an advantage.
Omair Majid [Thu, 11 Oct 2018 18:28:49 +0000 (14:28 -0400)]
Fix: address shellcheck warnings/errors in example scripts
ShellCheck points out a number of warnings in the example scripts. In
particular, a number of normal and special shell variables are not
quoted correctly.
Fix: check for event class/instance prototype mismatch
The TP_ARGS() for an event instance belonging to an event class
must have compatible types with the event class TP_ARGS().
Failure to follow this rule leads to a prototype mismatch between the
tracepoint call site and the probe function. A common effect perceived
is that events with prototype mismatch between call site and probe
function are never traced.
Fix this by enforcing a compile-time check of the event instance and
class prototypes, similarly to what is done in LTTng modules.
Fix: race between statedump and library destructor
The locking scheme for ust_lock() returns a teardown state (variable
lttng_ust_comm_should_quit) which is set by library destructor with lock
held.
It requires that when ust listener threads use this lock to protect
against concurrent accesses to a data structure, in addition to take
the lock, they need to check the return value of ust_lock() and
skip their critical section entirely if the return value indicates
that teardown is ongoing.
Iteration over all loaded libraries by lttng_ust_dl_update() starts by
iter_begin which grabs the lock, and sets data->cancel state
appropriately if teardown is ongoing. Then extract_bin_info_events()
uses the data->cancel state to skip over use of the protected structures
as needed, but iter_end() fails to take this data->cancel state into
account. Therefore, it can access data structures concurrently while
their teardown is ongoing which leads to crashes.
procname
Thread name, as set by exec(3) or prctl(2). It is recommended
that programs set their thread name with prctl(2) before
hitting the first tracepoint for that thread.
We can rightfully expect that this applies to the first thread created
within a child process upon fork. Reset the procname cache in the child
on fork.
Michael Jeanson [Fri, 2 Mar 2018 22:36:26 +0000 (17:36 -0500)]
Fix: cache the result of getpid() internally
On Linux we called getpid() directly on each tracepoint and relied on
the glibc pid cache. However, in glibc 2.25, released on 2017-02-05, the
pid cache was removed which results in a getpid syscall on each event
when the vpid context is enabled.
Remove the Linux specific case and use our internal cache all the time.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Michael Jeanson [Fri, 2 Mar 2018 22:36:25 +0000 (17:36 -0500)]
Fix: reset cached vpid context on fork
We currently reset the cached vtid on fork but not the vpid. This is not
a problem on Linux because we don't cache the vpid internally but call
getpid() directly and rely on the glibc pid cache.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
08:51:35 lttng-ust-fd-tracker.c: In function 'dup_std_fd':
08:51:35 lttng-ust-fd-tracker.c:174:2: error: 'for' loop initial
declarations are only allowed in C99 mode
08:51:35 for (int i = 0; i < STDERR_FILENO + 1; i++) {
08:51:35 ^
08:51:35 lttng-ust-fd-tracker.c:174:2: note: use option -std=c99 or
-std=gnu99 to compile your code
08:51:35 lttng-ust-fd-tracker.c:195:11: error: redefinition of 'i'
08:51:35 for (int i = 0; i < fd_to_close_count; i++) {
08:51:35 ^
08:51:35 lttng-ust-fd-tracker.c:174:11: note: previous definition of 'i'
was here
08:51:35 for (int i = 0; i < STDERR_FILENO + 1; i++) {
08:51:35 ^
08:51:35 lttng-ust-fd-tracker.c:195:2: error: 'for' loop initial
declarations are only allowed in C99 mode
08:51:35 for (int i = 0; i < fd_to_close_count; i++) {
08:51:35 ^
08:51:35 Makefile:412: recipe for target 'lttng-ust-fd-tracker.lo'
failed
08:51:35 make[2]: *** [lttng-ust-fd-tracker.lo] Error 1
08:51:35 make[2]: *** Waiting for unfinished jobs....
Michael Jeanson [Tue, 21 Nov 2017 16:11:15 +0000 (11:11 -0500)]
Fix: specify SONAME in python-lttngust LoadLibrary
When loading the python agent library with ctypes in the python
bindings, specify the SONAME. This will make sure we load the proper
library in the event of a SONAME bump and the bindings will work without
having to install the "dev" package which in most distros contains the
non-versionned ".so".
Signed-off-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Jonathan Rajotte [Fri, 10 Nov 2017 16:06:41 +0000 (11:06 -0500)]
Fix: fd of an elf object must be registered to the fd tracker
The open call take place inside ust, it must be tracked to prevent external
closing.
The bug can be hit during tracing of an application for which the probe
provider is loaded using LD_PRELOAD in combination with the fd utility
shared object. The application is responsible for closing all possible fd.
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fix: sync buffer file metadata on buffer allocation
Synchronizing the file metadata on disk after zeroing the whole file (on
buffer allocation) will make the crash extraction feature (--shm-path
create option) more robust. It ensures the content of the file metadata
backing the buffers does not have to be updated while tracing into the
memory map. Therefore, the on-disk metadata will never be out of sync at
the point where a system crash occurs.
Michael Jeanson [Tue, 9 May 2017 18:25:01 +0000 (14:25 -0400)]
Fix: Don't override user variables within the build system
Instead use the appropriatly prefixed AM_* variables as to not interfere
when a user variable is passed to a make command. The proper use of flag
variables is documented at :
Fix: race between lttng-ust getenv() and application setenv()
The LTTng-UST listener threads invoke getenv(), which can cause issues
if the application issues setenv() concurrently. This is a legitimate
use by the application because it may have a single thread and not be
aware that it runs with liblttng-ust.
Fix this by keeping our own environment variable table for the variables
we care about. Initialize this table within the lttng-ust library
constructor, when we don't race with the application.
As this thread shows:
https://sourceware.org/bugzilla/show_bug.cgi?id=5069#c10
getenv() does _not_ appear to be thread-safe if an application uses
setenv() or putenv().
Philippe Proulx [Wed, 15 Mar 2017 00:48:18 +0000 (20:48 -0400)]
doc/man: add typical `$` and `#` prompts to command lines
It is more instinctive for the typical reader to immediately recognize
command lines when they start with the classic prompts.
On the online version of the man pages, those prompts are treated
specially to make them non-selectable. This makes it possible to copy
multiple command lines at once (without copying the prompts) and to
paste them to your shell.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
This Makefile was using Distutils' setup.py to install the Python agent
but was using the Autoconf's $pkgpythondir variable for the uninstall
process. The two folders can be different on some distributions which
made the uninstall attempting to delete a non-existant folder and
effectively not uninstalling.
We now run a phony installation of the bindings in a temporary directory
and use the tree structure of the install folder to infere the location
of the files on the system to delete them.
Also, we print a warning if the install directory is not included in the
PYTHONPATH variable.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fix: loglevel and model_emf_uri with g++ compiled probes
Fix the loglevel and model_emf_uri features for probe providers compiled
with g++. They were previously effectless because of C++ symbol name
mangling. The weakref was refering to the non-mangled symbol, but C++
emits a mangled symbol for the static variable.
Fix this by emitting an extern "C" symbol with hidden visibility on C++.
With a C compiled, this simply turns a static variable into a variable
with hidden visibility.
Fix: perform statedump before replying to sessiond
If a stop command immediately follows a start command, the consumer
daemon will stop event recording in the ring buffers shared memory
control structures before the sessiond sends further commands to the
application. Therefore, a stop-after-start may be performed concurrently
with the statedump, leading to have parts of the statedump being
missing. This case may always happen if an application exits during
statedump, but it is not expected to have incomplete statedump in the
stop-after-start use case.
The session daemon statedump regeneration tests expect that the
statedump is completed when the regeneration command returns. This also
requires that we perform the statedump in lttng-ust before replying to
the session daemon command.
This library overrides close() and closeall() libc functions, and uses
lttng_ust_safe_close_fd() to check whether the application can
interact with the file descriptor or if it should be left to lttng-ust.
This takes care of bugs caused by applications doing bulk close() or
closefrom() of file descriptors soon after forking.
Introduce a tracker for file descriptors used by lttng-ust. It exposes
a new API in an internal header lttng_ust_safe_close_fd(), which is
meant to be used by a LD_PRELOADed library overriding close() and
closefrom() (BSD).
This takes care of bugs caused by applications doing bulk close() or
closefrom() of file descriptors soon after forking.
We need to hold the ust_lock() to protect the fd tracker lock against
fork. Since the fd tracker is needed across connect() (which allocates a
file descriptor), we need to hold the ust_lock across connect().
Needed if we want to hold the ust_lock() while we connect to the session
daemon without blocking the application forever if the session daemon is
hung on SIGSTOP.
This only triggers if we launchs _many_ applications with a session
daemon SIGSTOP'd (e.g. 1000 in parallel), so we fill the socket queue,
and applications hang there until the session daemon is SIGCONT'd.
Fix: perform TLS fixup in all UST entry points from each thread
Each entry point into lttng-ust that end up taking the ust lock need to
perform a TLS fixup for each thread. Add a TLS fixup in both listener
threads, in fork and base address dump helper libs, and in app context
and tracepoint probe registration/unregistration functions, which can be
called from application threads.
Those ensure we don't take the libc dl lock within the ust lock when
performing the TLS lazy fixup.
Because all length parameters received for serializing data coming from
applications go through a callback, they are never constant, and it
hurts performance to perform a call to memcpy each time.
Performance: mark ring buffer do_copy callers always inline
The underlying copy operation is more efficient if the size is a
constant, which only happens if this function is inlined in the caller.
Otherwise, we end up calling memcpy for each field.
Force inlining for performance reasons for:
- lib_ring_buffer_write,
- lib_ring_buffer_do_strcpy,
- lib_ring_buffer_strcpy.
Note that in lttng-ust, the probe provider serialization functions need
to call the lttng_event_write() client callback, which will fallback to
the memcpy operation.
Inlining those functions helps for the event header code, which can
inline them.
Performance: cache the backend pages pointer in context
Getting the backend pages pointer requires walking through the ring
buffer backend tables through multiple shmp operations. Cache the
current value so it can be re-used for all backend write operations
writing fields for the same event.
projects / lttng-ust.git / log
