[Lldb-commits] [PATCH] D96463: [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:16
+        self.assertTrue(reg_value.IsValid(),
+                        'Verify we have a register named "%s"' % (name))
+        self.assertEqual(reg_value.GetByteSize(), expected,
----------------
DavidSpickett wrote:
> These messages are printed on assert failure. So this one would be "Expected a register named...".
Agreed.


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:32
+                         for _ in range(z_reg_size)) + '}'
+            self.expect("register read " + 'z%i' %
+                        (i), substrs=[z_regs_value])
----------------
DavidSpickett wrote:
> For this and the other instances of this pattern you an make it one string like:
> ```
> self.expect("register read z%i" % i, ...
> ```
I am kinda old school so choose to write + will fix :)


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:45
+
+        self.expect("register read ffr", substrs=[p_regs_value])
+
----------------
DavidSpickett wrote:
> Do we need an ffr read if we have a read/write below?
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.


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:94
+            if 'Scalable Vector Extension Registers' in registerSet.GetName():
+                if not self.isAArch64SVE():
+                    self.fail(
----------------
DavidSpickett wrote:
> These if not could be self.assertFalse.
Yes self.assertTrue I guess. Will fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96463/new/

https://reviews.llvm.org/D96463