[Lldb-commits] [lldb] [lldb] Handle an empty SBMemoryRegionInfo from scripted process (PR #115963)

[Pavel Labath via lldb-commits]

================
@@ -288,8 +288,15 @@ Status ScriptedProcess::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
                                               MemoryRegionInfo &region) {
   Status error;
   if (auto region_or_err =
-          GetInterface().GetMemoryRegionContainingAddress(load_addr, error))
+          GetInterface().GetMemoryRegionContainingAddress(load_addr, error)) {
     region = *region_or_err;
+    if (region.GetRange().GetRangeBase() == 0 &&
+        (region.GetRange().GetByteSize() == 0 ||
+         region.GetRange().GetByteSize() == LLDB_INVALID_ADDRESS)) {
----------------
labath wrote:

> We had a gdb stub returning {0, UINT64_MAX} the other week and it broke IRMemory::FindSpace() which will avoid any memory region with read/write/execute flags if qMemoryRegionInfo packets are supported. The stub claimed the entire address space, FindSpace() said it could not find any address range available, and all expressions broke.

Okay, but what's the alternative? Picking a piece of memory that may overlap with some existing data? It sounds to me like the stub gets exactly what it asked for.

> Yeah, a range of {0, 1} would result in algorithms like FindSpace() looping for a very long time, and be nearly as bad. But so far the two instances I've seen of people return bad ranges are {0,0} and {0,UINT64_MAX}.

True, but if we can change the expression to catch both, why not do it? What I'm suggesting is to change the expression into something like `if (GetRange().GetRangeBase() > addr || GetRange().GetRangeEnd() <= addr)`. The `(0,0) case is subsumed by that, but this also catches any other incorrect response.

https://github.com/llvm/llvm-project/pull/115963