[lld] r193298 - [PECOFF] Support embedding resource file into executable.
Rui Ueyama
ruiu at google.com
Fri Oct 25 15:27:40 PDT 2013
Thank you for reviewing.
On Thu, Oct 24, 2013 at 10:18 AM, Reid Kleckner <rnk at google.com> wrote:
> Cool! Some thoughts below.
>
> On Wed, Oct 23, 2013 at 6:39 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> +// Quote double quotes and backslashes in the given string, so that we
>> can embed
>> +// the string into a resource script file.
>> +std::string quoteXml(StringRef str) {
>> + std::string ret;
>> + ret.reserve(str.size() * 2);
>> + StringRef line;
>> + for (;;) {
>> + if (str.empty())
>> + return std::move(ret);
>> + llvm::tie(line, str) = str.split("\n");
>> + if (!line.empty())
>> + continue;
>> + ret.append("\"");
>> + const char *p = line.data();
>> + for (int i = 0, size = line.size(); i < size; ++i) {
>> + switch (p[i]) {
>> + case '\"':
>> + case '\\':
>> + ret.append("\\");
>> + // fallthrough
>> + default:
>> + ret.append(1, p[i]);
>> + }
>> + }
>> + ret.append("\"\n");
>> + }
>> +}
>>
>
> Micro-optimization nit: use push_back over append for single character
> things because it won't contain a loop.
>
Will do.
> +
>> +// Create a resource file (.res file) containing the manifest XML. This
>> is done
>> +// in two steps:
>> +//
>> +// 1. Create a resource script file containing the XML as a literal
>> string.
>> +// 2. Run RC.EXE command to compile the script file to a resource file.
>> +//
>> +// The temporary file created in step 1 will be deleted on exit from this
>> +// function. The file created in step 2 will have the same lifetime as
>> the
>> +// PECOFFLinkingContext.
>> +bool createManifestResourceFile(PECOFFLinkingContext &ctx,
>> + raw_ostream &diagnostics,
>> + std::string &resFile) {
>> + // Create a temporary file for the resource script file.
>> + SmallString<128> rcFileSmallString;
>> + if (llvm::sys::fs::createTemporaryFile("tmp", "rc",
>> rcFileSmallString)) {
>> + diagnostics << "Cannot create a temporary file\n";
>> + return false;
>> + }
>> + StringRef rcFile(rcFileSmallString.str());
>> + llvm::FileRemover rcFileRemover((Twine(rcFile)));
>> +
>> + // Open the temporary file for writing.
>> + std::string errorInfo;
>> + llvm::raw_fd_ostream out(rcFile.data(), errorInfo);
>> + if (!errorInfo.empty()) {
>> + diagnostics << "Failed to open " << ctx.getManifestOutputPath() <<
>> ": "
>> + << errorInfo << "\n";
>> + return false;
>> + }
>> +
>> + // Write resource script to the RC file.
>> + out << "#define LANG_ENGLISH 9\n"
>> + << "#define SUBLANG_DEFAULT 1\n"
>> + << "#define APP_MANIFEST " << ctx.getManifestId() << "\n"
>> + << "#define RT_MANIFEST 24\n"
>> + << "LANGUAGE LANG_ENGLISH, SUBLANG_DEFAULT\n"
>> + << "APP_MANIFEST RT_MANIFEST {\n"
>> + << quoteXml(createManifestXml(ctx))
>>
>
> If this is the only use, then quoteXml should probably take out as a
> parameter and stream to it, rather than building up a temporary std::string.
>
That makes sense, and I think doing that would actually make the code a bit
cleaner. Will do later.
> + << "}\n";
>> + out.close();
>> +
>> + // Create output resource file.
>> + SmallString<128> resFileSmallString;
>> + if (llvm::sys::fs::createTemporaryFile("tmp", "res",
>> resFileSmallString)) {
>> + diagnostics << "Cannot create a temporary file";
>> + return false;
>> + }
>> + resFile = resFileSmallString.str();
>> +
>> + // Register the resource file path so that the file will be deleted
>> when the
>> + // context's destructor is called.
>> + ctx.registerTemporaryFile(resFile);
>> +
>> + // Run RC.EXE /fo tmp.res tmp.rc
>> + std::string program = "rc.exe";
>> + std::string programPath = llvm::sys::FindProgramByName(program);
>> + if (programPath.empty()) {
>> + diagnostics << "Unable to find " << program << " in PATH\n";
>> + return false;
>> + }
>> + std::vector<const char *> args;
>> + args.push_back(programPath.c_str());
>> + args.push_back("/fo");
>> + args.push_back(resFile.c_str());
>> + args.push_back(rcFile.data());
>>
>
> Are you sure this will be null-terminated? Seems like you do:
>
> + SmallString<128> rcFileSmallString;
> + if (llvm::sys::fs::createTemporaryFile("tmp", "rc", rcFileSmallString))
> {
> ...
> + StringRef rcFile(rcFileSmallString.str());
> ...
> + args.push_back(rcFile.data());
>
> I was able to prove that the temp file comes back null-terminated due to
> internal implementation details of the Windows UTF16ToUTF8 helper function,
> but it seems like you should null terminate here.
>
Fixed in r193442.
>
>
+ args.push_back(nullptr);
>> +
>> + if (llvm::sys::ExecuteAndWait(programPath.c_str(), &args[0]) != 0) {
>> + llvm::errs() << program << " failed\n";
>>
>
> llvm::errs() or diagnostics?
>
Fixed in r193387.
> + return false;
>> + }
>> + 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.
>> +bool createSideBySideManifestFile(PECOFFLinkingContext &ctx,
>> + raw_ostream &diagnostics) {
>> + std::string errorInfo;
>> + llvm::raw_fd_ostream out(ctx.getManifestOutputPath().data(),
>> errorInfo);
>> + if (!errorInfo.empty()) {
>> + diagnostics << "Failed to open " << ctx.getManifestOutputPath() <<
>> ": "
>> + << 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.
>> +bool createManifest(PECOFFLinkingContext &ctx, raw_ostream &diagnostics)
>> {
>> + if (ctx.getEmbedManifest()) {
>> + std::string resourceFilePath;
>> + if (!createManifestResourceFile(ctx, diagnostics, resourceFilePath))
>> + return false;
>> + std::unique_ptr<InputElement> inputElement(
>> + new PECOFFFileNode(ctx, resourceFilePath));
>> + ctx.inputGraph().addInputElement(std::move(inputElement));
>> + return true;
>> + }
>> + return createSideBySideManifestFile(ctx, diagnostics);
>> +}
>> +
>> // Handle /failifmismatch option.
>> bool handleFailIfMismatchOption(StringRef option,
>> std::map<StringRef, StringRef>
>> &mustMatch,
>> @@ -265,6 +402,10 @@ bool handleFailIfMismatchOption(StringRe
>> return false;
>> }
>>
>> +//
>> +// Environment variable
>> +//
>> +
>> // Process "LINK" environment variable. If defined, the value of the
>> variable
>> // should be processed as command line arguments.
>> std::vector<const char *> processLinkEnv(PECOFFLinkingContext &context,
>> @@ -344,6 +485,10 @@ parseArgs(int argc, const char *argv[],
>>
>> } // namespace
>>
>> +//
>> +// Main driver
>> +//
>> +
>> ErrorOr<StringRef> PECOFFFileNode::getPath(const LinkingContext &) const
>> {
>> if (_path.endswith(".lib"))
>> return _ctx.searchLibraryFile(_path);
>> @@ -365,6 +510,12 @@ bool WinLinkDriver::linkPECOFF(int argc,
>> processLibEnv(context);
>> if (!parse(newargv.size() - 1, &newargv[0], context, diagnostics))
>> return false;
>> +
>> + // Create the file if needed.
>> + if (context.getCreateManifest())
>> + if (!createManifest(context, diagnostics))
>> + return false;
>> +
>> return link(context, diagnostics);
>> }
>>
>> @@ -714,11 +865,6 @@ WinLinkDriver::parse(int argc, const cha
>> for (auto &e : inputElements)
>> ctx.inputGraph().addInputElement(std::move(e));
>>
>> - // Create the side-by-side manifest file if needed.
>> - if (!isReadingDirectiveSection && ctx.getCreateManifest())
>> - if (!createManifestFile(ctx, diagnostics))
>> - return false;
>> -
>> // Validate the combination of options used.
>> return ctx.validate(diagnostics);
>> }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131025/27a14889/attachment.html>
More information about the llvm-commits
mailing list