[Lldb-commits] [lldb] [lldb] Refactor InstrumentationRuntimeAsan and add a new plugin (PR #69388)

[Alex Langford via lldb-commits]

================
@@ -0,0 +1,108 @@
+//===-- InstrumentationRuntimeLibsanitizers.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 "InstrumentationRuntimeLibsanitizers.h"
+
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/PluginInterface.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Symbol/Symbol.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Utility/RegularExpression.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+LLDB_PLUGIN_DEFINE(InstrumentationRuntimeLibsanitizers)
+
+lldb::InstrumentationRuntimeSP
+InstrumentationRuntimeLibsanitizers::CreateInstance(const lldb::ProcessSP &process_sp) {
+  return InstrumentationRuntimeSP(new InstrumentationRuntimeLibsanitizers(process_sp));
+}
+
+void InstrumentationRuntimeLibsanitizers::Initialize() {
+  PluginManager::RegisterPlugin(
+      GetPluginNameStatic(), "AddressSanitizer instrumentation runtime plugin for Libsanitizers.",
+      CreateInstance, GetTypeStatic);
+}
+
+void InstrumentationRuntimeLibsanitizers::Terminate() {
+  PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+lldb::InstrumentationRuntimeType InstrumentationRuntimeLibsanitizers::GetTypeStatic() {
+  return eInstrumentationRuntimeTypeLibsanitizersAsan;
+}
+
+InstrumentationRuntimeLibsanitizers::~InstrumentationRuntimeLibsanitizers() { Deactivate(); }
+
+const RegularExpression &
+InstrumentationRuntimeLibsanitizers::GetPatternForRuntimeLibrary() {
+  // FIXME: This shouldn't include the "dylib" suffix.
+  static RegularExpression regex(
+      llvm::StringRef("libsystem_sanitizers\\.dylib"));
+  return regex;
+}
+
+bool InstrumentationRuntimeLibsanitizers::CheckIfRuntimeIsValid(
+    const lldb::ModuleSP module_sp) {
+  const Symbol *symbol = module_sp->FindFirstSymbolWithNameAndType(
+      ConstString("__asan_abi_init"), lldb::eSymbolTypeAny);
+
+  return symbol != nullptr;
+}
+
+bool InstrumentationRuntimeLibsanitizers::NotifyBreakpointHit(
+    void *baton, StoppointCallbackContext *context, user_id_t break_id,
+    user_id_t break_loc_id) {
+  assert(baton && "null baton");
+  if (!baton)
+    return false;
+
+  InstrumentationRuntimeLibsanitizers *const instance =
+      static_cast<InstrumentationRuntimeLibsanitizers *>(baton);
+
+  ProcessSP process_sp = instance->GetProcessSP();
+
+  return ReportRetriever::NotifyBreakpointHit(process_sp, context, break_id, break_loc_id);
+}
+
+void InstrumentationRuntimeLibsanitizers::Activate() {
+  if (IsActive())
+    return;
+
+  ProcessSP process_sp = GetProcessSP();
+  if (!process_sp)
+    return;
+
+  Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(GetRuntimeModuleSP(), process_sp, ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+
+  if (!breakpoint)
+    return;
+
+  bool sync = false;
+
+  breakpoint->SetCallback(InstrumentationRuntimeLibsanitizers::NotifyBreakpointHit, this,
+                          sync);
+  breakpoint->SetBreakpointKind("address-sanitizer-report");
+  SetBreakpointID(breakpoint->GetID());
+
+  SetActive(true);
+}
+
+void InstrumentationRuntimeLibsanitizers::Deactivate() {
+  if (GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
+    ProcessSP process_sp = GetProcessSP();
+    if (process_sp) {
+      process_sp->GetTarget().RemoveBreakpointByID(GetBreakpointID());
+      SetBreakpointID(LLDB_INVALID_BREAK_ID);
+    }
+  }
+  SetActive(false);
----------------
bulbazord wrote:

Does the ordering between the above block and `SetActive(false)` matter? If not, it might be clearer to follow if you move that above the first `if` block. There are a number of nested blocks here.

If you do that, you could also flip the initial condition and return early`GetBreakpointID() == LLDB_INVALID_BREAK_ID`

https://github.com/llvm/llvm-project/pull/69388