[PATCH] D150229: [bolt] Fix typo in BinaryFunction::parseLSDA

Wed May 10 19:01:38 PDT 2023

Amir added a comment.

In D150229#4333580 <https://reviews.llvm.org/D150229#4333580>, @rafauler wrote:

> Apparently this code was introduced at https://reviews.llvm.org/D128561 but no test case was provided showing when LPStart would be present.
>
> Honestly I do not understand why would we subtract Address when LPStart is encoded with absptr but not for other encodings. Of course, talking about the unknown case where LPStart is actually present and not omitted - I still need to find a case where this is actually used. I don't have any test case where LPStart encoding is not set to omit. @Amir any idea on which binary would have that? It would be better to have a testcase where LPStart is actually used, so we can better understand what we are fixing.
>
> I'm inclined towards always decrementing this Address instead of having the if(absptr). Another thing that is wrong is that LPStart can potentially be negative after subtracting Address from it, but the variable itself is declared as unsigned. Confusing. I think it needs to be clear that:
>
> - LPStart is ALWAYS a full pointer (when present). Therefore, if we want to extract an offset out of it, we ought to subtract Address for all cases.
> - LandingPad is ALWAYS an offset.

Sorry about that omission in test. The binary (HHVM) was compiled with Clang with `-fsplit-machine-functions`. Let me see if I can repro it and come up with a reduced test case.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150229