[lldb][Commands] Fix memory find for Swift expressions

[Michael137]

(depends on #143686)

There were two issues previously preventing memory find -e expressions
to succeed when stopped in Swift frames:

1. We weren't getting the dynamic type of the result ValueObject.
   For Swift this would fail when we tried to produce a scalar value
   out of it because the static VO wasn't sufficient to get to the
   integer value. Hence we add a call to GetQualifiedRepresentationIfAvailable
   (which is what we do for expressions in OptionArgParser::ToAddress too).
2. We weren't passing an ExecutionContextScope to GetByteSize, which
   Swift relied on to get the size of the result type.

My plan is to add an API test for this on the Apple swiftlang/llvm-project fork.

I considered an alternative where we use OptionArgParser::ToAddress
for memory find -e expressions, but it got a bit icky when trying to
figure out how many bytes we should copy out of the result into the
DataBufferHeap (currently we rely on the size of the result variable
type). This gets even trickier when we were to pass an expression that
was actually a hex digit or a number into ToAddress.

rdar://152113525

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

(depends on #143686)

There were two issues previously preventing memory find -e expressions
to succeed when stopped in Swift frames:

1. We weren't getting the dynamic type of the result ValueObject.
   For Swift this would fail when we tried to produce a scalar value
   out of it because the static VO wasn't sufficient to get to the
   integer value. Hence we add a call to GetQualifiedRepresentationIfAvailable
   (which is what we do for expressions in OptionArgParser::ToAddress too).
2. We weren't passing an ExecutionContextScope to GetByteSize, which
   Swift relied on to get the size of the result type.

My plan is to add an API test for this on the Apple swiftlang/llvm-project fork.

I considered an alternative where we use OptionArgParser::ToAddress
for memory find -e expressions, but it got a bit icky when trying to
figure out how many bytes we should copy out of the result into the
DataBufferHeap (currently we rely on the size of the result variable
type). This gets even trickier when we were to pass an expression that
was actually a hex digit or a number into ToAddress.

rdar://152113525

---

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

3 Files Affected:

• (modified) lldb/source/Commands/CommandObjectMemory.cpp (+68-43)
• (modified) lldb/test/API/functionalities/memory/find/TestMemoryFind.py (+31)
• (modified) lldb/test/API/functionalities/memory/find/main.cpp (+15)

diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 7140333bb3cde..be9ce5e5a3b6e 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -885,6 +885,61 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
 #define LLDB_OPTIONS_memory_find
 #include "CommandOptions.inc"
 
+static llvm::Error CopyExpressionResult(ValueObject &result,
+                                        DataBufferHeap &buffer,
+                                        ExecutionContextScope *scope) {
+  uint64_t value = result.GetValueAsUnsigned(0);
+  auto size_or_err = result.GetCompilerType().GetByteSize(scope);
+  if (!size_or_err)
+    return size_or_err.takeError();
+
+  switch (*size_or_err) {
+  case 1: {
+    uint8_t byte = (uint8_t)value;
+    buffer.CopyData(&byte, 1);
+  } break;
+  case 2: {
+    uint16_t word = (uint16_t)value;
+    buffer.CopyData(&word, 2);
+  } break;
+  case 4: {
+    uint32_t lword = (uint32_t)value;
+    buffer.CopyData(&lword, 4);
+  } break;
+  case 8: {
+    buffer.CopyData(&value, 8);
+  } break;
+  case 3:
+  case 5:
+  case 6:
+  case 7:
+    return llvm::createStringError("unknown type. pass a string instead");
+  default:
+    return llvm::createStringError(
+        "result size larger than 8 bytes. pass a string instead");
+  }
+
+  return llvm::Error::success();
+}
+
+static llvm::Expected<ValueObjectSP>
+EvaluateExpression(llvm::StringRef expression, StackFrame &frame,
+                   Process &process) {
+  ValueObjectSP result_sp;
+  auto status =
+      process.GetTarget().EvaluateExpression(expression, &frame, result_sp);
+  if (status != eExpressionCompleted || !result_sp)
+    return llvm::createStringError(
+        "expression evaluation failed. pass a string instead");
+
+  result_sp = result_sp->GetQualifiedRepresentationIfAvailable(
+      result_sp->GetDynamicValueType(), /*synthValue=*/true);
+  if (!result_sp)
+    return llvm::createStringError("failed to unwrap expression result type");
+
+  return result_sp;
+}
+
 // Find the specified data in memory
 class CommandObjectMemoryFind : public CommandObjectParsed {
 public:
@@ -1026,49 +1081,19 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
       }
       buffer.CopyData(str);
     } else if (m_memory_options.m_expr.OptionWasSet()) {
-      StackFrame *frame = m_exe_ctx.GetFramePtr();
-      ValueObjectSP result_sp;
-      if ((eExpressionCompleted ==
-           process->GetTarget().EvaluateExpression(
-               m_memory_options.m_expr.GetValueAs<llvm::StringRef>().value_or(
-                   ""),
-               frame, result_sp)) &&
-          result_sp) {
-        uint64_t value = result_sp->GetValueAsUnsigned(0);
-        std::optional<uint64_t> size = llvm::expectedToOptional(
-            result_sp->GetCompilerType().GetByteSize(nullptr));
-        if (!size)
-          return;
-        switch (*size) {
-        case 1: {
-          uint8_t byte = (uint8_t)value;
-          buffer.CopyData(&byte, 1);
-        } break;
-        case 2: {
-          uint16_t word = (uint16_t)value;
-          buffer.CopyData(&word, 2);
-        } break;
-        case 4: {
-          uint32_t lword = (uint32_t)value;
-          buffer.CopyData(&lword, 4);
-        } break;
-        case 8: {
-          buffer.CopyData(&value, 8);
-        } break;
-        case 3:
-        case 5:
-        case 6:
-        case 7:
-          result.AppendError("unknown type. pass a string instead");
-          return;
-        default:
-          result.AppendError(
-              "result size larger than 8 bytes. pass a string instead");
-          return;
-        }
-      } else {
-        result.AppendError(
-            "expression evaluation failed. pass a string instead");
+      auto result_or_err = EvaluateExpression(
+          m_memory_options.m_expr.GetValueAs<llvm::StringRef>().value_or(""),
+          m_exe_ctx.GetFrameRef(), *process);
+      if (!result_or_err) {
+        result.AppendError(llvm::toString(result_or_err.takeError()));
+        return;
+      }
+
+      ValueObjectSP result_sp = *result_or_err;
+
+      if (auto err = CopyExpressionResult(*result_sp, buffer,
+                                          m_exe_ctx.GetFramePtr())) {
+        result.AppendError(llvm::toString(std::move(err)));
         return;
       }
     } else {
diff --git a/lldb/test/API/functionalities/memory/find/TestMemoryFind.py b/lldb/test/API/functionalities/memory/find/TestMemoryFind.py
index 09611cc808777..1b9acb8d3090e 100644
--- a/lldb/test/API/functionalities/memory/find/TestMemoryFind.py
+++ b/lldb/test/API/functionalities/memory/find/TestMemoryFind.py
@@ -79,3 +79,34 @@ def test_memory_find(self):
             'memory find -s "nothere" `stringdata` `stringdata+10`',
             substrs=["data not found within the range."],
         )
+
+        # Expression results with unsupported result types.
+        self.expect(
+            'memory find -e "ThreeBytes{}" `&bytedata[0]` `&bytedata[2]`',
+            substrs=["unknown type."],
+            error=True,
+        )
+
+        self.expect(
+            'memory find -e "FiveBytes{}" `&bytedata[0]` `&bytedata[2]`',
+            substrs=["unknown type."],
+            error=True,
+        )
+
+        self.expect(
+            'memory find -e "SixBytes{}" `&bytedata[0]` `&bytedata[2]`',
+            substrs=["unknown type."],
+            error=True,
+        )
+
+        self.expect(
+            'memory find -e "SevenBytes{}" `&bytedata[0]` `&bytedata[2]`',
+            substrs=["unknown type."],
+            error=True,
+        )
+
+        self.expect(
+            'memory find -e "NineBytes{}" `&bytedata[0]` `&bytedata[2]`',
+            substrs=["result size larger than 8 bytes."],
+            error=True,
+        )
diff --git a/lldb/test/API/functionalities/memory/find/main.cpp b/lldb/test/API/functionalities/memory/find/main.cpp
index e3dcfc762ee0f..15c8df1a9fcf1 100644
--- a/lldb/test/API/functionalities/memory/find/main.cpp
+++ b/lldb/test/API/functionalities/memory/find/main.cpp
@@ -1,9 +1,24 @@
 #include <stdio.h>
 #include <stdint.h>
 
+template <size_t T> struct [[gnu::packed]] Payload {
+  uint8_t data[T];
+};
+
+using ThreeBytes = Payload<3>;
+using FiveBytes = Payload<5>;
+using SixBytes = Payload<5>;
+using SevenBytes = Payload<7>;
+using NineBytes = Payload<9>;
+
 int main (int argc, char const *argv[])
 {
     const char* stringdata = "hello world; I like to write text in const char pointers";
     uint8_t bytedata[] = {0xAA,0xBB,0xCC,0xDD,0xEE,0xFF,0x00,0x11,0x22,0x33,0x44,0x55,0x66,0x77,0x88,0x99};
+    ThreeBytes b1;
+    FiveBytes b2;
+    SixBytes b3;
+    SevenBytes b4;
+    NineBytes b5;
     return 0; // break here
 }