[llvm] r307460 - Revert "Revert "Revert "Revert "Switch external cvtres.exe for llvm's own resource library.""""

Robinson, Paul via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 8 12:32:30 PDT 2017


Let me observe that once you get past the second "Revert..." it becomes
noticeably harder to work out what's going on from the subject line.
At that point if you do want to reference the original commit, saying
"Reapply..." would be a lot clearer.
Thanks,
--paulr

> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Eric Beckmann via llvm-commits
> Sent: Friday, July 07, 2017 11:06 PM
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r307460 - Revert "Revert "Revert "Revert "Switch external
> cvtres.exe for llvm's own resource library.""""
> 
> Author: ecbeckmann
> Date: Fri Jul  7 20:06:10 2017
> New Revision: 307460
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=307460&view=rev
> Log:
> Revert "Revert "Revert "Revert "Switch external cvtres.exe for llvm's own
> resource library.""""
> 
> This reverts commit 147f45ff24456aea59575fa4ac16c8fa554df46a.
> 
> Revert "Revert "Revert "Revert "Replace trivial use of external rc.exe by
> writing our own .res file.""""
> 
> This reverts commit 61a90a67ed54a1f0dfeab457b65abffa129569e4.
> 
> The patches were intially reverted because they were causing a failure
> on CrWinClangLLD.  Unfortunately, this was done haphazardly and didn't
> compile, so the revert was reverted again quickly to fix this.  One that
> was done, the revert of the revert was itself reverted.  This allowed me
> to finally fix the actual bug in r307452.  This patch re-enables the
> code path that had originally been causing the bug, now that it (should)
> be fixed.
> 
> Modified:
>     llvm/trunk/include/llvm/BinaryFormat/COFF.h
>     llvm/trunk/include/llvm/Object/WindowsResource.h
>     llvm/trunk/lib/BinaryFormat/Magic.cpp
>     llvm/trunk/lib/Object/WindowsResource.cpp
>     llvm/trunk/tools/llvm-cvtres/llvm-cvtres.cpp
>     llvm/trunk/unittests/BinaryFormat/TestFileMagic.cpp
> 
> Modified: llvm/trunk/include/llvm/BinaryFormat/COFF.h
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/BinaryFormat/COFF.h?rev=307460&r1=307459&r
> 2=307460&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/include/llvm/BinaryFormat/COFF.h (original)
> +++ llvm/trunk/include/llvm/BinaryFormat/COFF.h Fri Jul  7 20:06:10 2017
> @@ -46,6 +46,12 @@ static const char ClGlObjMagic[] = {
>      '\xac', '\x9b', '\xd6', '\xb6', '\x22', '\x26', '\x53', '\xc2',
>  };
> 
> +// The signature bytes that start a .res file.
> +static const char WinResMagic[] = {
> +    '\x00', '\x00', '\x00', '\x00', '\x20', '\x00', '\x00', '\x00',
> +    '\xff', '\xff', '\x00', '\x00', '\xff', '\xff', '\x00', '\x00',
> +};
> +
>  // Sizes in bytes of various things in the COFF format.
>  enum {
>    Header16Size = 20,
> 
> Modified: llvm/trunk/include/llvm/Object/WindowsResource.h
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/include/llvm/Object/WindowsResource.h?rev=307460&r1=307
> 459&r2=307460&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/include/llvm/Object/WindowsResource.h (original)
> +++ llvm/trunk/include/llvm/Object/WindowsResource.h Fri Jul  7 20:06:10
> 2017
> @@ -43,12 +43,47 @@
>  #include <map>
> 
>  namespace llvm {
> -
>  namespace object {
> 
>  class WindowsResource;
> 
> -enum class Machine { UNKNOWN, ARM, X64, X86 };
> +const size_t WIN_RES_MAGIC_SIZE = 16;
> +const size_t WIN_RES_NULL_ENTRY_SIZE = 16;
> +const uint32_t WIN_RES_HEADER_ALIGNMENT = 4;
> +const uint32_t WIN_RES_DATA_ALIGNMENT = 4;
> +const uint16_t WIN_RES_PURE_MOVEABLE = 0x0030;
> +
> +struct WinResHeaderPrefix {
> +  support::ulittle32_t DataSize;
> +  support::ulittle32_t HeaderSize;
> +};
> +
> +// Type and Name may each either be an integer ID or a string.  This
> struct is
> +// only used in the case where they are both IDs.
> +struct WinResIDs {
> +  uint16_t TypeFlag;
> +  support::ulittle16_t TypeID;
> +  uint16_t NameFlag;
> +  support::ulittle16_t NameID;
> +
> +  void setType(uint16_t ID) {
> +    TypeFlag = 0xffff;
> +    TypeID = ID;
> +  }
> +
> +  void setName(uint16_t ID) {
> +    NameFlag = 0xffff;
> +    NameID = ID;
> +  }
> +};
> +
> +struct WinResHeaderSuffix {
> +  support::ulittle32_t DataVersion;
> +  support::ulittle16_t MemoryFlags;
> +  support::ulittle16_t Language;
> +  support::ulittle32_t Version;
> +  support::ulittle32_t Characteristics;
> +};
> 
>  class ResourceEntryRef {
>  public:
> @@ -73,14 +108,6 @@ private:
> 
>    Error loadNext();
> 
> -  struct HeaderSuffix {
> -    support::ulittle32_t DataVersion;
> -    support::ulittle16_t MemoryFlags;
> -    support::ulittle16_t Language;
> -    support::ulittle32_t Version;
> -    support::ulittle32_t Characteristics;
> -  };
> -
>    BinaryStreamReader Reader;
>    bool IsStringType;
>    ArrayRef<UTF16> Type;
> @@ -88,7 +115,7 @@ private:
>    bool IsStringName;
>    ArrayRef<UTF16> Name;
>    uint16_t NameID;
> -  const HeaderSuffix *Suffix = nullptr;
> +  const WinResHeaderSuffix *Suffix = nullptr;
>    ArrayRef<uint8_t> Data;
>    const WindowsResource *OwningRes = nullptr;
>  };
> 
> Modified: llvm/trunk/lib/BinaryFormat/Magic.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/BinaryFormat/Magic.cpp?rev=307460&r1=307459&r2=3074
> 60&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/lib/BinaryFormat/Magic.cpp (original)
> +++ llvm/trunk/lib/BinaryFormat/Magic.cpp Fri Jul  7 20:06:10 2017
> @@ -51,7 +51,8 @@ file_magic llvm::identify_magic(StringRe
>        return file_magic::coff_import_library;
>      }
>      // Windows resource file
> -    if (startswith(Magic, "\0\0\0\0\x20\0\0\0\xFF"))
> +    if (Magic.size() >= sizeof(COFF::WinResMagic) &&
> +        memcmp(Magic.data(), COFF::WinResMagic,
> sizeof(COFF::WinResMagic)) == 0)
>        return file_magic::windows_resource;
>      // 0x0000 = COFF unknown machine type
>      if (Magic[1] == 0)
> 
> Modified: llvm/trunk/lib/Object/WindowsResource.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Object/WindowsResource.cpp?rev=307460&r1=307459&r2=
> 307460&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/lib/Object/WindowsResource.cpp (original)
> +++ llvm/trunk/lib/Object/WindowsResource.cpp Fri Jul  7 20:06:10 2017
> @@ -36,23 +36,19 @@ const uint32_t MIN_HEADER_SIZE = 7 * siz
>  // 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;
> -
>  uint32_t WindowsResourceParser::TreeNode::StringCount = 0;
>  uint32_t WindowsResourceParser::TreeNode::DataCount = 0;
> 
>  WindowsResource::WindowsResource(MemoryBufferRef Source)
>      : Binary(Binary::ID_WinRes, Source) {
> -  size_t LeadingSize = ResourceMagicSize + NullEntrySize;
> +  size_t LeadingSize = WIN_RES_MAGIC_SIZE + WIN_RES_NULL_ENTRY_SIZE;
>    BBS = BinaryByteStream(Data.getBuffer().drop_front(LeadingSize),
>                           support::little);
>  }
> 
>  Expected<std::unique_ptr<WindowsResource>>
>  WindowsResource::createWindowsResource(MemoryBufferRef Source) {
> -  if (Source.getBufferSize() < ResourceMagicSize + NullEntrySize)
> +  if (Source.getBufferSize() < WIN_RES_MAGIC_SIZE +
> WIN_RES_NULL_ENTRY_SIZE)
>      return make_error<GenericBinaryError>(
>          "File too small to be a resource file",
>          object_error::invalid_file_type);
> @@ -105,12 +101,10 @@ static Error readStringOrId(BinaryStream
>  }
> 
>  Error ResourceEntryRef::loadNext() {
> -  uint32_t DataSize;
> -  RETURN_IF_ERROR(Reader.readInteger(DataSize));
> -  uint32_t HeaderSize;
> -  RETURN_IF_ERROR(Reader.readInteger(HeaderSize));
> +  const WinResHeaderPrefix *Prefix;
> +  RETURN_IF_ERROR(Reader.readObject(Prefix));
> 
> -  if (HeaderSize < MIN_HEADER_SIZE)
> +  if (Prefix->HeaderSize < MIN_HEADER_SIZE)
>      return make_error<GenericBinaryError>("Header size is too small.",
>                                            object_error::parse_failed);
> 
> @@ -118,13 +112,13 @@ Error ResourceEntryRef::loadNext() {
> 
>    RETURN_IF_ERROR(readStringOrId(Reader, NameID, Name, IsStringName));
> 
> -  RETURN_IF_ERROR(Reader.padToAlignment(sizeof(uint32_t)));
> +  RETURN_IF_ERROR(Reader.padToAlignment(WIN_RES_HEADER_ALIGNMENT));
> 
>    RETURN_IF_ERROR(Reader.readObject(Suffix));
> 
> -  RETURN_IF_ERROR(Reader.readArray(Data, DataSize));
> +  RETURN_IF_ERROR(Reader.readArray(Data, Prefix->DataSize));
> 
> -  RETURN_IF_ERROR(Reader.padToAlignment(sizeof(uint32_t)));
> +  RETURN_IF_ERROR(Reader.padToAlignment(WIN_RES_DATA_ALIGNMENT));
> 
>    return Error::success();
>  }
> @@ -350,6 +344,7 @@ WindowsResourceCOFFWriter::WindowsResour
>      : MachineType(MachineType), Resources(Parser.getTree()),
>        Data(Parser.getData()), StringTable(Parser.getStringTable()) {
>    performFileLayout();
> +
>    OutputBuffer = MemoryBuffer::getNewMemBuffer(FileSize);
>  }
> 
> @@ -467,8 +462,6 @@ void WindowsResourceCOFFWriter::writeFir
>    SectionOneHeader->PointerToLinenumbers = 0;
>    SectionOneHeader->NumberOfRelocations = Data.size();
>    SectionOneHeader->NumberOfLinenumbers = 0;
> -  SectionOneHeader->Characteristics = COFF::IMAGE_SCN_ALIGN_1BYTES;
> -  SectionOneHeader->Characteristics +=
> COFF::IMAGE_SCN_CNT_INITIALIZED_DATA;
>    SectionOneHeader->Characteristics +=
> COFF::IMAGE_SCN_CNT_INITIALIZED_DATA;
>    SectionOneHeader->Characteristics += COFF::IMAGE_SCN_MEM_READ;
>  }
> 
> Modified: llvm/trunk/tools/llvm-cvtres/llvm-cvtres.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-
> cvtres/llvm-cvtres.cpp?rev=307460&r1=307459&r2=307460&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/tools/llvm-cvtres/llvm-cvtres.cpp (original)
> +++ llvm/trunk/tools/llvm-cvtres/llvm-cvtres.cpp Fri Jul  7 20:06:10 2017
> @@ -207,6 +207,7 @@ int main(int argc_, const char *argv_[])
>    std::copy(OutputBuffer->getBufferStart(), OutputBuffer->getBufferEnd(),
>              FileBuffer->getBufferStart());
>    error(FileBuffer->commit());
> +
>    if (Verbose) {
>      Expected<OwningBinary<Binary>> BinaryOrErr =
> createBinary(OutputFile);
>      if (!BinaryOrErr)
> 
> Modified: llvm/trunk/unittests/BinaryFormat/TestFileMagic.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/unittests/BinaryFormat/TestFileMagic.cpp?rev=307460&r1=
> 307459&r2=307460&view=diff
> ==========================================================================
> ====
> --- llvm/trunk/unittests/BinaryFormat/TestFileMagic.cpp (original)
> +++ llvm/trunk/unittests/BinaryFormat/TestFileMagic.cpp Fri Jul  7
> 20:06:10 2017
> @@ -76,7 +76,8 @@ const char macho_dsym_companion[] =
>      "\xfe\xed\xfa\xce........\x00\x00\x00\x0a............";
>  const char macho_kext_bundle[] =
>      "\xfe\xed\xfa\xce........\x00\x00\x00\x0b............";
> -const char windows_resource[] = "\x00\x00\x00\x00\x020\x00\x00\x00\xff";
> +const char windows_resource[] =
> +    "\x00\x00\x00\x00\x020\x00\x00\x00\xff\xff\x00\x00\xff\xff\x00\x00";
>  const char macho_dynamically_linked_shared_lib_stub[] =
>      "\xfe\xed\xfa\xce........\x00\x00\x00\x09............";
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list