Introduce "LogBeforeReturn" templated function to simplify code that

logs something and then returns.

Author: delcypher

lldb/source/Plugins/InstrumentationRuntime/BoundsSafety/InstrumentationRuntimeBoundsSafety.cpp

@@ -29,6 +29,7 @@
 #include "clang/CodeGen/ModuleBuilder.h"
 
 #include <memory>
+#include <type_traits>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -51,6 +52,9 @@ const std::vector<std::string> &getBoundsSafetySoftTrapRuntimeFuncs() {
 #define SOFT_TRAP_FALLBACK_CATEGORY                                            \
   SOFT_TRAP_CATEGORY_PREFIX "Bounds check failed"
 
+using ComputedStopInfo =
+    std::pair<std::optional<std::string>, std::optional<uint32_t>>;
+
 class InstrumentationBoundsSafetyStopInfo : public StopInfo {
 public:
   ~InstrumentationBoundsSafetyStopInfo() override = default;
@@ -76,16 +80,14 @@ class InstrumentationBoundsSafetyStopInfo : public StopInfo {
 private:
   InstrumentationBoundsSafetyStopInfo(Thread &thread);
 
-  std::pair<std::optional<std::string>, std::optional<uint32_t>>
+  ComputedStopInfo
   ComputeStopReasonAndSuggestedStackFrame(bool &warning_emitted_for_failure);
 
-  std::pair<std::string, std::optional<uint32_t>>
-  ComputeStopReasonAndSuggestedStackFrameWithDebugInfo(
+  ComputedStopInfo 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(
+  ComputedStopInfo ComputeStopReasonAndSuggestedStackFrameWithoutDebugInfo(
       ThreadSP thread_sp, lldb::user_id_t debugger_id,
       bool &warning_emitted_for_failure);
 };
@@ -118,24 +120,33 @@ InstrumentationBoundsSafetyStopInfo::InstrumentationBoundsSafetyStopInfo(
   }
 }
 
-std::pair<std::optional<std::string>, std::optional<uint32_t>>
+// Helper functions to make it convenient to log a failure and then return.
+template <typename T, typename... ArgTys>
+[[nodiscard]] T LogBeforeReturn(ArgTys &&...Args) {
+  Log *log_category = GetLog(LLDBLog::InstrumentationRuntime);
+  LLDB_LOG(log_category, Args...);
+  return T();
+}
+
+template <typename... ArgTys>
+[[nodiscard]] ComputedStopInfo LogBeforeReturnCSI(ArgTys &&...Args) {
+  return LogBeforeReturn<ComputedStopInfo>(Args...);
+}
+
+ComputedStopInfo
 InstrumentationBoundsSafetyStopInfo::ComputeStopReasonAndSuggestedStackFrame(
     bool &warning_emitted_for_failure) {
-  Log *log_category = GetLog(LLDBLog::InstrumentationRuntime);
   ThreadSP thread_sp = GetThread();
-  if (!thread_sp) {
-    LLDB_LOG(log_category, "failed to get thread while stopped");
-    return {};
-  }
+  if (!thread_sp)
+    return LogBeforeReturnCSI("failed to get thread while stopped");
 
   lldb::user_id_t debugger_id =
       thread_sp->GetProcess()->GetTarget().GetDebugger().GetID();
 
   StackFrameSP parent_sf = thread_sp->GetStackFrameAtIndex(1);
-  if (!parent_sf) {
-    LLDB_LOG(log_category, "got nullptr when fetching stackframe at index 1");
-    return {};
-  }
+  if (!parent_sf)
+    return LogBeforeReturnCSI(
+        "got nullptr when fetching stackframe at index 1");
 
   if (parent_sf->HasDebugInformation())
     return ComputeStopReasonAndSuggestedStackFrameWithDebugInfo(
@@ -147,8 +158,7 @@ InstrumentationBoundsSafetyStopInfo::ComputeStopReasonAndSuggestedStackFrame(
       thread_sp, debugger_id, warning_emitted_for_failure);
 }
 
-std::pair<std::string, std::optional<uint32_t>>
-InstrumentationBoundsSafetyStopInfo::
+ComputedStopInfo InstrumentationBoundsSafetyStopInfo::
     ComputeStopReasonAndSuggestedStackFrameWithDebugInfo(
         lldb::StackFrameSP parent_sf, lldb::user_id_t debugger_id,
         bool &warning_emitted_for_failure) {
@@ -165,13 +175,11 @@ InstrumentationBoundsSafetyStopInfo::
 
   auto MaybeTrapReason =
       clang::CodeGen::DemangleTrapReasonInDebugInfo(TrapReasonFuncName);
-  if (!MaybeTrapReason.has_value()) {
-    LLDB_LOG(
-        GetLog(LLDBLog::InstrumentationRuntime),
+  if (!MaybeTrapReason.has_value())
+    return LogBeforeReturnCSI(
         "clang::CodeGen::DemangleTrapReasonInDebugInfo(\"{0}\") call failed",
         TrapReasonFuncName);
-    return {};
-  }
+
   llvm::StringRef category = MaybeTrapReason.value().first;
   llvm::StringRef message = MaybeTrapReason.value().second;
 
@@ -198,18 +206,15 @@ InstrumentationBoundsSafetyStopInfo::
   return std::make_pair(stop_reason, parent_sf->GetFrameIndex() + 1);
 }
 
-std::pair<std::optional<std::string>, std::optional<uint32_t>>
-InstrumentationBoundsSafetyStopInfo::
+ComputedStopInfo InstrumentationBoundsSafetyStopInfo::
     ComputeStopReasonAndSuggestedStackFrameWithoutDebugInfo(
         ThreadSP thread_sp, lldb::user_id_t debugger_id,
         bool &warning_emitted_for_failure) {
 
-  Log *log_category = GetLog(LLDBLog::InstrumentationRuntime);
   StackFrameSP softtrap_sf = thread_sp->GetStackFrameAtIndex(0);
-  if (!softtrap_sf) {
-    LLDB_LOG(log_category, "got nullptr when fetching stackframe at index 0");
-    return {};
-  }
+  if (!softtrap_sf)
+    return LogBeforeReturnCSI(
+        "got nullptr when fetching stackframe at index 0");
   llvm::StringRef trap_reason_func_name = softtrap_sf->GetFunctionName();
 
   if (trap_reason_func_name == BoundsSafetySoftTrapMinimal) {
@@ -242,29 +247,25 @@ InstrumentationBoundsSafetyStopInfo::
   // __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 != BoundsSafetySoftTrapStr) {
-    LLDB_LOG(log_category,
-             "unexpected function name. Expected \"{0}\" but got \"{1}\"",
-             BoundsSafetySoftTrapStr.data(), trap_reason_func_name.data());
     assert(0 && "hit breakpoint for unexpected function name");
-    return {};
+    return LogBeforeReturnCSI(
+        "unexpected function name. Expected \"{0}\" but got \"{1}\"",
+        BoundsSafetySoftTrapStr.data(), trap_reason_func_name.data());
   }
 
   RegisterContextSP rc = thread_sp->GetRegisterContext();
-  if (!rc) {
-    LLDB_LOG(log_category, "failed to get register context");
-    return {};
-  }
+  if (!rc)
+    return LogBeforeReturnCSI("failed to get register context");
 
   // 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.
   ProcessSP process = thread_sp->GetProcess();
-  if (!process) {
-    LLDB_LOG(log_category, "failed to get process");
-    return {};
-  }
+  if (!process)
+    return LogBeforeReturnCSI("failed to get process");
+
   switch (process->GetTarget().GetArchitecture().GetCore()) {
   case ArchSpec::eCore_x86_32_i386:
   case ArchSpec::eCore_x86_32_i486:
@@ -288,22 +289,16 @@ InstrumentationBoundsSafetyStopInfo::
   // Examine the register for the first argument.
   const RegisterInfo *arg0_info = rc->GetRegisterInfo(
       lldb::RegisterKind::eRegisterKindGeneric, LLDB_REGNUM_GENERIC_ARG1);
-  if (!arg0_info) {
-    LLDB_LOG(log_category,
-             "failed to get register info for LLDB_REGNUM_GENERIC_ARG1");
-    return {};
-  }
+  if (!arg0_info)
+    return LogBeforeReturnCSI(
+        "failed to get register info for LLDB_REGNUM_GENERIC_ARG1");
   RegisterValue reg_value;
-  if (!rc->ReadRegister(arg0_info, reg_value)) {
-    LLDB_LOG(log_category, "failed to read register {0}", arg0_info->name);
-    return {};
-  }
+  if (!rc->ReadRegister(arg0_info, reg_value))
+    return LogBeforeReturnCSI("failed to read register {0}", arg0_info->name);
   uint64_t reg_value_as_int = reg_value.GetAsUInt64(UINT64_MAX);
-  if (reg_value_as_int == UINT64_MAX) {
-    LLDB_LOG(log_category, "failed to read register {0} as a UInt64",
-             arg0_info->name);
-    return {};
-  }
+  if (reg_value_as_int == UINT64_MAX)
+    return LogBeforeReturnCSI("failed to read register {0} as a UInt64",
+                              arg0_info->name);
 
   if (reg_value_as_int == 0) {
     // nullptr arg. The compiler will pass that if no trap reason string was
@@ -322,12 +317,11 @@ InstrumentationBoundsSafetyStopInfo::
   Status error_status;
   thread_sp->GetProcess()->ReadCStringFromMemory(reg_value_as_int, out_string,
                                                  error_status);
-  if (error_status.Fail()) {
-    LLDB_LOG(log_category, "failed to read C string from address {0}",
-             (void *)reg_value_as_int);
-    return {};
-  }
-  LLDB_LOG(log_category,
+  if (error_status.Fail())
+    return LogBeforeReturnCSI("failed to read C string from address {0}",
+                              (void *)reg_value_as_int);
+
+  LLDB_LOG(GetLog(LLDBLog::InstrumentationRuntime),
            "read C string from {0} found in register {1}: \"{2}\"",
            (void *)reg_value_as_int, arg0_info->name, out_string.c_str());
   std::string stop_reason;
@@ -402,31 +396,23 @@ bool InstrumentationRuntimeBoundsSafety::NotifyBreakpointHit(
   InstrumentationRuntimeBoundsSafety *const instance =
       static_cast<InstrumentationRuntimeBoundsSafety *>(baton);
 
-  Log *log_category = GetLog(LLDBLog::InstrumentationRuntime);
   ProcessSP process_sp = instance->GetProcessSP();
-  if (!process_sp) {
-    LLDB_LOG(log_category, "failed to get process from baton");
-    return false;
-  }
+  if (!process_sp)
+    return LogBeforeReturn<bool>("failed to get process from baton");
   ThreadSP thread_sp = context->exe_ctx_ref.GetThreadSP();
-  if (!thread_sp) {
-    LLDB_LOG(log_category,
-             "failed to get thread from StoppointCallbackContext");
-    return false;
-  }
-  if (process_sp != context->exe_ctx_ref.GetProcessSP()) {
-    LLDB_LOG(log_category,
-             "process from baton ({0}) and StoppointCallbackContext ({1}) do "
-             "not match",
-             (void *)process_sp.get(),
-             (void *)context->exe_ctx_ref.GetProcessSP().get());
-    return false;
-  }
+  if (!thread_sp)
+    return LogBeforeReturn<bool>(
+        "failed to get thread from StoppointCallbackContext");
 
-  if (process_sp->GetModIDRef().IsLastResumeForUserExpression()) {
-    LLDB_LOG(log_category, "IsLastResumeForUserExpression is true");
-    return false;
-  }
+  if (process_sp != context->exe_ctx_ref.GetProcessSP())
+    return LogBeforeReturn<bool>(
+        "process from baton ({0}) and StoppointCallbackContext ({1}) do "
+        "not match",
+        (void *)process_sp.get(),
+        (void *)context->exe_ctx_ref.GetProcessSP().get());
+
+  if (process_sp->GetModIDRef().IsLastResumeForUserExpression())
+    return LogBeforeReturn<bool>("IsLastResumeForUserExpression is true");
 
   // Maybe the stop reason and stackframe selection should be done by
   // a stackframe recognizer instead?
@@ -440,12 +426,9 @@ void InstrumentationRuntimeBoundsSafety::Activate() {
   if (IsActive())
     return;
 
-  Log *log_category = GetLog(LLDBLog::InstrumentationRuntime);
   ProcessSP process_sp = GetProcessSP();
-  if (!process_sp) {
-    LLDB_LOG(log_category, "could not get process during Activate()");
-    return;
-  }
+  if (!process_sp)
+    return LogBeforeReturn<void>("could not get process during Activate()");
 
   BreakpointSP breakpoint = process_sp->GetTarget().CreateBreakpoint(
       /*containingModules=*/nullptr,
@@ -457,15 +440,15 @@ void InstrumentationRuntimeBoundsSafety::Activate() {
       /*request_hardware*/ false);
 
   if (!breakpoint)
-    return;
+    return LogBeforeReturn<void>("failed to create breakpoint");
+
   if (!breakpoint->HasResolvedLocations()) {
-    LLDB_LOG(log_category,
-             "breakpoint {0} 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;
+    return LogBeforeReturn<void>(
+        "breakpoint {0} for BoundsSafety soft traps did not resolve to "
+        "any locations",
+        breakpoint->GetID());
   }
 
   // Note: When `sync=true` the suggested stackframe is completely ignored. So
@@ -475,7 +458,8 @@ void InstrumentationRuntimeBoundsSafety::Activate() {
       /*sync=*/false);
   breakpoint->SetBreakpointKind("bounds-safety-soft-trap");
   SetBreakpointID(breakpoint->GetID());
-  LLDB_LOG(log_category, "created breakpoint {0} for BoundsSafety soft traps",
+  LLDB_LOG(GetLog(LLDBLog::InstrumentationRuntime),
+           "created breakpoint {0} for BoundsSafety soft traps",
            breakpoint->GetID());
   SetActive(true);
 }