Re-land r361206 "[COFF] Store alignment in log2 form, NFC"

The previous patch lost the call to PowerOf2Ceil, which causes LLD to
crash when handling common symbols with a non-power-of-2 size. I tweaked
the existing common.test to make the bsspad16 common symbol be 15 bytes
to add coverage for this case.

llvm-svn: 361426

8 files changed, 56 insertions, 32 deletions

diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 88ab1103d42..76b0c8301b9 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -41,7 +41,7 @@ SectionChunk::SectionChunk(ObjFile *F, const coff_section *H)
   SectionNameData = SectionName.data();
   SectionNameSize = SectionName.size();
 
-  Alignment = Header->getAlignment();
+  setAlignment(Header->getAlignment());
 
   // If linker GC is disabled, every chunk starts out alive.  If linker GC is
   // enabled, treat non-comdat sections as roots. Generally optimized object
@@ -53,7 +53,7 @@ SectionChunk::SectionChunk(ObjFile *F, const coff_section *H)
 // SectionChunk is one of the most frequently allocated classes, so it is
 // important to keep it as compact as possible. As of this writing, the number
 // below is the size of this class on x64 platforms.
-static_assert(sizeof(SectionChunk) <= 112, "SectionChunk grew unexpectedly");
+static_assert(sizeof(SectionChunk) <= 104, "SectionChunk grew unexpectedly");
 
 static void add16(uint8_t *P, int16_t V) { write16le(P, read16le(P) + V); }
 static void add32(uint8_t *P, int32_t V) { write32le(P, read32le(P) + V); }
@@ -625,7 +625,7 @@ SectionChunk *SectionChunk::findByName(ArrayRef<SectionChunk *> Sections,
 }
 
 void SectionChunk::replace(SectionChunk *Other) {
-  Alignment = std::max(Alignment, Other->Alignment);
+  P2Align = std::max(P2Align, Other->P2Align);
   Other->Repl = Repl;
   Other->Live = false;
 }
@@ -638,9 +638,10 @@ uint32_t SectionChunk::getSectionNumber() const {
 }
 
 CommonChunk::CommonChunk(const COFFSymbolRef S) : Sym(S) {
-  // Common symbols are aligned on natural boundaries up to 32 bytes.
+  // The value of a common symbol is its size. Align all common symbols smaller
+  // than 32 bytes naturally, i.e. round the size up to the next power of two.
   // This is what MSVC link.exe does.
-  Alignment = std::min(uint64_t(32), PowerOf2Ceil(Sym.getValue()));
+  setAlignment(std::min(32U, uint32_t(PowerOf2Ceil(Sym.getValue()))));
 }
 
 uint32_t CommonChunk::getOutputCharacteristics() const {
@@ -656,7 +657,7 @@ void StringChunk::writeTo(uint8_t *Buf) const {
 ImportThunkChunkX64::ImportThunkChunkX64(Defined *S) : ImpSymbol(S) {
   // Intel Optimization Manual says that all branch targets
   // should be 16-byte aligned. MSVC linker does this too.
-  Alignment = 16;
+  setAlignment(16);
 }
 
 void ImportThunkChunkX64::writeTo(uint8_t *Buf) const {
@@ -854,17 +855,20 @@ uint8_t Baserel::getDefaultType() {
   }
 }
 
-std::map<uint32_t, MergeChunk *> MergeChunk::Instances;
+MergeChunk *MergeChunk::Instances[Log2MaxSectionAlignment + 1] = {};
 
 MergeChunk::MergeChunk(uint32_t Alignment)
     : Builder(StringTableBuilder::RAW, Alignment) {
-  this->Alignment = Alignment;
+  setAlignment(Alignment);
 }
 
 void MergeChunk::addSection(SectionChunk *C) {
-  auto *&MC = Instances[C->Alignment];
+  assert(isPowerOf2_32(C->getAlignment()));
+  uint8_t P2Align = llvm::Log2_32(C->getAlignment());
+  assert(P2Align >= 0 && P2Align < array_lengthof(Instances));
+  auto *&MC = Instances[P2Align];
   if (!MC)
-    MC = make<MergeChunk>(C->Alignment);
+    MC = make<MergeChunk>(C->getAlignment());
   MC->Sections.push_back(C);
 }
 
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index 2b3bd29e1c2..8525feedf2e 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -44,6 +44,9 @@ const uint32_t PermMask = 0xFE000000;
 // Mask for section types (code, data, bss).
 const uint32_t TypeMask = 0x000000E0;
 
+// The log base 2 of the largest section alignment, which is log2(8192), or 13.
+enum : unsigned { Log2MaxSectionAlignment = 13 };
+
 // A Chunk represents a chunk of data that will occupy space in the
 // output (if the resolver chose that). It may or may not be backed by
 // a section of an input file. It could be linker-created data, or
@@ -57,6 +60,18 @@ public:
   // Returns the size of this chunk (even if this is a common or BSS.)
   virtual size_t getSize() const = 0;
 
+  // Returns chunk alignment in power of two form. Value values are powers of
+  // two from 1 to 8192.
+  uint32_t getAlignment() const { return 1U << P2Align; }
+  void setAlignment(uint32_t Align) {
+    // Treat zero byte alignment as 1 byte alignment.
+    Align = Align ? Align : 1;
+    assert(llvm::isPowerOf2_32(Align) && "alignment is not a power of 2");
+    P2Align = llvm::Log2_32(Align);
+    assert(P2Align <= Log2MaxSectionAlignment &&
+           "impossible requested alignment");
+  }
+
   // Write this chunk to a mmap'ed file, assuming Buf is pointing to
   // beginning of the file. Because this function may use RVA values
   // of other chunks for relocations, you need to set them properly
@@ -103,9 +118,6 @@ public:
   // bytes, so this is used only for logging or debugging.
   virtual StringRef getDebugName() { return ""; }
 
-  // The alignment of this chunk. The writer uses the value.
-  uint32_t Alignment = 1;
-
   virtual bool isHotPatchable() const { return false; }
 
 protected:
@@ -118,6 +130,10 @@ public:
   bool KeepUnique = false;
 
 protected:
+  // The alignment of this chunk, stored in log2 form. The writer uses the
+  // value.
+  uint8_t P2Align = 0;
+
   // The RVA of this chunk in the output. The writer sets a value.
   uint32_t RVA = 0;
 
@@ -313,7 +329,7 @@ public:
   size_t getSize() const override;
   void writeTo(uint8_t *Buf) const override;
 
-  static std::map<uint32_t, MergeChunk *> Instances;
+  static MergeChunk *Instances[Log2MaxSectionAlignment + 1];
   std::vector<SectionChunk *> Sections;
 
 private:
@@ -437,7 +453,7 @@ public:
 class LocalImportChunk : public Chunk {
 public:
   explicit LocalImportChunk(Defined *S) : Sym(S) {
-    Alignment = Config->Wordsize;
+    setAlignment(Config->Wordsize);
   }
   size_t getSize() const override;
   void getBaserels(std::vector<Baserel> *Res) override;
@@ -528,7 +544,7 @@ class PseudoRelocTableChunk : public Chunk {
 public:
   PseudoRelocTableChunk(std::vector<RuntimePseudoReloc> &Relocs)
       : Relocs(std::move(Relocs)) {
-    Alignment = 4;
+    setAlignment(4);
   }
   size_t getSize() const override;
   void writeTo(uint8_t *Buf) const override;
@@ -558,7 +574,7 @@ public:
 class AbsolutePointerChunk : public Chunk {
 public:
   AbsolutePointerChunk(uint64_t Value) : Value(Value) {
-    Alignment = getSize();
+    setAlignment(getSize());
   }
   size_t getSize() const override;
   void writeTo(uint8_t *Buf) const override;
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 815ba275ef2..b2bf53285af 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -59,7 +59,9 @@ private:
 // A chunk for the import descriptor table.
 class LookupChunk : public Chunk {
 public:
-  explicit LookupChunk(Chunk *C) : HintName(C) { Alignment = Config->Wordsize; }
+  explicit LookupChunk(Chunk *C) : HintName(C) {
+    setAlignment(Config->Wordsize);
+  }
   size_t getSize() const override { return Config->Wordsize; }
 
   void writeTo(uint8_t *Buf) const override {
@@ -78,7 +80,7 @@ public:
 class OrdinalOnlyChunk : public Chunk {
 public:
   explicit OrdinalOnlyChunk(uint16_t V) : Ordinal(V) {
-    Alignment = Config->Wordsize;
+    setAlignment(Config->Wordsize);
   }
   size_t getSize() const override { return Config->Wordsize; }
 
@@ -364,7 +366,7 @@ public:
 class DelayAddressChunk : public Chunk {
 public:
   explicit DelayAddressChunk(Chunk *C) : Thunk(C) {
-    Alignment = Config->Wordsize;
+    setAlignment(Config->Wordsize);
   }
   size_t getSize() const override { return Config->Wordsize; }
 
@@ -576,7 +578,7 @@ void DelayLoadContents::create(Defined *H) {
     for (int I = 0, E = Syms.size(); I < E; ++I)
       Syms[I]->setLocation(Addresses[Base + I]);
     auto *MH = make<NullChunk>(8);
-    MH->Alignment = 8;
+    MH->setAlignment(8);
     ModuleHandles.push_back(MH);
 
     // Fill the delay import table header fields.
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 04db2339c8a..605f52e11a2 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -1743,7 +1743,7 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
       continue;
 
     CommonChunk *C = DC->getChunk();
-    C->Alignment = std::max(C->Alignment, Alignment);
+    C->setAlignment(std::max(C->getAlignment(), Alignment));
   }
 
   // Windows specific -- Create a side-by-side manifest file.
diff --git a/lld/COFF/ICF.cpp b/lld/COFF/ICF.cpp
index 7ddb7b0f22a..7e105ddad75 100644
--- a/lld/COFF/ICF.cpp
+++ b/lld/COFF/ICF.cpp
@@ -256,9 +256,10 @@ void ICF::run(ArrayRef<Chunk *> Vec) {
 
   // Make sure that ICF doesn't merge sections that are being handled by string
   // tail merging.
-  for (auto &P : MergeChunk::Instances)
-    for (SectionChunk *SC : P.second->Sections)
-      SC->Class[0] = NextId++;
+  for (MergeChunk *MC : MergeChunk::Instances)
+    if (MC)
+      for (SectionChunk *SC : MC->Sections)
+        SC->Class[0] = NextId++;
 
   // Initially, we use hash values to partition sections.
   parallelForEach(Chunks, [&](SectionChunk *SC) {
diff --git a/lld/COFF/MapFile.cpp b/lld/COFF/MapFile.cpp
index fc5af18b3de..aade7ad5229 100644
--- a/lld/COFF/MapFile.cpp
+++ b/lld/COFF/MapFile.cpp
@@ -114,7 +114,7 @@ void coff::writeMapFile(ArrayRef<OutputSection *> OutputSections) {
       if (!SC)
         continue;
 
-      writeHeader(OS, SC->getRVA(), SC->getSize(), SC->Alignment);
+      writeHeader(OS, SC->getRVA(), SC->getSize(), SC->getAlignment());
       OS << Indent8 << SC->File->getName() << ":(" << SC->getSectionName()
          << ")\n";
       for (DefinedRegular *Sym : SectionSyms[SC])
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 46e8ee66393..f629715fbd2 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -865,8 +865,9 @@ void Writer::createSections() {
 }
 
 void Writer::createMiscChunks() {
-  for (auto &P : MergeChunk::Instances)
-    RdataSec->addChunk(P.second);
+  for (MergeChunk *P : MergeChunk::Instances)
+    if (P)
+      RdataSec->addChunk(P);
 
   // Create thunks for locally-dllimported symbols.
   if (!Symtab->LocalImportChunks.empty()) {
@@ -1159,7 +1160,7 @@ void Writer::assignAddresses() {
     for (Chunk *C : Sec->Chunks) {
       if (Padding && C->isHotPatchable())
         VirtualSize += Padding;
-      VirtualSize = alignTo(VirtualSize, C->Alignment);
+      VirtualSize = alignTo(VirtualSize, C->getAlignment());
       C->setRVA(RVA + VirtualSize);
       C->finalizeContents();
       VirtualSize += C->getSize();
@@ -1519,8 +1520,8 @@ void Writer::createGuardCFTables() {
 
   // Ensure sections referenced in the gfid table are 16-byte aligned.
   for (const ChunkAndOffset &C : AddressTakenSyms)
-    if (C.InputChunk->Alignment < 16)
-      C.InputChunk->Alignment = 16;
+    if (C.InputChunk->getAlignment() < 16)
+      C.InputChunk->setAlignment(16);
 
   maybeAddRVATable(std::move(AddressTakenSyms), "__guard_fids_table",
                    "__guard_fids_count");
diff --git a/lld/test/COFF/common.test b/lld/test/COFF/common.test
index 7d11da2ae15..23f746a58f8 100644
--- a/lld/test/COFF/common.test
+++ b/lld/test/COFF/common.test
@@ -79,7 +79,7 @@ symbols:
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
   - Name:            bssdata16
-    Value:           16
+    Value:           15
     SectionNumber:   0
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL