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

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked 2 inline comments as done.
aganea added a comment.

@hans @alvinhochun The rationale is to simply remove noise from the output when there are sparse ordinals in the table. I'm not quite sure how useful is to keeps those empty entries, since they will be skipped over by the kernel anyway. The check in COFFObjectFile.cpp, L1567 checks if the RVA is in the output sections. If it's outside the sections, that entry isn't exporting anything, and there will be no dynamic linking at runtime.

In D149610#4311847 <https://reviews.llvm.org/D149610#4311847>, @alvinhochun wrote:

> I don't think we need to follow exactly the behaviour of dumpbin. Can you check what Binutils objdump does just for comparison?

`objdump` skips over empty values, as this patch does (using a test added by D149611 <https://reviews.llvm.org/D149611>):

  C:\git\llvm-project>objdump --version
  GNU objdump (Binutils for MinGW-W64 x86_64, built by Brecht Sanders) 2.36.1
  Copyright (C) 2021 Free Software Foundation, Inc.
  This program is free software; you may redistribute it under the terms of
  the GNU General Public License version 3 or (at your option) any later version.
  This program has absolutely no warranty.
  
  C:\git\llvm-project>objdump -p "C:\git\llvm-project\stage1_msvc_debug\tools\lld\test\COFF\Output\ordinals-override.test.tmp.dll"
  ...
  The Export Tables (interpreted .rdata section contents)
  
  Export Flags                    0
  Time/Date stamp                 0
  Major/Minor                     0/0
  Name                            0000000000016848 ordinals-override.test.tmp.dll
  Ordinal Base                    122
  Number in:
          Export Address Table            00002698
          [Name Pointer/Ordinal] Table    00000003
  Table Addresses
          Export Address Table            0000000000016867
          Name Pointer Table              00000000000202c7
          Ordinal Table                   00000000000202d3
  
  Export Address Table -- Ordinal Base 122
          [   0] +base[ 122] 1010 Export RVA
          [9878] +base[10000] 1000 Export RVA
          [9879] +base[10001] 1020 Export RVA
  
  [Ordinal/Name Pointer] Table
          [   0] ?bar@@YAXXZ
          [9878] ?foo@@YAXXZ
          [9879] baz



================
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
----------------
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`. 


================
Comment at: llvm/tools/llvm-objdump/COFFDump.cpp:569
     } else {
-      outs() << format("    % 4d %# 8x", Ordinal, RVA);
+      outs() << format("   %5d %# 8x", Ordinal, RVA);
     }
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149610