[Lldb-commits] [PATCH] D96460: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:1128
+  if (error.Success())
   m_sve_header_is_valid = true;
 
----------------
Indent here

Actually, for these you could just do `m_sve_header_is_valid = error.Success()`, no if needed. Assuming setting it false here is also fine, which it might not be.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:1143
+
+  error = ReadRegisterSet(&ioVec, GetSVEHeaderSize(), NT_ARM_PAC_MASK);
+
----------------
Should be `GetPACMaskSize`


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:1146
+  if (error.Success())
+  m_pac_mask_is_valid = true;
+
----------------
see above


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:1218
+
+  if (error.Success())
+    m_mte_ctrl_is_valid = true;
----------------
see above


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:276
+    if (opt_regsets & eRegsetEnableMTE)
+      m_mte_regset_enabled = true;
   }
----------------
(like earlier) would it be ok to remove the if?
```
m_mte_regset_enabled = opt_regsets & eRegsetEnableMTE;
```


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:29
+	eRegsetEnablePAuth = 2,
+	eRegsetEnableMTE = 4
   };
----------------
indentation


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

https://reviews.llvm.org/D96460