[PECOFF] Fix section alignment.

If you have something like

  __declspec(align(8192)) int foo = 1;

in your code, the compiler makes the data to be aligned to 8192-byte
boundary, and the linker align the section containing the data to 8192.

LLD always aligned the section to 4192. So, as long as alignment
requirement is smaller than 4192, it was correct, but for larger
requirements, it's wrong.

This patch fixes the issue.

llvm-svn: 222043

diff --git a/lld/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp b/lld/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
index a205fad..13f6a2e 100644 (file)
--- a/lld/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
+++ b/lld/lib/ReaderWriter/PECOFF/WriterPECOFF.cpp
@@ -219,6 +219,7 @@ public:
   uint64_t align() const override { return SECTOR_SIZE; }
   uint32_t getCharacteristics() const { return _characteristics; }
   StringRef getSectionName() const { return _sectionName; }
+  virtual uint64_t memAlign() const { return _memAlign; }
 
   static bool classof(const Chunk *c) {
     Kind kind = c->getKind();
@@ -232,16 +233,18 @@ public:
   void setStringTableOffset(uint32_t offset) { _stringTableOffset = offset; }
 
 protected:
-  SectionChunk(Kind kind, StringRef sectionName, uint32_t characteristics)
+  SectionChunk(Kind kind, StringRef sectionName, uint32_t characteristics,
+               const PECOFFLinkingContext &ctx)
       : Chunk(kind), _sectionName(sectionName),
         _characteristics(characteristics), _virtualAddress(0),
-        _stringTableOffset(0) {}
+        _stringTableOffset(0), _memAlign(ctx.getPageSize()) {}
 
 private:
   StringRef _sectionName;
   const uint32_t _characteristics;
   uint64_t _virtualAddress;
   uint32_t _stringTableOffset;
+  uint64_t _memAlign;
 };
 
 /// An AtomChunk represents a section containing atoms.
@@ -252,6 +255,7 @@ public:
 
   void write(uint8_t *buffer) override;
 
+  uint64_t memAlign() const override;
   void appendAtom(const DefinedAtom *atom);
   void buildAtomRvaMap(std::map<const Atom *, uint64_t> &atomRva) const;
 
@@ -293,6 +297,7 @@ private:
 
   mutable llvm::BumpPtrAllocator _alloc;
   llvm::COFF::MachineTypes _machineType;
+  const PECOFFLinkingContext &_ctx;
 };
 
 /// A DataDirectoryChunk represents data directory entries that follows the PE
@@ -334,7 +339,7 @@ class BaseRelocChunk : public SectionChunk {
 
 public:
   BaseRelocChunk(ChunkVectorT &chunks, const PECOFFLinkingContext &ctx)
-      : SectionChunk(kindSection, ".reloc", characteristics),
+      : SectionChunk(kindSection, ".reloc", characteristics, ctx),
         _ctx(ctx), _contents(createContents(chunks)) {}
 
   void write(uint8_t *buffer) override {
@@ -485,8 +490,8 @@ void PEHeaderChunk<PEHeader>::write(uint8_t *buffer) {
 AtomChunk::AtomChunk(const PECOFFLinkingContext &ctx, StringRef sectionName,
                      const std::vector<const DefinedAtom *> &atoms)
     : SectionChunk(kindAtomChunk, sectionName,
-                   computeCharacteristics(ctx, sectionName, atoms)),
-      _virtualAddress(0), _machineType(ctx.getMachineType()) {
+                   computeCharacteristics(ctx, sectionName, atoms), ctx),
+      _virtualAddress(0), _machineType(ctx.getMachineType()), _ctx(ctx) {
   for (auto *a : atoms)
     appendAtom(a);
 }
@@ -727,6 +732,19 @@ void DataDirectoryChunk::write(uint8_t *buffer) {
   std::memcpy(buffer, &_data[0], size());
 }
 
+uint64_t AtomChunk::memAlign() const {
+  // ReaderCOFF propagated the section alignment to the first atom in
+  // the section. We restore that here.
+  if (_atomLayouts.empty())
+    return _ctx.getPageSize();
+  int align = _ctx.getPageSize();
+  for (auto atomLayout : _atomLayouts) {
+    auto *atom = cast<const DefinedAtom>(atomLayout->_atom);
+    align = std::max(align, 1 << atom->alignment().powerOf2);
+  }
+  return align;
+}
+
 void AtomChunk::appendAtom(const DefinedAtom *atom) {
   // Atom may have to be at a proper alignment boundary. If so, move the
   // pointer to make a room after the last atom before adding new one.
@@ -1230,23 +1248,24 @@ void PECOFFWriter::addChunk(Chunk *chunk) {
 void PECOFFWriter::addSectionChunk(std::unique_ptr<SectionChunk> chunk,
                                    SectionHeaderTableChunk *table,
                                    StringTableChunk *stringTable) {
-  SectionChunk &ref = *chunk;
-  _chunks.push_back(std::move(chunk));
-  table->addSection(&ref);
+  table->addSection(chunk.get());
   _numSections++;
 
-  StringRef sectionName = ref.getSectionName();
+  StringRef sectionName = chunk->getSectionName();
   if (sectionName.size() > llvm::COFF::NameSize) {
     uint32_t stringTableOffset = stringTable->addSectionName(sectionName);
-    ref.setStringTableOffset(stringTableOffset);
+    chunk->setStringTableOffset(stringTableOffset);
   }
 
   // Compute and set the starting address of sections when loaded in
   // memory. They are different from positions on disk because sections need
   // to be sector-aligned on disk but page-aligned in memory.
-  ref.setVirtualAddress(_imageSizeInMemory);
   _imageSizeInMemory = llvm::RoundUpToAlignment(
-      _imageSizeInMemory + ref.size(), _ctx.getPageSize());
+      _imageSizeInMemory, chunk->memAlign());
+  chunk->setVirtualAddress(_imageSizeInMemory);
+  _imageSizeInMemory = llvm::RoundUpToAlignment(
+      _imageSizeInMemory + chunk->size(), _ctx.getPageSize());
+  _chunks.push_back(std::move(chunk));
 }
 
 void PECOFFWriter::setImageSizeOnDisk() {

diff --git a/lld/test/pecoff/Inputs/alignment.obj.yaml b/lld/test/pecoff/Inputs/alignment.obj.yaml
index 0d4da1a..1b12da0 100644 (file)
--- a/lld/test/pecoff/Inputs/alignment.obj.yaml
+++ b/lld/test/pecoff/Inputs/alignment.obj.yaml
@@ -31,6 +31,14 @@ sections:
     Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
     Alignment:       16
     SectionData:     0000
+  - Name:            .yyy
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    Alignment:       4096
+    SectionData:     0000
+  - Name:            .zzz
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    Alignment:       16384
+    SectionData:     0000
 symbols:
   - Name:            .text
     Value:           0
@@ -80,4 +88,16 @@ symbols:
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .foo
+    Value:           0
+    SectionNumber:   8
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+  - Name:            .bar
+    Value:           0
+    SectionNumber:   9
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
 ...

diff --git a/lld/test/pecoff/alignment.test b/lld/test/pecoff/alignment.test
index 78f1c97..bdf8bba 100644 (file)
--- a/lld/test/pecoff/alignment.test
+++ b/lld/test/pecoff/alignment.test
@@ -12,3 +12,11 @@ CHECK-NEXT: VirtualSize: 0x6
 
 CHECK:      Name: .text (2E 74 65 78 74 00 00 00)
 CHECK-NEXT: VirtualSize: 0x1001
+
+CHECK:      Name: .yyy
+CHECK-NEXT: VirtualSize: 0x2
+CHECK-NEXT: VirtualAddress: 0x5000
+
+CHECK:      Name: .zzz
+CHECK-NEXT: VirtualSize: 0x2
+CHECK-NEXT: VirtualAddress: 0x8000