From 31b080d57faa4fb56aa8dbd3a054ab344e0f4c9e Mon Sep 17 00:00:00 2001
From: David Majnemer <david.majnemer@gmail.com>
Date: Sun, 21 Sep 2014 09:18:07 +0000
Subject: [PATCH] MC: Support aligned COMMON symbols for COFF

link.exe:
Fuzz testing has shown that COMMON symbols with size > 32 will always
have an alignment of at least 32 and all symbols with size < 32 will
have an alignment of at least the largest power of 2 less than the size
of the symbol.

binutils:
The BFD linker essentially work like the link.exe behavior but with
alignment 4 instead of 32.  The BFD linker also supports an extension to
COFF which adds an -aligncomm argument to the .drectve section which
permits specifying a precise alignment for a variable but MC currently
doesn't support editing .drectve in this way.

With all of this in mind, we decide to play a little trick: we can
ensure that the alignment will be respected by bumping the size of the
global to it's alignment.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@218201 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/MC/MCObjectFileInfo.cpp |  4 +---
 lib/MC/WinCOFFStreamer.cpp  | 17 +++++++++++++++--
 test/MC/COFF/comm.ll        |  4 ++--
 test/MC/COFF/comm.s         | 14 +++++++++++++-
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/lib/MC/MCObjectFileInfo.cpp b/lib/MC/MCObjectFileInfo.cpp
index da707d860a6..547630ea37d 100644
--- a/lib/MC/MCObjectFileInfo.cpp
+++ b/lib/MC/MCObjectFileInfo.cpp
@@ -595,9 +595,7 @@ void MCObjectFileInfo::InitELFMCObjectFileInfo(Triple T) {
 void MCObjectFileInfo::InitCOFFMCObjectFileInfo(Triple T) {
   bool IsWoA = T.getArch() == Triple::arm || T.getArch() == Triple::thumb;
 
-  // The object file format cannot represent common symbols with explicit
-  // alignments.
-  CommDirectiveSupportsAlignment = false;
+  CommDirectiveSupportsAlignment = true;
 
   // COFF
   BSSSection =
diff --git a/lib/MC/WinCOFFStreamer.cpp b/lib/MC/WinCOFFStreamer.cpp
index 1d03e2d742f..2829a8ff0b5 100644
--- a/lib/MC/WinCOFFStreamer.cpp
+++ b/lib/MC/WinCOFFStreamer.cpp
@@ -183,8 +183,21 @@ void MCWinCOFFStreamer::EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
           Symbol->getSection().getVariant() == MCSection::SV_COFF) &&
          "Got non-COFF section in the COFF backend!");
 
-  if (ByteAlignment > 32)
-    report_fatal_error("alignment is limited to 32-bytes");
+  const Triple &T = getContext().getObjectFileInfo()->getTargetTriple();
+  if (T.isKnownWindowsMSVCEnvironment()) {
+    if (ByteAlignment > 32)
+      report_fatal_error("alignment is limited to 32-bytes");
+  } else {
+    // The bfd linker from binutils only supports alignments less than 4 bytes
+    // without inserting -aligncomm arguments into the .drectve section.
+    // TODO: Support inserting -aligncomm into the .drectve section.
+    if (ByteAlignment > 4)
+      report_fatal_error("alignment is limited to 4-bytes");
+  }
+  // Round size up to alignment so that we will honor the alignment request.
+  // TODO: We don't need to do this if we are targeting the bfd linker once we
+  // add support for adding -aligncomm into the .drectve section.
+  Size = std::max(Size, static_cast<uint64_t>(ByteAlignment));
 
   AssignSection(Symbol, nullptr);
 
diff --git a/test/MC/COFF/comm.ll b/test/MC/COFF/comm.ll
index 6fe122ef1d9..74da557fb5c 100644
--- a/test/MC/COFF/comm.ll
+++ b/test/MC/COFF/comm.ll
@@ -9,5 +9,5 @@
 ; CHECK: .lcomm	_a,1
 ; CHECK: .lcomm	_b,8,8
 ; .comm uses log2 alignment
-; CHECK: .comm	_c,1
-; CHECK: .comm	_d,8
+; CHECK: .comm	_c,1,0
+; CHECK: .comm	_d,8,3
diff --git a/test/MC/COFF/comm.s b/test/MC/COFF/comm.s
index 28d9726fc5e..773ebde0b9d 100644
--- a/test/MC/COFF/comm.s
+++ b/test/MC/COFF/comm.s
@@ -1,7 +1,9 @@
 // RUN: llvm-mc -filetype=obj -triple i686-pc-win32 %s | llvm-readobj -t | FileCheck %s
 
 .lcomm _a,4,4
-.comm	_b, 4
+.comm	_b, 4, 2
+// _c has size 1 but align 32, the value field is the max of size and align.
+.comm	_c, 1, 5
 
 
 // CHECK:       Symbol {
@@ -23,3 +25,13 @@
 // CHECK-NEXT:    StorageClass: External
 // CHECK-NEXT:    AuxSymbolCount: 0
 // CHECK-NEXT:  }
+
+// CHECK:       Symbol {
+// CHECK:         Name: _c
+// CHECK-NEXT:    Value: 32
+// CHECK-NEXT:    Section:  IMAGE_SYM_UNDEFINED (0)
+// CHECK-NEXT:    BaseType: Null
+// CHECK-NEXT:    ComplexType: Null
+// CHECK-NEXT:    StorageClass: External
+// CHECK-NEXT:    AuxSymbolCount: 0
+// CHECK-NEXT:  }
-- 
2.34.1