[COFF] Add support for aligncomm directives

mstorsjo · mstorsjo · commit d2752aa9ecd9 · 2017-08-14T19:07:27.000Z
These are emitted for comm symbols in object files, when targeting a GNU environment. Alternatively, just ignore them since we already align CommonChunk to the natural size of the content (up to 32 bytes). That would only trade away the possibility to overalign small symbols, which doesn't sound like something that might not need to be handled? Differential Revision: https://reviews.llvm.org/D36304 llvm-svn: 310871
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
@@ -377,6 +377,10 @@ CommonChunk::CommonChunk(const COFFSymbolRef S) : Sym(S) {
   Align = std::min(uint64_t(32), PowerOf2Ceil(Sym.getValue()));
 }
 
+void CommonChunk::setAlign(uint32_t NewAlign) {
+  Align = std::max(Align, NewAlign);
+}
+
 uint32_t CommonChunk::getPermissions() const {
   return IMAGE_SCN_CNT_UNINITIALIZED_DATA | IMAGE_SCN_MEM_READ |
          IMAGE_SCN_MEM_WRITE;
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
@@ -243,6 +243,7 @@ class CommonChunk : public Chunk {
   bool hasData() const override { return false; }
   uint32_t getPermissions() const override;
   StringRef getSectionName() const override { return ".bss"; }
+  void setAlign(uint32_t NewAlign);
 
 private:
   const COFFSymbolRef Sym;
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
@@ -139,6 +139,9 @@ struct Configuration {
   StringRef ManifestUIAccess = "'false'";
   StringRef ManifestFile;
 
+  // Used for /aligncomm.
+  std::map<std::string, int> AlignComm;
+
   // Used for /failifmismatch.
   std::map<StringRef, StringRef> MustMatch;
 
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
@@ -202,6 +202,9 @@ void LinkerDriver::parseDirectives(StringRef S) {
 
   for (auto *Arg : Args) {
     switch (Arg->getOption().getUnaliasedOption().getID()) {
+    case OPT_aligncomm:
+      parseAligncomm(Arg->getValue());
+      break;
     case OPT_alternatename:
       parseAlternateName(Arg->getValue());
       break;
@@ -899,6 +902,10 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   for (auto *Arg : Args.filtered(OPT_section))
     parseSection(Arg->getValue());
 
+  // Handle /aligncomm
+  for (auto *Arg : Args.filtered(OPT_aligncomm))
+    parseAligncomm(Arg->getValue());
+
   // Handle /manifestdependency. This enables /manifest unless /manifest:no is
   // also passed.
   if (auto *Arg = Args.getLastArg(OPT_manifestdependency)) {
@@ -1158,6 +1165,23 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
     assignExportOrdinals();
   }
 
+  // Set extra alignment for .comm symbols
+  for (auto Pair : Config->AlignComm) {
+    StringRef Name = Pair.first;
+    int Align = Pair.second;
+    Symbol *Sym = Symtab.find(Name);
+    if (!Sym) {
+      warn("/aligncomm symbol " + Name + " not found");
+      continue;
+    }
+    auto *DC = dyn_cast<DefinedCommon>(Sym->body());
+    if (!DC) {
+      warn("/aligncomm symbol " + Name + " of wrong kind");
+      continue;
+    }
+    DC->getChunk()->setAlign(Align);
+  }
+
   // Windows specific -- Create a side-by-side manifest file.
   if (Config->Manifest == Configuration::SideBySide)
     createSideBySideManifest();
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
@@ -145,6 +145,7 @@ void parseSubsystem(StringRef Arg, WindowsSubsystem *Sys, uint32_t *Major,
 void parseAlternateName(StringRef);
 void parseMerge(StringRef);
 void parseSection(StringRef);
+void parseAligncomm(StringRef);
 
 // Parses a string in the form of "EMBED[,=<integer>]|NO".
 void parseManifest(StringRef Arg);
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
@@ -221,6 +221,22 @@ void parseSection(StringRef S) {
   Config->Section[Name] = parseSectionAttributes(Attrs);
 }
 
+// Parses /aligncomm option argument.
+void parseAligncomm(StringRef S) {
+  StringRef Name, Align;
+  std::tie(Name, Align) = S.split(',');
+  if (Name.empty() || Align.empty()) {
+    error("/aligncomm: invalid argument: " + S);
+    return;
+  }
+  int V;
+  if (Align.getAsInteger(0, V)) {
+    error("/aligncomm: invalid argument: " + S);
+    return;
+  }
+  Config->AlignComm[Name] = std::max(Config->AlignComm[Name], 1 << V);
+}
+
 // Parses a string in the form of "EMBED[,=<integer>]|NO".
 // Results are directly written to Config.
 void parseManifest(StringRef Arg) {
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
@@ -16,6 +16,7 @@ multiclass B<string name, string help> {
 }
 
 def align   : P<"align", "Section alignment">;
+def aligncomm : P<"aligncomm", "Set common symbol alignment">;
 def alternatename : P<"alternatename", "Define weak alias">;
 def base    : P<"base", "Base address of the program">;
 def defaultlib : P<"defaultlib", "Add the library to the list of input files">;
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
@@ -182,7 +182,7 @@ class DefinedCommon : public DefinedCOFF {
   }
 
   uint64_t getRVA() { return Data->getRVA(); }
-  Chunk *getChunk() { return Data; }
+  CommonChunk *getChunk() { return Data; }
 
 private:
   friend SymbolTable;
diff --git a/lld/test/COFF/common-alignment.test b/lld/test/COFF/common-alignment.test
@@ -0,0 +1,78 @@
+# REQUIRES: x86
+# RUN: yaml2obj %s > %t.obj
+# RUN: lld-link /out:%t.exe /entry:main %t.obj %t.obj
+# RUN: llvm-objdump -d %t.exe | FileCheck %s
+
+# Operands of B8 (MOV EAX) are common symbols
+# CHECK: 3000: b8 00 10 00 40
+# CHECK: 3005: b8 10 10 00 40
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     b800000000b800000000
+    Relocations:
+      - VirtualAddress:  1
+        SymbolName:      bssdata4
+        Type:            IMAGE_REL_AMD64_ADDR32
+      - VirtualAddress:  6
+        SymbolName:      bssdata4_align16
+        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
+  - Name:            .drectve
+    Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+    Alignment:       1
+    SectionData:     202d616c69676e636f6d6d3a62737364617461345f616c69676e31362c340a  # -aligncomm:bssdata4_align16,4
+
+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: 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:            main
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            bssdata4
+    Value:           4
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            bssdata4_align16
+    Value:           4
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ multiclass B<string name, string help> {`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`def align : P<"align", "Section alignment">;`
	`19`	`+def aligncomm : P<"aligncomm", "Set common symbol alignment">;`
`19`	`20`	`def alternatename : P<"alternatename", "Define weak alias">;`
`20`	`21`	`def base : P<"base", "Base address of the program">;`
`21`	`22`	`def defaultlib : P<"defaultlib", "Add the library to the list of input files">;`
Original file line number	Diff line number	Diff line change
`@@ -182,7 +182,7 @@ class DefinedCommon : public DefinedCOFF {`
`182`	`182`	`}`
`183`	`183`
`184`	`184`	`uint64_t getRVA() { return Data->getRVA(); }`
`185`		`- Chunk *getChunk() { return Data; }`
	`185`	`+ CommonChunk *getChunk() { return Data; }`
`186`	`186`
`187`	`187`	`private:`
`188`	`188`	`friend SymbolTable;`