[PATCH] D50569: Change how we handle -wrap.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/SymbolTable.cpp:159
+  int &Idx2 = Symtab->SymMap[CachedHashStringRef(Real->getName())];
+  int &Idx3 = Symtab->SymMap[CachedHashStringRef(Wrap->getName())];
 
----------------
Maybe naming with a something consistent with the arguments, like `SymIdx`, `RealIdx` and `WrapIdx` would be better?


================
Comment at: lld/ELF/SymbolTable.cpp:162
+  Idx2 = Idx1;
+  Idx1 = Idx3;
 
----------------
It is technically correct I think, but no tests are failing without these 2 lines.
Can it be tested?


================
Comment at: lld/test/ELF/wrap-plt.s:31
+// DISASM-NEXT: jmp    36
+// DISASM-NEXT: jmp    47
+
----------------
I would reveal more information here to show where the `jmp`s are actually pointing to.

```
// DISASM:       _start:
// DISASM-NEXT:   1002:       {{.*}}  jmp     41
// DISASM-NEXT:   1007:       {{.*}}  jmp     36
// DISASM-NEXT:   100c:       {{.*}}  jmp     47

// 0x1002 + 41 + 5 == 0x1030 <__wrap_foo at plt>
// 0x1007 + 36 + 5 == 0x1030 <__wrap_foo at plt>
// 0x100c + 47 + 5 == 0x1040 <_start at plt>
```


https://reviews.llvm.org/D50569