[PECOFF] Fix common symbol alignment.

[ruiu]

Summary

This is to fix common symbol alignment.

[majnemer]

LGTM

[ruiu]

Closed by commit rL205826 (authored by @ruiu).

lib/ReaderWriter/PECOFF/ReaderCOFF.cpp

 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Memory.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/system_error.h"
 
+#include <algorithm>
 #include <map>
 #include <set>
 #include <vector>
 
 using std::vector;
 using lld::pecoff::COFFAbsoluteAtom;
 using lld::pecoff::COFFBSSAtom;
 using lld::pecoff::COFFDefinedAtom;
for (const coff_symbol *sym : symbols) {
     // mergeable atom. Implement the above semantcis.
     if (sym->SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED &&
         sym->Value > 0) {
       StringRef name = _symbolName[sym];
       uint32_t size = sym->Value;
       auto *atom = new (_alloc)
           COFFBSSAtom(*this, name, getScope(sym), DefinedAtom::permRW_,
                       DefinedAtom::mergeAsWeakAndAddressUsed, size, _ordinal++);
+
+      // Common symbols should be aligned on natural boundaries with the maximum
+      // of 32 byte. It's not documented anywhere, but it's what MSVC link.exe
+      // seems to do.
+      uint64_t alignment = std::min((uint64_t)32, llvm::NextPowerOf2(size));
+      atom->setAlignment(DefinedAtom::Alignment(
+          llvm::countTrailingZeros(alignment)));
       result.push_back(atom);
       continue;
     }
 
     // Skip if it's not for defined atom.
     if (sym->SectionNumber == llvm::COFF::IMAGE_SYM_ABSOLUTE ||
         sym->SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED)
       continue;

test/pecoff/Inputs/common-symbol.obj.yaml

 ---
 header:
   Machine:         IMAGE_FILE_MACHINE_I386
   Characteristics: [  ]
 sections:
   - Name:            .text
     Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
     Alignment:       4
-    SectionData:     ""
+    SectionData:     b800000000b800000000b800000000b800000000b800000000
+    Relocations:
+      - VirtualAddress:  1
+        SymbolName:      _bssdata4
+        Type:            IMAGE_REL_AMD64_ADDR32
+      - VirtualAddress:  6
+        SymbolName:      _bsspad1
+        Type:            IMAGE_REL_AMD64_ADDR32
+      - VirtualAddress:  11
+        SymbolName:      _bssdata64
+        Type:            IMAGE_REL_AMD64_ADDR32
+      - VirtualAddress:  16
+        SymbolName:      _bsspad2
+        Type:            IMAGE_REL_AMD64_ADDR32
+      - VirtualAddress:  21
+        SymbolName:      _bssdata16
+        Type:            IMAGE_REL_AMD64_ADDR32
   - Name:            .data
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
     Alignment:       4
     SectionData:     03000000
 symbols:
   - Name:            .text
     Value:           0
     SectionNumber:   1
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_STATIC
     SectionDefinition:
       Length:          0
-      NumberOfRelocations: 0
+      NumberOfRelocations: 5
       NumberOfLinenumbers: 0
       CheckSum:        0
       Number:          0
   - Name:            .data
     Value:           0
     SectionNumber:   2
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_STATIC
     SectionDefinition:
       Length:          4
       NumberOfRelocations: 0
       NumberOfLinenumbers: 0
       CheckSum:        0
       Number:          0
-  - Name:            _bssdata1
+  - Name:            _bssdata4
     Value:           4
     SectionNumber:   0
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
-  - Name:            _bssdata2
-    Value:           4
+  - Name:            _bsspad1
+    Value:           1
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            _bssdata64
+    Value:           64
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            _bsspad2
+    Value:           1
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            _bssdata16
+    Value:           16
     SectionNumber:   0
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
 ...

test/pecoff/common-symbol.test

 # RUN: yaml2obj %p/Inputs/common-symbol.obj.yaml > %t.obj
 #
-# RUN: lld -flavor link /out:%t.exe /subsystem:console /force /opt:noref \
-# RUN:    -- %t.obj %t.obj
-# RUN: llvm-readobj -sections %t.exe | FileCheck %s
+# RUN: lld -flavor link /machine:x64 /out:%t.exe /subsystem:console /force \
+# RUN:    /opt:noref -- %t.obj %t.obj
+# RUN: llvm-objdump -d %t.exe | FileCheck %s
 
-CHECK:       Section {
-CHECK:         Number: 1
-CHECK-NEXT:    Name: .bss (2E 62 73 73 00 00 00 00)
-CHECK-NEXT:    VirtualSize: 0x0
-CHECK-NEXT:    VirtualAddress: 0x1000
-CHECK-NEXT:    RawDataSize: 8
-CHECK-NEXT:    PointerToRawData: 0x0
-CHECK-NEXT:    PointerToRelocations: 0x0
-CHECK-NEXT:    PointerToLineNumbers: 0x0
-CHECK-NEXT:    RelocationCount: 0
-CHECK-NEXT:    LineNumberCount: 0
-CHECK-NEXT:    Characteristics [
-CHECK-NEXT:      IMAGE_SCN_CNT_UNINITIALIZED_DATA
-CHECK-NEXT:      IMAGE_SCN_MEM_READ
-CHECK-NEXT:      IMAGE_SCN_MEM_WRITE
-CHECK-NEXT:    ]
-CHECK-NEXT:  }
+# Operands of B8 (MOV EAX) are common symbols
+CHECK: 3000: b8 00 10 00 40
+CHECK: 3005: b8 04 10 00 40
+CHECK: 300a: b8 20 10 00 40
+CHECK: 300f: b8 60 10 00 40
+CHECK: 3014: b8 80 10 00 40