Skip to content

Commit 5f63098

Browse files
cataphractcataphract
committed
Fix flakiness in appsec test
1 parent 546fae6 commit 5f63098

File tree

5 files changed

+106
-47
lines changed

5 files changed

+106
-47
lines changed

‎appsec/src/extension/ddtrace.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ static void dd_trace_load_symbols(void)
7575

7676
#define ASSIGN_DLSYM(var, export) \
7777
do { \
78-
var = (typeof(var))(uintptr_t)dlsym(handle, export ""); \
79-
if (var == NULL && !testing) { \
78+
(var) = (typeof(var))(uintptr_t)dlsym(handle, export ""); \
79+
if ((var) == NULL && !testing) { \
8080
/* NOLINTNEXTLINE(concurrency-mt-unsafe) */ \
8181
mlog(dd_log_error, "Failed to load %s: %s", export, dlerror()); \
8282
} \

‎appsec/src/helper/engine.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ std::optional<engine::result> engine::context::publish(
124124
return std::nullopt;
125125
}
126126

127-
bool force_keep = limiter_.allow();
127+
const bool force_keep = limiter_.allow();
128128
dds::engine::result res{{}, std::move(event_.data), force_keep};
129129
// Currently the only action the extension can perform is block
130130
if (event_.actions.empty()) {

‎appsec/src/helper/engine_settings.cpp‎

Lines changed: 71 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -51,45 +51,82 @@ std::filesystem::path get_helper_path()
5151

5252
namespace dds {
5353

54-
const std::string &engine_settings::default_rules_file()
55-
{
56-
struct def_rules_file {
57-
def_rules_file()
58-
{
59-
std::filesystem::path base_path;
54+
namespace {
55+
struct def_rules_file {
56+
std::atomic<std::string *> stored_file{};
57+
58+
def_rules_file() = default;
59+
def_rules_file(const def_rules_file &) = delete;
60+
def_rules_file(def_rules_file &&) = delete;
61+
def_rules_file &operator=(const def_rules_file &) = delete;
62+
def_rules_file &operator=(def_rules_file &&) = delete;
63+
64+
const std::string &get()
65+
{
66+
auto *cur_val = stored_file.load();
67+
if (cur_val == nullptr) {
68+
auto *new_val = initialize().release();
69+
if (!stored_file.compare_exchange_strong(cur_val, new_val)) {
70+
delete new_val; // NOLINT
71+
// cur_val is now the currently stored value
72+
} else {
73+
cur_val = new_val;
74+
}
75+
}
76+
return *cur_val;
77+
}
78+
79+
static std::unique_ptr<std::string> initialize()
80+
{
81+
std::filesystem::path base_path;
82+
std::unique_ptr<std::string> file;
6083

84+
try {
85+
base_path = get_helper_path().parent_path();
86+
} catch (const std::exception &e) {
87+
file = std::make_unique<std::string>(
88+
"<error resolving helper library path: " +
89+
std::string(e.what()) + ">");
90+
91+
// try relative to the executable instead
6192
try {
62-
base_path = get_helper_path().parent_path();
63-
} catch (const std::exception &e) {
64-
file = "<error resolving helper library path: " +
65-
std::string(e.what()) + ">";
66-
67-
// try relative to the executable instead
68-
try {
69-
auto psp = std::filesystem::path{"/proc/self/exe"};
70-
if (std::filesystem::is_symlink(psp)) {
71-
psp = std::filesystem::read_symlink(psp);
72-
}
73-
base_path = psp.parent_path();
74-
} catch (const std::exception &e) {
75-
file =
76-
"<error resolving both library and executable path: " +
77-
std::string(e.what()) + ">";
78-
return;
93+
auto psp = std::filesystem::path{"/proc/self/exe"};
94+
if (std::filesystem::is_symlink(psp)) {
95+
psp = std::filesystem::read_symlink(psp);
7996
}
97+
base_path = psp.parent_path();
98+
} catch (const std::exception &e) {
99+
file = std::make_unique<std::string>(
100+
"<error resolving both library and executable path: " +
101+
std::string(e.what()) + ">");
102+
return file;
80103
}
104+
}
81105

82-
file = base_path / "../etc/recommended.json";
83-
if (!std::filesystem::exists(file)) {
84-
// This fallback file is set by a custom/old installer on
85-
// appsec repository
86-
file = base_path / "../etc/dd-appsec/recommended.json";
87-
}
106+
file = std::make_unique<std::string>(
107+
base_path / "../etc/recommended.json");
108+
if (!std::filesystem::exists(*file)) {
109+
// This fallback file is set by a custom/old installer on
110+
// appsec repository
111+
file = std::make_unique<std::string>(
112+
base_path / "../etc/dd-appsec/recommended.json");
88113
}
89-
std::string file;
90-
};
91114

92-
static def_rules_file const drf; // NOLINT
93-
return drf.file;
94-
}
115+
return file;
116+
}
117+
118+
~def_rules_file()
119+
{
120+
auto *val = stored_file.load();
121+
delete val; // NOLINT
122+
stored_file.store(nullptr);
123+
}
124+
};
125+
126+
// can't be a local static inside default_rules_file() because those
127+
// register their atexit() handlers lazily, which is too late for us.
128+
def_rules_file drf{}; // NOLINT
129+
} // namespace
130+
131+
const std::string &engine_settings::default_rules_file() { return drf.get(); }
95132
} // namespace dds

‎appsec/tests/fuzzer/CMakeLists.txt‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,21 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSIO
2121
target_compile_definitions(ddappsec_helper_fuzzer PUBLIC ZLIB_CONST=1)
2222
target_link_directories(ddappsec_helper_fuzzer PRIVATE ${LLVM_RUNTIME_DIR})
2323
target_link_libraries(ddappsec_helper_fuzzer
24-
PRIVATE libddwaf_objects pthread spdlog cpp-base64 msgpack_c RapidJSON::rapidjson Boost::system libclang_rt.fuzzer_no_main-${ARCHITECTURE}.a zlibstatic)
24+
PRIVATE libddwaf_objects pthread spdlog cpp-base64 msgpack_c RapidJSON::rapidjson Boost::system zlibstatic)
25+
26+
set(FUZZER_LIB_NAME "libclang_rt.fuzzer_no_main-${ARCHITECTURE}.a")
27+
find_library(FUZZER_LIB ${FUZZER_LIB_NAME} PATHS ${LLVM_RUNTIME_DIR})
28+
29+
if(NOT FUZZER_LIB)
30+
set(FUZZER_LIB_NAME_FALLBACK "libclang_rt.fuzzer_no_main.a")
31+
find_library(FUZZER_LIB ${FUZZER_LIB_NAME_FALLBACK} PATHS ${LLVM_RUNTIME_DIR})
32+
endif()
33+
34+
if(NOT FUZZER_LIB)
35+
message(FATAL_ERROR "Could not find fuzzer library (tried ${FUZZER_LIB_NAME} and ${FUZZER_LIB_NAME_FALLBACK}); extra path ${LLVM_RUNTIME_DIR}")
36+
endif()
37+
38+
target_link_libraries(ddappsec_helper_fuzzer PRIVATE ${FUZZER_LIB})
2539

2640
add_executable(corpus_generator corpus_generator.cpp)
2741
target_link_libraries(corpus_generator PRIVATE helper_objects libddwaf_objects pthread spdlog)

‎appsec/tests/fuzzer/main.cpp‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
#include "mutators.hpp"
88
#include "network.hpp"
9-
#include "network/acceptor.hpp"
109
#include <cstdlib>
1110
#include <iostream>
1211
#include <runner.hpp>
@@ -173,17 +172,26 @@ int main(int argc, char **argv)
173172

174173
auto acceptor_ptr = std::make_unique<dds::fuzzer::acceptor>();
175174
acceptor = acceptor_ptr.get();
176-
std::atomic<bool> interrupted{};
177-
dds::runner runner{config, std::move(acceptor_ptr), interrupted};
178-
179-
std::thread runner_thread([&runner] { runner.run(); });
175+
static std::atomic<bool> interrupted{};
176+
dds::runner runner{std::move(acceptor_ptr), interrupted};
177+
static std::thread *runner_thread =
178+
new std::thread{[&runner] { runner.run(); }};
179+
180+
static auto cleanup = []() {
181+
SPDLOG_INFO("atexit() hook, interrupting runner");
182+
interrupted.store(true, std::memory_order_release);
183+
acceptor->exit();
184+
185+
SPDLOG_INFO("Waiting for runner thread to finish");
186+
runner_thread->join();
187+
SPDLOG_INFO("Runner thread finished");
188+
delete runner_thread;
189+
};
190+
std::atexit(cleanup);
180191

181192
int result = LLVMFuzzerRunDriver(&argc, &argv, LLVMFuzzerTestOneInput);
182193

183-
interrupted.store(true, std::memory_order_release);
184-
acceptor->exit();
185-
186-
runner_thread.join();
194+
// generally not reached
187195

188196
return result;
189197
}

0 commit comments

Comments
 (0)