Skip to content

Commit ff24295

Browse files
committed
helper: refactor ffi resolution; wait for sidecar in telemetry
1 parent 41b2489 commit ff24295

File tree

12 files changed

+271
-191
lines changed

12 files changed

+271
-191
lines changed

appsec/.clang-tidy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
# readability-function-cognitive-complexity temporarily disabled until clang-tidy is fixed
33
# right now emalloc causes it to misbehave
4-
Checks: '*,-bugprone-reserved-identifier,-hicpp-signed-bitwise,-llvmlibc-restrict-system-libc-headers,-altera-unroll-loops,-hicpp-named-parameter,-cert-dcl37-c,-cert-dcl51-cpp,-read,-cppcoreguidelines-init-variables,-cppcoreguidelines-avoid-non-const-global-variables,-altera-id-dependent-backward-branch,-performance-no-int-to-ptr,-altera-struct-pack-align,-google-readability-casting,-modernize-use-trailing-return-type,-llvmlibc-implementation-in-namespace,-llvmlibc-callee-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-fuchsia-default-arguments-declarations,-fuchsia-overloaded-operator,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-cppcoreguidelines-non-private-member-variables-in-classes,-misc-non-private-member-variables-in-classes,-google-readability-todo,-llvm-header-guard,-readability-function-cognitive-complexity,-readability-identifier-length,-modernize-macro-to-enum,-misc-include-cleaner,-bugprone-empty-catch,-cppcoreguidelines-avoid-do-while,-hicpp-no-array-decay,-llvmlibc-*'
4+
Checks: '*,-bugprone-reserved-identifier,-hicpp-signed-bitwise,-llvmlibc-restrict-system-libc-headers,-altera-unroll-loops,-hicpp-named-parameter,-cert-dcl37-c,-cert-dcl51-cpp,-read,-cppcoreguidelines-init-variables,-cppcoreguidelines-avoid-non-const-global-variables,-altera-id-dependent-backward-branch,-performance-no-int-to-ptr,-altera-struct-pack-align,-google-readability-casting,-modernize-use-trailing-return-type,-llvmlibc-implementation-in-namespace,-llvmlibc-callee-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-fuchsia-default-arguments-declarations,-fuchsia-overloaded-operator,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-cppcoreguidelines-non-private-member-variables-in-classes,-misc-non-private-member-variables-in-classes,-google-readability-todo,-llvm-header-guard,-readability-function-cognitive-complexity,-readability-identifier-length,-modernize-macro-to-enum,-misc-include-cleaner,-bugprone-empty-catch,-cppcoreguidelines-avoid-do-while,-hicpp-no-array-decay,-hicpp-avoid-c-arrays,-modernize-avoid-c-arrays,-hicpp-no-reinterpret-cast,-llvmlibc-*'
55
WarningsAsErrors: '*'
66
HeaderFilterRegex: ''
77
CheckOptions:

appsec/src/helper/ffi.hpp

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
#include <array>
2+
#include <atomic>
3+
#include <concepts>
4+
#include <cstddef>
5+
#include <functional>
6+
#include <stdexcept>
7+
#include <string>
8+
#include <type_traits>
9+
#include <utility>
10+
11+
extern "C" {
12+
#include <dlfcn.h>
13+
// push -Wno-nested-anon-types and -Wno-gnu-anonymous-struct on clang
14+
#if defined(__clang__)
15+
# pragma clang diagnostic push
16+
# pragma clang diagnostic ignored "-Wnested-anon-types"
17+
# pragma clang diagnostic ignored "-Wgnu-anonymous-struct"
18+
#endif
19+
#include <sidecar.h>
20+
#if defined(__clang__)
21+
# pragma clang diagnostic pop
22+
#endif
23+
}
24+
25+
#define SIDECAR_FFI_SYMBOL(symbol_name) \
26+
namespace dds::ffi { \
27+
constinit inline ::dds::ffi::sidecar_function<decltype((symbol_name)), \
28+
::dds::ffi::fixed_string{#symbol_name}> \
29+
symbol_name = {} /* NOLINT(misc-use-internal-linkage, \
30+
cert-err58-cpp) */ \
31+
; \
32+
} // namespace ::dds::ffi
33+
34+
namespace dds::ffi {
35+
36+
template <std::size_t N> struct fixed_string {
37+
std::array<char, N> value{};
38+
39+
// NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays)
40+
explicit consteval fixed_string(const char (&str)[N])
41+
{
42+
for (std::size_t i = 0; i < N; i++) { value.at(i) = str[i]; }
43+
}
44+
45+
[[nodiscard]] constexpr const char *c_str() const { return value.data(); }
46+
};
47+
48+
template <typename Func, auto SymbolName>
49+
requires std::is_function_v<std::remove_reference_t<Func>>
50+
class sidecar_function {
51+
public:
52+
using function_type = std::remove_reference_t<Func>;
53+
54+
constexpr sidecar_function() noexcept = default;
55+
56+
template <typename... Args>
57+
requires std::invocable<std::remove_reference_t<Func> *, Args...>
58+
decltype(auto) operator()(Args &&...args) const
59+
{
60+
resolve();
61+
return std::invoke(get_fn(), std::forward<Args>(args)...);
62+
}
63+
64+
[[nodiscard]] function_type *get_fn() const { return resolve(); }
65+
66+
private:
67+
static function_type *failed_sentinel() noexcept
68+
{
69+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
70+
return reinterpret_cast<function_type *>(
71+
static_cast<std::uintptr_t>(-1));
72+
}
73+
74+
mutable std::atomic<function_type *> resolved_fn_{nullptr};
75+
76+
function_type *resolve() const
77+
{
78+
function_type *fn = resolved_fn_.load(std::memory_order_relaxed);
79+
if (fn == failed_sentinel()) {
80+
throw std::runtime_error{
81+
std::string{"Failed to resolve symbol: "} + SymbolName.c_str()};
82+
}
83+
if (fn != nullptr) {
84+
return fn;
85+
}
86+
87+
void *found = dlsym(RTLD_DEFAULT, SymbolName.c_str());
88+
if (found == nullptr) {
89+
fn = failed_sentinel();
90+
} else {
91+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
92+
fn = reinterpret_cast<function_type *>(found);
93+
}
94+
95+
(void)resolved_fn_.store(fn, std::memory_order_relaxed);
96+
97+
return fn;
98+
}
99+
};
100+
101+
} // namespace dds::ffi
102+
103+
namespace dds {
104+
inline std::string_view to_sv(ddog_CharSlice &slice) noexcept
105+
{
106+
return std::string_view{slice.ptr, static_cast<std::size_t>(slice.len)};
107+
}
108+
} // namespace dds

appsec/src/helper/main.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,6 @@ int appsec_helper_main_impl()
187187
return 1;
188188
}
189189

190-
dds::remote_config::resolve_symbols();
191-
dds::runner::resolve_symbols();
192-
dds::service::resolve_symbols();
193-
194190
auto runner = std::make_shared<dds::runner>(config, interrupted);
195191
SPDLOG_INFO("starting runner on new thread");
196192

appsec/src/helper/remote_config/client.cpp

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
//
44
// This product includes software developed at Datadog
55
// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.
6+
67
#include "client.hpp"
8+
#include "../ffi.hpp"
79
#include "product.hpp"
810
#include <set>
911
#include <spdlog/spdlog.h>
@@ -14,16 +16,11 @@ extern "C" {
1416
#include <dlfcn.h>
1517
}
1618

19+
SIDECAR_FFI_SYMBOL(ddog_remote_config_reader_for_path);
20+
SIDECAR_FFI_SYMBOL(ddog_remote_config_read);
21+
SIDECAR_FFI_SYMBOL(ddog_remote_config_reader_drop);
22+
1723
namespace {
18-
struct ddog_CharSlice { // NOLINT(readability-identifier-naming)
19-
const char *ptr;
20-
uintptr_t len;
21-
};
22-
ddog_RemoteConfigReader *(*ddog_remote_config_reader_for_path)(
23-
const char *path);
24-
bool (*ddog_remote_config_read)(
25-
ddog_RemoteConfigReader *reader, ddog_CharSlice *data);
26-
void (*ddog_remote_config_reader_drop)(struct ddog_RemoteConfigReader *);
2724

2825
bool sets_are_indentical_for_subbed_products(
2926
const std::unordered_set<dds::remote_config::product> &products,
@@ -48,40 +45,12 @@ bool sets_are_indentical_for_subbed_products(
4845

4946
namespace dds::remote_config {
5047

51-
void resolve_symbols()
52-
{
53-
ddog_remote_config_reader_for_path =
54-
// NOLINTNEXTLINE
55-
reinterpret_cast<decltype(ddog_remote_config_reader_for_path)>(
56-
dlsym(RTLD_DEFAULT, "ddog_remote_config_reader_for_path"));
57-
if (ddog_remote_config_reader_for_path == nullptr) {
58-
throw std::runtime_error{
59-
"Failed to resolve ddog_remote_config_reader_for_path"};
60-
}
61-
62-
ddog_remote_config_read =
63-
// NOLINTNEXTLINE
64-
reinterpret_cast<decltype(ddog_remote_config_read)>(
65-
dlsym(RTLD_DEFAULT, "ddog_remote_config_read"));
66-
if (ddog_remote_config_read == nullptr) {
67-
throw std::runtime_error{"Failed to resolve ddog_remote_config_read"};
68-
}
69-
70-
ddog_remote_config_reader_drop =
71-
// NOLINTNEXTLINE
72-
reinterpret_cast<decltype(ddog_remote_config_reader_drop)>(
73-
dlsym(RTLD_DEFAULT, "ddog_remote_config_reader_drop"));
74-
if (ddog_remote_config_reader_drop == nullptr) {
75-
throw std::runtime_error{
76-
"Failed to resolve ddog_remote_config_reader_drop"};
77-
}
78-
}
79-
8048
client::client(remote_config::settings settings,
8149
std::vector<std::shared_ptr<listener_base>> listeners,
8250
std::shared_ptr<telemetry::telemetry_submitter> msubmitter)
83-
: reader_{ddog_remote_config_reader_for_path(settings.shmem_path.c_str()),
84-
ddog_remote_config_reader_drop},
51+
: reader_{ffi::ddog_remote_config_reader_for_path(
52+
settings.shmem_path.c_str()),
53+
ffi::ddog_remote_config_reader_drop.get_fn()},
8554
settings_{std::move(settings)}, listeners_{std::move(listeners)},
8655
msubmitter_{std::move(msubmitter)}
8756
{
@@ -113,7 +82,7 @@ bool client::poll()
11382
SPDLOG_DEBUG("Polling remote config");
11483

11584
ddog_CharSlice slice{};
116-
const bool has_update = ddog_remote_config_read(reader_.get(), &slice);
85+
const bool has_update = ffi::ddog_remote_config_read(reader_.get(), &slice);
11786
if (!has_update) {
11887
SPDLOG_DEBUG("No update available for {}", settings_.shmem_path);
11988
return false;

appsec/src/helper/remote_config/client.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ struct ddog_RemoteConfigReader;
2121

2222
namespace dds::remote_config {
2323

24-
void resolve_symbols();
25-
2624
struct config_path {
2725
static config_path from_path(const std::string &path);
2826

appsec/src/helper/runner.cpp

Lines changed: 27 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,27 @@
66
#include "runner.hpp"
77

88
#include "client.hpp"
9+
#include "ffi.hpp"
10+
#include <chrono>
911
#include <csignal>
10-
#include <cstdio>
1112
#include <spdlog/spdlog.h>
1213
#include <stdexcept>
1314
#include <sys/stat.h>
15+
#include <thread>
1416

1517
extern "C" {
18+
#include <common.h>
1619
#include <dlfcn.h>
1720
}
1821

19-
namespace {
20-
struct ConfigInvariants;
21-
struct Arc_Target;
22-
2322
using in_proc_notify_fn = void (*)(
24-
const ConfigInvariants *invariants, const Arc_Target *target);
23+
const ddog_ConfigInvariants *invariants, const ddog_Arc_Target *target);
2524

26-
void (*ddog_set_rc_notify_fn)(in_proc_notify_fn notify_fn);
27-
char *(*ddog_remote_config_path)(const ConfigInvariants *, const Arc_Target *);
28-
void (*ddog_remote_config_path_free)(char *);
29-
} // namespace
25+
extern "C" void ddog_set_rc_notify_fn(in_proc_notify_fn notify_fn);
26+
27+
SIDECAR_FFI_SYMBOL(ddog_set_rc_notify_fn);
28+
SIDECAR_FFI_SYMBOL(ddog_remote_config_path);
29+
SIDECAR_FFI_SYMBOL(ddog_remote_config_path_free);
3030

3131
namespace dds {
3232

@@ -109,31 +109,25 @@ void runner::register_for_rc_notifications()
109109
SPDLOG_INFO("Register RC update callback");
110110
std::atomic_store(&runner::RUNNER_FOR_NOTIFICATIONS, shared_from_this());
111111

112-
ddog_set_rc_notify_fn(
113-
[](const ConfigInvariants *invariants, const Arc_Target *target) {
114-
char *path = ddog_remote_config_path(invariants, target);
115-
116-
if (path == nullptr) {
117-
// NOLINTNEXTLINE(bugprone-lambda-function-name)
118-
SPDLOG_ERROR("Failed to get remote config path");
119-
return;
120-
}
121-
122-
const std::shared_ptr<runner> runner =
123-
std::atomic_load(&RUNNER_FOR_NOTIFICATIONS);
124-
if (!runner) {
125-
// NOLINTNEXTLINE(bugprone-lambda-function-name)
126-
SPDLOG_WARN("No runner to notify of remote config updates");
127-
ddog_remote_config_path_free(path);
128-
return;
129-
}
112+
ffi::ddog_set_rc_notify_fn([](const ddog_ConfigInvariants *invariants,
113+
const ddog_Arc_Target *target) {
114+
char *path = ffi::ddog_remote_config_path(invariants, target);
130115

116+
const std::shared_ptr<runner> runner =
117+
std::atomic_load(&RUNNER_FOR_NOTIFICATIONS);
118+
if (!runner) {
131119
// NOLINTNEXTLINE(bugprone-lambda-function-name)
132-
SPDLOG_INFO("Remote config updated notification for {}", path);
133-
// TODO: move the updates to a separate thread
134-
runner->service_manager_->notify_of_rc_updates(path);
135-
ddog_remote_config_path_free(path);
136-
});
120+
SPDLOG_WARN("No runner to notify of remote config updates");
121+
ffi::ddog_remote_config_path_free(path);
122+
return;
123+
}
124+
125+
// NOLINTNEXTLINE(bugprone-lambda-function-name)
126+
SPDLOG_INFO("Remote config updated notification for {}", path);
127+
// TODO: move the updates to a separate thread
128+
runner->service_manager_->notify_of_rc_updates(path);
129+
ffi::ddog_remote_config_path_free(path);
130+
});
137131
}
138132

139133
void runner::unregister_for_rc_notifications()
@@ -189,31 +183,4 @@ void runner::run()
189183
SPDLOG_INFO("Pool stopped");
190184
}
191185

192-
void runner::resolve_symbols()
193-
{
194-
// NOLINTNEXTLINE
195-
ddog_set_rc_notify_fn = reinterpret_cast<decltype(ddog_set_rc_notify_fn)>(
196-
dlsym(RTLD_DEFAULT, "ddog_set_rc_notify_fn"));
197-
if (ddog_set_rc_notify_fn == nullptr) {
198-
throw std::runtime_error{"Failed to resolve ddog_set_rc_notify_fn"};
199-
}
200-
201-
ddog_remote_config_path =
202-
// NOLINTNEXTLINE
203-
reinterpret_cast<decltype(ddog_remote_config_path)>(
204-
dlsym(RTLD_DEFAULT, "ddog_remote_config_path"));
205-
if (ddog_remote_config_path == nullptr) {
206-
throw std::runtime_error{"Failed to resolve ddog_remote_config_path"};
207-
}
208-
209-
ddog_remote_config_path_free =
210-
// NOLINTNEXTLINE
211-
reinterpret_cast<decltype(ddog_remote_config_path_free)>(
212-
dlsym(RTLD_DEFAULT, "ddog_remote_config_path_free"));
213-
if (ddog_remote_config_path_free == nullptr) {
214-
throw std::runtime_error{
215-
"Failed to resolve ddog_remote_config_path_free"};
216-
}
217-
}
218-
219186
} // namespace dds

appsec/src/helper/runner.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ class runner : public std::enable_shared_from_this<runner> {
2525
runner &operator=(runner &&) = delete;
2626
~runner() = default;
2727

28-
static void resolve_symbols();
29-
3028
void run() noexcept(false);
3129

3230
void register_for_rc_notifications();

0 commit comments

Comments
 (0)