[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)

[Pavel Labath via lldb-commits]

================
@@ -494,6 +528,23 @@ Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
     }
     return std::make_unique<MemoryListStream>(std::move(Ranges));
   }
+  case StreamKind::Memory64List: {
+    Error Err = Error::success();
+    auto Memory64List = File.getMemory64List(Err);
+    if (Err)
+      return Err;
+    std::vector<Memory64ListStream::entry_type> Ranges;
+    for (const auto &Pair : Memory64List) {
+      Ranges.push_back({Pair.first, Pair.second});
+    }
+
+    // If we don't have an error, or if any of the reads succeed, return ranges
+    // this would also work if we have no descriptors.
+    if (!Err || Ranges.size() > 0)
+      return std::make_unique<Memory64ListStream>(std::move(Ranges));
+
+    return Err;
----------------
labath wrote:

If we encounter an error after reading at least one element, this will assert due to not consuming the error. If this is the behavior you want, you should thrown in an extra `consumeError(std::move(Err))` here.

*However*, I'm not sure this is the best behavior, as we will swallow the error and the caller of this function has no way of knowing that he has given an incomplete range. I *think* it'd be better to just pass the error instead of an incomplete range. I.e., this:
```suggestion
    if (Err) return Err;
    return std::make_unique<Memory64ListStream>(std::move(Ranges));
```

I realize this may seem to contradict what I said earlier about the `getMemory64List`, but there is a logic to that: `getMemory64List` can (thanks to the iterator pattern) return both a partial result *and* and error, and it is this flexibility which allows the callers (such as this function) to choose what they want to do. It's just that for this particular caller, I don't think that a partial result is the most useful mode of operation. Things would be different e.g. for a function that was looking for a particular piece of memory, in which case it could return early (and successfully) even if the rest of the stream was corrupted.

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