[llvm] r305259 - Revert "Revert "Fix alignment bug in COFF emission.""
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 13 11:58:51 PDT 2017
This breaks ubsan again, please fix asap
http://lab.llvm.org:8011/builders/sanitizer-x86_64-
linux-fast/builds/5640/steps/check-llvm%20ubsan/logs/stdio
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Object/WindowsResource.cpp:603:3:
runtime error: store to misaligned address 0x7fe1e5facdb2 for type
'unsigned int', which requires 4 byte alignment
0x7fe1e5facdb2: note: pointer points here
00 00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00
^
#0 0x44fabc in
llvm::object::WindowsResourceCOFFWriter::writeStringTable()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Object/WindowsResource.cpp:603:20
#1 0x44f13b in llvm::object::WindowsResourceCOFFWriter::write()
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Object/WindowsResource.cpp:432:3
#2 0x450840 in
llvm::object::writeWindowsResourceCOFF(llvm::StringRef,
llvm::object::Machine, llvm::object::WindowsResourceParser const&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Object/WindowsResource.cpp:743:17
#3 0x4335af in main
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-cvtres/llvm-cvtres.cpp:198:7
#4 0x7fe1e48f582f in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
On Mon, Jun 12, 2017 at 5:19 PM, Eric Beckmann via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> 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);
> }
> }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170613/bab163a9/attachment.html>
More information about the llvm-commits
mailing list