[libcxx-commits] [PATCH] D59694: [PPC64][libunwind] Fix r2 not properly restored

[Nemanja Ivanovic via Phabricator via libcxx-commits]

nemanjai added a comment.

Also, is it possible to have a test case for this? I don't actually know what `libunwind` test cases look like, but seems pertinent.



================
Comment at: libunwind/src/DwarfInstructions.hpp:238
+      // then r2 was saved and needs to be restored
+      if (R::getArch() == REGISTERS_PPC64 && returnAddress != 0 &&
+          addressSpace.get32(returnAddress)
----------------
If I follow this correctly, this will just look at the return address and look at the 4 bytes at the address (i.e. the TOC restore that the linker filled in). This seems perfectly reasonable to me but I have a couple of questions:
1. Does `get32()` account for endianness correctly?
2. Why the macro checks at all?

For 2. what I am suggesting is something like:
```
// Look for the TOC restore at the return address.
if (R::getArch() == REGISTERS_PPC64 && returnAddress != 0) {
  pint_t sp = addressSpace.get64(sp + 24);
  pint_t r2 = 0;
  switch (addressSpace.get32(returnAddress)) {
    case 0xe8410018: // ld r2,24(r1) - ELFv2
      pint_t r2 = addressSpace.get64(sp + 24);
      break;
    case 0xe8410028: // ld r2,40(r1) - ELFv1
      r2 = addressSpace.get64(sp + 40);
      break;
  }
  if (r2)
    newRegisters.setRegister(UNW_PPC64_R2, r2);
}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59694