Use llvm's own resource libraries instead of shelling out to external cvtres.

[ecbeckmann]

Summary

Changed WindowsResourceCOFFWriter to write to a memory buffer instead of file. Also got rid of my own Machine enum and use the one defined in COFF.h

[ecbeckmann]

rebase

[ecbeckmann]

Fixed a bug where .rsrc sections were marked as discardable memory, and were therefore incorrectly moved to the end of the image.

[ecbeckmann]

Being reviewed in D34265.

lld/COFF/DriverUtils.cpp

 #include "Config.h"
 #include "Driver.h"
 #include "Error.h"
 #include "Memory.h"
 #include "Symbols.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Object/COFF.h"
+#include "llvm/Object/WindowsResource.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
fatal("/failifmismatch: mismatch detected: " + Existing + " and " + V +
           " for key " + K);
   Config->MustMatch[K] = V;
 }
 
 // Convert Windows resource files (.res files) to a .obj file
 // using cvtres.exe.
 std::unique_ptr<MemoryBuffer>
 convertResToCOFF(const std::vector<MemoryBufferRef> &MBs) {
-  // Create an output file path.
-  TemporaryFile File("resource-file", "obj");
+  object::WindowsResourceParser Parser;
 
-  // Execute cvtres.exe.
-  Executor E("cvtres.exe");
-  E.add("/machine:" + machineToStr(Config->Machine));
-  E.add("/readonly");
-  E.add("/nologo");
-  E.add("/out:" + Twine(File.Path));
-
-  // We must create new files because the memory buffers we have may have no
-  // underlying file still existing on the disk.
-  // It happens if it was created from a TemporaryFile, which usually delete
-  // the file just after creating the MemoryBuffer.
-  std::vector<TemporaryFile> ResFiles;
-  ResFiles.reserve(MBs.size());
   for (MemoryBufferRef MB : MBs) {
-    // We store the temporary file in a vector to avoid deletion
-    // before running cvtres
-    ResFiles.emplace_back("resource-file", "res");
-    TemporaryFile& ResFile = ResFiles.back();
-    // Write the content of the resource in a temporary file
-    std::error_code EC;
-    raw_fd_ostream OS(ResFile.Path, EC, sys::fs::F_None);
-    if (EC)
-      fatal(EC, "failed to open " + ResFile.Path);
-    OS << MB.getBuffer();
-    OS.close();
-
-    E.add(ResFile.Path);
+    auto Binary = check(object::createBinary(MB));
+    object::WindowsResource *RF =
+        dyn_cast<object::WindowsResource>(Binary.get());
+    if (!RF)
+      fatal("Cannot compile non-resource file as resource");
+    if (auto EC = Parser.parse(RF))
+      fatal(EC, "Failed to parse .res file.");
   }
 
-  E.run();
-  return File.getMemoryBuffer();
+  std::unique_ptr<MemoryBuffer> OutputBuffer;
+  if (auto EC = llvm::object::writeWindowsResourceCOFF(OutputBuffer,
+                                                       Config->Machine, Parser))
+    fatal(EC, "Failed to write resources to buffer.");
+
+  return OutputBuffer;
 }
 
 // Run MSVC link.exe for given in-memory object files.
 // Command line options are copied from those given to LLD.
 // This is for the /msvclto option.
 void runMSVCLinker(std::string Rsp, ArrayRef<StringRef> Objects) {
   // Write the in-memory object files to disk.
   std::vector<TemporaryFile> Temps;

lld/COFF/Writer.cpp

void Writer::createSections() {
   std::map<StringRef, std::vector<Chunk *>> Map;
   for (Chunk *C : Symtab->getChunks()) {
     auto *SC = dyn_cast<SectionChunk>(C);
     if (SC && !SC->isLive()) {
       if (Config->Verbose)
         SC->printDiscardedMessage();
       continue;
     }
+    llvm::outs() << "binning " << C->getSectionName() << "\n";
     Map[C->getSectionName()].push_back(C);
   }
 
   // Then create an OutputSection for each section.
   // '$' and all following characters in input section names are
   // discarded when determining output section. So, .text$foo
   // contributes to .text, for example. See PE/COFF spec 3.2.
   SmallDenseMap<StringRef, OutputSection *> Sections;
   for (auto Pair : Map) {
     StringRef Name = getOutputSection(Pair.first);
     OutputSection *&Sec = Sections[Name];
+    llvm::outs() << "got " << Name << "\n";
     if (!Sec) {
+      llvm::outs() << "pushing\n";
       Sec = make<OutputSection>(Name);
       OutputSections.push_back(Sec);
     }
     std::vector<Chunk *> &Chunks = Pair.second;
     for (Chunk *C : Chunks) {
       Sec->addChunk(C);
       Sec->addPermissions(C->getPermissions());
     }
void Writer::assignAddresses() {
   // the loader cannot handle holes.
   std::stable_partition(
       OutputSections.begin(), OutputSections.end(), [](OutputSection *S) {
         return (S->getPermissions() & IMAGE_SCN_MEM_DISCARDABLE) == 0;
       });
   for (OutputSection *Sec : OutputSections) {
     if (Sec->getName() == ".reloc")
       addBaserels(Sec);
+    char hexRVA[20];
+    sprintf(hexRVA, "%x", RVA);
+    llvm::outs() << Sec->getName() << ": " << hexRVA << "\n";
     Sec->setRVA(RVA);
     Sec->setFileOffset(FileSize);
     RVA += alignTo(Sec->getVirtualSize(), PageSize);
     FileSize += alignTo(Sec->getRawSize(), SectorSize);
   }
   SizeOfImage = SizeOfHeaders + alignTo(RVA - 0x1000, PageSize);
 }
 

lld/test/COFF/resource.test

-# REQUIRES: winres
-
 # RUN: yaml2obj < %p/Inputs/ret42.yaml > %t.obj
 # RUN: lld-link /out:%t.exe /entry:main %t.obj %p/Inputs/resource.res
 
 # Check if the binary contains UTF-16LE string "Hello" copied from resource.res.
 # RUN: FileCheck --check-prefix=EXE %s < %t.exe
 
 EXE: {{H.e.l.l.o}}
 
 # RUN: llvm-readobj -file-headers %t.exe | FileCheck  --check-prefix=HEADER %s
 
 HEADER: ResourceTableRVA: 0x1000
 HEADER: ResourceTableSize: 0x88

lld/test/lit.cfg

 if re.search(r'PowerPC', archs):
     config.available_features.add('ppc')
 if re.search(r'AMDGPU', archs):
     config.available_features.add('amdgpu')
 llvm_config_cmd.wait()
 
 # Set a fake constant version so that we get consitent output.
 config.environment['LLD_VERSION'] = 'LLD 1.0'
-
-# Check if Windows resource file compiler exists.
-cvtres = lit.util.which('cvtres', config.environment['PATH'])
-rc = lit.util.which('rc', config.environment['PATH'])
-if cvtres and rc:
-    config.available_features.add('winres')

llvm/include/llvm/Object/WindowsResource.h

 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include <map>
 
 namespace llvm {
 
-class FileOutputBuffer;
-
 namespace object {
 
 class WindowsResource;
 
-enum class Machine { UNKNOWN, ARM, X64, X86 };
-
 class ResourceEntryRef {
 public:
   Error moveNext(bool &End);
   bool checkTypeString() const { return IsStringType; }
   ArrayRef<UTF16> getTypeString() const { return Type; }
   uint16_t getTypeID() const { return TypeID; }
   bool checkNameString() const { return IsStringName; }
   ArrayRef<UTF16> getNameString() const { return Name; }
public:
   };
 
 private:
   TreeNode Root;
   std::vector<std::vector<uint8_t>> Data;
   std::vector<std::vector<UTF16>> StringTable;
 };
 
-Error writeWindowsResourceCOFF(StringRef OutputFile, Machine MachineType,
+Error writeWindowsResourceCOFF(std::unique_ptr<MemoryBuffer> &OutputBuffer,
+                               llvm::COFF::MachineTypes MachineType,
                                const WindowsResourceParser &Parser);
 
 } // namespace object
 } // namespace llvm
 
 #endif

llvm/lib/Object/WindowsResource.cpp

 #include "llvm/Object/COFF.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/MathExtras.h"
 #include <ctime>
 #include <queue>
 #include <sstream>
 #include <system_error>
 
+using namespace llvm;
+
 namespace llvm {
 namespace object {
 
 #define RETURN_IF_ERROR(X)                                                     \
   if (auto EC = X)                                                             \
     return EC;
 
 const uint32_t MIN_HEADER_SIZE = 7 * sizeof(uint32_t) + 2 * sizeof(uint16_t);
uint32_t WindowsResourceParser::TreeNode::getTreeSize() const {
   for (auto const &Child : IDChildren) {
     Size += Child.second->getTreeSize();
   }
   return Size;
 }
 
 class WindowsResourceCOFFWriter {
 public:
-  WindowsResourceCOFFWriter(StringRef OutputFile, Machine MachineType,
+  WindowsResourceCOFFWriter(std::unique_ptr<MemoryBuffer> &OutputBuffer,
+                            llvm::COFF::MachineTypes MachineType,
                             const WindowsResourceParser &Parser, Error &E);
   Error write();
 
 private:
   void performFileLayout();
   void performSectionOneLayout();
   void performSectionTwoLayout();
   void writeCOFFHeader();
   void writeFirstSectionHeader();
   void writeSecondSectionHeader();
   void writeFirstSection();
   void writeSecondSection();
   void writeSymbolTable();
   void writeStringTable();
   void writeDirectoryTree();
   void writeDirectoryStringTable();
   void writeFirstSectionRelocations();
-  std::unique_ptr<FileOutputBuffer> Buffer;
-  uint8_t *BufferStart;
+  std::unique_ptr<MemoryBuffer> &OutputBuffer;
+  char *BufferStart;
   uint64_t CurrentOffset = 0;
-  Machine MachineType;
+  llvm::COFF::MachineTypes MachineType;
   const WindowsResourceParser::TreeNode &Resources;
   const ArrayRef<std::vector<uint8_t>> Data;
   uint64_t FileSize;
   uint32_t SymbolTableOffset;
   uint32_t SectionOneSize;
   uint32_t SectionOneOffset;
   uint32_t SectionOneRelocations;
   uint32_t SectionTwoSize;
   uint32_t SectionTwoOffset;
   const ArrayRef<std::vector<UTF16>> StringTable;
   std::vector<uint32_t> StringTableOffsets;
   std::vector<uint32_t> DataOffsets;
   std::vector<uint32_t> RelocationAddresses;
 };
 
 WindowsResourceCOFFWriter::WindowsResourceCOFFWriter(
-    StringRef OutputFile, Machine MachineType,
-    const WindowsResourceParser &Parser, Error &E)
-    : MachineType(MachineType), Resources(Parser.getTree()),
-      Data(Parser.getData()), StringTable(Parser.getStringTable()) {
+    std::unique_ptr<MemoryBuffer> &OutputBuffer,
+    llvm::COFF::MachineTypes MachineType, const WindowsResourceParser &Parser,
+    Error &E)
+    : OutputBuffer(OutputBuffer), MachineType(MachineType),
+      Resources(Parser.getTree()), Data(Parser.getData()),
+      StringTable(Parser.getStringTable()) {
   performFileLayout();
 
-  ErrorOr<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
-      FileOutputBuffer::create(OutputFile, FileSize);
-  if (!BufferOrErr) {
-    E = errorCodeToError(BufferOrErr.getError());
-    return;
-  }
-
-  Buffer = std::move(*BufferOrErr);
+  OutputBuffer = std::move(MemoryBuffer::getNewMemBuffer(FileSize));
 }
 
 void WindowsResourceCOFFWriter::performFileLayout() {
   // Add size of COFF header.
   FileSize = llvm::COFF::Header16Size;
 
   // one .rsrc section header for directory tree, another for resource data.
   FileSize += 2 * llvm::COFF::SectionSize;

 static std::time_t getTime() {
   std::time_t Now = time(nullptr);
   if (Now < 0 || !isUInt<32>(Now))
     return UINT32_MAX;
   return Now;
 }
 
 Error WindowsResourceCOFFWriter::write() {
-  BufferStart = Buffer->getBufferStart();
+  BufferStart = const_cast<char *>(OutputBuffer->getBufferStart());
 
   writeCOFFHeader();
   writeFirstSectionHeader();
   writeSecondSectionHeader();
   writeFirstSection();
   writeSecondSection();
   writeSymbolTable();
   writeStringTable();
 
-  if (auto EC = Buffer->commit()) {
-    return errorCodeToError(EC);
-  }
-
   return Error::success();
 }
 
 void WindowsResourceCOFFWriter::writeCOFFHeader() {
   // Write the COFF header.
   auto *Header =
       reinterpret_cast<llvm::object::coff_file_header *>(BufferStart);
   switch (MachineType) {
-  case Machine::ARM:
+  case COFF::IMAGE_FILE_MACHINE_ARMNT:
     Header->Machine = llvm::COFF::IMAGE_FILE_MACHINE_ARMNT;
     break;
-  case Machine::X64:
+  case COFF::IMAGE_FILE_MACHINE_AMD64:
     Header->Machine = llvm::COFF::IMAGE_FILE_MACHINE_AMD64;
     break;
-  case Machine::X86:
+  case COFF::IMAGE_FILE_MACHINE_I386:
     Header->Machine = llvm::COFF::IMAGE_FILE_MACHINE_I386;
     break;
   default:
     Header->Machine = llvm::COFF::IMAGE_FILE_MACHINE_UNKNOWN;
   }
   Header->NumberOfSections = 2;
   Header->TimeDateStamp = getTime();
   Header->PointerToSymbolTable = SymbolTableOffset;
void WindowsResourceCOFFWriter::writeFirstSectionHeader() {
   SectionOneHeader->PointerToRawData = SectionOneOffset;
   SectionOneHeader->PointerToRelocations = SectionOneRelocations;
   SectionOneHeader->PointerToLinenumbers = 0;
   SectionOneHeader->NumberOfRelocations = Data.size();
   SectionOneHeader->NumberOfLinenumbers = 0;
   SectionOneHeader->Characteristics = llvm::COFF::IMAGE_SCN_ALIGN_1BYTES;
   SectionOneHeader->Characteristics +=
       llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA;
-  SectionOneHeader->Characteristics += llvm::COFF::IMAGE_SCN_MEM_DISCARDABLE;
+  SectionOneHeader->Characteristics +=
+      llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA;
   SectionOneHeader->Characteristics += llvm::COFF::IMAGE_SCN_MEM_READ;
 }
 
 void WindowsResourceCOFFWriter::writeSecondSectionHeader() {
   // Write the second section header.
   CurrentOffset += sizeof(llvm::object::coff_section);
   auto *SectionTwoHeader = reinterpret_cast<llvm::object::coff_section *>(
       BufferStart + CurrentOffset);
void WindowsResourceCOFFWriter::writeFirstSectionRelocations() {
   // .rsrc section.
   uint32_t NextSymbolIndex = 5;
   for (unsigned i = 0; i < Data.size(); i++) {
     auto *Reloc = reinterpret_cast<llvm::object::coff_relocation *>(
         BufferStart + CurrentOffset);
     Reloc->VirtualAddress = RelocationAddresses[i];
     Reloc->SymbolTableIndex = NextSymbolIndex++;
     switch (MachineType) {
-    case Machine::ARM:
+    case COFF::IMAGE_FILE_MACHINE_ARMNT:
       Reloc->Type = llvm::COFF::IMAGE_REL_ARM_ADDR32NB;
       break;
-    case Machine::X64:
+    case COFF::IMAGE_FILE_MACHINE_AMD64:
       Reloc->Type = llvm::COFF::IMAGE_REL_AMD64_ADDR32NB;
       break;
-    case Machine::X86:
+    case COFF::IMAGE_FILE_MACHINE_I386:
       Reloc->Type = llvm::COFF::IMAGE_REL_I386_DIR32NB;
       break;
     default:
       Reloc->Type = 0;
     }
     CurrentOffset += sizeof(llvm::object::coff_relocation);
   }
 }
 
-Error writeWindowsResourceCOFF(StringRef OutputFile, Machine MachineType,
+Error writeWindowsResourceCOFF(std::unique_ptr<MemoryBuffer> &OutputBuffer,
+                               COFF::MachineTypes MachineType,
                                const WindowsResourceParser &Parser) {
   Error E = Error::success();
-  WindowsResourceCOFFWriter Writer(OutputFile, MachineType, Parser, E);
+  WindowsResourceCOFFWriter Writer(OutputBuffer, MachineType, Parser, E);
   if (E)
     return E;
   return Writer.write();
 }
 
 } // namespace object
 } // namespace llvm

llvm/tools/llvm-cvtres/llvm-cvtres.cpp

int main(int argc_, const char *argv_[]) {
 
   if (InputArgs.hasArg(OPT_HELP)) {
     T.PrintHelp(outs(), "cvtres", "Resource Converter", false);
     return 0;
   }
 
   bool Verbose = InputArgs.hasArg(OPT_VERBOSE);
 
-  Machine MachineType;
+  COFF::MachineTypes MachineType;
 
   if (InputArgs.hasArg(OPT_MACHINE)) {
     std::string MachineString = InputArgs.getLastArgValue(OPT_MACHINE).upper();
-    MachineType = StringSwitch<Machine>(MachineString)
-                      .Case("ARM", Machine::ARM)
-                      .Case("X64", Machine::X64)
-                      .Case("X86", Machine::X86)
-                      .Default(Machine::UNKNOWN);
-    if (MachineType == Machine::UNKNOWN)
+    MachineType = StringSwitch<COFF::MachineTypes>(MachineString)
+                      .Case("ARM", COFF::IMAGE_FILE_MACHINE_ARMNT)
+                      .Case("X64", COFF::IMAGE_FILE_MACHINE_AMD64)
+                      .Case("X86", COFF::IMAGE_FILE_MACHINE_I386)
+                      .Default(COFF::IMAGE_FILE_MACHINE_UNKNOWN);
+    if (MachineType == COFF::IMAGE_FILE_MACHINE_UNKNOWN)
       reportError("Unsupported machine architecture");
   } else {
     if (Verbose)
       outs() << "Machine architecture not specified; assumed X64.\n";
-    MachineType = Machine::X64;
+    MachineType = COFF::IMAGE_FILE_MACHINE_AMD64;
   }
 
   std::vector<std::string> InputFiles = InputArgs.getAllArgValues(OPT_INPUT);
 
   if (InputFiles.size() == 0) {
     reportError("No input file specified.\n");
   }
 
   SmallString<128> OutputFile;
 
   if (InputArgs.hasArg(OPT_OUT)) {
     OutputFile = InputArgs.getLastArgValue(OPT_OUT);
   } else {
     OutputFile = llvm::sys::path::filename(StringRef(InputFiles[0]));
     llvm::sys::path::replace_extension(OutputFile, ".obj");
   }
 
   if (Verbose) {
     outs() << "Machine: ";
     switch (MachineType) {
-    case Machine::ARM:
+    case COFF::IMAGE_FILE_MACHINE_ARMNT:
       outs() << "ARM\n";
       break;
-    case Machine::X86:
+    case COFF::IMAGE_FILE_MACHINE_I386:
       outs() << "X86\n";
       break;
     default:
       outs() << "X64\n";
     }
   }
 
   WindowsResourceParser Parser;
for (const auto &File : InputFiles) {
     error(Parser.parse(RF));
   }
 
   if (Verbose) {
     Parser.printTree(outs());
     Parser.printTree(errs());
   }
 
-  error(
-      llvm::object::writeWindowsResourceCOFF(OutputFile, MachineType, Parser));
+  std::unique_ptr<MemoryBuffer> OutputBuffer;
+  error(llvm::object::writeWindowsResourceCOFF(OutputBuffer, MachineType,
+                                               Parser));
+  auto FileOrErr =
+      FileOutputBuffer::create(OutputFile, OutputBuffer->getBufferSize());
+  if (!FileOrErr)
+    reportError(OutputFile, FileOrErr.getError());
+  std::unique_ptr<FileOutputBuffer> FileBuffer = std::move(*FileOrErr);
+  std::copy(OutputBuffer->getBufferStart(), OutputBuffer->getBufferEnd(),
+            FileBuffer->getBufferStart());
+  ;
+  error(FileBuffer->commit());
 
   if (Verbose) {
     Expected<object::OwningBinary<object::Binary>> BinaryOrErr =
         object::createBinary(OutputFile);
     if (!BinaryOrErr)
       reportError(OutputFile, errorToErrorCode(BinaryOrErr.takeError()));
     Binary &Binary = *BinaryOrErr.get().getBinary();
     ScopedPrinter W(errs());
     W.printBinaryBlock("Output File Raw Data", Binary.getData());
   }
 
   return 0;
 }