[PATCH] D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 11:19:06 PDT 2023

mstorsjo added inline comments.

Comment at: lld/test/COFF/export32.test:83
 # CHECK7:      Ordinal      RVA  Name
-# CHECK7-NEXT:       1        0
+# CHECK7-NEXT:       1   0x1010  exportfn3
 # CHECK7-NEXT:       2   0x1010  foo
aganea wrote:
> mstorsjo wrote:
> > This change doesn't seem consistent and/or directly explainable by the change itself?
> The previous pattern was wrong. The RVA `0` was actually matching the first character of `0x1010`. 
Ok, that makes sense. Please mention and explain that bit in the commit message then!

Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:569
     } else {
-      outs() << format("    % 4d %# 8x", Ordinal, RVA);
+      outs() << format("   %5d %# 8x", Ordinal, RVA);
aganea wrote:
> mstorsjo wrote:
> > This part of the change also seems a bit unrelated to the main change. If you want it bundled here, make sure to at least mention/explain it in the commit message.
> I was planning to commit it separately as a NFC patch, is that ok with you?
Yes, that sounds good, I’d rather have that as a separate commit.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list