[llvm-size] Fix missing file name for darwin output format with non-Mach-O

[xgupta]

Summary

llvm-size falls back to printing in Berkeley format, if --format=darwin is specified and a non-Mach-O object has been provided. However, it does not print the input filename when it should:

Before -
(base) xgupta@archlinux ~/llvm/llvm-project/build (main*) $ llvm-size ~/hello.o --format=darwin

text	   data	    bss	    dec	    hex	filename
 291	      0	      0	    291	    123	%

After -
(base) xgupta@archlinux ~/llvm/llvm-project/build (main*) $ bin/llvm-size ~/hello.o --format=darwin

text	   data	    bss	    dec	    hex	filename
 291	      0	      0	    291	    123	/home/xgupta/hello.o

Fix #42316

[jhenderson]

Thanks for the patch @xgupta. Please could you write a new test (or update an existing one) in the lit testsuite at llvm\test\tools\llvm-size, to make sure this fix is covered by a test.

llvm/tools/llvm-size/llvm-size.cpp

512–513

I'm not sure this is the right place for this patch. The filenames are usually printed elsewhere, at the call site.

[xgupta]

Address comment and update test case

[jhenderson]

There are several other call sites to printObjectSectionSizes in llvm-size. At least one of those (to do with printing archive members) needs updating too (and a corresponding test added). There may be others too, but I think most of the other call sites would result in darwin format printing, since they are for handling Mach-O files or variations thereof. However, you should check.

llvm/test/tools/llvm-size/elf-m.test

9

It might be worth adding {{$}} to the end of this line. This will prevent FileCheck from matching successfully, if there's garbage after the filename.

[xgupta]

Address comment for printing archive statistics

[jhenderson]

Looks good to me, thanks! Do you have commit access?

[xgupta]

In D132364#3757698, @jhenderson wrote:

Looks good to me, thanks! Do you have commit access?

Thank you very much for the review. I do have commit access. I will run check-llvm and then commit it.

llvm/test/tools/llvm-size/archive.test

 # RUN: llvm-size -A %t1.a | count 0
 
 ## Case 2: Single member.
 # RUN: rm -f %t2.a
 # RUN: llvm-ar rc %t2.a %t1
 
 # RUN: llvm-size -B %t2.a | FileCheck %s -DARCHIVE=%t2.a --check-prefix=BERKELEY-1
 # RUN: llvm-size -A %t2.a | FileCheck %s -DARCHIVE=%t2.a --check-prefix=SYSV-1
+# RUN: llvm-size -m %t2.a | FileCheck %s -DARCHIVE=%t2.a --check-prefix=DARWIN-1
 
 ## Case 3: Multiple members.
 # RUN: rm -f %t3.a
 # RUN: llvm-ar rc %t3.a %t1 %t2
 
 # RUN: llvm-size -B %t3.a | FileCheck %s -DARCHIVE=%t3.a --check-prefixes=BERKELEY-1,BERKELEY-2
 # RUN: llvm-size -A %t3.a | FileCheck %s -DARCHIVE=%t3.a --check-prefixes=SYSV-1,SYSV-2
+# RUN: llvm-size -m %t3.a | FileCheck %s -DARCHIVE=%t3.a --check-prefixes=DARWIN-1,DARWIN-2
 
 ## Case 4: Mixing archives and non-archives produces sensible output:
 # RUN: llvm-size -B %t1 %t2.a %t2 %t3.a \
 # RUN:   | FileCheck %s -DARCHIVE=%t3.a -DARCHIVE2=%t2.a -DFILE1=%t1 -DFILE2=%t2 \
 # RUN:                  --check-prefixes=BERKELEY-1,BERKELEY-2,BERKELEY-3
 # RUN: llvm-size -A %t1 %t2.a %t2 %t3.a \
 # RUN:   | FileCheck %s -DARCHIVE=%t3.a -DARCHIVE2=%t2.a -DFILE1=%t1 -DFILE2=%t2 \
 # RUN:                  --check-prefixes=SYSV-1,SYSV-2,SYSV-3
+# RUN: llvm-size -B %t1 %t2.a %t2 %t3.a \
+# RUN:   | FileCheck %s -DARCHIVE=%t3.a -DARCHIVE2=%t2.a -DFILE1=%t1 -DFILE2=%t2 \
+# RUN:                  --check-prefixes=DARWIN-1,DARWIN-2,DARWIN-3
 
 # BERKELEY-1:     text    data     bss     dec     hex filename
 # BERKELEY-3-NEXT:   1       2       4       7       7 [[FILE1]]
 # BERKELEY-3-NEXT:   1       2       4       7       7 archive.test.tmp1 (ex [[ARCHIVE2]])
 # BERKELEY-3-NEXT:   8      16      32      56      38 [[FILE2]]
 # BERKELEY-1-NEXT:   1       2       4       7       7 archive.test.tmp1 (ex [[ARCHIVE]])
 # BERKELEY-2-NEXT:   8      16      32      56      38 archive.test.tmp2 (ex [[ARCHIVE]])
 # BERKELEY-1-NOT:{{.}}

 # SYSV-2-NEXT: archive.test.tmp2   (ex [[ARCHIVE]]):
 # SYSV-2-NEXT: section     size   addr
 # SYSV-2-NEXT: .text          8      32
 # SYSV-2-NEXT: .data         16      16
 # SYSV-2-NEXT: .bss          32      8
 # SYSV-2-NEXT: Total         56
 # SYSV-1-NOT:{{.}}
 
+# DARWIN-1:     text    data     bss     dec     hex filename
+# DARWIN-3-NEXT:   1       2       4       7       7 [[FILE1]]
+# DARWIN-3-NEXT:   1       2       4       7       7 archive.test.tmp1 (ex [[ARCHIVE2]])
+# DARWIN-3-NEXT:   8      16      32      56      38 [[FILE2]]
+# DARWIN-1-NEXT:   1       2       4       7       7 archive.test.tmp1 (ex [[ARCHIVE]])
+# DARWIN-2-NEXT:   8      16      32      56      38 archive.test.tmp2 (ex [[ARCHIVE]])
+# DARWIN-1-NOT:{{.}}
+
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name:    .text

llvm/test/tools/llvm-size/elf-m.test

 ## Show that llvm-size falls back to Berkeley output if --format=darwin is
 ## specified and the input is not Mach-O.
 
 # RUN: yaml2obj %s -o %t
-# RUN: llvm-size -m %t | FileCheck %s
-# RUN: llvm-size --format=darwin %t | FileCheck %s
+# RUN: llvm-size -m %t | FileCheck -DFILE=%t %s
+# RUN: llvm-size --format=darwin %t | FileCheck -DFILE=%t %s
 
-## FIXME: The input filename should be printed but isn't currently due to
-##        https://bugs.llvm.org/show_bug.cgi?id=42971.
 # CHECK:         text    data     bss     dec     hex filename
-# CHECK-NEXT:       0       0       0       0       0
+# CHECK-NEXT:       0       0       0       0       0 [[FILE]]{{$}}
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64

llvm/tools/llvm-size/llvm-size.cpp

fmt << "%#7" << radix_fmt << "\t"
         << "%#7" << radix_fmt << "\t"
         << "%#7" << radix_fmt << "\t";
     outs() << format(fmt.str().c_str(), total_text, total_data, total_bss);
     fmtbuf.clear();
     fmt << "%7" << (Radix == octal ? PRIo64 : PRIu64) << "\t"
         << "%7" PRIx64 "\t";
     outs() << format(fmt.str().c_str(), total, total);
   }
 }
 
 /// Checks to see if the @p O ObjectFile is a Mach-O file and if it is and there
 /// is a list of architecture flags specified then check to make sure this
 /// Mach-O file is one of those architectures or all architectures was
 /// specificed.  If not then an error is generated and this routine returns
 /// false.  Else it returns true.
 static bool checkMachOAndArchFlags(ObjectFile *O, StringRef Filename) {
   auto *MachO = dyn_cast<MachOObjectFile>(O);
 
for (auto &C : a->children(Err)) {
         MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
         if (!checkMachOAndArchFlags(o, file))
           return;
         if (OutputFormat == sysv)
           outs() << o->getFileName() << "   (ex " << a->getFileName() << "):\n";
         else if (MachO && OutputFormat == darwin)
           outs() << a->getFileName() << "(" << o->getFileName() << "):\n";
         printObjectSectionSizes(o);
+        if (!MachO && OutputFormat == darwin)
+          outs() << o->getFileName() << " (ex " << a->getFileName() << ")\n";
         if (OutputFormat == berkeley) {
           if (MachO)
             outs() << a->getFileName() << "(" << o->getFileName() << ")\n";
           else
             outs() << o->getFileName() << " (ex " << a->getFileName() << ")\n";
         }
       }
     }
if (Archive *a = dyn_cast<Archive>(&Bin)) {
     if (!checkMachOAndArchFlags(o, file))
       return;
     MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(o);
     if (OutputFormat == sysv)
       outs() << o->getFileName() << "  :\n";
     else if (MachO && OutputFormat == darwin && MoreThanOneFile)
       outs() << o->getFileName() << ":\n";
     printObjectSectionSizes(o);
+    if (!MachO && OutputFormat == darwin)
+      outs() << o->getFileName() << "\n";
     if (OutputFormat == berkeley) {
       if (!MachO || MoreThanOneFile)
         outs() << o->getFileName();
       outs() << "\n";
     }
   } else {
     error("unsupported file type", file);
   }