[Lldb-commits] [PATCH] D77000: [lldb] [PECOFF] Check that PECallFrameInfo operates on the expected architecture before interpreting RuntimeFunction structs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77000#1949247 <https://reviews.llvm.org/D77000#1949247>, @aleksandr.urakov wrote:

> Hello! But does the format of an exception directory for aarch64 differ completely from x86-64 one? Can we somehow adopt the existing parser to support it? If we can, I think that this check looks good (we encapsulate the difference inside `PECallFrameInfo`). Or may be it is worth to make a different parser for aarch64? Then I think the check would look better in `ObjectFilePECOFF::CreateEHCallFrameInfo` (and then we will choose a right exception directory parser depending on an architecture). What do you think on this?


I think it would make sense to put the function there (I guess you meant `ObjectFilePECOFF::CreateCallFrameInfo`) even if we later do end up adding arm support to the `PECallFrameInfo` class).

> Not sure how easy it is to make a sensible test for this; it makes a difference when the aarch64 RuntimeFunction struct, when interpreted as an x86_64 RuntimeFunction, happens to falsely match address ranges that the unwinder looks for

One could construct a yaml file with a "falsely matching record" and then run "image show-unwind" to demonstrate that the record is _not_ parsed. The tricky part there is that this command only works on  "live" processes (one day i'd like to change that, but right now it's hard because of... reasons). The "breakpad" unwinding tests work around that by creating a process out of a minidump core file. That might be overkill for a change of this type...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77000