Author: David Spickett
Date: 2025-05-02T09:14:16Z
New Revision: daa4061d61216456baa83ab404e096200e327fb4

URL: 
https://github.com/llvm/llvm-project/commit/daa4061d61216456baa83ab404e096200e327fb4
DIFF: 
https://github.com/llvm/llvm-project/commit/daa4061d61216456baa83ab404e096200e327fb4.diff

LOG: Revert "[lldb] Do not bump memory modificator ID when "internal" debugger 
memory is updated (#129092)"

And a follow up warning fix.

This reverts commit 6aa963f780d63d4c8fa80de97dd79c932bc35f4e
and 2bff80f25d51e24d3c552e033a2863dd36ef648b.

This is failing on Windows on Arm: 
https://lab.llvm.org/buildbot/#/builders/141/builds/8375

Seems to produce the line the test wants but not in the right place.
Reverting while I investigate.

Added: 
    

Modified: 
    lldb/include/lldb/Target/Memory.h
    lldb/include/lldb/Target/Process.h
    lldb/source/Commands/CommandObjectProcess.cpp
    lldb/source/Commands/Options.td
    lldb/source/Target/Memory.cpp
    lldb/source/Target/Process.cpp
    lldb/source/Target/TargetProperties.td
    lldb/test/API/commands/settings/TestSettings.py

Removed: 
    lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
    lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVar.cpp
    lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVarWindows.cpp
    lldb/test/Shell/Expr/TestProcessModificationIdOnExpr.cpp


################################################################################
diff  --git a/lldb/include/lldb/Target/Memory.h 
b/lldb/include/lldb/Target/Memory.h
index 864ef6ca00802..2d1489a2c96ea 100644
--- a/lldb/include/lldb/Target/Memory.h
+++ b/lldb/include/lldb/Target/Memory.h
@@ -125,8 +125,6 @@ class AllocatedMemoryCache {
 
   bool DeallocateMemory(lldb::addr_t ptr);
 
-  bool IsInCache(lldb::addr_t addr) const;
-
 protected:
   typedef std::shared_ptr<AllocatedBlock> AllocatedBlockSP;
 
@@ -135,7 +133,7 @@ class AllocatedMemoryCache {
 
   // Classes that inherit from MemoryCache can see and modify these
   Process &m_process;
-  mutable std::recursive_mutex m_mutex;
+  std::recursive_mutex m_mutex;
   typedef std::multimap<uint32_t, AllocatedBlockSP> PermissionsToBlockMap;
   PermissionsToBlockMap m_memory_map;
 

diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 536a69fb89759..f5ee01d3fa713 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -111,7 +111,6 @@ class ProcessProperties : public Properties {
   void SetOSPluginReportsAllThreads(bool does_report);
   bool GetSteppingRunsAllThreads() const;
   FollowForkMode GetFollowForkMode() const;
-  bool TrackMemoryCacheChanges() const;
 
 protected:
   Process *m_process; // Can be nullptr for global ProcessProperties
@@ -313,18 +312,6 @@ class ProcessModID {
     return lldb::EventSP();
   }
 
-  void Dump(Stream &stream) const {
-    stream.Format("ProcessModID:\n"
-                  "  m_stop_id: {0}\n  m_last_natural_stop_id: {1}\n"
-                  "  m_resume_id: {2}\n  m_memory_id: {3}\n"
-                  "  m_last_user_expression_resume: {4}\n"
-                  "  m_running_user_expression: {5}\n"
-                  "  m_running_utility_function: {6}\n",
-                  m_stop_id, m_last_natural_stop_id, m_resume_id, m_memory_id,
-                  m_last_user_expression_resume, m_running_user_expression,
-                  m_running_utility_function);
-  }
-
 private:
   uint32_t m_stop_id = 0;
   uint32_t m_last_natural_stop_id = 0;

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index d0f5eaf2dfd9a..ed80c854ed66e 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -1388,9 +1388,6 @@ class CommandObjectProcessStatus : public 
CommandObjectParsed {
       case 'v':
         m_verbose = true;
         break;
-      case 'd':
-        m_dump = true;
-        break;
       default:
         llvm_unreachable("Unimplemented option");
       }
@@ -1400,7 +1397,6 @@ class CommandObjectProcessStatus : public 
CommandObjectParsed {
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_verbose = false;
-      m_dump = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
@@ -1409,7 +1405,6 @@ class CommandObjectProcessStatus : public 
CommandObjectParsed {
 
     // Instance variables to hold the values for command options.
     bool m_verbose = false;
-    bool m_dump = false;
   };
 
 protected:
@@ -1464,14 +1459,6 @@ class CommandObjectProcessStatus : public 
CommandObjectParsed {
         crash_info_sp->GetDescription(strm);
       }
     }
-
-    if (m_options.m_dump) {
-      StateType state = process->GetState();
-      if (state == eStateStopped) {
-        ProcessModID process_mod_id = process->GetModID();
-        process_mod_id.Dump(result.GetOutputStream());
-      }
-    }
   }
 
 private:

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index eb2e5113ed83e..346ca4aeb76a0 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -788,8 +788,6 @@ let Command = "process handle" in {
 let Command = "process status" in {
   def process_status_verbose : Option<"verbose", "v">, Group<1>,
     Desc<"Show verbose process status including extended crash information.">;
-  def process_status_dump : Option<"dump-modification-id", "d">, Group<1>,
-    Desc<"Dump the state of the ProcessModID of the stopped process.">;
 }
 
 let Command = "process save_core" in {

diff  --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp
index a23a50718712d..5cdd84f6640f0 100644
--- a/lldb/source/Target/Memory.cpp
+++ b/lldb/source/Target/Memory.cpp
@@ -433,11 +433,3 @@ bool AllocatedMemoryCache::DeallocateMemory(lldb::addr_t 
addr) {
             (uint64_t)addr, success);
   return success;
 }
-
-bool AllocatedMemoryCache::IsInCache(lldb::addr_t addr) const {
-  std::lock_guard<std::recursive_mutex> 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 ce64b44846a5d..25579249f73c9 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -370,12 +370,6 @@ FollowForkMode ProcessProperties::GetFollowForkMode() 
const {
                g_process_properties[idx].default_uint_value));
 }
 
-bool ProcessProperties::TrackMemoryCacheChanges() const {
-  const uint32_t idx = ePropertyTrackMemoryCacheChanges;
-  return GetPropertyAtIndexAs<bool>(
-      idx, g_process_properties[idx].default_uint_value != 0);
-}
-
 ProcessSP Process::FindPlugin(lldb::TargetSP target_sp,
                               llvm::StringRef plugin_name,
                               ListenerSP listener_sp,
@@ -2291,7 +2285,6 @@ 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())
@@ -2304,12 +2297,7 @@ 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 (TrackMemoryCacheChanges() || !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
@@ -2438,6 +2426,7 @@ 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/source/Target/TargetProperties.td 
b/lldb/source/Target/TargetProperties.td
index 14fa33cc22c18..3940ac00a2bd9 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -299,9 +299,6 @@ let Definition = "process" in {
     DefaultEnumValue<"eFollowParent">,
     EnumValues<"OptionEnumValues(g_follow_fork_mode_values)">,
     Desc<"Debugger's behavior upon fork or vfork.">;
-  def TrackMemoryCacheChanges: Property<"track-memory-cache-changes", 
"Boolean">,
-    DefaultTrue,
-    Desc<"If true, memory cache modifications (which happen often during 
expressions evaluation) will bump process state ID (and invalidate all 
synthetic children). Disabling this option helps to avoid synthetic children 
reevaluation when pretty printers heavily use expressions. The downside of 
disabled setting is that convenience variables won't reevaluate synthetic 
children automatically.">;
 }
 
 let Definition = "platform" in {

diff  --git a/lldb/test/API/commands/settings/TestSettings.py 
b/lldb/test/API/commands/settings/TestSettings.py
index 4f4fa0e032b50..4eb868430b006 100644
--- a/lldb/test/API/commands/settings/TestSettings.py
+++ b/lldb/test/API/commands/settings/TestSettings.py
@@ -905,7 +905,6 @@ def test_all_settings_exist(self):
                 "target.use-hex-immediates",
                 "target.process.disable-memory-cache",
                 "target.process.extra-startup-command",
-                "target.process.track-memory-cache-changes",
                 "target.process.thread.trace-thread",
                 "target.process.thread.step-avoid-regexp",
             ],

diff  --git a/lldb/test/Shell/Expr/TestExprWithSideEffect.cpp 
b/lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
deleted file mode 100644
index 4fc88b62730c7..0000000000000
--- a/lldb/test/Shell/Expr/TestExprWithSideEffect.cpp
+++ /dev/null
@@ -1,55 +0,0 @@
-// Tests evaluating expressions with side effects.
-// Applied side effect should be visible to the debugger.
-
-// RUN: %build %s -o %t
-// RUN: %lldb %t \
-// RUN:   -o "settings set target.process.track-memory-cache-changes false" \
-// RUN:   -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 "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();
-  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)

diff  --git a/lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVar.cpp 
b/lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVar.cpp
deleted file mode 100644
index b41a81f6f31a9..0000000000000
--- a/lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVar.cpp
+++ /dev/null
@@ -1,52 +0,0 @@
-// Tests evaluating expressions with side effects on convenience variable.
-// Applied side effect should be visible to the debugger.
-
-// UNSUPPORTED: system-windows
-
-// RUN: %build %s -o %t
-// RUN: %lldb %t \
-// RUN:   -o "settings set target.process.track-memory-cache-changes false" \
-// RUN:   -o "run" \
-// 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 "expr X \$mine = {100, 200}" \
-// RUN:   -o "expr \$mine.a = 300" \
-// RUN:   -o "expr \$mine" \
-// RUN:   -o "exit" | FileCheck %s -dump-input=fail
-
-struct X {
-  int a;
-  int b;
-};
-
-int main() {
-  X x;
-
-  __builtin_debugtrap();
-  __builtin_debugtrap();
-  return 0;
-}
-
-// CHECK-LABEL: expr int $y = 11
-// CHECK-LABEL: expr $y
-// CHECK: (int) $y = 11
-
-// CHECK-LABEL: expr $y = 100
-// CHECK: (int) $0 = 100
-
-// CHECK-LABEL: expr $y
-// CHECK: (int) $y = 100
-
-// CHECK-LABEL: continue
-// CHECK-LABEL: expr $y
-// CHECK: (int) $y = 100
-
-// CHECK-LABEL: expr X $mine = {100, 200}
-// CHECK-LABEL: expr $mine.a = 300
-// CHECK: (int) $1 = 300
-// CHECK-LABEL: expr $mine
-// CHECK: (X) $mine = (a = 300, b = 200)

diff  --git 
a/lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVarWindows.cpp 
b/lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVarWindows.cpp
deleted file mode 100644
index 8ac4b961f1ca9..0000000000000
--- a/lldb/test/Shell/Expr/TestExprWithSideEffectOnConvenienceVarWindows.cpp
+++ /dev/null
@@ -1,51 +0,0 @@
-// Same as TestExprWithSideEffectOnConvenienceVar.cpp but without $ escaping
-
-// REQUIRES: target-windows
-
-// RUN: %build %s -o %t
-// RUN: %lldb %t \
-// RUN:   -o "settings set target.process.track-memory-cache-changes false" \
-// RUN:   -o "run" \
-// 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 "expr X $mine = {100, 200}" \
-// RUN:   -o "expr $mine.a = 300" \
-// RUN:   -o "expr $mine" \
-// RUN:   -o "exit" | FileCheck %s -dump-input=fail
-
-struct X {
-  int a;
-  int b;
-};
-
-int main() {
-  X x;
-
-  __builtin_debugtrap();
-  __builtin_debugtrap();
-  return 0;
-}
-
-// CHECK-LABEL: expr int $y = 11
-// CHECK-LABEL: expr $y
-// CHECK: (int) $y = 11
-
-// CHECK-LABEL: expr $y = 100
-// CHECK: (int) $0 = 100
-
-// CHECK-LABEL: expr $y
-// CHECK: (int) $y = 100
-
-// CHECK-LABEL: continue
-// CHECK-LABEL: expr $y
-// CHECK: (int) $y = 100
-
-// CHECK-LABEL: expr X $mine = {100, 200}
-// CHECK-LABEL: expr $mine.a = 300
-// CHECK: (int) $1 = 300
-// CHECK-LABEL: expr $mine
-// CHECK: (X) $mine = (a = 300, b = 200)

diff  --git a/lldb/test/Shell/Expr/TestProcessModificationIdOnExpr.cpp 
b/lldb/test/Shell/Expr/TestProcessModificationIdOnExpr.cpp
deleted file mode 100644
index eea02665c0749..0000000000000
--- a/lldb/test/Shell/Expr/TestProcessModificationIdOnExpr.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-// Tests that ProcessModID.m_memory_id is not bumped when evaluating 
expressions without side effects.
-
-// REQUIRES: target-windows
-// Due to 
diff erent implementations exact numbers (m_stop_id) are 
diff erent on 
diff erent OSs. So we lock this test to specific platform.
-
-// RUN: %build %s -o %t
-// RUN: %lldb %t \
-// RUN:   -o "settings set target.process.track-memory-cache-changes false" \
-// RUN:   -o "run" \
-// RUN:   -o "process status -d" \
-// RUN:   -o "expr x.i != 42" \
-// RUN:   -o "process status -d" \
-// RUN:   -o "expr x.get()" \
-// RUN:   -o "process status -d" \
-// RUN:   -o "expr x.i = 10" \
-// RUN:   -o "process status -d" \
-// RUN:   -o "continue" \
-// RUN:   -o "process status -d" \
-// RUN:   -o "exit" | FileCheck %s -dump-input=fail
-
-class X {
-  int i = 0;
-
-public:
-  int get() { return i; }
-};
-
-int main() {
-  X x;
-  x.get();
-
-  __builtin_debugtrap();
-  __builtin_debugtrap();
-  return 0;
-}
-
-// CHECK-LABEL: process status -d
-// CHECK: m_stop_id: 2
-// CHECK: m_memory_id: 0
-
-// CHECK-LABEL: expr x.i != 42
-// IDs are not changed when executing simple expressions
-
-// CHECK-LABEL: process status -d
-// CHECK: m_stop_id: 2
-// CHECK: m_memory_id: 0
-
-// CHECK-LABEL: expr x.get()
-// Expression causes ID to be bumped because LLDB has to execute function
-
-// CHECK-LABEL: process status -d
-// CHECK: m_stop_id: 3
-// CHECK: m_memory_id: 1
-
-// CHECK-LABEL: expr x.i = 10
-// Expression causes MemoryID to be bumped because LLDB writes to non-cache 
memory
-
-// CHECK-LABEL: process status -d
-// CHECK: m_stop_id: 3
-// CHECK: m_memory_id: 2
-
-// CHECK-LABEL: continue
-// Continue causes StopID to be bumped because process is resumed
-
-// CHECK-LABEL: process status -d
-// CHECK: m_stop_id: 4
-// CHECK: m_memory_id: 2


        
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to