MB-54866: Inject non-elided task and bucket names on the call stack

Adds a new utility class called DebugVariable which uses folly's
compiler_must_not_elide to allow values to be kept around on the stack,
and uses that to put useful information about the task that is being
run on the program stack, so that they are available in crash dumps.

```
(gdb) bt full
No locals.
        phosphor_internal_category_enabled_71 = {_M_b = {_M_p = 0x0}, static is_always_lock_free = <optimised out>}
        phosphor_internal_category_enabled_temp_71 = <optimised out>
        phosphor_internal_tpi_71 = {category = 0x0, name = 0x0, type = phosphor::TraceEventType::AsyncStart, argument_names = {_M_elems = {0x0,
              0x0}}, argument_types = {_M_elems = {phosphor::TraceArgumentType::is_bool, phosphor::TraceArgumentType::is_bool}}}
        phosphor_internal_guard_71 = {tpi = 0x10a31a0 <ConnManager::run()::phosphor_internal_tpi_71>, enabled = true, arg1 = {<No data fields>},
          arg2 = {<No data fields>}, start = {__d = {__r = 88724143709634368}}}
        guard = {previous = 0x0}
        taskableName = {value = {_M_elems = "asdasd", '\000' <repeats 25 times>}}
        taskName = {value = {_M_elems = "ConnManager", '\000' <repeats 20 times>}}
        executedAt = {__d = {__r = <optimised out>}}
        scheduleOverhead = {__r = <optimised out>}
        start = {__d = {__r = <optimised out>}}
        runAgain = <optimised out>
        end = {__d = {__r = <optimised out>}}
        runtime = {__r = <optimised out>}
```

Change-Id: Ie2298ea18df493ffe2ad07b4c6ba7a0eab1017e6
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/184426
Reviewed-by: James H <james.harrison@couchbase.com>
Tested-by: Vesko Karaganev <vesko.karaganev@couchbase.com>

engines/ep/src/vb_adapters.cc

@@ -12,6 +12,7 @@
 
 #include "ep_engine.h"
 #include "kv_bucket.h"
+#include "utilities/debug_variable.h"
 #include "vb_visitors.h"
 
 VBCBAdaptor::VBCBAdaptor(KVBucket* s,
@@ -48,13 +49,18 @@ std::string VBCBAdaptor::getDescription() const {
 }
 
 bool VBCBAdaptor::run() {
+    // It might be useful to have the visitor that is running recorded
+    // in minidumps.
+    cb::DebugVariable visitorName{cb::toCharArrayN<32>(label)};
     visitor->begin();
 
     while (!vbucketsToVisit.empty()) {
         const auto vbid = vbucketsToVisit.front();
         VBucketPtr vb = store->getVBucket(vbid);
         if (vb) {
             currentvb = vbid.get();
+            // Also record the vbid.
+            cb::DebugVariable debugVbid{vbid.get()};
 
             using State = InterruptableVBucketVisitor::ExecutionState;
             switch (visitor->shouldInterrupt()) {

executor/globaltask.cc

@@ -14,6 +14,7 @@
 #include <engines/ep/src/ep_engine.h>
 #include <engines/ep/src/objectregistry.h>
 #include <folly/lang/Hint.h>
+#include <utilities/debug_variable.h>
 #include <climits>
 
 // These static_asserts previously were in priority_test.cc
@@ -72,6 +73,10 @@ bool GlobalTask::execute(std::string_view threadName) {
     // Invoke run with the engine as the target for alloc/dalloc
     BucketAllocationGuard guard(engine);
 
+    // Put the taskable name on the stack so we know which bucket's task was
+    // running if we happen to crash.
+    cb::DebugVariable taskableName(cb::toCharArrayN<32>(taskable.getName()));
+
     // Call GlobalTask::run(), noting the result.
     // If true: Read GlobalTask::wakeTime. If "now", then re-queue
     // directly on the CPUThreadPool. If some time in the future,

tests/CMakeLists.txt

@@ -1,6 +1,7 @@
 add_compile_options_disable_optimization()
 
 add_subdirectory(cert)
+add_subdirectory(dbgvar)
 ADD_SUBDIRECTORY(dockey)
 ADD_SUBDIRECTORY(engine_error)
 add_subdirectory(gocode)

tests/dbgvar/CMakeLists.txt

@@ -0,0 +1,7 @@
+cb_add_test_executable(memcached_dbgvar_test
+        dbgvar_test.cc)
+target_link_libraries(memcached_dbgvar_test
+        PRIVATE Folly::headers GTest::gtest GTest::gtest_main)
+add_test(NAME memcached_dbgvar_test
+        WORKING_DIRECTORY ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}
+        COMMAND memcached_dbgvar_test)

tests/dbgvar/dbgvar_test.cc

@@ -0,0 +1,162 @@
+/*
+ *     Copyright 2022-Present Couchbase, Inc.
+ *
+ *   Use of this software is governed by the Business Source License included
+ *   in the file licenses/BSL-Couchbase.txt.  As of the Change Date specified
+ *   in that file, in accordance with the Business Source License, use of this
+ *   software will be governed by the Apache License, Version 2.0, included in
+ *   the file licenses/APL2.txt.
+ */
+
+#include <folly/CPortability.h>
+#include <folly/portability/GTest.h>
+
+#include <cstdint>
+#include <cstdlib>
+#include <iostream>
+#include <string_view>
+
+#include "utilities/debug_variable.h"
+
+class DebugVariableTest : public ::testing::Test {};
+
+/**
+ * Creates a readable representation of the contents of the string_view.
+ */
+std::string dumpBytes(std::string_view sv) {
+    std::stringstream ss;
+    ss << std::hex;
+
+    int i = 0;
+    for (char ch : sv) {
+        if (isprint(ch)) {
+            ss << ' ' << ch << ' ';
+        } else {
+            ss << std::setfill('0') << std::setw(2) << int(uint8_t(ch)) << ' ';
+        }
+        if (i++ % 8 == 0) {
+            ss << '\n';
+        }
+    }
+    ss << '\n';
+    return ss.str();
+}
+
+template <typename F, typename... Args>
+FOLLY_NOINLINE auto callNonInline(F&& f, Args&&... args) -> decltype(auto) {
+    return f(std::forward<Args>(args)...);
+}
+
+/**
+ * Memcpy implementation which is not checked by sanitizers. We use this to be
+ * able to copy bytes off the stack in StackCapture.
+ */
+FOLLY_NOINLINE FOLLY_DISABLE_SANITIZERS void* memcpy_nosanitize(
+        void* dest, const void* src, std::size_t count) {
+    char* it = reinterpret_cast<char*>(dest);
+    char* end = it + count;
+    const char* inputIt = reinterpret_cast<const char*>(src);
+
+    while (it != end) {
+        *it = *inputIt;
+        it++;
+        inputIt++;
+    }
+
+    return dest;
+}
+
+/**
+ * A callable object which will record the contents of program memory between
+ * the address of the *this object and the stack frame of the operator().
+ */
+struct StackCapture {
+public:
+    StackCapture() = default;
+
+    StackCapture(const StackCapture&) = delete;
+    StackCapture(StackCapture&&) = delete;
+    StackCapture& operator=(const StackCapture&) = delete;
+    StackCapture& operator=(StackCapture&&) = delete;
+
+    FOLLY_NOINLINE void operator()() {
+        const char* startMarker = reinterpret_cast<const char*>(this);
+        const char* endMarker = reinterpret_cast<const char*>(&startMarker);
+        // Adjust for different stack growth strategies.
+        if (startMarker > endMarker) {
+            std::swap(startMarker, endMarker);
+        }
+
+        // Record the stack between *this and the stack frame of this method.
+        std::size_t bytesToCopy = endMarker - startMarker;
+        stackBytes.resize(bytesToCopy);
+        memcpy_nosanitize(stackBytes.data(), startMarker, bytesToCopy);
+    }
+
+    std::string_view getBytes() const {
+        return stackBytes;
+    }
+
+private:
+    std::string stackBytes;
+};
+
+template <typename F>
+FOLLY_NOINLINE std::string stackCapture(F&& f) {
+    StackCapture sc;
+    callNonInline(f, sc);
+    return std::string(sc.getBytes());
+}
+
+TEST_F(DebugVariableTest, StringPresentInStack) {
+    auto bytes = stackCapture([](auto& capture) {
+        cb::DebugVariable var1(cb::toCharArrayN<9>("b"));
+        capture();
+    });
+
+    // Sanity check.
+    ASSERT_EQ(bytes.find("nonexistent"), std::string_view::npos);
+
+    EXPECT_NE(bytes.find({"b\0\0\0\0\0\0\0\0", 9}), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+}
+
+TEST_F(DebugVariableTest, TruncatedStringPresentInStack) {
+    auto val = cb::toCharArrayN<8>("blarblarblar");
+    auto bytes = stackCapture([&](auto& capture) {
+        cb::DebugVariable var1(val);
+        capture();
+    });
+    EXPECT_NE(bytes.find("blarblar"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+    EXPECT_EQ(bytes.find("blarblarblar"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+}
+
+TEST_F(DebugVariableTest, VarsDoNotLeak) {
+    auto bytes = stackCapture([](auto& capture) {
+        cb::DebugVariable var1(cb::toCharArrayN<3>("foo"));
+        capture();
+        cb::DebugVariable var2(cb::toCharArrayN<3>("bar"));
+    });
+
+    EXPECT_NE(bytes.find("foo"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+    EXPECT_EQ(bytes.find("bar"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+}
+
+TEST_F(DebugVariableTest, MultipleVars) {
+    auto bytes = stackCapture([](auto& capture) {
+        cb::DebugVariable var1(cb::toCharArrayN<3>("abc"));
+        cb::DebugVariable var2(cb::toCharArrayN<3>("def"));
+        cb::DebugVariable var3(cb::toCharArrayN<3>("ghi"));
+        capture();
+    });
+    EXPECT_NE(bytes.find("abc"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+    EXPECT_NE(bytes.find("def"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+    EXPECT_NE(bytes.find("ghi"), std::string_view::npos)
+            << "Actual: " << dumpBytes(bytes);
+}

utilities/debug_variable.h

@@ -0,0 +1,104 @@
+/*
+ *     Copyright 2022-Present Couchbase, Inc.
+ *
+ *   Use of this software is governed by the Business Source License included
+ *   in the file licenses/BSL-Couchbase.txt.  As of the Change Date specified
+ *   in that file, in accordance with the Business Source License, use of this
+ *   software will be governed by the Apache License, Version 2.0, included in
+ *   the file licenses/APL2.txt.
+ */
+
+#pragma once
+
+#include <folly/lang/Hint.h>
+#include <algorithm>
+#include <array>
+#include <cstring>
+#include <stdexcept>
+#include <string_view>
+#include <utility>
+
+namespace cb {
+
+/**
+ * Copies the input into a fixed-size char array, truncating it if longer than
+ * the size of the array.
+ *
+ * If N == s.size(), no null-terminator will be inserted.
+ */
+template <std::size_t N>
+constexpr std::array<char, N> toCharArrayN(std::string_view s) noexcept {
+    static_assert(N > 1, "String will always be empty.");
+    std::array<char, N> m{};
+    std::memcpy(m.data(), s.data(), std::min(N, s.size()));
+    return m;
+}
+
+/**
+ * A container for a value which uses compiler hints to keep the object on the
+ * program stack. This means that DebugVariables show up in crash dumps, and
+ * makes them a useful debugging aid.
+ *
+ * If the value is a string type, it is best to use toCharArrayN to create a
+ * fixed-size char array, because std::string with SVO is hard to read and
+ * char* will only show the address of the string.
+ *
+ * ### Implementation
+ *
+ * Folly provides a "compiler hint" compiler_must_not_elide, which prevents
+ * elision by the compiler of the given value.
+ *
+ * By applying compiler_must_not_elide to the `this` pointer, we effectively
+ * require that the object to be present somewhere in memory (because the
+ * pointer must be valid and not elided). We do this compiler_must_not_elide
+ * trick twice - once in the constructor, and once in the destructor, ensuring
+ * that the object "exists" in memory at both times.
+ *
+ * This is reliant on how the compiler decides to allocate objects, however,
+ * in practice, gcc and clang will allocate this object on the stack and keep it
+ * there until the destructor runs.
+ *
+ * There is an alternative possible implementation that an imaginary compiler
+ * could use -- push the object on the stack, call the constructor, pop the
+ * object off the stack, do some stuff, then push on the stack again, call the
+ * destructor and pop it off for good. This is less optimal and would make sense
+ * if the compiler were trying to save stack memory, however no compiler I
+ * tested with does this.
+ */
+template <typename T>
+class [[maybe_unused]] DebugVariable {
+public:
+    // Dynamic allocation does not make sense for this object. It must be
+    // allocated on the stack.
+    void* operator new(std::size_t) = delete;
+
+    /**
+     * Construct a DebugVariable.
+     */
+    constexpr DebugVariable(const T& v) : value(std::move(v)) {
+        static_assert(!std::is_pointer_v<T> && !std::is_same_v<void*, T> &&
+                              !std::is_same_v<const void*, T>,
+                      "Storing a pointer doesn't make sense in most cases. "
+                      "Use a void* if you want the address to be visible on "
+                      "the stack.");
+
+        // Value must be in memory when the constructor runs.
+        folly::compiler_must_not_elide(this);
+    }
+
+    DebugVariable(const DebugVariable&) = delete;
+    DebugVariable(DebugVariable&&) = delete;
+
+    DebugVariable& operator=(const DebugVariable&) = delete;
+    DebugVariable& operator=(DebugVariable&&) = delete;
+
+    ~DebugVariable() {
+        // Value must be in memory when the destructor runs.
+        folly::compiler_must_not_elide(this);
+    }
+
+private:
+    T value;
+};
+
+} // namespace cb