This is an archive of the discontinued LLVM Phabricator instance.

[lldb-test] Add a testing harness for the JIT's IRMemoryMap
ClosedPublic

Authored by vsk on May 29 2018, 6:20 PM.

Details

Summary

This teaches lldb-test how to launch a process, set up an IRMemoryMap,
and issue memory allocations in the target process through the map. This
makes it possible to test IRMemoryMap in a targeted way.

This has uncovered two bugs so far. The first bug is that Malloc
performs an adjustment on the pointer returned from AllocateMemory (for
alignment purposes) which ultimately allows overlapping memory regions
to be created. The second bug is that after most of the address space on
the host side is exhausted, Malloc may return the same address multiple
times. These bugs (and hopefully more!) can be uncovered and tested for
with targeted lldb-test commands.

At an even higher level, the motivation for addressing these bugs is
that they can lead to strange user-visible failures (e.g, variables
assume the wrong value during expression evaluation, or the debugger
crashes). See my third comment on this swift-lldb PR for an example:

https://github.com/apple/swift-lldb/pull/652

I hope lldb-test is the right place to add this testing harness. Setting
up a gtest-style unit test proved too cumbersome (you need to recreate
or mock way too much debugger state), as did writing end-to-end tests
(it's hard to write a test that actually hits a buggy path).

With lldb-test, it's easy to read/generate the test input and parse the
test output. I'll attach a simple "fuzz" tester which generates failing
test cases to the Phab review. Here's an example:

Command: malloc(size=1024, alignment=32)
Malloc: address = 0xca000
Command: malloc(size=64, alignment=16)
Malloc: address = 0xca400
Command: malloc(size=1024, alignment=16)
Malloc: address = 0xca440
Command: malloc(size=16, alignment=8)
Malloc: address = 0xca840
Command: malloc(size=2048, alignment=16)
Malloc: address = 0xcb000
Command: malloc(size=64, alignment=32)
Malloc: address = 0xca860
Command: malloc(size=1024, alignment=16)
Malloc: address = 0xca890
Malloc error: overlapping allocation detected, previous allocation at [0xca860, 0xca8a0)

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.May 29 2018, 6:20 PM
vsk edited the summary of this revision. (Show Details)

The idea that came to me while looking at this is testing this gdb-client style. This would allow you to mock the server responses to allocation and e.g. test handling of allocation failures. However, the problem is these tests sit on top of SBAPI and there seems to be no way to issue "raw" allocation requests through that (although maybe there is a case to be made for SBProcess.AllocateMemory as a generally useful API).

However, if this does the job you want, then I'm fine with that too.

tools/lldb-test/lldb-test.cpp
503 ↗(On Diff #149015)

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.

536–542 ↗(On Diff #149015)

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).

vsk updated this revision to Diff 149159.May 30 2018, 10:43 AM
vsk edited the summary of this revision. (Show Details)
  • Use %zu, and improve detection of overlapping allocations.
tools/lldb-test/lldb-test.cpp
503 ↗(On Diff #149015)

Yes, Line is null-terminated because MemoryBuffer::getFileOrSTDIN defaults to adding a null terminator.

503 ↗(On Diff #149015)

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.

536–542 ↗(On Diff #149015)

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.

vsk updated this revision to Diff 149173.May 30 2018, 11:55 AM
vsk edited the summary of this revision. (Show Details)
  • Really fix the allocation overlap test. The previous version of this patch would not detect overlaps in which the end of the new allocation is contained within an existing allocation.

The idea that came to me while looking at this is testing this gdb-client style. This would allow you to mock the server responses to allocation and e.g. test handling of allocation failures. However, the problem is these tests sit on top of SBAPI and there seems to be no way to issue "raw" allocation requests through that (although maybe there is a case to be made for SBProcess.AllocateMemory as a generally useful API).

However, if this does the job you want, then I'm fine with that too.

Testing at this level looks to be sufficient to uncover the bugs I'm concerned about, so I'd prefer not to extend the SB API if possible.

labath accepted this revision.May 30 2018, 11:57 AM

Thank you for making the changes. This looks fine to me. The more testing, the better.

tools/lldb-test/lldb-test.cpp
532 ↗(On Diff #149159)

%x is not right here. Maybe use formatv?

This revision is now accepted and ready to land.May 30 2018, 11:57 AM
This revision was automatically updated to reflect the committed changes.