From 31fd9d09b2ac07e0bae0a81d387db89d5090769d Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Thu, 10 Jul 2014 20:53:37 +0000 Subject: [PATCH] [PECOFF] Invoke cvtres.exe in the driver. Previously we invoked cvtres.exe for each compiled Windows resource file. The generated files were then concatenated and embedded to the executable. That was not the correct way to merge compiled Windows resource files. If you just concatenate generated files, only the first file would be recognized and the rest would be ignored as trailing garbage. The right way to merge them is to call cvtres.exe with multiple input files. In this patch we do that in the Windows driver. llvm-svn: 212763 --- lld/include/lld/ReaderWriter/Reader.h | 1 - lld/lib/Driver/WinLinkDriver.cpp | 137 +++++++++++++++++------- lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp | 109 ------------------- lld/test/pecoff/resource.test | 8 +- lld/unittests/DriverTests/WinLinkDriverTest.cpp | 19 ---- 5 files changed, 106 insertions(+), 168 deletions(-) diff --git a/lld/include/lld/ReaderWriter/Reader.h b/lld/include/lld/ReaderWriter/Reader.h index 3d21a0d..0bfe2ad 100644 --- a/lld/include/lld/ReaderWriter/Reader.h +++ b/lld/include/lld/ReaderWriter/Reader.h @@ -122,7 +122,6 @@ public: void addSupportNativeObjects(); void addSupportCOFFObjects(PECOFFLinkingContext &); void addSupportCOFFImportLibraries(); - void addSupportWindowsResourceFiles(); void addSupportMachOObjects(StringRef archName); void addSupportELFObjects(bool atomizeStrings, TargetHandlerBase *handler); void addSupportELFDynamicSharedObjects(bool useShlibUndefines, diff --git a/lld/lib/Driver/WinLinkDriver.cpp b/lld/lib/Driver/WinLinkDriver.cpp index 33c374c..d97d9df 100644 --- a/lld/lib/Driver/WinLinkDriver.cpp +++ b/lld/lib/Driver/WinLinkDriver.cpp @@ -282,6 +282,53 @@ static bool parseManifest(StringRef option, bool &enable, bool &embed, return true; } +// Returns true if the given file is a Windows resource file. +static bool isResoruceFile(StringRef path) { + llvm::sys::fs::file_magic fileType; + if (llvm::sys::fs::identify_magic(path, fileType)) { + // If we cannot read the file, assume it's not a resource file. + // The further stage will raise an error on this unreadable file. + return false; + } + return fileType == llvm::sys::fs::file_magic::windows_resource; +} + +// Merge Windows resource files and convert them to a single COFF file. +// The temporary file path is set to result. +static bool convertResourceFiles(std::vector inFiles, + std::string &result) { + // Create an output file path. + SmallString<128> outFile; + if (llvm::sys::fs::createTemporaryFile("resource", "obj", outFile)) + return false; + std::string outFileArg = ("/out:" + outFile).str(); + + // Construct CVTRES.EXE command line and execute it. + std::string program = "cvtres.exe"; + std::string programPath = llvm::sys::FindProgramByName(program); + if (programPath.empty()) { + llvm::errs() << "Unable to find " << program << " in PATH\n"; + return false; + } + + std::vector args; + args.push_back(programPath.c_str()); + args.push_back("/machine:x86"); + args.push_back("/readonly"); + args.push_back("/nologo"); + args.push_back(outFileArg.c_str()); + for (const std::string &path : inFiles) + args.push_back(path.c_str()); + args.push_back(nullptr); + + if (llvm::sys::ExecuteAndWait(programPath.c_str(), &args[0]) != 0) { + llvm::errs() << program << " failed\n"; + return false; + } + result = outFile.str(); + return true; +} + // Parse /manifestuac:(level=|uiAccess=). // // The arguments will be embedded to the manifest XML file with no error check, @@ -503,43 +550,37 @@ static bool createManifestResourceFile(PECOFFLinkingContext &ctx, return true; } -// Create a side-by-side manifest file. The side-by-side manifest file is a -// separate XML file having ".manifest" extension. It will be created in the -// same directory as the resulting executable. + +// Create the a side-by-side manifest file. +// +// The manifest file will convey some information to the linker, such as whether +// the binary needs to run as Administrator or not. Instead of being placed in +// the PE/COFF header, it's in XML format for some reason -- I guess it's +// probably because it's invented in the early dot-com era. +// +// The side-by-side manifest file is a separate XML file having ".manifest" +// extension. It will be created in the same directory as the resulting +// executable. static bool createSideBySideManifestFile(PECOFFLinkingContext &ctx, raw_ostream &diag) { + std::string path = ctx.getManifestOutputPath(); + if (path.empty()) { + // Default name of the manifest file is "foo.exe.manifest" where "foo.exe" is + // the output path. + path = ctx.outputPath(); + path.append(".manifest"); + } + std::string errorInfo; - llvm::raw_fd_ostream out(ctx.getManifestOutputPath().data(), errorInfo, - llvm::sys::fs::F_Text); + llvm::raw_fd_ostream out(path.c_str(), errorInfo, llvm::sys::fs::F_Text); if (!errorInfo.empty()) { - diag << "Failed to open " << ctx.getManifestOutputPath() << ": " - << errorInfo << "\n"; + diag << errorInfo << "\n"; return false; } out << createManifestXml(ctx); return true; } -// Create the a side-by-side manifest file, or create a resource file for the -// manifest file and add it to the input graph. -// -// The manifest file will convey some information to the linker, such as whether -// the binary needs to run as Administrator or not. Instead of being placed in -// the PE/COFF header, it's in XML format for some reason -- I guess it's -// probably because it's invented in the early dot-com era. -static bool createManifest(PECOFFLinkingContext &ctx, raw_ostream &diag) { - if (ctx.getEmbedManifest()) { - std::string resourceFilePath; - if (!createManifestResourceFile(ctx, diag, resourceFilePath)) - return false; - std::unique_ptr inputElement( - new PECOFFFileNode(ctx, ctx.allocate(resourceFilePath))); - ctx.getInputGraph().addInputElement(std::move(inputElement)); - return true; - } - return createSideBySideManifestFile(ctx, diag); -} - // Handle /failifmismatch option. static bool handleFailIfMismatchOption(StringRef option, @@ -770,14 +811,13 @@ bool WinLinkDriver::linkPECOFF(int argc, const char **argv, raw_ostream &diag) { return false; // Create the file if needed. - if (context.getCreateManifest()) - if (!createManifest(context, diag)) + if (context.getCreateManifest() && !context.getEmbedManifest()) + if (!createSideBySideManifestFile(context, diag)) return false; // Register possible input file parsers. context.registry().addSupportCOFFObjects(context); context.registry().addSupportCOFFImportLibraries(); - context.registry().addSupportWindowsResourceFiles(); context.registry().addSupportArchives(context.logInputFiles()); context.registry().addSupportNativeObjects(); context.registry().addSupportYamlFiles(); @@ -1202,6 +1242,35 @@ bool WinLinkDriver::parse(int argc, const char *argv[], for (const StringRef value : dashdash->getValues()) inputFiles.push_back(value); + // Compile Windows resource files to compiled resource file. + if (ctx.getCreateManifest() && ctx.getEmbedManifest() && + !isReadingDirectiveSection) { + std::string resFile; + if (!createManifestResourceFile(ctx, diag, resFile)) + return false; + inputFiles.push_back(ctx.allocate(resFile)); + } + + // A Windows Resource file is not an object file. It contains data, + // such as an icon image, and is not in COFF file format. If resource + // files are given, the linker merge them into one COFF file using + // CVTRES.EXE and then link the resulting file. + { + auto it = std::partition(inputFiles.begin(), inputFiles.end(), + isResoruceFile); + if (it != inputFiles.begin()) { + std::vector resFiles(inputFiles.begin(), it); + std::string resObj; + if (!convertResourceFiles(resFiles, resObj)) { + diag << "Failed to convert resource files\n"; + return false; + } + inputFiles = std::vector(it, inputFiles.end()); + inputFiles.push_back(ctx.allocate(resObj)); + ctx.registerTemporaryFile(resObj); + } + } + // Prepare objects to add them to input graph. for (StringRef path : inputFiles) { path = ctx.allocate(path); @@ -1253,14 +1322,6 @@ bool WinLinkDriver::parse(int argc, const char *argv[], ctx.setOutputPath(replaceExtension(ctx, path, ".exe")); } - // Default name of the manifest file is "foo.exe.manifest" where "foo.exe" is - // the output path. - if (ctx.getManifestOutputPath().empty()) { - std::string path = ctx.outputPath(); - path.append(".manifest"); - ctx.setManifestOutputPath(ctx.allocate(path)); - } - // Add the input files to the input graph. if (!ctx.hasInputGraph()) ctx.setInputGraph(std::unique_ptr(new InputGraph())); diff --git a/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp b/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp index a9c1d31..f1d833c 100644 --- a/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp +++ b/lld/lib/ReaderWriter/PECOFF/ReaderCOFF.cpp @@ -935,112 +935,6 @@ StringRef FileCOFF::ArrayRefToString(ArrayRef array) { return StringRef(*contents).trim(); } -// Convert .res file to .coff file and then parse it. Resource file is a file -// containing various types of data, such as icons, translation texts, -// etc. "cvtres.exe" command reads an RC file to create a COFF file which -// encapsulates resource data into rsrc$N sections, where N is an integer. -// -// The linker is not capable to handle RC files directly. Instead, it runs -// cvtres.exe on RC files and then then link its outputs. -class ResourceFileReader : public Reader { -public: - bool canParse(file_magic magic, StringRef ext, - const MemoryBuffer &) const override { - return (magic == llvm::sys::fs::file_magic::windows_resource); - } - - std::error_code - parseFile(std::unique_ptr &mb, const class Registry &, - std::vector> &result) const override { - // Convert RC file to COFF - ErrorOr coffPath = convertResourceFileToCOFF(std::move(mb)); - if (std::error_code ec = coffPath.getError()) - return ec; - llvm::FileRemover coffFileRemover(*coffPath); - - // Read and parse the COFF - ErrorOr> newmb = - MemoryBuffer::getFile(*coffPath); - if (std::error_code ec = newmb.getError()) - return ec; - std::error_code ec; - std::unique_ptr file(new FileCOFF(std::move(newmb.get()), ec)); - if (ec) - return ec; - if (std::error_code ec = file->parse()) - return ec; - result.push_back(std::move(file)); - return std::error_code(); - } - -private: - static ErrorOr - writeResToTemporaryFile(std::unique_ptr mb) { - // Get a temporary file path for .res file. - SmallString<128> tempFilePath; - if (std::error_code ec = - llvm::sys::fs::createTemporaryFile("tmp", "res", tempFilePath)) - return ec; - - // Write the memory buffer contents to .res file, so that we can run - // cvtres.exe on it. - std::unique_ptr buffer; - if (std::error_code ec = llvm::FileOutputBuffer::create( - tempFilePath.str(), mb->getBufferSize(), buffer)) - return ec; - memcpy(buffer->getBufferStart(), mb->getBufferStart(), mb->getBufferSize()); - if (std::error_code ec = buffer->commit()) - return ec; - - // Convert SmallString -> StringRef -> std::string. - return tempFilePath.str().str(); - } - - static ErrorOr - convertResourceFileToCOFF(std::unique_ptr mb) { - // Write the resource file to a temporary file. - ErrorOr inFilePath = writeResToTemporaryFile(std::move(mb)); - if (std::error_code ec = inFilePath.getError()) - return ec; - llvm::FileRemover inFileRemover(*inFilePath); - - // Create an output file path. - SmallString<128> outFilePath; - if (std::error_code ec = - llvm::sys::fs::createTemporaryFile("tmp", "obj", outFilePath)) - return ec; - std::string outFileArg = ("/out:" + outFilePath).str(); - - // Construct CVTRES.EXE command line and execute it. - std::string program = "cvtres.exe"; - std::string programPath = llvm::sys::FindProgramByName(program); - if (programPath.empty()) { - llvm::errs() << "Unable to find " << program << " in PATH\n"; - return llvm::errc::broken_pipe; - } - std::vector args; - args.push_back(programPath.c_str()); - args.push_back("/machine:x86"); - args.push_back("/readonly"); - args.push_back("/nologo"); - args.push_back(outFileArg.c_str()); - args.push_back(inFilePath->c_str()); - args.push_back(nullptr); - - DEBUG({ - for (const char **p = &args[0]; *p; ++p) - llvm::dbgs() << *p << " "; - llvm::dbgs() << "\n"; - }); - - if (llvm::sys::ExecuteAndWait(programPath.c_str(), &args[0]) != 0) { - llvm::errs() << program << " failed\n"; - return llvm::errc::broken_pipe; - } - return outFilePath.str().str(); - } -}; - class COFFObjectReader : public Reader { public: COFFObjectReader(PECOFFLinkingContext &ctx) : _ctx(ctx) {} @@ -1204,7 +1098,4 @@ void Registry::addSupportCOFFObjects(PECOFFLinkingContext &ctx) { kindStringsAMD64); } -void Registry::addSupportWindowsResourceFiles() { - add(std::unique_ptr(new ResourceFileReader())); -} } diff --git a/lld/test/pecoff/resource.test b/lld/test/pecoff/resource.test index 243d02b..4d26a67 100644 --- a/lld/test/pecoff/resource.test +++ b/lld/test/pecoff/resource.test @@ -1,9 +1,15 @@ # REQUIRES: winres # RUN: yaml2obj %p/Inputs/nop.obj.yaml > %t.obj -# + # RUN: lld -flavor link /out:%t.exe /subsystem:console /entry:start /opt:noref \ # RUN: -- %t.obj %p/Inputs/resource.res # Check if the binary contains UTF-16LE string "Hello" copied from resource.res. # RUN: cat %t.exe | grep 'H.e.l.l.o' + +# RUN: lld -flavor link /out:%t.exe /subsystem:console /entry:start /opt:noref \ +# RUN: -- %t.obj %p/Inputs/resource.res %p/Inputs/resource.res + +# Multiple resource files are concatenated. +# RUN: cat %t.exe | grep 'H.e.l.l.o.H.e.l.l.o.' diff --git a/lld/unittests/DriverTests/WinLinkDriverTest.cpp b/lld/unittests/DriverTests/WinLinkDriverTest.cpp index e49d122..2246d82 100644 --- a/lld/unittests/DriverTests/WinLinkDriverTest.cpp +++ b/lld/unittests/DriverTests/WinLinkDriverTest.cpp @@ -64,7 +64,6 @@ TEST_F(WinLinkParserTest, Basic) { EXPECT_TRUE(_context.isTerminalServerAware()); EXPECT_TRUE(_context.getDynamicBaseEnabled()); EXPECT_TRUE(_context.getCreateManifest()); - EXPECT_EQ("a.exe.manifest", _context.getManifestOutputPath()); EXPECT_EQ("", _context.getManifestDependency()); EXPECT_FALSE(_context.getEmbedManifest()); EXPECT_EQ(1, _context.getManifestId()); @@ -595,24 +594,6 @@ TEST_F(WinLinkParserTest, Manifest_No) { EXPECT_FALSE(_context.getCreateManifest()); } -TEST_F(WinLinkParserTest, Manifest_Embed) { - EXPECT_TRUE(parse("link.exe", "/manifest:embed", "a.out", nullptr)); - EXPECT_TRUE(_context.getCreateManifest()); - EXPECT_TRUE(_context.getEmbedManifest()); - EXPECT_EQ(1, _context.getManifestId()); - EXPECT_EQ("'asInvoker'", _context.getManifestLevel()); - EXPECT_EQ("'false'", _context.getManifestUiAccess()); -} - -TEST_F(WinLinkParserTest, Manifest_Embed_ID42) { - EXPECT_TRUE(parse("link.exe", "/manifest:embed,id=42", "a.out", nullptr)); - EXPECT_TRUE(_context.getCreateManifest()); - EXPECT_TRUE(_context.getEmbedManifest()); - EXPECT_EQ(42, _context.getManifestId()); - EXPECT_EQ("'asInvoker'", _context.getManifestLevel()); - EXPECT_EQ("'false'", _context.getManifestUiAccess()); -} - TEST_F(WinLinkParserTest, Manifestuac_no) { EXPECT_TRUE(parse("link.exe", "/manifestuac:NO", "a.out", nullptr)); EXPECT_FALSE(_context.getManifestUAC()); -- 2.7.4