Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,34 @@ target_include_directories(cyclescope INTERFACE
)
target_compile_features(cyclescope INTERFACE cxx_std_20)

set(CYCLESCOPE_SANITIZE "" CACHE STRING "Sanitizer for tests: thread, address, undefined, or empty")

# Compiler-instrumentation backend: hooks for -finstrument-functions,
# compiled without instrumentation. Not available with MSVC.
if(NOT MSVC)
add_library(cyclescope_instrument STATIC src/instrument.cpp)
add_library(cyclescope::instrument ALIAS cyclescope_instrument)
target_link_libraries(cyclescope_instrument
PUBLIC cyclescope::cyclescope ${CMAKE_DL_LIBS})
target_compile_features(cyclescope_instrument PUBLIC cxx_std_20)
target_compile_options(cyclescope_instrument PRIVATE -Wall -Wextra)
# The hooks and frame stacks must be visible to the sanitizer too.
# Consumers opt in to the sanitizer runtime themselves (tests and
# examples do), keeping instrumentation consistent per binary.
if(CYCLESCOPE_SANITIZE AND NOT MSVC)
target_compile_options(cyclescope_instrument PRIVATE
-fsanitize=${CYCLESCOPE_SANITIZE} -g -fno-omit-frame-pointer)
endif()
endif()

if(PROJECT_IS_TOP_LEVEL)
include(CTest)
option(CYCLESCOPE_BUILD_TESTS "Build cyclescope tests" ON)
set(CYCLESCOPE_SANITIZE "" CACHE STRING "Sanitizer for tests: thread, address, undefined, or empty")
option(CYCLESCOPE_BUILD_EXAMPLES "Build cyclescope examples" ON)
if(CYCLESCOPE_BUILD_TESTS AND BUILD_TESTING)
add_subdirectory(tests)
endif()
if(CYCLESCOPE_BUILD_EXAMPLES AND NOT MSVC)
add_subdirectory(examples)
endif()
endif()
12 changes: 12 additions & 0 deletions examples/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
add_executable(cyclescope_demo demo.cpp)
target_link_libraries(cyclescope_demo PRIVATE cyclescope::cyclescope
cyclescope::instrument)
target_compile_options(cyclescope_demo PRIVATE -finstrument-functions
-Wall -Wextra)
set_target_properties(cyclescope_demo PROPERTIES ENABLE_EXPORTS ON)
if(CYCLESCOPE_SANITIZE)
target_compile_options(cyclescope_demo PRIVATE
-fsanitize=${CYCLESCOPE_SANITIZE} -g -fno-omit-frame-pointer)
target_link_options(cyclescope_demo PRIVATE
-fsanitize=${CYCLESCOPE_SANITIZE})
endif()
35 changes: 35 additions & 0 deletions examples/demo.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Instrumented demo: every function call in this file becomes a trace
// event. Build is compiled with -finstrument-functions and linked against
// cyclescope::instrument; running it writes demo.trace.json, which loads
// in the Perfetto UI (ui.perfetto.dev) or chrome://tracing.

#include <cstdio>
#include <cyclescope/trace.hpp>

namespace {

__attribute__((noinline)) long fib(int n) {
return n < 2 ? n : fib(n - 1) + fib(n - 2);
}

__attribute__((noinline)) long work(void) {
long total = 0;
for (int i = 0; i < 18; ++i) {
total += fib(i);
}
return total;
}

} // namespace

int main() {
const long total = work();
std::uint64_t dropped = 0;
const bool ok =
cyclescope::collector::instance().write_json("demo.trace.json",
&dropped);
std::printf("fib total %ld; trace %s, %llu dropped\n", total,
ok ? "written to demo.trace.json" : "FAILED",
static_cast<unsigned long long>(dropped));
return ok ? 0 : 1;
}
44 changes: 42 additions & 2 deletions include/cyclescope/trace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,39 @@ namespace detail {
// thread's nontrivial thread_locals (including the buffer handle) are
// gone. Set during TLS teardown; record() then drops late events.
inline thread_local bool tls_shutdown = false;

// True while collector internals run on this thread. Instrumentation
// hooks check it and stand down, so a hook firing inside record() or
// write_json() can never re-enter a lock this thread already holds.
inline thread_local bool tls_suppress = false;

// The constructor and destructor are forced inline: an out-of-line copy
// emitted by an instrumented translation unit could be selected by the
// linker, and its own entry would fire a hook before the flag is set,
// recursing without bound in unoptimized builds.
#if defined(_MSC_VER)
#define CYCLESCOPE_ALWAYS_INLINE __forceinline
#define CYCLESCOPE_NO_INSTRUMENT
#else
#define CYCLESCOPE_ALWAYS_INLINE __attribute__((always_inline)) inline
#define CYCLESCOPE_NO_INSTRUMENT __attribute__((no_instrument_function))
#endif

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CYCLESCOPE_ALWAYS_INLINE macro leaks into consumer namespace.

The macro is defined in a public header but never #undef'd. Any translation unit that includes <cyclescope/trace.hpp> inherits it and could conflict with a consumer-defined macro of the same name. Add #undef CYCLESCOPE_ALWAYS_INLINE immediately after the suppress_scope struct that uses it:

Suggested change
#endif
#define CYCLESCOPE_ALWAYS_INLINE __attribute__((always_inline)) inline
#endif

(with a #undef CYCLESCOPE_ALWAYS_INLINE inserted after the closing brace of suppress_scope)


struct suppress_scope {
bool previous;
CYCLESCOPE_ALWAYS_INLINE CYCLESCOPE_NO_INSTRUMENT suppress_scope() noexcept
: previous(tls_suppress) {
tls_suppress = true;
}
suppress_scope(const suppress_scope&) = delete;
suppress_scope& operator=(const suppress_scope&) = delete;
CYCLESCOPE_ALWAYS_INLINE CYCLESCOPE_NO_INSTRUMENT ~suppress_scope() {
tls_suppress = previous;
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Belt-and-suspenders: suppress_scope ctor/dtor could also carry no_instrument_function.

The comment explains the correctness argument: always_inline forces the ctor/dtor to be inlined at every call site, so -finstrument-functions never inserts a function-entry/exit hook for them. This is sound in practice.

However, if an instrumented TU happens to emit an out-of-line copy (e.g. via a pointer-to-member, a virtual dispatch, or a compiler that declines to inline in certain debug/LTO paths), the entry hook would fire before tls_suppress = true is set, allowing a hook to re-enter and recurse unboundedly — exactly the failure mode the comment warns about.

Adding __attribute__((no_instrument_function)) to both the constructor and destructor provides an independent, compiler-enforced guarantee at zero runtime cost:

struct suppress_scope {
  bool previous;
  CYCLESCOPE_ALWAYS_INLINE __attribute__((no_instrument_function))
  suppress_scope() noexcept : previous(tls_suppress) {
    tls_suppress = true;
  }
  suppress_scope(const suppress_scope&) = delete;
  suppress_scope& operator=(const suppress_scope&) = delete;
  CYCLESCOPE_ALWAYS_INLINE __attribute__((no_instrument_function))
  ~suppress_scope() { tls_suppress = previous; }
};

This is a defence-in-depth change, not a fix for a currently-triggered bug.


#undef CYCLESCOPE_ALWAYS_INLINE
#undef CYCLESCOPE_NO_INSTRUMENT
} // namespace detail

struct trace_event {
Expand All @@ -50,9 +83,13 @@ inline std::uint64_t trace_now_ns() {

class collector {
public:
// Deliberately immortal (never destroyed): instrumentation hooks and
// traced destructors can run during process teardown, and a destroyed
// collector would turn those into use-after-free. The buffers are
// reclaimed by the operating system at process exit.
static collector& instance() {
static collector self;
return self;
static collector* self = new collector();
return *self;
}

void set_capacity_per_thread(std::size_t capacity) {
Expand All @@ -69,6 +106,7 @@ class collector {
// destroyed; the event is dropped rather than touching dead TLS.
return;
}
detail::suppress_scope suppress;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same suppress_scope recursion risk applies to all three inline collector methods

record(), write_json() (line 112), and clear() (line 174) all create a suppress_scope to prevent re-entrancy. When any of these inline functions is instantiated in an instrumented TU (e.g. instrument_fixture.cpp or demo.cpp) and the linker selects that instrumented copy program-wide, the entry hook for the function fires before suppress_scope is constructed. If the compiler also emits a call to the un-inlined suppress_scope::suppress_scope() (likely at -O0), the constructor's entry hook fires with tls_suppress == false, and we have the same recursion chain described on the hooks.

record() is the most critical case: it is called on the hot path from every exit hook, so if the linker picks an instrumented copy the recursion is reached on every traced call, not just during flush.

The cleanest fix across the board is to mark both suppress_scope methods always_inline:

struct suppress_scope {
  bool previous;
  [[gnu::always_inline]] suppress_scope() noexcept
      : previous(tls_suppress) { tls_suppress = true; }
  suppress_scope(const suppress_scope&) = delete;
  suppress_scope& operator=(const suppress_scope&) = delete;
  [[gnu::always_inline]] ~suppress_scope() { tls_suppress = previous; }
};

This guarantees no call instruction is emitted for the guard regardless of TU, optimization level, or linker symbol selection.

thread_buffer& mine = local_buffer();
std::lock_guard guard(mine.mutex);
if (mine.events.size() >= capacity_.load(std::memory_order_relaxed)) {
Expand All @@ -91,6 +129,7 @@ class collector {
// snapshotted first (a transient copy of the event data), so recording
// threads and thread registration never stall behind a slow disk.
bool write_json(const char* path, std::uint64_t* dropped_total = nullptr) {
detail::suppress_scope suppress;
std::vector<std::shared_ptr<thread_buffer>> registered;
{
std::lock_guard registry_guard(registry_mutex_);
Expand Down Expand Up @@ -152,6 +191,7 @@ class collector {
// runs so their events still reach the trace; long-running programs with
// thread churn should clear() after each flush.
void clear() {
detail::suppress_scope suppress;
std::lock_guard registry_guard(registry_mutex_);
std::vector<std::shared_ptr<thread_buffer>> kept;
kept.reserve(buffers_.size());
Expand Down
135 changes: 135 additions & 0 deletions src/instrument.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
// Compiler-instrumentation backend: implements the hooks that
// -finstrument-functions inserts at every function entry and exit, and
// records each call as a complete event in the cyclescope collector.
//
// This translation unit must be compiled WITHOUT -finstrument-functions
// (the build system guarantees it); the attribute on each hook is a second
// line of defense. Usage: compile the code to trace with
// -finstrument-functions and link cyclescope::instrument.
//
// Names resolve lazily: the first return from a given function looks it up
// with dladdr, demangles it when possible, and interns the string for the
// process lifetime (the collector stores name pointers, not copies).
// Symbols outside the dynamic symbol table fall back to their address; on
// Linux, executables should enable exports (CMake ENABLE_EXPORTS) so
// dladdr can see them.

#include <cstdint>
#include <cstdio>
#include <cstring>
#include <cyclescope/trace.hpp>
#include <mutex>
#include <unordered_map>

#if !defined(_WIN32)
#include <dlfcn.h>
#endif
#if defined(__has_include)
#if __has_include(<cxxabi.h>)
#include <cxxabi.h>
#define CYCLESCOPE_HAS_CXXABI 1
#endif
#endif

namespace {

constexpr int kMaxDepth = 512;

struct frame {
void* fn;
std::uint64_t start_ns;
};

// Trivially destructible on purpose: hooks can run during TLS teardown,
// and POD thread_local storage stays readable there.
struct frame_stack {
int depth = 0;
int beyond = 0; // Calls deeper than kMaxDepth, tracked but not recorded.
};

thread_local frame_stack tls_stack;
thread_local frame tls_frames[kMaxDepth];

const char* interned_name(void* fn) {
// Deliberately immortal, mutex included: exit hooks can fire during
// process teardown, after function-local statics with destructors are
// gone, and locking a destroyed mutex throws from a noexcept context.
static auto* mutex = new std::mutex();
static auto* names = new std::unordered_map<void*, const char*>();

std::lock_guard guard(*mutex);
if (const auto it = names->find(fn); it != names->end()) {
return it->second;
}

const char* resolved = nullptr;
#if !defined(_WIN32)
Dl_info info;
if (dladdr(fn, &info) != 0 && info.dli_sname != nullptr) {
#if defined(CYCLESCOPE_HAS_CXXABI)
int status = -1;
char* demangled =
abi::__cxa_demangle(info.dli_sname, nullptr, nullptr, &status);
resolved = (status == 0 && demangled != nullptr)
? demangled
: ::strdup(info.dli_sname);
#else
resolved = ::strdup(info.dli_sname);
#endif
}
#endif
if (resolved == nullptr) {
char buffer[32];
std::snprintf(buffer, sizeof(buffer), "fn@%p", fn);
resolved = ::strdup(buffer);
}
names->emplace(fn, resolved);
return resolved;
}

} // namespace

extern "C" {

// Both hooks stand down while the suppress flag is set: it covers hook
// reentrancy AND collector internals. Without the latter, an instrumented
// copy of an inline collector function (the linker may pick one from an
// instrumented translation unit) fires a hook while this thread already
// holds a collector mutex, and record() self-deadlocks.
__attribute__((no_instrument_function)) void __cyg_profile_func_enter(
void* fn, void* /*call_site*/) {
if (cyclescope::detail::tls_suppress) {
return;
}
cyclescope::detail::suppress_scope suppress;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential infinite recursion in debug builds

suppress_scope::suppress_scope() is an inline function defined in trace.hpp. When a TU compiled with -finstrument-functions (e.g., demo.cpp, which is the sole non-library TU in the demo and includes trace.hpp) is linked before cyclescope_instrument.a, the linker selects the instrumented weak symbol for the constructor. At -O0, instrument.cpp emits a real call instruction here rather than inlining the two-line body.

Consequence: entering that instrumented constructor fires __cyg_profile_func_enter(suppress_scope_ctor_addr, …). At that point tls_suppress is still false (the constructor hasn't run yet), so the check at line 101 passes and the hook tries to construct another suppress_scope → infinite recursion → stack overflow.

The same risk applies at line 119 in the exit hook.

Because the hooks are no_instrument_function, no recursion guard exists for the hook's own entry, so the suppression mechanism must be in place before any potentially-instrumented function is called.

Fix – avoid the function call entirely:

Suggested change
cyclescope::detail::suppress_scope suppress;
cyclescope::detail::tls_suppress = true;
frame_stack& stack = tls_stack;

Then at the end of the hook body (before the closing }) restore with cyclescope::detail::tls_suppress = false;. The hooks are non-throwing C functions, so no RAII is needed for the restore.

Alternatively, add [[gnu::always_inline]] / __attribute__((always_inline)) to both suppress_scope::suppress_scope() and ~suppress_scope() in trace.hpp so the constructor is never emitted as an out-of-line call.

frame_stack& stack = tls_stack;
if (stack.beyond > 0 || stack.depth == kMaxDepth) {
++stack.beyond;
} else {
tls_frames[stack.depth] = {fn, cyclescope::trace_now_ns()};
++stack.depth;
}
}

__attribute__((no_instrument_function)) void __cyg_profile_func_exit(
void* fn, void* /*call_site*/) {
if (cyclescope::detail::tls_suppress) {
return;
}
cyclescope::detail::suppress_scope suppress;
frame_stack& stack = tls_stack;
if (stack.beyond > 0) {
--stack.beyond;
} else if (stack.depth > 0) {
--stack.depth;
const frame entered = tls_frames[stack.depth];
// Longjmp or exceptions can skip exits; only record matched frames.
if (entered.fn == fn) {
cyclescope::collector::instance().record(
interned_name(fn), entered.start_ns,
cyclescope::trace_now_ns() - entered.start_ns);
}
}
}

} // extern "C"
23 changes: 23 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,26 @@ endfunction()

cyclescope_add_test(log_test)
cyclescope_add_test(trace_test)

# The instrumentation backend test: only the fixture translation unit is
# compiled with -finstrument-functions, so the assertions run over a known
# call set instead of every doctest internal. ENABLE_EXPORTS lets dladdr
# resolve the fixture symbols from the executable on Linux.
if(NOT MSVC)
add_executable(instrument_test instrument_test.cpp instrument_fixture.cpp)
set_source_files_properties(instrument_fixture.cpp PROPERTIES
COMPILE_OPTIONS "-finstrument-functions")
target_link_libraries(instrument_test PRIVATE cyclescope::cyclescope
cyclescope::instrument doctest::doctest Threads::Threads)
target_compile_options(instrument_test PRIVATE -Wall -Wextra)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cyclescope_instrument library not compiled with sanitizer flags

When CYCLESCOPE_SANITIZE=thread (or address), the compile/link options are applied only to instrument_test but not to the cyclescope_instrument static library. Object code from src/instrument.cpp (the hooks, interned_name, frame_stack accesses) enters the final binary without sanitizer instrumentation, so TSan/ASan cannot observe data races or memory errors in that code.

The top-level CMakeLists.txt builds cyclescope_instrument unconditionally before the test option is read, so the simplest fix is to propagate the flag there or use an INTERFACE propagation:

if(CYCLESCOPE_SANITIZE)
  target_compile_options(cyclescope_instrument PUBLIC
    -fsanitize=${CYCLESCOPE_SANITIZE} -g -fno-omit-frame-pointer)
  target_link_options(cyclescope_instrument PUBLIC
    -fsanitize=${CYCLESCOPE_SANITIZE})
endif()

(Use PUBLIC rather than PRIVATE so the flag is consistently applied to any target linking the library.)

set_target_properties(instrument_test PROPERTIES ENABLE_EXPORTS ON)
if(CYCLESCOPE_SANITIZE)
target_compile_options(instrument_test PRIVATE
-fsanitize=${CYCLESCOPE_SANITIZE} -g -fno-omit-frame-pointer)
target_link_options(instrument_test PRIVATE
-fsanitize=${CYCLESCOPE_SANITIZE})
endif()
add_test(NAME instrument_test COMMAND instrument_test)
# A suppression regression deadlocks rather than fails; bound it.
set_tests_properties(instrument_test PROPERTIES TIMEOUT 120)
endif()
27 changes: 27 additions & 0 deletions tests/instrument_fixture.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Compiled WITH -finstrument-functions (see tests/CMakeLists.txt); the
// hooks in cyclescope::instrument trace every call made here. extern "C"
// keeps the symbol names stable for the assertions.

#include <cyclescope/trace.hpp>

extern "C" {

// Regression shape: an instrumented translation unit instantiates the
// inline collector functions, the linker may select those copies
// program-wide, and a hook firing inside them must stand down instead of
// re-entering a lock this thread holds.
__attribute__((noinline)) int cyclescope_fixture_flush(const char* path) {
return cyclescope::collector::instance().write_json(path) ? 1 : 0;
}

__attribute__((noinline)) void cyclescope_fixture_leaf(void) {
asm volatile(""); // Keeps the call from being optimized away.
}

__attribute__((noinline)) void cyclescope_fixture_parent(void) {
for (int i = 0; i < 3; ++i) {
cyclescope_fixture_leaf();
}
}

} // extern "C"
Loading
Loading