[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.


================
Comment at: lldb/include/lldb/Target/AppleArm64ExceptionClass.h:14
+
+enum class AppleArm64ExceptionClass : unsigned {
+#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code) Name = Code,
----------------
shafik wrote:
> We should use fixed sized integer types whenever possible, I suppose in this case `uint32_t`.
I don't think we should use fixed size integer types in cases where the size doesn't matter (e.g. when uint8_t would do, as it would in this case), since it's distracting.


================
Comment at: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp:112
+          auto exception_class =
+              static_cast<AppleArm64ExceptionClass>(esr_val >> 26);
+          if (exception_class !=
----------------
jasonmolenda wrote:
> shafik wrote:
> > Does `26` have a meaning? I am guessing we are shifting to get the EC bits?
> Yeah I think the comment table in AppleArm64ExceptionClass.def would be better placed here, showing the bit positions of the three fields in the esr register probably.  AppleArm64ExceptionClass.def should make it clear that it is operating on the exception class field of the esr register, but the bit positions are more relevant here.
Good point - actually, maybe it's cleaner to keep knowledge of the EC bit position in the header. Lmk what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795