Implement common symbol size calculation in llvm-size

[khemant]

Summary

Commons do not live in any sections. Their sizes need to be added to total size. This option adds them to total size. In Berkely format, this is shown in bss size. In Sys V format a new row named "*COM*" is added that shows the common totals. Both format will have this added to the grand total if the option is set.

This is similar to GNU size binutils tool.

[khemant]

This change needs update to test case after Rafael's rework on previous patch.

[davide]

This looks fine. Update, I'll take another look and sign off.

Thanks!

Davide

[• rafael]

test/tools/llvm-size/basic.test

14 ↗

(On Diff #46691)

It is really easy to create common symbols with a given size from assembly. Please use llvm-mc instead of checking in a binary.

[davide]

Indeed, as Rafael pointed already out, please convert the test to use MC. Thanks.

[khemant]

Changed test case.

[khemant]

Ping

[• rafael]

test/tools/llvm-size/X86/test-common.s

17

Do you actually need all this? To test just the common size I don't see you need any asm instruction.

tools/llvm-size/llvm-size.cpp

62

git-clang-format

137

TotalCommons

370

CommonSize

[khemant]

test/tools/llvm-size/X86/test-common.s

17

I needed some section with non zero size for case where no common is specified.
Does that matter?

tools/llvm-size/llvm-size.cpp

370

There are variables with all lower case. Do you want me to converst all locals to upper before merging this patch with upper case variables pertaining to this change?

[khemant]

Addressed comments

[khemant]

I will make the blakent style change after this patch. For now I am making the changes you mentioned.

Thanks.

Hemant Kulkarni
[e-mail]
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation

[khemant]

ping

[• rafael]

LGTM

test/tools/llvm-size/X86/test-common.s

+// #Check that with common switch commons are added to bss or 
+// #Shown as *COM* otherwise their size is discounted
+// RUN: llvm-mc %s -o %t.o -filetype=obj -triple=x86_64-pc-linux
+// RUN: llvm-size -A -common %t.o | FileCheck --check-prefix="SYSV" %s
+// RUN: llvm-size -B -common %t.o| FileCheck --check-prefix="BSD" %s
+// RUN: llvm-size -A %t.o | FileCheck --check-prefix="SYSVNOCOMM" %s
+// RUN: llvm-size -B %t.o| FileCheck --check-prefix="BSDNOCOMM" %s
+  .text
+  .globl  main
+  .align  16, 0x90
+  .type main,@function
+main:                                   # @main
+  pushq %rbp
+  movq  %rsp, %rbp
+  movl  $0, -4(%rbp)
+  movl  $1, x
+  movl  $1, y
+  movl  $1, z
+  movl  x, %eax
+  addl  y, %eax
+  addl  z, %eax
+  popq  %rbp
+  retq
+.Ltmp3:
+  .size main, .Ltmp3-main
+  .type x,@object             
+  .comm x,4,4
+  .type y,@object            
+  .comm y,4,4
+  .type z,@object           
+  .comm z,4,4
+// SYSV:      {{[ -\(\)_A-Za-z0-9.\\/:]+}}  :
+// SYSV-NEXT: section   size   addr
+// SYSV-NEXT: .text     67      0
+// SYSV-NEXT: *COM*     12      0
+// SYSV-NEXT: Total     79
+
+// SYSVNOCOMM:      {{[ -\(\)_A-Za-z0-9.\\/:]+}}  :
+// SYSVNOCOMM-NEXT: section   size   addr
+// SYSVNOCOMM-NEXT: .text     67      0
+// SYSVNOCOMM-NEXT: Total     67
+
+// BSD:      text    data     bss     dec     hex filename
+// BSD-NEXT:   67       0      12      79      4f  {{[ -\(\)_A-Za-z0-9.\\/:]+}}
+
+// BSDNOCOMM:      text    data     bss     dec     hex filename
+// BSDNOCOMM-NEXT:   67       0       0      67      43  {{[ -\(\)_A-Za-z0-9.\\/:]+}}

tools/llvm-size/llvm-size.cpp

 DarwinLongFormat("l", cl::desc("When format is darwin, use long format "
                                "to include addresses and offsets."));
 
+cl::opt<bool> ELFCommons("common",
+                         cl::desc("Print common symbols in the ELF file. \
+When using Berkely format, this is added to bss."),
+                         cl::init(false));
+
 static cl::list<std::string>
 ArchFlags("arch", cl::desc("architecture(s) from a Mach-O file to dump"),
           cl::ZeroOrMore);

   return true;
 }
 
+/// Total size of all ELF common symbols
+static uint64_t getCommonSize(ObjectFile *Obj) {
+  uint64_t total_commons = 0;
+  for (auto &Sym : Obj->symbols())
+    if (Obj->getSymbolFlags(Sym.getRawDataRefImpl()) & SymbolRef::SF_Common)
+      total_commons += Obj->getCommonSymbolSize(Sym.getRawDataRefImpl());
+  return total_commons;
+}
+
 /// Print the size of each Mach-O segment and section in @p MachO.
 ///
 /// This is when used when @c OutputFormat is darwin and produces the same

       outs() << format(fmt.str().c_str(), namestr.c_str(), size, addr);
     }
 
+    if (ELFCommons) {
+      uint64_t common_size = getCommonSize(Obj);
+      total += common_size;
+      outs() << format(fmt.str().c_str(), std::string("*COM*").c_str(),
+                       common_size, static_cast<uint64_t>(0));
+    }
+
     // Print total.
     fmtbuf.clear();
     fmt << "%-" << max_name_len << "s "

         total_bss += size;
     }
 
+    if (ELFCommons)
+      total_bss += getCommonSize(Obj);
+
     total = total_text + total_data + total_bss;
 
     if (!BerkeleyHeaderPrinted) {