https://github.com/real-mikhail created
https://github.com/llvm/llvm-project/pull/129092
This change prevents invalidating and updating values in
`ValueObject::UpdateValueIfNeeded` when only "internal" debugger memory is
updated. Writing to "internal" debugger memory happens when, for instance,
expressions are evaluated by visualizers (pretty printers).
One of the examples when broken caching has a particularly heavy impact is
visualizations of some collections: in some collections getting collection size
is an expensive operation (it requires traversal of the collection). But the
change fixes the general issue when modifying debugger-only-visible memory
invalidates caching of all variables.
At the same time evaluating user expression with side effects (visible to
target, not only to debugger) will still bump memory ID because:
1. If expression is evaluated via interpreter: it will cause write to
"non-internal" memory
2. If expression is JIT-compiled: then to call the function LLDB will write to
"non-internal" stack memory
>From 1d2da22bceb83cbda54567e5f819e55555eef4e6 Mon Sep 17 00:00:00 2001
From: Mikhail Zakharov <mikhail.zakha...@jetbrains.com>
Date: Thu, 27 Feb 2025 18:43:33 +0100
Subject: [PATCH] [lldb] Do not bump memory modificator ID when "internal"
debugger memory is updated
This change prevents invalidating and updating values in
`ValueObject::UpdateValueIfNeeded` when only "internal" debugger memory is
updated.
Writing to "internal" debugger memory happens when, for instance, user
expressions are evaluated by pretty printers.
For some collections getting collection size is an expensive operation (it
requires traversal of the collection) and broken value caching (being
invalidated after executing expressions) make things worse.
At the same time evaluating user expression with side effects (visible to
target, not only to debugger) will still bump memory ID because:
1. If expression is evaluated via interpreter: it will cause write to
"non-internal" memory
2. If expression is JIT-compiled: then to call the function LLDB will write to
"non-internal" stack memory
---
lldb/include/lldb/Target/Memory.h | 4 +-
lldb/source/Target/Memory.cpp | 8 ++
lldb/source/Target/Process.cpp | 7 +-
.../Shell/Expr/TestExprWithSideEffect.cpp | 82 +++++++++++++++++++
4 files changed, 99 insertions(+), 2 deletions(-)
create mode 100644 lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
diff --git a/lldb/include/lldb/Target/Memory.h
b/lldb/include/lldb/Target/Memory.h
index 2d1489a2c96ea..864ef6ca00802 100644
--- a/lldb/include/lldb/Target/Memory.h
+++ b/lldb/include/lldb/Target/Memory.h
@@ -125,6 +125,8 @@ class AllocatedMemoryCache {
bool DeallocateMemory(lldb::addr_t ptr);
+ bool IsInCache(lldb::addr_t addr) const;
+
protected:
typedef std::shared_ptr<AllocatedBlock> AllocatedBlockSP;
@@ -133,7 +135,7 @@ class AllocatedMemoryCache {
// Classes that inherit from MemoryCache can see and modify these
Process &m_process;
- std::recursive_mutex m_mutex;
+ mutable std::recursive_mutex m_mutex;
typedef std::multimap<uint32_t, AllocatedBlockSP> PermissionsToBlockMap;
PermissionsToBlockMap m_memory_map;
diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp
index 5cdd84f6640f0..9bfb8c349aa2c 100644
--- a/lldb/source/Target/Memory.cpp
+++ b/lldb/source/Target/Memory.cpp
@@ -433,3 +433,11 @@ bool AllocatedMemoryCache::DeallocateMemory(lldb::addr_t
addr) {
(uint64_t)addr, success);
return success;
}
+
+bool AllocatedMemoryCache::IsInCache(lldb::addr_t addr) const {
+ std::lock_guard guard(m_mutex);
+
+ return llvm::any_of(m_memory_map, [addr](const auto &block) {
+ return block.second->Contains(addr);
+ });
+}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6db582096155f..1150fabf2d0ed 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2267,6 +2267,7 @@ size_t Process::WriteMemoryPrivate(addr_t addr, const
void *buf, size_t size,
return bytes_written;
}
+#define USE_ALLOCATE_MEMORY_CACHE 1
size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
Status &error) {
if (ABISP abi_sp = GetABI())
@@ -2279,7 +2280,12 @@ size_t Process::WriteMemory(addr_t addr, const void
*buf, size_t size,
if (buf == nullptr || size == 0)
return 0;
+#if defined(USE_ALLOCATE_MEMORY_CACHE)
+ if (!m_allocated_memory_cache.IsInCache(addr))
+ m_mod_id.BumpMemoryID();
+#else
m_mod_id.BumpMemoryID();
+#endif
// We need to write any data that would go where any current software traps
// (enabled software breakpoints) any software traps (breakpoints) that we
@@ -2408,7 +2414,6 @@ Status
Process::WriteObjectFile(std::vector<ObjectFile::LoadableData> entries) {
return error;
}
-#define USE_ALLOCATE_MEMORY_CACHE 1
addr_t Process::AllocateMemory(size_t size, uint32_t permissions,
Status &error) {
if (GetPrivateState() != eStateStopped) {
diff --git a/lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
b/lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
new file mode 100644
index 0000000000000..d072fe3121031
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
@@ -0,0 +1,82 @@
+// Tests evaluating expressions with side effects.
+// Applied side effect should be visible to the debugger.
+
+// RUN: %build %s -o %t
+// RUN: %lldb %t -o run \
+// RUN: -o "frame variable x" \
+// RUN: -o "expr x.inc()" \
+// RUN: -o "frame variable x" \
+// RUN: -o "continue" \
+// RUN: -o "frame variable x" \
+// RUN: -o "expr x.i = 10" \
+// RUN: -o "frame variable x" \
+// RUN: -o "continue" \
+// RUN: -o "frame variable x" \
+// RUN: -o "expr int $y = 11" \
+// RUN: -o "expr $y" \
+// RUN: -o "expr $y = 100" \
+// RUN: -o "expr $y" \
+// RUN: -o "continue" \
+// RUN: -o "expr $y" \
+// RUN: -o exit | FileCheck %s -dump-input=fail
+
+class X
+{
+ int i = 0;
+public:
+ void inc()
+ {
+ ++i;
+ }
+};
+
+int main()
+{
+ X x;
+ x.inc();
+
+ __builtin_debugtrap();
+ __builtin_debugtrap();
+ __builtin_debugtrap();
+ __builtin_debugtrap();
+ return 0;
+}
+
+// CHECK-LABEL: frame variable x
+// CHECK: (X) x = (i = 1)
+
+// CHECK-LABEL: expr x.inc()
+// CHECK-LABEL: frame variable x
+// CHECK: (X) x = (i = 2)
+
+// CHECK-LABEL: continue
+// CHECK-LABEL: frame variable x
+// CHECK: (X) x = (i = 2)
+
+
+
+// CHECK-LABEL: expr x.i = 10
+// CHECK: (int) $0 = 10
+
+// CHECK-LABEL: frame variable x
+// CHECK: (X) x = (i = 10)
+
+// CHECK-LABEL: continue
+// CHECK-LABEL: frame variable x
+// CHECK: (X) x = (i = 10)
+
+
+
+// CHECK-LABEL: expr int $y = 11
+// CHECK-LABEL: expr $y
+// CHECK: (int) $y = 11
+
+// CHECK-LABEL: expr $y = 100
+// CHECK: (int) $1 = 100
+
+// CHECK-LABEL: expr $y
+// CHECK: (int) $y = 100
+
+// CHECK-LABEL: continue
+// CHECK-LABEL: expr $y
+// CHECK: (int) $y = 100
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
