[lldb] Fix setting CanJIT if memory cannot be allocated

[igorkudrin]

When a server is unable to allocate memory for the _M packet, it may respond with an error code. In this case,
GDBRemoteCommunicationClient::AllocateMemory() sets m_supports_alloc_dealloc_memory to eLazyBoolYes; eLazyBoolNo is only used if the server cannot handle the packet at all. Before this patch, ProcessGDBRemote::DoAllocateMemory() checked this flag and returned LLDB_INVALID_ADDRESS without setting an error, which caused Process::CanJIT() to set m_can_jit = eCanJITYes, resulting in IRMemoryMap::FindSpace() attempting to allocate memory in the inferior process and failing. With the patch,
ProcessGDBRemote::DoAllocateMemory() returns an error and m_can_jit is set to eCanJITNo.

Example debug session:

(lldb) platform connect...
(lldb) file test
(lldb) br set...
(lldb) run
Process 100 launched:...
Process 100 stopped
* thread #1,...
(lldb) expr $x0
error: Couldn't allocate space for materialized struct: Couldn't malloc: address space is full
error: errored out in virtual lldb_private::LLVMUserExpression::DoExecute, couldn't PrepareToExecuteJITExpression

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Igor Kudrin (igorkudrin)

Changes

When a server is unable to allocate memory for the _M packet, it may respond with an error code. In this case,
GDBRemoteCommunicationClient::AllocateMemory() sets m_supports_alloc_dealloc_memory to eLazyBoolYes; eLazyBoolNo is only used if the server cannot handle the packet at all. Before this patch, ProcessGDBRemote::DoAllocateMemory() checked this flag and returned LLDB_INVALID_ADDRESS without setting an error, which caused Process::CanJIT() to set m_can_jit = eCanJITYes, resulting in IRMemoryMap::FindSpace() attempting to allocate memory in the inferior process and failing. With the patch,
ProcessGDBRemote::DoAllocateMemory() returns an error and m_can_jit is set to eCanJITNo.

Example debug session:

(lldb) platform connect...
(lldb) file test
(lldb) br set...
(lldb) run
Process 100 launched:...
Process 100 stopped
* thread #<!-- -->1,...
(lldb) expr $x0
error: Couldn't allocate space for materialized struct: Couldn't malloc: address space is full
error: errored out in virtual lldb_private::LLVMUserExpression::DoExecute, couldn't PrepareToExecuteJITExpression

---

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

3 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/lldbgdbclient.py (+5-2)
• (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+3-3)
• (added) lldb/test/API/functionalities/gdb_remote_client/TestExprNoAlloc.py (+44)

diff --git a/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py b/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
index 9b2a89e934132..6c68254d42628 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
@@ -32,7 +32,7 @@ def tearDown(self):
         self.server.stop()
         TestBase.tearDown(self)
 
-    def createTarget(self, yaml_path):
+    def createTarget(self, yaml_path, triple=""):
         """
         Create a target by auto-generating the object based on the given yaml
         instructions.
@@ -43,7 +43,10 @@ def createTarget(self, yaml_path):
         yaml_base, ext = os.path.splitext(yaml_path)
         obj_path = self.getBuildArtifact(yaml_base)
         self.yaml2obj(yaml_path, obj_path)
-        return self.dbg.CreateTarget(obj_path)
+        if triple:
+            return self.dbg.CreateTargetWithFileAndTargetTriple(obj_path, triple)
+        else:
+            return self.dbg.CreateTarget(obj_path)
 
     def connect(self, target, plugin="gdb-remote"):
         """
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 80a8f441da12e..445ce820030d3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3170,9 +3170,9 @@ lldb::addr_t ProcessGDBRemote::DoAllocateMemory(size_t size,
 
   if (m_gdb_comm.SupportsAllocDeallocMemory() != eLazyBoolNo) {
     allocated_addr = m_gdb_comm.AllocateMemory(size, permissions);
-    if (allocated_addr != LLDB_INVALID_ADDRESS ||
-        m_gdb_comm.SupportsAllocDeallocMemory() == eLazyBoolYes)
-      return allocated_addr;
+    assert((allocated_addr == LLDB_INVALID_ADDRESS ||
+            m_gdb_comm.SupportsAllocDeallocMemory() == eLazyBoolYes) &&
+           "Memory can only be allocated if the support is enabled");
   }
 
   if (m_gdb_comm.SupportsAllocDeallocMemory() == eLazyBoolNo) {
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestExprNoAlloc.py b/lldb/test/API/functionalities/gdb_remote_client/TestExprNoAlloc.py
new file mode 100644
index 0000000000000..3a77ceed32b57
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestExprNoAlloc.py
@@ -0,0 +1,44 @@
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class MyResponder(MockGDBServerResponder):
+    def other(self, packet) -> str:
+        if packet.startswith("_M"):
+            return "E04"
+        else:
+            return super().other(packet)
+
+    def readRegister(self, regnum):
+        return "E01"
+
+    def readRegisters(self):
+        return "20000000000000002000000000000000f0c154bfffff00005daa985a8fea0b48f0b954bfffff0000ad13cce570150b48380000000000000070456abfffff0000a700000000000000000000000000000001010101010101010000000000000000f0c154bfffff00000f2700000000000008e355bfffff0000080e55bfffff0000281041000000000010de61bfffff00005c05000000000000f0c154bfffff000090fcffffffff00008efcffffffff00008ffcffffffff00000000000000000000001000000000000090fcffffffff000000d06cbfffff0000f0c154bfffff00000100000000000000d0b954bfffff0000e407400000000000d0b954bfffff0000e40740000000000000100000"
+
+
+class TestExprNoAlloc(GDBRemoteTestBase):
+    @skipIfRemote
+    @skipIfLLVMTargetMissing("AArch64")
+    def test(self):
+        """
+        Test that a simple expression can be evaluated when the server supports the '_M'
+        packet, but memory cannot be allocated, and it returns an error code response.
+        In this case, 'CanJIT' used to be set to 'eCanJITYes', so 'IRMemoryMap' tried to
+        allocated memory in the inferior process and failed.
+        """
+
+        self.server.responder = MyResponder()
+        # Note: DynamicLoaderStatic disables JIT by calling 'm_process->SetCanJIT(false)'
+        # in LoadAllImagesAtFileAddresses(). Specifying a triple with "-linux" enables
+        # DynamicLoaderPOSIXDYLD to be used instead.
+        self.target = self.createTarget("basic_eh_frame-aarch64.yaml", "aarch64-linux")
+        process = self.connect(self.target)
+        lldbutil.expect_state_changes(
+            self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+        )
+
+        self.expect_expr("$x1", result_type="unsigned long", result_value="32")

[DavidSpickett]

The logic of the functions involved here is a bit intertwined which gave me some trouble understanding your fix, but this is not your fault and I do agree with the change.

[DavidSpickett]

I think you mean here that:

• We should be able to evaluate an expression that requires no allocations, even if the server responds to _M with an error.

I got a bit confused what "it" was in "and it returns an error code response", was it the expression evaluation or the server.

But anyway, that implies that if we evaluate an expression that does require allocation, then evaluation itself would fail.

Please add another expression to check for that.

[jimingham]

We allocate memory in expression evaluation both to hold the JIT code and the expression results. We inject the result into the target so that you can pass the result of one expression to a subsequent expression even if the expression wants a reference to the value.

If the expression is sufficiently simple that we can IR interpret it in lldb, we won't need to allocate memory for the JIT code, but we will still need to insert the result into the target for reuse. We handle this in the case where there isn't a process, but I'm not sure what happens if we fail to allocate memory for the result when we have a live process.

[igorkudrin]

But anyway, that implies that if we evaluate an expression that does require allocation, then evaluation itself would fail.
Please add another expression to check for that.

I've added an expression (int)foo(); it cannot be evaluated without JIT. I'm not sure it proves anything, since the mock server cannot execute it anyway. Did you mean anything different?

[DavidSpickett]

In light of what Jim said, it's probably not testing what I intended but it can stay.

[DavidSpickett]

LGTM

[augusto2112]

Hi @igorkudrin, this patch broke green dragon: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/37989/console.

[augusto2112]

I've reverted your change for now to unblock the blocks and so you can investigate the failure later.

[igorkudrin]

I've reverted your change for now to unblock the blocks and so you can investigate the failure later.

Thanks!

[igorkudrin]

The buildbot failure was probably caused by a peculiarity of the debug server for macOS. Perhaps it cannot allocate 8 bytes of memory with all permissions set? Unfortunately, I don't have access to any macOS hardware to confirm this and fix the patch.