Page MenuHomePhabricator

[LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset
Needs ReviewPublic

Authored by omjavaid on Wed, Feb 10, 4:09 PM.

Details

Summary

This patch adds a test case to test dynamic register sets. This tests for the availability of certain register sets and query their registers accordingly.

Diff Detail

Event Timeline

omjavaid created this revision.Wed, Feb 10, 4:09 PM
omjavaid requested review of this revision.Wed, Feb 10, 4:09 PM
DavidSpickett added a subscriber: DavidSpickett.
DavidSpickett added inline comments.
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
36

Can you explain the logic for the values here?

40

You don't need the brackets around i for % (i).

50

Is it slightly more strict if we have one loop that reads then writes?

for i in range(32):
   write
   read

Since we write the same value, if you write them all then read them all you aren't totally sure that each one is lined up right. Then again I'm sure you've tested sve register writes elsewhere.

Anyway, a single loop for each would be a bit neater.

112

If you move all this code from after the for, into the for loop, you could skip having the bools per register set.

lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c
15

(I'm not familiar with SVE assembly but anyway..)

Is the .b/.h/.s etc. pattern specific or does it not matter?

32–33

Same here, is the p0/p5/p10/p15 pattern affecting values used in the test or just for fun? (which isn't a bad thing)

69

This appears to be defined but not set.

74

Should there be a #ifndef HWCAP_SVE above too?

83

Would be neat to do these vars like:

unsigned int mte_is_enabled = hwcap2 & HWCAP2_MTE;

We are using SVE read/write in this test to make sure we do not overlap register offsets while calculating offsets for the dynamic registers. Further dynamic register sets should also be readable.

lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
36

P reg sets predicate lanes. P0 will have all lanes set while P5 will have no lanes set. These are just random values testing read/write.

40

Ack.

50

I think you are right. If we do a read after write in a single loop we ll be doing 64 ptrace calls on lldb-server side. More the better.

112

I was actually trying to see whether what cpuinfo reports and what is reported by prctl is same or not.
I am going to revisit this piece and adjust.

lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c
15

Replied in comment below.

32–33

I copied this from SVE test case that I wrote last year.

P is a predicate register and ptrue/pfalse instruction is used to set a predicate lane with a pattern. pattern is decide based on size specifier, b, h, s and d.

if size specified is b all lanes will be set to 1. which is needed to set al bytes in a Z registers to the specified value.

We are setting p0, p5, p10 and p15 to enable all lanes other p registers are not enabling all lanes thats why they were not used as predicate for setting Z registers in following lines which set a constant value to Z register byte size elements.

69

Ack. I ll adjust these as per your suggestion below.

74

Most distros have now backported ptrace.h to include HWCAP_SVE but other two are still in the process.

83

Cool!

DavidSpickett added inline comments.Wed, Feb 17, 2:57 AM
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
36

I would put that in a comment to save some time for those unfamiliar with SVE.

lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c
32–33

I'd put that in a comment. Enough for someone triaging a failure to know what's arbitrary numbers and what's very specific.

omjavaid updated this revision to Diff 325441.Mon, Feb 22, 7:16 AM

Review comments resolved.

Some python nits otherwise the test looks good.

lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
17

These messages are printed on assert failure. So this one would be "Expected a register named...".

33

For this and the other instances of this pattern you an make it one string like:

self.expect("register read z%i" % i, ...
46

Do we need an ffr read if we have a read/write below?

95

These if not could be self.assertFalse.

omjavaid added inline comments.Mon, Feb 22, 11:39 AM
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
17

Agreed.

33

I am kinda old school so choose to write + will fix :)

46

Yes.
There are two values which are being verified:
First we verify value that was written by our assembly code.
After that we use debugger to write and read back values.

95

Yes self.assertTrue I guess. Will fix.

omjavaid updated this revision to Diff 325517.Mon, Feb 22, 11:40 AM
DavidSpickett added inline comments.Tue, Feb 23, 2:35 AM
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
46

Ok but then should the second r/w be of a different value?

program writes A - we read A, so the read works
we write A - we read A, was that our A or the program's A?