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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to