[PATCH] D134169: [LLD][COFF] Improve symbol table info for import thunk

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

> Import thunks themselves contain a jump or branch, which is code by nature. Therefore the import thunk symbol should be marked as function type in the symbol table to help with debugging.

Totally agree about this.

> The __imp_ import symbol associated to the import thunk is also useful for debugging. However, when the import symbol isn't directly referenced outside of the import thunk, it doesn't normally get added to the symbol table. This change teaches LLD to add the import symbol explicitly.

My initial reaction here is that I wonder when you'd want to look at these symbols in "normal" application level debugging. But on the other hand, I presume we're already inconsistently adding some `__imp_` symbols to the symbol table, but not all of them, so maybe it's just for the best to add all of them. Additionally, if it makes things more consistent, and is useful for lower level debugging (like when figuring out which part of the toolchain is misbehaving), then it's probably good to add. So yeah, +1 from me to this too.



================
Comment at: lld/test/COFF/symtab.test:25
+# DWARF-NEXT:     Value: 128
+# NODWF-NEXT:     Value: 72
+# CHECK-NEXT:     Section: .rdata (2)
----------------
If the actual value isn't very important for the test, one can also just strip this down to `CHECK-NEXT: Value:`, leaving out the value itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134169