Add type-checked versions of allocation and deallocations functions
authorSimon Marchi <simon.marchi@efficios.com>
Wed, 17 Nov 2021 02:36:17 +0000 (21:36 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 31 Mar 2022 02:59:33 +0000 (22:59 -0400)
commit64803277bbdbe0a943360d918298a48157d9da55
tree4f1e383db290172ba923cacc2f695b8c06be5a1c
parent60f1b42d6280b6bd386abb726dca4fd3b31d8491
Add type-checked versions of allocation and deallocations functions

A common mistake when porting things from C to C++ is to use malloc for
types that have a on-trivial constructor (and to not call the
constructor explicitly either). For example:

struct foo {
std::vector<int> field;
};

foo *f = (foo *) zmalloc(sizeof(*f));

This allocates a `foo` without calling the constructor, leaving it in an
invalid state. Same idea when free-ing with free something that has a
non-trivial destructor.

To avoid this, I suggest adding templated allocation functions that we
will use throughout, that verify if the given type is safe to malloc
(and generate a compilation failure if not). The existing code barely
needs changes. For example:

- (foo *) zmalloc(sizeof(*f))
+ zmalloc<foo>()

For simplicity I propose that as soon as a type is non-POD
(std::is_pod<T>::value is false), we prevent using malloc/free on it. It
would be ok in theory to allocate such a type with malloc and free with
free, but call the constructor (using placement-new) and destructor
explicitly, but I don't see why we would want to do that. It might also
be technically more correct to use a combination of
std::is_trivially_constructible and std::is_trivially_destructible
(std::is_pod being not fine-grained enough), but using std::is_pod just
keeps things simpler.

This patch introduces the following templated allocation functions:

1. zmalloc<T>()
2. zmalloc<T>(size)
3. malloc<T>()
4. malloc<T>(size)
5. calloc<T>(nmemb)

1. Allocate one T, zero-initialized
2. Allocate a buffer of size `size`, zero-initialized, this is used when
   the caller calculates the size to allocate, like when using flexible
   array members
3. Same as 1, but without the zero-initialization
4. Same as 2, but without the zero-initialization
5. Allocate an array of `nmemb` elements of type T, zero-initialized

For the de-allocation side, add templated `free` function declaration
that uses std::enable_if (SFINAE) to declare a deleted prototype if the
type T isn't safe to free (causing a compilation error).

There are a lot of places where we pass pointers to void to free. These
can't be checked, as we don't know what type of object the pointer
really points to. We could forbid that and fix all callers to pass a
typed pointer, but that seems a bit too much to chew for the moment. So
for now, simply accept that freeing pointers to void won't be checked.
It's a best effort.

As an example, if I add an explicit constructor to type ctf_trace (in
src/bin/lttng-relayd/ctf-trace.h), I get the following errors with
clang. For the allocation:

/home/simark/src/lttng-tools/src/common/macros.h:57:2: error: static_assert failed due to requirement 'std::is_pod<ctf_trace>::value' "type is POD"
static_assert (std::is_pod<T>::value, "type is POD");
^              ~~~~~~~~~~~~~~~~~~~~~
/home/simark/src/lttng-tools/src/bin/lttng-relayd/ctf-trace.cpp:84:10: note: in instantiation of function template specialization 'zmalloc<ctf_trace>' requested here
trace = zmalloc<ctf_trace>();
^

For the de-allocation:

/home/simark/src/lttng-tools/src/bin/lttng-relayd/ctf-trace.cpp:29:2: error: call to deleted function 'free'
free(trace);
^~~~
/home/simark/src/lttng-tools/src/common/macros.h:125:6: note: candidate function [with T = ctf_trace, $1 = void] has been explicitly deleted
void free(T *p) = delete;
     ^
/usr/include/stdlib.h:565:13: note: candidate function
extern void free (void *__ptr) __THROW;
    ^

Change-Id: I246a9113d08fa36b81a49137f4e80a5e808de913
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
135 files changed:
src/bin/lttng-crash/lttng-crash.cpp
src/bin/lttng-relayd/backward-compatibility-group-by.cpp
src/bin/lttng-relayd/connection.cpp
src/bin/lttng-relayd/ctf-trace.cpp
src/bin/lttng-relayd/index.cpp
src/bin/lttng-relayd/live.cpp
src/bin/lttng-relayd/session.cpp
src/bin/lttng-relayd/sessiond-trace-chunks.cpp
src/bin/lttng-relayd/stream.cpp
src/bin/lttng-relayd/tracefile-array.cpp
src/bin/lttng-relayd/viewer-session.cpp
src/bin/lttng-relayd/viewer-stream.cpp
src/bin/lttng-sessiond/action-executor.cpp
src/bin/lttng-sessiond/agent-thread.cpp
src/bin/lttng-sessiond/agent.cpp
src/bin/lttng-sessiond/buffer-registry.cpp
src/bin/lttng-sessiond/channel.cpp
src/bin/lttng-sessiond/clear.cpp
src/bin/lttng-sessiond/client.cpp
src/bin/lttng-sessiond/cmd.cpp
src/bin/lttng-sessiond/consumer.cpp
src/bin/lttng-sessiond/dispatch.cpp
src/bin/lttng-sessiond/event-notifier-error-accounting.cpp
src/bin/lttng-sessiond/health.cpp
src/bin/lttng-sessiond/kernel.cpp
src/bin/lttng-sessiond/lttng-syscall.cpp
src/bin/lttng-sessiond/manage-apps.cpp
src/bin/lttng-sessiond/manage-consumer.cpp
src/bin/lttng-sessiond/manage-kernel.cpp
src/bin/lttng-sessiond/modprobe.cpp
src/bin/lttng-sessiond/notification-thread-commands.cpp
src/bin/lttng-sessiond/notification-thread-events.cpp
src/bin/lttng-sessiond/notification-thread.cpp
src/bin/lttng-sessiond/notify-apps.cpp
src/bin/lttng-sessiond/register.cpp
src/bin/lttng-sessiond/rotation-thread.cpp
src/bin/lttng-sessiond/save.cpp
src/bin/lttng-sessiond/session.cpp
src/bin/lttng-sessiond/snapshot.cpp
src/bin/lttng-sessiond/thread.cpp
src/bin/lttng-sessiond/trace-kernel.cpp
src/bin/lttng-sessiond/trace-ust.cpp
src/bin/lttng-sessiond/tracker.cpp
src/bin/lttng-sessiond/ust-app.cpp
src/bin/lttng-sessiond/ust-registry.cpp
src/bin/lttng/commands/add_context.cpp
src/bin/lttng/commands/enable_events.cpp
src/bin/lttng/commands/list.cpp
src/bin/lttng/conf.cpp
src/bin/lttng/uprobe.cpp
src/common/actions/list.cpp
src/common/actions/notify.cpp
src/common/actions/path.cpp
src/common/actions/rate-policy.cpp
src/common/actions/rotate-session.cpp
src/common/actions/snapshot-session.cpp
src/common/actions/start-session.cpp
src/common/actions/stop-session.cpp
src/common/argpar-utils/argpar-utils.hpp
src/common/bytecode/bytecode.cpp
src/common/channel.cpp
src/common/compat/directory-handle.cpp
src/common/compat/poll.cpp
src/common/compat/string.hpp
src/common/conditions/buffer-usage.cpp
src/common/conditions/event-rule-matches.cpp
src/common/conditions/session-consumed-size.cpp
src/common/conditions/session-rotation.cpp
src/common/config/session-config.cpp
src/common/consumer/consumer-metadata-cache.cpp
src/common/consumer/consumer-stream.cpp
src/common/consumer/consumer.cpp
src/common/consumer/metadata-bucket.cpp
src/common/context.cpp
src/common/error-query.cpp
src/common/event-expr/event-expr.cpp
src/common/event-field-value.cpp
src/common/event-rule/jul-logging.cpp
src/common/event-rule/kernel-kprobe.cpp
src/common/event-rule/kernel-syscall.cpp
src/common/event-rule/kernel-tracepoint.cpp
src/common/event-rule/kernel-uprobe.cpp
src/common/event-rule/log4j-logging.cpp
src/common/event-rule/python-logging.cpp
src/common/event-rule/user-tracepoint.cpp
src/common/event.cpp
src/common/fd-handle.cpp
src/common/fd-tracker/fd-tracker.cpp
src/common/fd-tracker/inode.cpp
src/common/filter.cpp
src/common/filter/filter-parser.ypp
src/common/filter/filter-visitor-generate-ir.cpp
src/common/hashtable/hashtable.cpp
src/common/health/health.cpp
src/common/index-allocator.cpp
src/common/index/index.cpp
src/common/ini-config/ini-config.cpp
src/common/ini-config/ini.cpp
src/common/kernel-ctl/kernel-ctl.cpp
src/common/kernel-probe.cpp
src/common/location.cpp
src/common/log-level-rule.cpp
src/common/lttng-elf.cpp
src/common/macros.hpp
src/common/mi-lttng.cpp
src/common/notification.cpp
src/common/path.cpp
src/common/pipe.cpp
src/common/relayd/relayd.cpp
src/common/runas.cpp
src/common/session-descriptor.cpp
src/common/sessiond-comm/sessiond-comm.cpp
src/common/spawn-viewer.cpp
src/common/string-utils/string-utils.cpp
src/common/trace-chunk.cpp
src/common/tracker.cpp
src/common/trigger.cpp
src/common/uri.cpp
src/common/userspace-probe.cpp
src/common/ust-consumer/ust-consumer.cpp
src/common/utils.cpp
src/lib/lttng-ctl/channel.cpp
src/lib/lttng-ctl/clear.cpp
src/lib/lttng-ctl/destruction-handle.cpp
src/lib/lttng-ctl/event.cpp
src/lib/lttng-ctl/load.cpp
src/lib/lttng-ctl/lttng-ctl-health.cpp
src/lib/lttng-ctl/lttng-ctl.cpp
src/lib/lttng-ctl/rotate.cpp
src/lib/lttng-ctl/save.cpp
src/lib/lttng-ctl/snapshot.cpp
src/lib/lttng-ctl/tracker.cpp
tests/regression/tools/live/live_test.cpp
tests/unit/test_ust_data.cpp
tests/unit/test_utils_expand_path.cpp
This page took 0.039965 seconds and 4 git commands to generate.