[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.


================
Comment at: tools/lldb-test/lldb-test.cpp:503
+  uint8_t Alignment;
+  int Matches = sscanf(Line.data(), "malloc %lu %hhu", &Size, &Alignment);
+  if (Matches != 2)
----------------
labath wrote:
> is `Line` null-terminated here? Also a size_t arg should have a `%zu` modifier, but I am not sure if all msvc versions support that. It might be best to make the type uint64_t and then use SCNu64.
Yes, `Line` is null-terminated because `MemoryBuffer::getFileOrSTDIN` defaults to adding a null terminator.


================
Comment at: tools/lldb-test/lldb-test.cpp:503
+  uint8_t Alignment;
+  int Matches = sscanf(Line.data(), "malloc %lu %hhu", &Size, &Alignment);
+  if (Matches != 2)
----------------
vsk wrote:
> labath wrote:
> > is `Line` null-terminated here? Also a size_t arg should have a `%zu` modifier, but I am not sure if all msvc versions support that. It might be best to make the type uint64_t and then use SCNu64.
> Yes, `Line` is null-terminated because `MemoryBuffer::getFileOrSTDIN` defaults to adding a null terminator.
LLVM currently requires MSVC >= 2015 Update 3 (see: https://reviews.llvm.org/D47073), which supports %zu (see: https://blogs.msdn.microsoft.com/vcblog/2014/06/03/visual-studio-14-ctp/#div-comment-77743). I'll just use %zu.


================
Comment at: tools/lldb-test/lldb-test.cpp:536-542
+  bool Overlaps = AllocatedIntervals.lookup(Addr, false);
+  if (Size && !Overlaps)
+    Overlaps = AllocatedIntervals.lookup(Addr + Size - 1, false);
+  if (Overlaps) {
+    outs() << "Malloc error: overlapping allocation detected\n";
+    exit(1);
+  }
----------------
labath wrote:
> It looks like this won't detect the case when a larger interval is placed on top of a smaller one (e.g. `0x1000-0x4000` and `0x2000-0x3000`).
Thanks for pointing this out. I wasn't sure how to do this efficiently. Taking another look at things, it looks like IntervalMap surfaces iterators which can be used to scan a range quickly. I'll try that out.


https://reviews.llvm.org/D47508