[llvm] r305259 - Revert "Revert "Fix alignment bug in COFF emission.""

Eric Beckmann via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 17:19:44 PDT 2017


Author: ecbeckmann
Date: Mon Jun 12 19:19:43 2017
New Revision: 305259

URL: http://llvm.org/viewvc/llvm-project?rev=305259&view=rev
Log:
Revert "Revert "Fix alignment bug in COFF emission.""

This revert was done so that my other patch to add test framework could
land separately.  Now the revert can be reverted and this patch can
reland.

This reverts commit 18b3c75b2b0d32601fb60a06b9672c33d6f0dff9.

Modified:
    llvm/trunk/lib/Object/WindowsResource.cpp

Modified: llvm/trunk/lib/Object/WindowsResource.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/WindowsResource.cpp?rev=305259&r1=305258&r2=305259&view=diff
==============================================================================
--- llvm/trunk/lib/Object/WindowsResource.cpp (original)
+++ llvm/trunk/lib/Object/WindowsResource.cpp Mon Jun 12 19:19:43 2017
@@ -30,6 +30,10 @@ namespace object {
 
 const uint32_t MIN_HEADER_SIZE = 7 * sizeof(uint32_t) + 2 * sizeof(uint16_t);
 
+// COFF files seem to be inconsistent with alignment between sections, just use
+// 8-byte because it makes everyone happy.
+const uint32_t SECTION_ALIGNMENT = sizeof(uint64_t);
+
 static const size_t ResourceMagicSize = 16;
 
 static const size_t NullEntrySize = 16;
@@ -320,7 +324,8 @@ private:
   void writeDirectoryStringTable();
   void writeFirstSectionRelocations();
   std::unique_ptr<FileOutputBuffer> Buffer;
-  uint8_t *Current;
+  uint8_t *BufferStart;
+  uint64_t CurrentOffset = 0;
   Machine MachineType;
   const WindowsResourceParser::TreeNode &Resources;
   const ArrayRef<std::vector<uint8_t>> Data;
@@ -392,6 +397,7 @@ void WindowsResourceCOFFWriter::performS
   FileSize += SectionOneSize;
   FileSize += Data.size() *
               llvm::COFF::RelocationSize; // one relocation for each resource.
+  FileSize = alignTo(FileSize, SECTION_ALIGNMENT);
 }
 
 void WindowsResourceCOFFWriter::performSectionTwoLayout() {
@@ -404,6 +410,7 @@ void WindowsResourceCOFFWriter::performS
     SectionTwoSize += llvm::alignTo(Entry.size(), sizeof(uint64_t));
   }
   FileSize += SectionTwoSize;
+  FileSize = alignTo(FileSize, SECTION_ALIGNMENT);
 }
 
 static std::time_t getTime() {
@@ -414,7 +421,7 @@ static std::time_t getTime() {
 }
 
 Error WindowsResourceCOFFWriter::write() {
-  Current = Buffer->getBufferStart();
+  BufferStart = Buffer->getBufferStart();
 
   writeCOFFHeader();
   writeFirstSectionHeader();
@@ -433,7 +440,8 @@ Error WindowsResourceCOFFWriter::write()
 
 void WindowsResourceCOFFWriter::writeCOFFHeader() {
   // Write the COFF header.
-  auto *Header = reinterpret_cast<llvm::object::coff_file_header *>(Current);
+  auto *Header =
+      reinterpret_cast<llvm::object::coff_file_header *>(BufferStart);
   switch (MachineType) {
   case Machine::ARM:
     Header->Machine = llvm::COFF::IMAGE_FILE_MACHINE_ARMNT;
@@ -458,9 +466,9 @@ void WindowsResourceCOFFWriter::writeCOF
 
 void WindowsResourceCOFFWriter::writeFirstSectionHeader() {
   // Write the first section header.
-  Current += sizeof(llvm::object::coff_file_header);
-  auto *SectionOneHeader =
-      reinterpret_cast<llvm::object::coff_section *>(Current);
+  CurrentOffset += sizeof(llvm::object::coff_file_header);
+  auto *SectionOneHeader = reinterpret_cast<llvm::object::coff_section *>(
+      BufferStart + CurrentOffset);
   strncpy(SectionOneHeader->Name, ".rsrc$01", (size_t)llvm::COFF::NameSize);
   SectionOneHeader->VirtualSize = 0;
   SectionOneHeader->VirtualAddress = 0;
@@ -479,9 +487,9 @@ void WindowsResourceCOFFWriter::writeFir
 
 void WindowsResourceCOFFWriter::writeSecondSectionHeader() {
   // Write the second section header.
-  Current += sizeof(llvm::object::coff_section);
-  auto *SectionTwoHeader =
-      reinterpret_cast<llvm::object::coff_section *>(Current);
+  CurrentOffset += sizeof(llvm::object::coff_section);
+  auto *SectionTwoHeader = reinterpret_cast<llvm::object::coff_section *>(
+      BufferStart + CurrentOffset);
   strncpy(SectionTwoHeader->Name, ".rsrc$02", (size_t)llvm::COFF::NameSize);
   SectionTwoHeader->VirtualSize = 0;
   SectionTwoHeader->VirtualAddress = 0;
@@ -498,75 +506,85 @@ void WindowsResourceCOFFWriter::writeSec
 
 void WindowsResourceCOFFWriter::writeFirstSection() {
   // Write section one.
-  Current += sizeof(llvm::object::coff_section);
+  CurrentOffset += sizeof(llvm::object::coff_section);
 
   writeDirectoryTree();
   writeDirectoryStringTable();
   writeFirstSectionRelocations();
+
+  CurrentOffset = alignTo(CurrentOffset, SECTION_ALIGNMENT);
 }
 
 void WindowsResourceCOFFWriter::writeSecondSection() {
   // Now write the .rsrc$02 section.
   for (auto const &RawDataEntry : Data) {
-    std::copy(RawDataEntry.begin(), RawDataEntry.end(), Current);
-    Current += alignTo(RawDataEntry.size(), sizeof(uint64_t));
+    std::copy(RawDataEntry.begin(), RawDataEntry.end(),
+              BufferStart + CurrentOffset);
+    CurrentOffset += alignTo(RawDataEntry.size(), sizeof(uint64_t));
   }
+
+  CurrentOffset = alignTo(CurrentOffset, SECTION_ALIGNMENT);
 }
 
 void WindowsResourceCOFFWriter::writeSymbolTable() {
   // Now write the symbol table.
   // First, the feat symbol.
-  auto *Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(Current);
+  auto *Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(BufferStart +
+                                                                 CurrentOffset);
   strncpy(Symbol->Name.ShortName, "@feat.00", (size_t)llvm::COFF::NameSize);
   Symbol->Value = 0x11;
   Symbol->SectionNumber = 0xffff;
   Symbol->Type = llvm::COFF::IMAGE_SYM_DTYPE_NULL;
   Symbol->StorageClass = llvm::COFF::IMAGE_SYM_CLASS_STATIC;
   Symbol->NumberOfAuxSymbols = 0;
-  Current += sizeof(llvm::object::coff_symbol16);
+  CurrentOffset += sizeof(llvm::object::coff_symbol16);
 
   // Now write the .rsrc1 symbol + aux.
-  Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(Current);
+  Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(BufferStart +
+                                                           CurrentOffset);
   strncpy(Symbol->Name.ShortName, ".rsrc$01", (size_t)llvm::COFF::NameSize);
   Symbol->Value = 0;
   Symbol->SectionNumber = 1;
   Symbol->Type = llvm::COFF::IMAGE_SYM_DTYPE_NULL;
   Symbol->StorageClass = llvm::COFF::IMAGE_SYM_CLASS_STATIC;
   Symbol->NumberOfAuxSymbols = 1;
-  Current += sizeof(llvm::object::coff_symbol16);
-  auto *Aux =
-      reinterpret_cast<llvm::object::coff_aux_section_definition *>(Current);
+  CurrentOffset += sizeof(llvm::object::coff_symbol16);
+  auto *Aux = reinterpret_cast<llvm::object::coff_aux_section_definition *>(
+      BufferStart + CurrentOffset);
   Aux->Length = SectionOneSize;
   Aux->NumberOfRelocations = Data.size();
   Aux->NumberOfLinenumbers = 0;
   Aux->CheckSum = 0;
   Aux->NumberLowPart = 0;
   Aux->Selection = 0;
-  Current += sizeof(llvm::object::coff_aux_section_definition);
+  CurrentOffset += sizeof(llvm::object::coff_aux_section_definition);
 
   // Now write the .rsrc2 symbol + aux.
-  Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(Current);
+  Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(BufferStart +
+                                                           CurrentOffset);
   strncpy(Symbol->Name.ShortName, ".rsrc$02", (size_t)llvm::COFF::NameSize);
   Symbol->Value = 0;
   Symbol->SectionNumber = 2;
   Symbol->Type = llvm::COFF::IMAGE_SYM_DTYPE_NULL;
   Symbol->StorageClass = llvm::COFF::IMAGE_SYM_CLASS_STATIC;
   Symbol->NumberOfAuxSymbols = 1;
-  Current += sizeof(llvm::object::coff_symbol16);
-  Aux = reinterpret_cast<llvm::object::coff_aux_section_definition *>(Current);
+  CurrentOffset += sizeof(llvm::object::coff_symbol16);
+  Aux = reinterpret_cast<llvm::object::coff_aux_section_definition *>(
+      BufferStart + CurrentOffset);
   Aux->Length = SectionTwoSize;
   Aux->NumberOfRelocations = 0;
   Aux->NumberOfLinenumbers = 0;
   Aux->CheckSum = 0;
   Aux->NumberLowPart = 0;
   Aux->Selection = 0;
-  Current += sizeof(llvm::object::coff_aux_section_definition);
+  CurrentOffset += sizeof(llvm::object::coff_aux_section_definition);
 
   // Now write a symbol for each relocation.
   for (unsigned i = 0; i < Data.size(); i++) {
     char RelocationName[9];
     sprintf(RelocationName, "$R%06X", DataOffsets[i]);
-    Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(Current);
+    Symbol = reinterpret_cast<llvm::object::coff_symbol16 *>(BufferStart +
+                                                             CurrentOffset);
     strncpy(Symbol->Name.ShortName, RelocationName,
             (size_t)llvm::COFF::NameSize);
     Symbol->Value = DataOffsets[i];
@@ -574,14 +592,15 @@ void WindowsResourceCOFFWriter::writeSym
     Symbol->Type = llvm::COFF::IMAGE_SYM_DTYPE_NULL;
     Symbol->StorageClass = llvm::COFF::IMAGE_SYM_CLASS_STATIC;
     Symbol->NumberOfAuxSymbols = 0;
-    Current += sizeof(llvm::object::coff_symbol16);
+    CurrentOffset += sizeof(llvm::object::coff_symbol16);
   }
 }
 
 void WindowsResourceCOFFWriter::writeStringTable() {
   // Just 4 null bytes for the string table.
-  auto COFFStringTable = reinterpret_cast<void *>(Current);
-  memset(COFFStringTable, 0, 4);
+  auto COFFStringTable =
+      reinterpret_cast<uint32_t *>(BufferStart + CurrentOffset);
+  *COFFStringTable = 0;
 }
 
 void WindowsResourceCOFFWriter::writeDirectoryTree() {
@@ -599,8 +618,8 @@ void WindowsResourceCOFFWriter::writeDir
   while (!Queue.empty()) {
     auto CurrentNode = Queue.front();
     Queue.pop();
-    auto *Table =
-        reinterpret_cast<llvm::object::coff_resource_dir_table *>(Current);
+    auto *Table = reinterpret_cast<llvm::object::coff_resource_dir_table *>(
+        BufferStart + CurrentOffset);
     Table->Characteristics = CurrentNode->getCharacteristics();
     Table->TimeDateStamp = 0;
     Table->MajorVersion = CurrentNode->getMajorVersion();
@@ -609,13 +628,13 @@ void WindowsResourceCOFFWriter::writeDir
     auto &StringChildren = CurrentNode->getStringChildren();
     Table->NumberOfNameEntries = StringChildren.size();
     Table->NumberOfIDEntries = IDChildren.size();
-    Current += sizeof(llvm::object::coff_resource_dir_table);
+    CurrentOffset += sizeof(llvm::object::coff_resource_dir_table);
     CurrentRelativeOffset += sizeof(llvm::object::coff_resource_dir_table);
 
     // Write the directory entries immediately following each directory table.
     for (auto const &Child : StringChildren) {
-      auto *Entry =
-          reinterpret_cast<llvm::object::coff_resource_dir_entry *>(Current);
+      auto *Entry = reinterpret_cast<llvm::object::coff_resource_dir_entry *>(
+          BufferStart + CurrentOffset);
       Entry->Identifier.NameOffset =
           StringTableOffsets[Child.second->getStringIndex()];
       if (Child.second->checkIsDataNode()) {
@@ -630,12 +649,12 @@ void WindowsResourceCOFFWriter::writeDir
                                sizeof(llvm::object::coff_resource_dir_entry);
         Queue.push(Child.second.get());
       }
-      Current += sizeof(llvm::object::coff_resource_dir_entry);
+      CurrentOffset += sizeof(llvm::object::coff_resource_dir_entry);
       CurrentRelativeOffset += sizeof(llvm::object::coff_resource_dir_entry);
     }
     for (auto const &Child : IDChildren) {
-      auto *Entry =
-          reinterpret_cast<llvm::object::coff_resource_dir_entry *>(Current);
+      auto *Entry = reinterpret_cast<llvm::object::coff_resource_dir_entry *>(
+          BufferStart + CurrentOffset);
       Entry->Identifier.ID = Child.first;
       if (Child.second->checkIsDataNode()) {
         Entry->Offset.DataEntryOffset = NextLevelOffset;
@@ -649,7 +668,7 @@ void WindowsResourceCOFFWriter::writeDir
                                sizeof(llvm::object::coff_resource_dir_entry);
         Queue.push(Child.second.get());
       }
-      Current += sizeof(llvm::object::coff_resource_dir_entry);
+      CurrentOffset += sizeof(llvm::object::coff_resource_dir_entry);
       CurrentRelativeOffset += sizeof(llvm::object::coff_resource_dir_entry);
     }
   }
@@ -657,14 +676,14 @@ void WindowsResourceCOFFWriter::writeDir
   RelocationAddresses.resize(Data.size());
   // Now write all the resource data entries.
   for (auto DataNodes : DataEntriesTreeOrder) {
-    auto *Entry =
-        reinterpret_cast<llvm::object::coff_resource_data_entry *>(Current);
+    auto *Entry = reinterpret_cast<llvm::object::coff_resource_data_entry *>(
+        BufferStart + CurrentOffset);
     RelocationAddresses[DataNodes->getDataIndex()] = CurrentRelativeOffset;
     Entry->DataRVA = 0; // Set to zero because it is a relocation.
     Entry->DataSize = Data[DataNodes->getDataIndex()].size();
     Entry->Codepage = 0;
     Entry->Reserved = 0;
-    Current += sizeof(llvm::object::coff_resource_data_entry);
+    CurrentOffset += sizeof(llvm::object::coff_resource_data_entry);
     CurrentRelativeOffset += sizeof(llvm::object::coff_resource_data_entry);
   }
 }
@@ -673,16 +692,17 @@ void WindowsResourceCOFFWriter::writeDir
   // Now write the directory string table for .rsrc$01
   uint32_t TotalStringTableSize = 0;
   for (auto String : StringTable) {
-    auto *LengthField = reinterpret_cast<uint16_t *>(Current);
+    auto *LengthField =
+        reinterpret_cast<uint16_t *>(BufferStart + CurrentOffset);
     uint16_t Length = String.size();
     *LengthField = Length;
-    Current += sizeof(uint16_t);
-    auto *Start = reinterpret_cast<UTF16 *>(Current);
+    CurrentOffset += sizeof(uint16_t);
+    auto *Start = reinterpret_cast<UTF16 *>(BufferStart + CurrentOffset);
     std::copy(String.begin(), String.end(), Start);
-    Current += Length * sizeof(UTF16);
+    CurrentOffset += Length * sizeof(UTF16);
     TotalStringTableSize += Length * sizeof(UTF16) + sizeof(uint16_t);
   }
-  Current +=
+  CurrentOffset +=
       alignTo(TotalStringTableSize, sizeof(uint32_t)) - TotalStringTableSize;
 }
 
@@ -693,7 +713,8 @@ void WindowsResourceCOFFWriter::writeFir
   // .rsrc section.
   uint32_t NextSymbolIndex = 5;
   for (unsigned i = 0; i < Data.size(); i++) {
-    auto *Reloc = reinterpret_cast<llvm::object::coff_relocation *>(Current);
+    auto *Reloc = reinterpret_cast<llvm::object::coff_relocation *>(
+        BufferStart + CurrentOffset);
     Reloc->VirtualAddress = RelocationAddresses[i];
     Reloc->SymbolTableIndex = NextSymbolIndex++;
     switch (MachineType) {
@@ -709,7 +730,7 @@ void WindowsResourceCOFFWriter::writeFir
     default:
       Reloc->Type = 0;
     }
-    Current += sizeof(llvm::object::coff_relocation);
+    CurrentOffset += sizeof(llvm::object::coff_relocation);
   }
 }
 




More information about the llvm-commits mailing list