[PATCH] D135359: [lld-macho][nfc] define command UNWIND_MODE_MASK for convenience and rewrite mode-mask checking logic for clarity

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo marked 2 inline comments as done.
oontvoo added a comment.

In D135359#3859014 <https://reviews.llvm.org/D135359#3859014>, @int3 wrote:

>> The previous form is currently "harmless" and happened to work (because there's no 0x05000000 in the enum set, but as soon as it's defined, it'll stop working)
>
> I can see why there might be an issue if 0x11000000 was a defined mode, but I don't think 0x05000000 would be a problem?
> <....>
> I guess we can remove this now? Or if you want to be more precise, I think it happens to work because we don't usually get values like `0x04111111` in the input, so there's usually nothing to mask out

Sorry, I'd missed your question about the 0x05000000. 
Consider the struct: (for x86-64, but same issue can be said for the ARM/64 families):

  UNWIND_X86_64_MODE_MASK                              = 0x0F000000,
  UNWIND_X86_64_MODE_RBP_FRAME                   = 0x01000000,
  UNWIND_X86_64_MODE_STACK_IMMD                = 0x02000000,
  UNWIND_X86_64_MODE_STACK_IND                    = 0x03000000,
  UNWIND_X86_64_MODE_DWARF                          = 0x04000000,

Previously, we were doing: `(encoding & MODE_DWARF) == MODE_DWARF`

What I meant was, as soon as a new `UNWIND_X86_64_MODE_FOO = 0x05000000` is defined, then the check above would always return true for `encoding=MODE_FOO` (because `(0b0101 & 0b0100) == 0b0100 `
So no, the explanation still sticks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135359