Page MenuHomePhabricator

[IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands
ClosedPublic

Authored by vsk on Jun 1 2018, 11:36 AM.

Details

Summary

Change the syntax of the malloc and free commands in lldb-test's
ir-memory-map subcommand to:

<malloc> ::= <label> = malloc <size> <alignment>

<free> ::= free <label>

This should make it easier to read and extend tests in the future, e.g
to test IRMemoryMap::WriteMemory or double-free behavior.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Jun 1 2018, 11:36 AM

While you are at it, can you make sure this works on Windows? The current version of the test that is checked in fails.

vsk added a comment.Jun 1 2018, 12:43 PM

While you are at it, can you make sure this works on Windows? The current version of the test that is checked in fails.

Sorry about that. Could you point me to the error message?

In D47646#1119471, @vsk wrote:

While you are at it, can you make sure this works on Windows? The current version of the test that is checked in fails.

Sorry about that. Could you point me to the error message?

"Cannot use process to test IRMemoryMap"

vsk added a comment.Jun 1 2018, 1:02 PM
In D47646#1119471, @vsk wrote:

While you are at it, can you make sure this works on Windows? The current version of the test that is checked in fails.

Sorry about that. Could you point me to the error message?

"Cannot use process to test IRMemoryMap"

Hm, I'm afraid this just tells us that the debugger either failed to create a process, or that the process is not alive / incapable of JIT'ing. As I don't have a Windows machine on hand, I can't dig into this further. Would you mind taking a look? I can simply disable these tests on Windows for now.

vsk added a comment.Jun 1 2018, 1:03 PM
In D47646#1119487, @vsk wrote:
In D47646#1119471, @vsk wrote:

While you are at it, can you make sure this works on Windows? The current version of the test that is checked in fails.

Sorry about that. Could you point me to the error message?

"Cannot use process to test IRMemoryMap"

Hm, I'm afraid this just tells us that the debugger either failed to create a process, or that the process is not alive / incapable of JIT'ing. As I don't have a Windows machine on hand, I can't dig into this further. Would you mind taking a look? I can simply disable these tests on Windows for now.

One additional thing to try: you can run the lldb-test command with -log path/to/log/file appended. The resulting logging output may help narrow down the process creation issue.

I can look into the failure - but can you XFAIL the test rather than skipping it and log a bug, so that we can track the failure rather than potentially assuming down the line that the test is not meant for windows?

vsk added a comment.Jun 1 2018, 1:37 PM

I can look into the failure - but can you XFAIL the test rather than skipping it and log a bug, so that we can track the failure rather than potentially assuming down the line that the test is not meant for windows?

Sure, I've xfailed the test in r333787 and filed llvm.org/PR37656.

labath accepted this revision.Jun 4 2018, 1:50 AM

Looks great, thanks for doing this.

As far as windows goes, I believe that there we simply don't have the memory allocation part implemented, so it's not surprising that these tests fail. It may be possible that the host-only tests would work (if you remove the CanJIT check in lldb-test), but I can't say that for sure.

tools/lldb-test/lldb-test.cpp
606 ↗(On Diff #149517)

s/startswith/consumeFront. Then, you don't need to do the .find(' ') below.

This revision is now accepted and ready to land.Jun 4 2018, 1:50 AM

Looks great, thanks for doing this.

As far as windows goes, I believe that there we simply don't have the memory allocation part implemented, so it's not surprising that these tests fail. It may be possible that the host-only tests would work (if you remove the CanJIT check in lldb-test), but I can't say that for sure.

I believe you are correct about the implementation - the failure occurs before CanJit is even called though, so I don't know if any of the tests can run on Windows today.

This revision was automatically updated to reflect the committed changes.