Skip to content

Commit

Permalink
Backtraces (kuzudb#3456)
Browse files Browse the repository at this point in the history
* Print backtraces when exceptions are thrown

Currently does so by printing them in the exception constructor, as otherwise we wouldn't be able to check for expected query result messages.
Also adds a std::terminate handler to handle some unexpected exits which would otherwise not print traces

* Add support for SIGSEGV backtraces (and other signals)

* Ignore false positive GCC warning in relwithdebinfo mode
  • Loading branch information
benjaminwinger authored May 10, 2024
1 parent 7251ee6 commit 4e1a9fd
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 8 deletions.
22 changes: 22 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,35 @@ option(BUILD_SINGLE_FILE_HEADER "Build single file header. Requires Python >= 3.
option(BUILD_TESTS "Build C++ tests." FALSE)
option(BUILD_EXTENSION_TESTS "Build C++ extension tests." FALSE)
option(BUILD_KUZU "Build Kuzu." TRUE)
option(ENABLE_BACKTRACES "Enable backtrace printing for exceptions and segfaults" FALSE)

option(BUILD_LCOV "Build coverage report." FALSE)
if(${BUILD_LCOV})
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
endif()

if (ENABLE_BACKTRACES)
find_package(cpptrace)
if (NOT cpptrace_FOUND)
include(FetchContent)
FetchContent_Declare(
cpptrace
GIT_REPOSITORY https://github.com/jeremy-rifkin/cpptrace.git
GIT_TAG v0.5.4
GIT_SHALLOW TRUE
)
FetchContent_MakeAvailable(cpptrace)
endif()
add_compile_definitions(KUZU_BACKTRACE)
endif()

function(add_kuzu_test TEST_NAME)
set(SRCS ${ARGN})
add_executable(${TEST_NAME} ${SRCS})
target_link_libraries(${TEST_NAME} PRIVATE test_helper test_runner graph_test)
if (ENABLE_BACKTRACES)
target_link_libraries(${TEST_NAME} PRIVATE register_backtrace_signal_handler)
endif()
target_include_directories(${TEST_NAME} PRIVATE ${PROJECT_SOURCE_DIR}/test/include)
include(GoogleTest)
gtest_discover_tests(${TEST_NAME} DISCOVERY_TIMEOUT 600 DISCOVERY_MODE PRE_TEST)
Expand All @@ -186,6 +205,9 @@ function(add_kuzu_api_test TEST_NAME)
set(SRCS ${ARGN})
add_executable(${TEST_NAME} ${SRCS})
target_link_libraries(${TEST_NAME} PRIVATE api_graph_test api_test_helper)
if (ENABLE_BACKTRACES)
target_link_libraries(${TEST_NAME} PRIVATE register_backtrace_signal_handler)
endif()
target_include_directories(${TEST_NAME} PRIVATE ${PROJECT_SOURCE_DIR}/test/include)
include(GoogleTest)
gtest_discover_tests(${TEST_NAME})
Expand Down
31 changes: 24 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
clangd tidy clangd-diagnostics \
install \
clean-extension clean-python-api clean-java clean \
extension-test shell-test
extension-test shell-test

.ONESHELL:
.SHELLFLAGS = -ec
Expand Down Expand Up @@ -57,6 +57,9 @@ endif
release:
$(call run-cmake-release,)

relwithdebinfo:
$(call run-cmake-relwithdebinfo,)

debug:
$(call run-cmake-debug,)

Expand Down Expand Up @@ -90,8 +93,8 @@ alldebug:

# Main tests
test:
$(call run-cmake-release, -DBUILD_TESTS=TRUE)
ctest --test-dir build/release/test --output-on-failure -j ${TEST_JOBS}
$(call run-cmake-relwithdebinfo, -DBUILD_TESTS=TRUE -DENABLE_BACKTRACES=TRUE)
ctest --test-dir build/relwithdebinfo/test --output-on-failure -j ${TEST_JOBS}

lcov:
$(call run-cmake-release, -DBUILD_TESTS=TRUE -DBUILD_LCOV=TRUE)
Expand Down Expand Up @@ -161,12 +164,13 @@ example:
$(call run-cmake-release, -DBUILD_EXAMPLES=TRUE)

extension-test:
$(call run-cmake-release, \
$(call run-cmake-relwithdebinfo, \
-DBUILD_EXTENSIONS="httpfs;duckdb;postgres" \
-DBUILD_EXTENSION_TESTS=TRUE \
-DENABLE_ADDRESS_SANITIZER=TRUE \
-DENABLE_BACKTRACES=TRUE \
)
ctest --test-dir build/release/extension --output-on-failure -j ${TEST_JOBS}
ctest --test-dir build/relwithdebinfo/extension --output-on-failure -j ${TEST_JOBS}
aws s3 rm s3://kuzu-dataset-us/${RUN_ID}/ --recursive

extension-debug:
Expand All @@ -182,10 +186,10 @@ extension-release:
)

shell-test:
$(call run-cmake-release, \
$(call run-cmake-relwithdebinfo, \
-DBUILD_SHELL=TRUE \
)
cd tools/shell/test && python3 -m pytest -v
cd tools/shell/test && python3 -m pytest -v

# Clang-related tools and checks

Expand Down Expand Up @@ -247,11 +251,24 @@ define build-cmake-release
$(call build-cmake,release,Release,$1)
endef

define build-cmake-relwithdebinfo
$(call build-cmake,relwithdebinfo,RelWithDebInfo,$1)
endef

define config-cmake-release
$(call config-cmake,release,Release,$1)
endef

define config-cmake-relwithdebinfo
$(call config-cmake,relwithdebinfo,RelWithDebInfo,$1)
endef

define run-cmake-release
$(call config-cmake-release,$1)
$(call build-cmake-release,$1)
endef

define run-cmake-relwithdebinfo
$(call config-cmake-relwithdebinfo,$1)
$(call build-cmake-relwithdebinfo,$1)
endef
3 changes: 3 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ set(KUZU_LIBRARIES antlr4_cypher antlr4_runtime brotlidec brotlicommon fast_floa
if(NOT WIN32)
set(KUZU_LIBRARIES dl ${KUZU_LIBRARIES})
endif()
if (ENABLE_BACKTRACES)
set(KUZU_LIBRARIES ${KUZU_LIBRARIES} cpptrace::cpptrace)
endif()
target_link_libraries(kuzu PUBLIC ${KUZU_LIBRARIES})
target_link_libraries(kuzu_shared PUBLIC ${KUZU_LIBRARIES})
unset(KUZU_LIBRARIES)
Expand Down
1 change: 1 addition & 0 deletions src/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ add_subdirectory(data_chunk)
add_subdirectory(enums)
add_subdirectory(exception)
add_subdirectory(serializer)
add_subdirectory(signal)
add_subdirectory(task_system)
add_subdirectory(types)
add_subdirectory(vector)
Expand Down
5 changes: 5 additions & 0 deletions src/common/exception/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
add_library(kuzu_common_exception
OBJECT
exception.cpp
message.cpp)

if (ENABLE_BACKTRACES)
target_link_libraries(kuzu_common_exception PRIVATE cpptrace::cpptrace)
endif()

set(ALL_OBJECT_FILES
${ALL_OBJECT_FILES} $<TARGET_OBJECTS:kuzu_common_exception>
PARENT_SCOPE)
17 changes: 17 additions & 0 deletions src/common/exception/exception.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#include "common/exception/exception.h"

#ifdef KUZU_BACKTRACE
#include <cpptrace/cpptrace.hpp>
#endif

namespace kuzu {
namespace common {

Exception::Exception(std::string msg) : exception(), exception_message_(std::move(msg)) {
#ifdef KUZU_BACKTRACE
cpptrace::generate_trace(1 /*skip this function's frame*/).print();
#endif
}

} // namespace common
} // namespace kuzu
4 changes: 4 additions & 0 deletions src/common/signal/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
if (ENABLE_BACKTRACES)
add_library(register_backtrace_signal_handler OBJECT register.cpp)
target_link_libraries(register_backtrace_signal_handler cpptrace::cpptrace)
endif()
32 changes: 32 additions & 0 deletions src/common/signal/register.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#ifdef KUZU_BACKTRACE
#include <csignal>
#include <cstdlib>
#include <cstring>
#include <iostream>

#include <cpptrace/cpptrace.hpp>

namespace {

void handler(int signo) {
// Not safe. Safe method would be writing directly to stderr with a pre-defined string
// But since the below isn't safe either...
std::cerr << "Fatal signal " << signo << std::endl;
// This is not safe, however the safe version, described at the link below,
// was causing hangs when the tracer program can't be found.
// Since this is only used in CI, the occasional failure/hang is probably acceptable.
// https://github.com/jeremy-rifkin/cpptrace/blob/main/docs/signal-safe-tracing.md
cpptrace::generate_trace(1 /*skip this function's frame*/).print();
std::_Exit(1);
}

int register_signal_handlers() noexcept {
std::signal(SIGSEGV, handler);
std::signal(SIGFPE, handler);
cpptrace::register_terminate_handler();
return 0;
}

static int ignore = register_signal_handlers();
}; // namespace
#endif
2 changes: 1 addition & 1 deletion src/include/common/exception/exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace common {

class KUZU_API Exception : public std::exception {
public:
explicit Exception(std::string msg) : exception(), exception_message_(std::move(msg)){};
explicit Exception(std::string msg);

public:
const char* what() const noexcept override { return exception_message_.c_str(); }
Expand Down
8 changes: 8 additions & 0 deletions third_party/spdlog/spdlog/fmt/bundled/format.h
Original file line number Diff line number Diff line change
Expand Up @@ -922,8 +922,16 @@ FMT_CONSTEXPR20 void basic_memory_buffer<T, SIZE, Allocator>::grow(
T* new_data =
std::allocator_traits<Allocator>::allocate(alloc_, new_capacity);
// The following code doesn't throw, so the raw pointer above doesn't leak.

// False positive warning when built in RelWithDebInfo mode
// see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
#pragma clang diagnostic ignored "-Wunknown-warning-option"
#pragma GCC diagnostic ignored "-Wstringop-overflow"
std::uninitialized_copy(old_data, old_data + this->size(),
detail::make_checked(new_data, new_capacity));
#pragma GCC diagnostic pop
this->set(new_data, new_capacity);
// deallocate must not throw according to the standard, but even if it does,
// the buffer already uses the new storage and will deallocate it in
Expand Down
3 changes: 3 additions & 0 deletions tools/shell/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ add_executable(kuzu_shell
shell_runner.cpp)

target_link_libraries(kuzu_shell kuzu)
if (ENABLE_BACKTRACES)
target_link_libraries(kuzu_shell register_backtrace_signal_handler)
endif()
if (MSVC)
target_link_libraries(kuzu_shell ws2_32)
endif()
Expand Down

0 comments on commit 4e1a9fd

Please sign in to comment.