[PATCH] D50203: Find PLT entries for x86, x86_64, and AArch64

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lib/Object/ELFObjectFile.cpp:340
+    return {};
+  uint64_t ExpectedType = 0;
+  switch (Triple.getArch()) {
----------------
Maybe this should be named `JumpSlotReloc`?


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp:165
+         Byte += 4) {
+      uint32_t Insn = (PltContents[Byte + 3] << 24) |
+                      (PltContents[Byte + 2] << 16) |
----------------
You could use the `support::endian::read32le` function here (and elsewhere in the patch).


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp:170
+      // Check for adrp
+      if (Insn >> 31 == 1 && ((Insn >> 24) & 0x1f) == 0x10) {
+        Imm = (((PltSectionVA + Byte) >> 12) << 12) +
----------------
To avoid excessive indentation I'd write this with an early continue.
```
if (Insn >> 31 != 1 || ((Insn >> 24) & 0x1f) != 0x10)
  continue;
uint64_t Imm = ...;
```
I might also simplify the condition like this:
```
if ((Insn & 0x9f000000) != 0x90000000)
  continue;
```


================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp:176
+            (PltContents[Byte + 5] << 8) | (PltContents[Byte + 4] << 0);
+        // Check for ldr
+        if (Insn2 >> 22 == 0x3e5) {
----------------
I'd write out the instruction: `ldr Xt, [Xn, #pimm]` so that it's clear which `ldr` variant you're matching for.


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:449
+  for (uint64_t Byte = 0, End = PltContents.size(); Byte + 6 < End; ) {
+    // Recognize a jmp followed by a push.
+    if (PltContents[Byte] == 0xff && PltContents[Byte + 1] == 0xa3 &&
----------------
I wouldn't try to match the push here (and in the 64-bit version) since it's not essential for decoding the plt entry. It's also theoretically possible for it to be missing, e.g. a linker would be able to avoid emitting it when passed `-z now`.


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:450
+    // Recognize a jmp followed by a push.
+    if (PltContents[Byte] == 0xff && PltContents[Byte + 1] == 0xa3 &&
+        PltContents[Byte + 6] == 0x68) {
----------------
I think you also need to be able to decode the `0xff 0x25` form here, it's used in non-PIC executables.


Repository:
  rL LLVM

https://reviews.llvm.org/D50203