[PATCH] D78921: [llvm-objcopy] [COFF] Fix a misconception about debug directory payloads

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/COFF/debug-dir-unmapped.test:1
+# RUN: yaml2obj %s -o %t.in.exe
+
----------------
Could you add a top-level comment to this and the other test saying exactly what this is testing, and how it is testing it (if not obvious from the "what"), please?


================
Comment at: llvm/tools/llvm-objcopy/COFF/Writer.cpp:386
 
+Expected<uint32_t> COFFWriter::virtualAddressToFileAddress(uint32_t Rva) {
+  for (const auto &S : Obj.getSections()) {
----------------
Are COFF addresses limited to 32-bits? Or should this really be a 64-bit value?


================
Comment at: llvm/tools/llvm-objcopy/COFF/Writer.h:48
   Error patchDebugDirectory();
+  Expected<uint32_t> virtualAddressToFileAddress(uint32_t Rva);
 
----------------
I think `RVA` is more correct for acronyms. Same applies in the .cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78921