[BoundsSafety][LLDB] Implement instrumentation plugin for -fbounds-safety soft traps #169117
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
delcypher
requested review from
Michael137,
adrian-prantl,
hnrklssn,
jimingham,
patrykstefanski and
rapidsna
delcypher
added
lldb
clang:bounds-safety
Member
|
@llvm/pr-subscribers-testing-tools @llvm/pr-subscribers-lldb Author: Dan Liew (delcypher) Changes[BoundsSafety][LLDB] Implement instrumentation plugin for -fbounds-safety soft traps This patch tries to upstream code landed downstream in This patch implements an instrumentation plugin for the
The current implementation of -fbounds-safety traps works by emitting When debug info is available:
When debug info is not available:
This patch includes shell and api tests. The shell tests seemed like the rdar://163230807 Patch is 50.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/169117.diff 25 Files Affected:
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 1a7db8faecd94..79f22be1c95d3 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -542,6 +542,7 @@ enum InstrumentationRuntimeType {
eInstrumentationRuntimeTypeMainThreadChecker = 0x0003,
eInstrumentationRuntimeTypeSwiftRuntimeReporting = 0x0004,
eInstrumentationRuntimeTypeLibsanitizersAsan = 0x0005,
+ eInstrumentationRuntimeTypeBoundsSafety = 0x0006,
eNumInstrumentationRuntimeTypes
};
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index 23d2165e07f7e..7311b17f97e01 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -1059,6 +1059,16 @@ def is_compiler_with_address_sanitizer():
return skipTestIfFn(is_compiler_with_address_sanitizer)(func)
+def skipUnlessBoundsSafety(func):
+ """Decorate the item to skip test unless Clang -fbounds-safety is supported."""
+
+ def is_compiler_with_bounds_safety():
+ if not _compiler_supports(lldbplatformutil.getCompiler(), "-fbounds-safety"):
+ return "Compiler cannot compile with -fbounds-safety"
+ return None
+
+ return skipTestIfFn(is_compiler_with_bounds_safety)(func)
+
def skipIfAsan(func):
"""Skip this test if the environment is set up to run LLDB *itself* under ASAN."""
return skipTestIfFn(is_running_under_asan)(func)
diff --git a/lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/CMakeLists.txt b/lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/CMakeLists.txt
new file mode 100644
index 0000000000000..adbd6c45e45af
--- /dev/null
+++ b/lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_lldb_library(lldbPluginInstrumentationRuntimeBoundsSafety PLUGIN
+ InstrumentationRuntimeBoundsSafety.cpp
+
+ LINK_LIBS
+ lldbBreakpoint
+ lldbCore
+ lldbSymbol
+ lldbTarget
+ lldbPluginInstrumentationRuntimeUtility
+
+ CLANG_LIBS
+ clangCodeGen
+ )
diff --git a/lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp b/lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
new file mode 100644
index 0000000000000..788fe9867c36e
--- /dev/null
+++ b/lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
@@ -0,0 +1,491 @@
+//===-- InstrumentationRuntimeBoundsSafety.cpp -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "InstrumentationRuntimeBoundsSafety.h"
+
+#include "Plugins/Process/Utility/HistoryThread.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Symbol/Block.h"
+#include "lldb/Symbol/Symbol.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Symbol/Variable.h"
+#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/InstrumentationRuntimeStopInfo.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/SectionLoadList.h"
+#include "lldb/Target/StopInfo.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/RegularExpression.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+
+#include <memory>
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(InstrumentationRuntimeBoundsSafety)
+
+#define BOUNDS_SAFETY_SOFT_TRAP_MINIMAL "__bounds_safety_soft_trap"
+#define BOUNDS_SAFETY_SOFT_TRAP_S "__bounds_safety_soft_trap_s"
+
+const std::vector<std::string> &getBoundsSafetySoftTrapRuntimeFuncs() {
+ static std::vector<std::string> Funcs = {BOUNDS_SAFETY_SOFT_TRAP_MINIMAL,
+ BOUNDS_SAFETY_SOFT_TRAP_S};
+
+ return Funcs;
+}
+
+#define SOFT_TRAP_CATEGORY_PREFIX "Soft "
+#define SOFT_TRAP_FALLBACK_CATEGORY \
+ SOFT_TRAP_CATEGORY_PREFIX "Bounds check failed"
+
+class InstrumentationBoundsSafetyStopInfo : public StopInfo {
+public:
+ ~InstrumentationBoundsSafetyStopInfo() override = default;
+
+ lldb::StopReason GetStopReason() const override {
+ return lldb::eStopReasonInstrumentation;
+ }
+
+ std::optional<uint32_t>
+ GetSuggestedStackFrameIndex(bool inlined_stack) override {
+ return m_value;
+ }
+
+ const char *GetDescription() override { return m_description.c_str(); }
+
+ bool DoShouldNotify(Event *event_ptr) override { return true; }
+
+ static lldb::StopInfoSP
+ CreateInstrumentationBoundsSafetyStopInfo(Thread &thread) {
+ return StopInfoSP(new InstrumentationBoundsSafetyStopInfo(thread));
+ }
+
+private:
+ InstrumentationBoundsSafetyStopInfo(Thread &thread);
+
+ std::pair<std::optional<std::string>, std::optional<uint32_t>>
+ ComputeStopReasonAndSuggestedStackFrame(bool &warning_emitted_for_failure);
+
+ std::pair<std::string, std::optional<uint32_t>>
+ ComputeStopReasonAndSuggestedStackFrameWithDebugInfo(
+ lldb::StackFrameSP parent_sf, lldb::user_id_t debugger_id,
+ bool &warning_emitted_for_failure);
+
+ std::pair<std::optional<std::string>, std::optional<uint32_t>>
+ ComputeStopReasonAndSuggestedStackFrameWithoutDebugInfo(
+ ThreadSP thread_sp, lldb::user_id_t debugger_id,
+ bool &warning_emitted_for_failure);
+};
+
+InstrumentationBoundsSafetyStopInfo::InstrumentationBoundsSafetyStopInfo(
+ Thread &thread)
+ : StopInfo(thread, 0) {
+ // No additional data describing the reason for stopping
+ m_extended_info = nullptr;
+ m_description = SOFT_TRAP_FALLBACK_CATEGORY;
+
+ bool warning_emitted_for_failure = false;
+ auto [Description, MaybeSuggestedStackIndex] =
+ ComputeStopReasonAndSuggestedStackFrame(warning_emitted_for_failure);
+ if (Description)
+ m_description = Description.value();
+ if (MaybeSuggestedStackIndex)
+ m_value = MaybeSuggestedStackIndex.value();
+
+ // Emit warning about the failure to compute the stop info if one wasn't
+ // already emitted
+ if ((!Description.has_value()) && !warning_emitted_for_failure) {
+ if (auto thread_sp = GetThread()) {
+ lldb::user_id_t debugger_id =
+ thread_sp->GetProcess()->GetTarget().GetDebugger().GetID();
+ Debugger::ReportWarning(
+ "specific BoundsSafety trap reason could not be computed",
+ debugger_id);
+ }
+ }
+}
+
+std::pair<std::optional<std::string>, std::optional<uint32_t>>
+InstrumentationBoundsSafetyStopInfo::ComputeStopReasonAndSuggestedStackFrame(
+ bool &warning_emitted_for_failure) {
+ auto *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+ ThreadSP thread_sp = GetThread();
+ if (!thread_sp) {
+ LLDB_LOGF(log_category, "failed to get thread while stopped");
+ return {};
+ }
+
+ lldb::user_id_t debugger_id =
+ thread_sp->GetProcess()->GetTarget().GetDebugger().GetID();
+
+ auto parent_sf = thread_sp->GetStackFrameAtIndex(1);
+ if (!parent_sf) {
+ LLDB_LOGF(log_category, "got nullptr when fetching stackframe at index 1");
+ return {};
+ }
+
+ if (parent_sf->HasDebugInformation()) {
+ return ComputeStopReasonAndSuggestedStackFrameWithDebugInfo(
+ parent_sf, debugger_id, warning_emitted_for_failure);
+ }
+
+ // If the debug info is missing we can still get some information
+ // from the parameter in the soft trap runtime call.
+ return ComputeStopReasonAndSuggestedStackFrameWithoutDebugInfo(
+ thread_sp, debugger_id, warning_emitted_for_failure);
+}
+
+std::pair<std::string, std::optional<uint32_t>>
+InstrumentationBoundsSafetyStopInfo::
+ ComputeStopReasonAndSuggestedStackFrameWithDebugInfo(
+ lldb::StackFrameSP parent_sf, lldb::user_id_t debugger_id,
+ bool &warning_emitted_for_failure) {
+ // First try to use debug info to understand the reason for trapping. The
+ // call stack will look something like this:
+ //
+ // ```
+ // frame #0: `__bounds_safety_soft_trap_s(reason="")
+ // frame #1: `__clang_trap_msg$Bounds check failed$<reason>'
+ // frame #2: `bad_read(index=10)
+ // ```
+ // ....
+ const auto *TrapReasonFuncName = parent_sf->GetFunctionName();
+
+ auto MaybeTrapReason =
+ clang::CodeGen::DemangleTrapReasonInDebugInfo(TrapReasonFuncName);
+ if (!MaybeTrapReason.has_value()) {
+ LLDB_LOGF(
+ GetLog(LLDBLog::InstrumentationRuntime),
+ "clang::CodeGen::DemangleTrapReasonInDebugInfo(\"%s\") call failed",
+ TrapReasonFuncName);
+ return {};
+ }
+ auto category = MaybeTrapReason.value().first;
+ auto message = MaybeTrapReason.value().second;
+
+ // TODO: Clang should probably be changed to emit the "Soft " prefix itself
+ std::string stop_reason;
+ llvm::raw_string_ostream ss(stop_reason);
+ ss << SOFT_TRAP_CATEGORY_PREFIX;
+ if (category.empty())
+ ss << "<empty category>";
+ else
+ ss << category;
+ if (message.empty()) {
+ // This is not a failure so leave `warning_emitted_for_failure` untouched.
+ Debugger::ReportWarning(
+ "specific BoundsSafety trap reason is not "
+ "available because the compiler omitted it from the debug info",
+ debugger_id);
+ } else {
+ ss << ": " << message;
+ }
+ // Use computed stop-reason and assume the parent of `parent_sf` is the
+ // the place in the user's code where the call to the soft trap runtime
+ // originated.
+ return std::make_pair(stop_reason, parent_sf->GetFrameIndex() + 1);
+}
+
+std::pair<std::optional<std::string>, std::optional<uint32_t>>
+InstrumentationBoundsSafetyStopInfo::
+ ComputeStopReasonAndSuggestedStackFrameWithoutDebugInfo(
+ ThreadSP thread_sp, lldb::user_id_t debugger_id,
+ bool &warning_emitted_for_failure) {
+
+ auto *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+ auto softtrap_sf = thread_sp->GetStackFrameAtIndex(0);
+ if (!softtrap_sf) {
+ LLDB_LOGF(log_category, "got nullptr when fetching stackframe at index 0");
+ return {};
+ }
+ llvm::StringRef trap_reason_func_name = softtrap_sf->GetFunctionName();
+
+ if (trap_reason_func_name == BOUNDS_SAFETY_SOFT_TRAP_MINIMAL) {
+ // This function has no arguments so there's no additional information
+ // that would allow us to identify the trap reason.
+ //
+ // Use the fallback stop reason and the current frame.
+ // While we "could" set the suggested frame to our parent (where the
+ // bounds check failed), doing this leads to very misleading output in
+ // LLDB. E.g.:
+ //
+ // ```
+ // 0x100003b40 <+104>: bl 0x100003d64 ; __bounds_safety_soft_trap
+ // -> 0x100003b44 <+108>: b 0x100003b48 ; <+112>
+ // ```
+ //
+ // This makes it look we stopped after finishing the call to
+ // `__bounds_safety_soft_trap` but actually we are in the middle of the
+ // call. To avoid this confusion just use the current frame.
+ Debugger::ReportWarning(
+ "specific BoundsSafety trap reason is not available because debug "
+ "info is missing on the caller of '" BOUNDS_SAFETY_SOFT_TRAP_MINIMAL
+ "'",
+ debugger_id);
+ warning_emitted_for_failure = true;
+ return {};
+ }
+
+ // BOUNDS_SAFETY_SOFT_TRAP_S has one argument which is a pointer to a string
+ // describing the trap or a nullptr.
+ if (trap_reason_func_name != BOUNDS_SAFETY_SOFT_TRAP_S) {
+ LLDB_LOGF(log_category,
+ "unexpected function name. Expected \"%s\" but got \"%s\"",
+ BOUNDS_SAFETY_SOFT_TRAP_S, trap_reason_func_name.data());
+ assert(0 && "hit breakpoint for unexpected function name");
+ return {};
+ }
+
+ auto rc = thread_sp->GetRegisterContext();
+ if (!rc) {
+ LLDB_LOGF(log_category, "failed to get register context");
+ return {};
+ }
+
+ // FIXME: LLDB should have an API that tells us for the current target if
+ // `LLDB_REGNUM_GENERIC_ARG1` can be used.
+ // https://github.com/llvm/llvm-project/issues/168602
+ // Don't try for architectures where examining the first register won't
+ // work.
+ auto process = thread_sp->GetProcess();
+ if (!process) {
+ LLDB_LOGF(log_category, "failed to get process");
+ return {};
+ }
+ switch (process->GetTarget().GetArchitecture().GetCore()) {
+ case ArchSpec::eCore_x86_32_i386:
+ case ArchSpec::eCore_x86_32_i486:
+ case ArchSpec::eCore_x86_32_i486sx:
+ case ArchSpec::eCore_x86_32_i686:
+ // Technically some x86 calling conventions do use a register for
+ // passing the first argument but let's ignore that for now.
+ Debugger::ReportWarning(
+ "specific BoundsSafety trap reason cannot be inferred on x86 when "
+ "the caller of '" BOUNDS_SAFETY_SOFT_TRAP_S "' is missing debug info",
+ debugger_id);
+ warning_emitted_for_failure = true;
+ return {};
+ default: {
+ }
+ };
+
+ // Examine the register for the first argument
+ auto *arg0_info = rc->GetRegisterInfo(
+ lldb::RegisterKind::eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG1);
+ if (!arg0_info) {
+ LLDB_LOGF(log_category,
+ "failed to get register info for LLDB_REGNUM_GENERIC_ARG1");
+ return {};
+ }
+ RegisterValue reg_value;
+ if (!rc->ReadRegister(arg0_info, reg_value)) {
+ LLDB_LOGF(log_category, "failed to read register %s", arg0_info->name);
+ return {};
+ }
+ uint64_t reg_value_as_int = reg_value.GetAsUInt64(UINT64_MAX);
+ if (reg_value_as_int == UINT64_MAX) {
+ LLDB_LOGF(log_category, "failed to read register %s as a UInt64",
+ arg0_info->name);
+ return {};
+ }
+
+ if (reg_value_as_int == 0) {
+ // nullptr arg. The compiler will pass that if no trap reason string was
+ // available.
+ Debugger::ReportWarning(
+ "specific BoundsSafety trap reason cannot be inferred because the "
+ "compiler omitted the reason",
+ debugger_id);
+ warning_emitted_for_failure = true;
+ return {};
+ }
+
+ // The first argument to the call is a pointer to a global C string
+ // containing the trap reason.
+ std::string out_string;
+ Status error_status;
+ thread_sp->GetProcess()->ReadCStringFromMemory(reg_value_as_int, out_string,
+ error_status);
+ if (error_status.Fail()) {
+ LLDB_LOGF(log_category, "failed to read C string from address %p",
+ (void *)reg_value_as_int);
+ return {};
+ }
+ LLDB_LOGF(log_category, "read C string from %p found in register %s: \"%s\"",
+ (void *)reg_value_as_int, arg0_info->name, out_string.c_str());
+ std::string stop_reason;
+ llvm::raw_string_ostream SS(stop_reason);
+ SS << SOFT_TRAP_FALLBACK_CATEGORY;
+ if (!stop_reason.empty()) {
+ SS << ": " << out_string;
+ }
+ // Use the current frame as the suggested frame for the same reason as for
+ // `BOUNDS_SAFETY_SOFT_TRAP_MINIMAL`.
+ return {stop_reason, 0};
+}
+
+InstrumentationRuntimeBoundsSafety::~InstrumentationRuntimeBoundsSafety() {
+ Deactivate();
+}
+
+lldb::InstrumentationRuntimeSP
+InstrumentationRuntimeBoundsSafety::CreateInstance(
+ const lldb::ProcessSP &process_sp) {
+ return InstrumentationRuntimeSP(
+ new InstrumentationRuntimeBoundsSafety(process_sp));
+}
+
+void InstrumentationRuntimeBoundsSafety::Initialize() {
+ PluginManager::RegisterPlugin(GetPluginNameStatic(),
+ "BoundsSafety instrumentation runtime plugin.",
+ CreateInstance, GetTypeStatic);
+}
+
+void InstrumentationRuntimeBoundsSafety::Terminate() {
+ PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+lldb::InstrumentationRuntimeType
+InstrumentationRuntimeBoundsSafety::GetTypeStatic() {
+ return lldb::eInstrumentationRuntimeTypeBoundsSafety;
+}
+
+const RegularExpression &
+InstrumentationRuntimeBoundsSafety::GetPatternForRuntimeLibrary() {
+ static RegularExpression regex;
+ return regex;
+}
+
+bool InstrumentationRuntimeBoundsSafety::CheckIfRuntimeIsValid(
+ const lldb::ModuleSP module_sp) {
+ auto *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+ for (const auto &SoftTrapFunc : getBoundsSafetySoftTrapRuntimeFuncs()) {
+ ConstString test_sym(SoftTrapFunc);
+
+ if (module_sp->FindFirstSymbolWithNameAndType(test_sym,
+ lldb::eSymbolTypeAny)) {
+ LLDB_LOGF(log_category, "found \"%s\" in %s", SoftTrapFunc.c_str(),
+ module_sp->GetObjectName().AsCString("<unknown>"));
+ return true;
+ }
+ }
+ LLDB_LOGF(log_category,
+ "did not findFound BoundsSafety soft trap functions in %s",
+ module_sp->GetObjectName().AsCString("<unknown>"));
+ return false;
+}
+
+bool InstrumentationRuntimeBoundsSafety::NotifyBreakpointHit(
+ void *baton, StoppointCallbackContext *context, user_id_t break_id,
+ user_id_t break_loc_id) {
+ assert(baton && "null baton");
+ if (!baton)
+ return false; ///< false => resume execution.
+
+ InstrumentationRuntimeBoundsSafety *const instance =
+ static_cast<InstrumentationRuntimeBoundsSafety *>(baton);
+
+ auto *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+ ProcessSP process_sp = instance->GetProcessSP();
+ if (!process_sp) {
+ LLDB_LOGF(log_category, "failed to get process from baton");
+ return false;
+ }
+ ThreadSP thread_sp = context->exe_ctx_ref.GetThreadSP();
+ if (!thread_sp) {
+ LLDB_LOGF(log_category,
+ "failed to get thread from StoppointCallbackContext");
+ return false;
+ }
+ if (process_sp != context->exe_ctx_ref.GetProcessSP()) {
+ LLDB_LOGF(log_category,
+ "process from baton (%p) and StoppointCallbackContext (%p) do "
+ "not match",
+ (void *)process_sp.get(),
+ (void *)context->exe_ctx_ref.GetProcessSP().get());
+ return false;
+ }
+
+ if (process_sp->GetModIDRef().IsLastResumeForUserExpression()) {
+ LLDB_LOGF(log_category, "IsLastResumeForUserExpression is true");
+ return false;
+ }
+
+ // Maybe the stop reason and stackframe selection should be done by
+ // a stackframe recognizer instead?
+ thread_sp->SetStopInfo(
+ InstrumentationBoundsSafetyStopInfo::
+ CreateInstrumentationBoundsSafetyStopInfo(*thread_sp));
+ return true;
+}
+
+void InstrumentationRuntimeBoundsSafety::Activate() {
+ if (IsActive())
+ return;
+
+ auto *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+ ProcessSP process_sp = GetProcessSP();
+ if (!process_sp) {
+ LLDB_LOGF(log_category, "could not get process during Activate()");
+ return;
+ }
+
+ auto breakpoint = process_sp->GetTarget().CreateBreakpoint(
+ /*containingModules=*/nullptr,
+ /*containingSourceFiles=*/nullptr, getBoundsSafetySoftTrapRuntimeFuncs(),
+ eFunctionNameTypeFull, eLanguageTypeUnknown,
+ /*m_offset=*/0,
+ /*skip_prologue*/ eLazyBoolNo,
+ /*internal=*/true,
+ /*request_hardware*/ false);
+
+ if (!breakpoint)
+ return;
+ if (!breakpoint->HasResolvedLocations()) {
+ LLDB_LOGF(log_category,
+ "breakpoint %d for BoundsSafety soft traps did not resolve to "
+ "any locations",
+ breakpoint->GetID());
+ assert(0 && "breakpoint has no resolved locations");
+ process_sp->GetTarget().RemoveBreakpointByID(breakpoint->GetID());
+ return;
+ }
+
+ // Note: When `sync=true` the suggested stackframe is completely ignored. So
+ // we use `sync=false`. Is that a bug?
+ breakpoint->SetCallback(
+ InstrumentationRuntimeBoundsSafety::NotifyBreakpointHit, this,
+ /*sync=*/false);
+ breakpoint->SetBreakpointKind("bounds-safety-soft-trap");
+ SetBreakpointID(breakpoint->GetID());
+ LLDB_LOGF(log_category, "created breakpoint %d for BoundsSafety soft traps",
+ breakpoint->GetID());
+ SetActive(true);
+}
+
+void InstrumentationRuntimeBoundsSafety::Deactivate() {
+ SetActive(false);
+ auto *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+ if (ProcessSP process_sp = GetProcessSP()) {
+ bool success =
+ process_sp->GetTarget().RemoveBreakpointByID(GetBreakpointID());
+ LLDB_LOGF(log_category...
[truncated]
|
Contributor
Author
|
This PR contains #169112 . Once that's merged into |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
delcypher
force-pushed
the
dliew/rdar-163230807
branch
from
3930daf to
95e2587
Compare
Member
JDevlieghere
left a comment
JDevlieghere
left a comment
There was a problem hiding this comment.
Functionally this looks fine, most of my comments are nits related to coding standards.
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.h
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
delcypher
force-pushed
the
dliew/rdar-163230807
branch
from
959e4f8 to
be6db43
Compare
delcypher
force-pushed
the
dliew/rdar-163230807
branch
2 times, most recently
from
33d9e3c to
492a8b5
Compare
JDevlieghere
approved these changes
Dec 1, 2025
Member
JDevlieghere
left a comment
JDevlieghere
left a comment
There was a problem hiding this comment.
LGTM modulo two small nits. Thanks Dan!
Comment on lines
44
to
49
| const std::vector<std::string> &getBoundsSafetySoftTrapRuntimeFuncs() { | ||
| static std::vector<std::string> Funcs = {BoundsSafetySoftTrapMinimal.str(), | ||
| BoundsSafetySoftTrapStr.str()}; | ||
|
|
||
| return Funcs; | ||
| } |
Member
There was a problem hiding this comment.
Now that the elements are contexpr, you can use a contexpr std::array for these.
Contributor
Author
There was a problem hiding this comment.
I've made the change. We still end up having to create a std::vector<std::string> later on though because we call an API to make a breakpoint and that has a std::vector<std::string>& parameter.
lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp
Outdated
Show resolved
Hide resolved
jimingham
approved these changes
Dec 1, 2025
Collaborator
jimingham
left a comment
jimingham
left a comment
There was a problem hiding this comment.
This is substantially the same as the version I already reviewed on the swiftlang fork. Still LGTM.
The only comment I have is trivial, but LogBeforeReturnCSI seems an awkward way to say "log the ComputedStopInfo before return". Seems to me LogCSIBeforeReturn is more natural, or even LogCSIThenReturn?
But that's more to fulfill the reviewer's need to find something to object to...
Contributor
Author
The meaning here is actually "log something then return the empty ComputedStopInfo because we failed to compute it". Maybe it should be |
Collaborator
Yes. The original name would also have been appropriate for a function that logs and then returns a successfully computed CSI, so that seems much clearer. |
…fety soft traps This patch tries to upstream code landed downstream in swiftlang#11835. This patch implements an instrumentation plugin for the `-fbounds-safety` soft trap mode first implemented in swiftlang#11645 (rdar://158088757). That functionality isn't supported in upstream Clang yet, however the instrumented plugin can be compiled without issue so this patch tries to upstream it. The included tests are all disabled when the clang used for testing doesn't support `-fbounds-safety`. This means the tests will be skipped. However, it's fairly easy to point LLDB at a clang that does support `-fbounds-safety. I've done this and confirmed the tests pass. To use a custom clang the following can be done: * For API tests set the `LLDB_TEST_COMPILER` CMake cache variable to point to appropriate compiler. * For shell tests applying a patch like this can be used to set the appropriate compiler: ``` --- a/lldb/test/Shell/helper/toolchain.py +++ b/lldb/test/Shell/helper/toolchain.py @@ -271,6 +271,7 @@ def use_support_substitutions(config): if config.lldb_lit_tools_dir: additional_tool_dirs.append(config.lldb_lit_tools_dir) + config.environment['CLANG'] = '/path/to/clang' llvm_config.use_clang( ``` The current implementation of -fbounds-safety traps works by emitting calls to runtime functions intended to log the occurrence of a soft trap. While the user could just set a breakpoint of these functions the instrumentation plugin sets it automatically and provides several additional features: When debug info is available: * It adjusts the stop reason to be the reason for trapping. This is extracted from the artificial frame in the debug info (similar to -fbounds-safety hard traps). * It adjusts the selected frame to be the frame where the soft trap occurred. When debug info is not available: * For the `call-with-str` soft trap mode the soft trap reason is read from the first argument register. * For the `call-minimal` soft trap mode the stop reason is adjusted to note its a bounds check failure but does not give further information because none is available. * In this situation the selected frame is not adjusted because in this mode the user will be looking at assembly and adjusting the frame makes things confusing. This patch includes shell and api tests. The shell tests seemed like the best way to test behavior when debug info is missing because those tests make it easy to disable building with debug info completely. rdar://163230807
…mentationRuntimeBoundsSafety.h Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
…mentationRuntimeBoundsSafety.cpp Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
…mentationRuntimeBoundsSafety.cpp Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
…mentationRuntimeBoundsSafety.cpp Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
…mentationRuntimeBoundsSafety.cpp Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
…mentationRuntimeBoundsSafety.cpp Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
logs something and then returns.
of the logging code while testing.
delcypher
force-pushed
the
dliew/rdar-163230807
branch
from
795ce2e to
4eea006
Compare
kcloudy0717
pushed a commit
to kcloudy0717/llvm-project
that referenced
this pull request
Dec 4, 2025
…fety soft traps (llvm#169117) This patch tries to upstream code landed downstream in swiftlang#11835. This patch implements an instrumentation plugin for the `-fbounds-safety` soft trap mode first implemented in swiftlang#11645 (rdar://158088757). That functionality isn't supported in upstream Clang yet, however the instrumented plugin can be compiled without issue so this patch tries to upstream it. The included tests are all disabled when the clang used for testing doesn't support `-fbounds-safety`. This means the tests will be skipped. However, it's fairly easy to point LLDB at a clang that does support `-fbounds-safety. I've done this and confirmed the tests pass. To use a custom clang the following can be done: * For API tests set the `LLDB_TEST_COMPILER` CMake cache variable to point to appropriate compiler. * For shell tests applying a patch like this can be used to set the appropriate compiler: ``` --- a/lldb/test/Shell/helper/toolchain.py +++ b/lldb/test/Shell/helper/toolchain.py @@ -271,6 +271,7 @@ def use_support_substitutions(config): if config.lldb_lit_tools_dir: additional_tool_dirs.append(config.lldb_lit_tools_dir) + config.environment['CLANG'] = '/path/to/clang' llvm_config.use_clang( ``` The current implementation of -fbounds-safety traps works by emitting calls to runtime functions intended to log the occurrence of a soft trap. While the user could just set a breakpoint of these functions the instrumentation plugin sets it automatically and provides several additional features: When debug info is available: * It adjusts the stop reason to be the reason for trapping. This is extracted from the artificial frame in the debug info (similar to -fbounds-safety hard traps). * It adjusts the selected frame to be the frame where the soft trap occurred. When debug info is not available: * For the `call-with-str` soft trap mode the soft trap reason is read from the first argument register. * For the `call-minimal` soft trap mode the stop reason is adjusted to note its a bounds check failure but does not give further information because none is available. * In this situation the selected frame is not adjusted because in this mode the user will be looking at assembly and adjusting the frame makes things confusing. This patch includes shell and api tests. The shell tests seemed like the best way to test behavior when debug info is missing because those tests make it easy to disable building with debug info completely. rdar://163230807
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[BoundsSafety][LLDB] Implement instrumentation plugin for -fbounds-safety soft traps
This patch tries to upstream code landed downstream in
swiftlang#11835.
This patch implements an instrumentation plugin for the
-fbounds-safetysoft trap mode first implemented inswiftlang#11645 (rdar://158088757).
That functionality isn't supported in upstream Clang yet, however the
instrumented plugin can be compiled without issue so this patch tries to
upstream it. The included tests are all disabled when the clang used for
testing doesn't support
-fbounds-safety. This means the tests will beskipped. However, it's fairly easy to point LLDB at a clang that does
support `-fbounds-safety. I've done this and confirmed the tests pass.
To use a custom clang the following can be done:
LLDB_TEST_COMPILERCMake cache variable topoint to appropriate compiler.
appropriate compiler:
The current implementation of -fbounds-safety traps works by emitting
calls to runtime functions intended to log the occurrence of a soft trap.
While the user could just set a breakpoint of these functions the
instrumentation plugin sets it automatically and provides several
additional features:
When debug info is available:
extracted from the artificial frame in the debug info (similar to
-fbounds-safety hard traps).
occurred.
When debug info is not available:
call-with-strsoft trap mode the soft trap reason isread from the first argument register.
call-minimalsoft trap mode the stop reason is adjustedto note its a bounds check failure but does not give further
information because none is available.
this mode the user will be looking at assembly and adjusting the
frame makes things confusing.
This patch includes shell and api tests. The shell tests seemed like the
best way to test behavior when debug info is missing because those tests
make it easy to disable building with debug info completely.
rdar://163230807