[lldb][core] Fix getting summary of a variable pointing to r/o memory

[igorkudrin]

Motivation example:

> lldb -c altmain2.core
...
(lldb) var F
(const char *) F = 0x0804a000 ""

The variable F points to a read-only memory page not dumped to the core file, so Process::ReadMemory() cannot read the data. The patch switches to Target::ReadMemory(), which can read data both from the process memory and the application binary.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Igor Kudrin (igorkudrin)

Changes

Motivation example:

> lldb -c altmain2.core
...
(lldb) var F
(const char *) F = 0x0804a000 ""

The variable F points to a read-only memory page not dumped to the core file, so Process::ReadMemory() cannot read the data. The patch switches to Target::ReadMemory(), which can read data both from the process memory and the application binary.

---

Full diff: https://github.com/llvm/llvm-project/pull/139196.diff

4 Files Affected:

• (modified) lldb/source/ValueObject/ValueObject.cpp (+12-1)
• (modified) lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py (+27)
• (added) lldb/test/API/functionalities/postmortem/elf-core/altmain2.core ()
• (added) lldb/test/API/functionalities/postmortem/elf-core/altmain2.out ()

diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index e1c66763ff0b8..aab78428d9103 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -735,7 +735,7 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
     case eAddressTypeLoad: {
       ExecutionContext exe_ctx(GetExecutionContextRef());
       Process *process = exe_ctx.GetProcessPtr();
-      if (process) {
+      if (process && process->IsLiveDebugSession()) {
         heap_buf_ptr->SetByteSize(bytes);
         size_t bytes_read = process->ReadMemory(
             addr + offset, heap_buf_ptr->GetBytes(), bytes, error);
@@ -743,6 +743,17 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
           data.SetData(data_sp);
           return bytes_read;
         }
+      } else if (Target *target = exe_ctx.GetTargetPtr()) {
+        Address target_addr;
+        target_addr.SetLoadAddress(addr + offset, target);
+        heap_buf_ptr->SetByteSize(bytes);
+        size_t bytes_read =
+            target->ReadMemory(target_addr, heap_buf_ptr->GetBytes(), bytes,
+                               error, /*force_live_memory=*/true);
+        if (error.Success() || bytes_read > 0) {
+          data.SetData(data_sp);
+          return bytes_read;
+        }
       }
     } break;
     case eAddressTypeHost: {
diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
index a287fd19ba352..d1e065a32efdc 100644
--- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -977,6 +977,33 @@ def test_get_core_file_api(self):
         self.assertEqual(process.GetCoreFile().GetFilename(), core_file_name)
         self.dbg.DeleteTarget(target)
 
+    @skipIfLLVMTargetMissing("X86")
+    def test_ro_cstring(self):
+        """
+        Test that we can show the summary for a cstring variable that points
+        to a r/o memory page which is not dumped to a core file.
+        """
+        target = self.dbg.CreateTarget("altmain2.out")
+        process = target.LoadCore("altmain2.core")
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        frame = process.GetSelectedThread().GetFrameAtIndex(0)
+        self.assertEqual(frame.GetFunctionName(), "_start")
+
+        var = frame.FindVariable("F")
+
+        # The variable points to a RO segment that is not dumped to the core
+        # file and thus process.ReadCStringFromMemory() cannot get the value.
+        error = lldb.SBError()
+        cstr = process.ReadCStringFromMemory(var.GetValueAsUnsigned(), 256, error)
+        self.assertFailure(error, error_str="core file does not contain 0x804a000")
+        self.assertEqual(cstr, "")
+
+        # Nevertheless, when getting the summary, the value can be read from the
+        # application binary.
+        cstr = var.GetSummary()
+        self.assertEqual(cstr, '"_start"')
+
     def check_memory_regions(self, process, region_count):
         region_list = process.GetMemoryRegions()
         self.assertEqual(region_list.GetSize(), region_count)
diff --git a/lldb/test/API/functionalities/postmortem/elf-core/altmain2.core b/lldb/test/API/functionalities/postmortem/elf-core/altmain2.core
new file mode 100755
index 0000000000000..b9dd8de08b813
Binary files /dev/null and b/lldb/test/API/functionalities/postmortem/elf-core/altmain2.core differ
diff --git a/lldb/test/API/functionalities/postmortem/elf-core/altmain2.out b/lldb/test/API/functionalities/postmortem/elf-core/altmain2.out
new file mode 100755
index 0000000000000..e930c3d8055e1
Binary files /dev/null and b/lldb/test/API/functionalities/postmortem/elf-core/altmain2.out differ

[DavidSpickett]

Surprised no one found this sooner, thanks for contributing.

[igorkudrin]

Ping.