[llvm] r315499 - [llvm-rc] Use proper search algorithm for finding resources.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 11 13:12:09 PDT 2017
Author: zturner
Date: Wed Oct 11 13:12:09 2017
New Revision: 315499
URL: http://llvm.org/viewvc/llvm-project?rev=315499&view=rev
Log:
[llvm-rc] Use proper search algorithm for finding resources.
Previously we would only look in the current directory for a
resource, which might not be the same as the directory of the
rc file. Furthermore, MSVC rc supports a /I option, and can
also look in the system environment. This patch adds support
for this search algorithm.
Differential Revision: https://reviews.llvm.org/D38740
Added:
llvm/trunk/test/tools/llvm-rc/Inputs/deep-include.rc
llvm/trunk/test/tools/llvm-rc/Inputs/include.rc
llvm/trunk/test/tools/llvm-rc/Inputs/nested/
llvm/trunk/test/tools/llvm-rc/Inputs/nested/nested-bitmap.bmp (with props)
llvm/trunk/test/tools/llvm-rc/include-paths.test
Modified:
llvm/trunk/include/llvm/Support/Process.h
llvm/trunk/lib/Support/Process.cpp
llvm/trunk/test/tools/llvm-rc/tag-html.test
llvm/trunk/tools/llvm-rc/ResourceFileWriter.cpp
llvm/trunk/tools/llvm-rc/ResourceFileWriter.h
llvm/trunk/tools/llvm-rc/ResourceScriptParser.cpp
llvm/trunk/tools/llvm-rc/ResourceScriptParser.h
llvm/trunk/tools/llvm-rc/llvm-rc.cpp
Modified: llvm/trunk/include/llvm/Support/Process.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Process.h?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Process.h (original)
+++ llvm/trunk/include/llvm/Support/Process.h Wed Oct 11 13:12:09 2017
@@ -80,9 +80,15 @@ public:
/// This function searches for an existing file in the list of directories
/// in a PATH like environment variable, and returns the first file found,
/// according to the order of the entries in the PATH like environment
- /// variable.
- static Optional<std::string> FindInEnvPath(const std::string& EnvName,
- const std::string& FileName);
+ /// variable. If an ignore list is specified, then any folder which is in
+ /// the PATH like environment variable but is also in IgnoreList is not
+ /// considered.
+ static Optional<std::string> FindInEnvPath(StringRef EnvName,
+ StringRef FileName,
+ ArrayRef<std::string> IgnoreList);
+
+ static Optional<std::string> FindInEnvPath(StringRef EnvName,
+ StringRef FileName);
/// This function returns a SmallVector containing the arguments passed from
/// the operating system to the program. This function expects to be handed
Modified: llvm/trunk/lib/Support/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Process.cpp?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Process.cpp (original)
+++ llvm/trunk/lib/Support/Process.cpp Wed Oct 11 13:12:09 2017
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/Process.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Config/config.h"
#include "llvm/Support/FileSystem.h"
@@ -26,9 +27,14 @@ using namespace sys;
//=== independent code.
//===----------------------------------------------------------------------===//
-Optional<std::string> Process::FindInEnvPath(const std::string& EnvName,
- const std::string& FileName)
-{
+Optional<std::string> Process::FindInEnvPath(StringRef EnvName,
+ StringRef FileName) {
+ return FindInEnvPath(EnvName, FileName, {});
+}
+
+Optional<std::string> Process::FindInEnvPath(StringRef EnvName,
+ StringRef FileName,
+ ArrayRef<std::string> IgnoreList) {
assert(!path::is_absolute(FileName));
Optional<std::string> FoundPath;
Optional<std::string> OptPath = Process::GetEnv(EnvName);
@@ -39,10 +45,13 @@ Optional<std::string> Process::FindInEnv
SmallVector<StringRef, 8> Dirs;
SplitString(OptPath.getValue(), Dirs, EnvPathSeparatorStr);
- for (const auto &Dir : Dirs) {
+ for (StringRef Dir : Dirs) {
if (Dir.empty())
continue;
+ if (any_of(IgnoreList, [&](StringRef S) { return fs::equivalent(S, Dir); }))
+ continue;
+
SmallString<128> FilePath(Dir);
path::append(FilePath, FileName);
if (fs::exists(Twine(FilePath))) {
Added: llvm/trunk/test/tools/llvm-rc/Inputs/deep-include.rc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-rc/Inputs/deep-include.rc?rev=315499&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-rc/Inputs/deep-include.rc (added)
+++ llvm/trunk/test/tools/llvm-rc/Inputs/deep-include.rc Wed Oct 11 13:12:09 2017
@@ -0,0 +1,3 @@
+// Whether this is found depends on whether the /I flag searches within the
+// "nested" subdirectory
+foo BITMAP "nested-bitmap.bmp"
\ No newline at end of file
Added: llvm/trunk/test/tools/llvm-rc/Inputs/include.rc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-rc/Inputs/include.rc?rev=315499&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-rc/Inputs/include.rc (added)
+++ llvm/trunk/test/tools/llvm-rc/Inputs/include.rc Wed Oct 11 13:12:09 2017
@@ -0,0 +1,2 @@
+// Found because bitmap.bmp is in same directory
+foo BITMAP "bitmap.bmp"
\ No newline at end of file
Added: llvm/trunk/test/tools/llvm-rc/Inputs/nested/nested-bitmap.bmp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-rc/Inputs/nested/nested-bitmap.bmp?rev=315499&view=auto
==============================================================================
Binary file - no diff available.
Propchange: llvm/trunk/test/tools/llvm-rc/Inputs/nested/nested-bitmap.bmp
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
Added: llvm/trunk/test/tools/llvm-rc/include-paths.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-rc/include-paths.test?rev=315499&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-rc/include-paths.test (added)
+++ llvm/trunk/test/tools/llvm-rc/include-paths.test Wed Oct 11 13:12:09 2017
@@ -0,0 +1,45 @@
+; Should find the bitmap if it is in the same folder as the rc file.
+; RUN: rm %t.include.res
+; RUN: llvm-rc /FO %t.include.res %p/Inputs/include.rc
+; RUN: llvm-readobj %t.include.res | FileCheck --check-prefix=FOUND %s
+
+; Should find the bitmap if the folder is explicitly specified.
+; RUN: rm %t.nested-include.res
+; RUN: llvm-rc /FO %t.nested-include.res /I %p/Inputs/nested %p/Inputs/deep-include.rc
+; RUN: llvm-readobj %t.nested-include.res | FileCheck --check-prefix=FOUND %s
+
+; Otherwise, it should not find the bitmap.
+; RUN: rm %t.nested-include.res
+; RUN: not llvm-rc /FO %t.nested-include.res %p/Inputs/deep-include.rc 2>&1 \
+; RUN: | FileCheck --check-prefix=MISSING %s
+
+; Should find the bitmap if the process's current working directory
+; contains the resource being searched for. Do this test last since it
+; changes the current working directory and could affect the success or
+; failure of other tests if run first.
+; RUN: rm %t.nested-include.res
+; RUN: cd %p/Inputs/nested
+; RUN: llvm-rc /FO %t.nested-include.res %p/Inputs/include.rc
+; RUN: llvm-readobj %t.nested-include.res | FileCheck --check-prefix=FOUND %s
+
+FOUND: Resource type (string): BITMAP
+FOUND-NEXT: Resource name (string): FOO
+FOUND-NEXT: Data version: 0
+FOUND-NEXT: Memory flags: 0x30
+FOUND-NEXT: Language ID: 1033
+FOUND-NEXT: Version (major): 0
+FOUND-NEXT: Version (minor): 0
+FOUND-NEXT: Characteristics: 0
+FOUND-NEXT: Data size: 110
+FOUND-NEXT: Data: (
+FOUND-NEXT: 0000: 424D6E00 00000000 00003600 00002800 |BMn.......6...(.|
+FOUND-NEXT: 0010: 00000200 00000700 00000100 18000000 |................|
+FOUND-NEXT: 0020: 00003800 00000000 00000000 00000000 |..8.............|
+FOUND-NEXT: 0030: 00000000 00005BB3 855BB385 0000FFFF |......[..[......|
+FOUND-NEXT: 0040: FFFFFFFF 0000FFFF FFFFFFFF 0000FFFF |................|
+FOUND-NEXT: 0050: FFFFFFFF 00005BB3 85FFFFFF 0000FFFF |......[.........|
+FOUND-NEXT: 0060: FF0EC9FF 0000241C EDFFFFFF 0000 |......$.......|
+FOUND-NEXT: )
+
+MISSING: llvm-rc: Error in BITMAP statement (ID foo):
+MISSING-NEXT: error : file not found : nested-bitmap.bmp
Modified: llvm/trunk/test/tools/llvm-rc/tag-html.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-rc/tag-html.test?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-rc/tag-html.test (original)
+++ llvm/trunk/test/tools/llvm-rc/tag-html.test Wed Oct 11 13:12:09 2017
@@ -33,9 +33,3 @@
; HTML-NEXT: 0020: 202D2D3E 0A3C696D 67207372 633D226B | -->.<img src="k|
; HTML-NEXT: 0030: 69747465 6E732E62 6D70223E 0A |ittens.bmp">.|
; HTML-NEXT: )
-
-
-; RUN: not llvm-rc /FO %t/tag-html-wrong.res %p/Inputs/tag-html-wrong.rc 2>&1 | FileCheck %s --check-prefix NOFILE
-
-; NOFILE: llvm-rc: Error in HTML statement (ID 1):
-; NOFILE-NEXT: Error opening file 'some-really-nonexistent-file.html':
Modified: llvm/trunk/tools/llvm-rc/ResourceFileWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rc/ResourceFileWriter.cpp?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-rc/ResourceFileWriter.cpp (original)
+++ llvm/trunk/tools/llvm-rc/ResourceFileWriter.cpp Wed Oct 11 13:12:09 2017
@@ -17,6 +17,8 @@
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/EndianStream.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
using namespace llvm::support;
@@ -41,12 +43,13 @@ public:
~ContextKeeper() { FileWriter->ObjectData = SavedInfo; }
};
-static Error createError(Twine Message,
+static Error createError(const Twine &Message,
std::errc Type = std::errc::invalid_argument) {
return make_error<StringError>(Message, std::make_error_code(Type));
}
-static Error checkNumberFits(uint32_t Number, size_t MaxBits, Twine FieldName) {
+static Error checkNumberFits(uint32_t Number, size_t MaxBits,
+ const Twine &FieldName) {
assert(1 <= MaxBits && MaxBits <= 32);
if (!(Number >> MaxBits))
return Error::success();
@@ -56,13 +59,13 @@ static Error checkNumberFits(uint32_t Nu
}
template <typename FitType>
-static Error checkNumberFits(uint32_t Number, Twine FieldName) {
+static Error checkNumberFits(uint32_t Number, const Twine &FieldName) {
return checkNumberFits(Number, sizeof(FitType) * 8, FieldName);
}
// A similar function for signed integers.
template <typename FitType>
-static Error checkSignedNumberFits(uint32_t Number, Twine FieldName,
+static Error checkSignedNumberFits(uint32_t Number, const Twine &FieldName,
bool CanBeNegative) {
int32_t SignedNum = Number;
if (SignedNum < std::numeric_limits<FitType>::min() ||
@@ -79,13 +82,13 @@ static Error checkSignedNumberFits(uint3
return Error::success();
}
-static Error checkRCInt(RCInt Number, Twine FieldName) {
+static Error checkRCInt(RCInt Number, const Twine &FieldName) {
if (Number.isLong())
return Error::success();
return checkNumberFits<uint16_t>(Number, FieldName);
}
-static Error checkIntOrString(IntOrString Value, Twine FieldName) {
+static Error checkIntOrString(IntOrString Value, const Twine &FieldName) {
if (!Value.isInt())
return Error::success();
return checkNumberFits<uint16_t>(Value.getInt(), FieldName);
@@ -356,15 +359,10 @@ Error ResourceFileWriter::appendFile(Str
bool IsLong;
stripQuotes(Filename, IsLong);
- // Filename path should be relative to the current working directory.
- // FIXME: docs say so, but reality is more complicated, script
- // location and include paths must be taken into account.
- ErrorOr<std::unique_ptr<MemoryBuffer>> File =
- MemoryBuffer::getFile(Filename, -1, false);
+ auto File = loadFile(Filename);
if (!File)
- return make_error<StringError>("Error opening file '" + Filename +
- "': " + File.getError().message(),
- File.getError());
+ return File.takeError();
+
*FS << (*File)->getBuffer();
return Error::success();
}
@@ -805,15 +803,10 @@ Error ResourceFileWriter::visitIconOrCur
bool IsLong;
stripQuotes(FileStr, IsLong);
- ErrorOr<std::unique_ptr<MemoryBuffer>> File =
- MemoryBuffer::getFile(FileStr, -1, false);
+ auto File = loadFile(FileStr);
if (!File)
- return make_error<StringError>(
- "Error opening " +
- Twine(Type == IconCursorGroupType::Icon ? "icon" : "cursor") +
- " '" + FileStr + "': " + File.getError().message(),
- File.getError());
+ return File.takeError();
BinaryStreamReader Reader((*File)->getBuffer(), support::little);
@@ -1413,5 +1406,43 @@ Error ResourceFileWriter::writeVersionIn
return Error::success();
}
+Expected<std::unique_ptr<MemoryBuffer>>
+ResourceFileWriter::loadFile(StringRef File) const {
+ SmallString<128> Path;
+ SmallString<128> Cwd;
+ std::unique_ptr<MemoryBuffer> Result;
+
+ // 1. The current working directory.
+ sys::fs::current_path(Cwd);
+ Path.assign(Cwd.begin(), Cwd.end());
+ sys::path::append(Path, File);
+ if (sys::fs::exists(Path))
+ return errorOrToExpected(MemoryBuffer::getFile(Path, -1i64, false));
+
+ // 2. The directory of the input resource file, if it is different from the
+ // current
+ // working directory.
+ StringRef InputFileDir = sys::path::parent_path(Params.InputFilePath);
+ Path.assign(InputFileDir.begin(), InputFileDir.end());
+ sys::path::append(Path, File);
+ if (sys::fs::exists(Path))
+ return errorOrToExpected(MemoryBuffer::getFile(Path, -1i64, false));
+
+ // 3. All of the include directories specified on the command line.
+ for (StringRef ForceInclude : Params.Include) {
+ Path.assign(ForceInclude.begin(), ForceInclude.end());
+ sys::path::append(Path, File);
+ if (sys::fs::exists(Path))
+ return errorOrToExpected(MemoryBuffer::getFile(Path, -1i64, false));
+ }
+
+ if (auto Result =
+ llvm::sys::Process::FindInEnvPath("INCLUDE", File, Params.NoInclude))
+ return errorOrToExpected(MemoryBuffer::getFile(*Result, -1i64, false));
+
+ return make_error<StringError>("error : file not found : " + Twine(File),
+ inconvertibleErrorCode());
+}
+
} // namespace rc
} // namespace llvm
Modified: llvm/trunk/tools/llvm-rc/ResourceFileWriter.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rc/ResourceFileWriter.h?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-rc/ResourceFileWriter.h (original)
+++ llvm/trunk/tools/llvm-rc/ResourceFileWriter.h Wed Oct 11 13:12:09 2017
@@ -22,10 +22,17 @@
namespace llvm {
namespace rc {
+struct SearchParams {
+ std::vector<std::string> Include; // Additional folders to search for files.
+ std::vector<std::string> NoInclude; // Folders to exclude from file search.
+ StringRef InputFilePath; // The full path of the input file.
+};
+
class ResourceFileWriter : public Visitor {
public:
- ResourceFileWriter(std::unique_ptr<raw_fd_ostream> Stream)
- : FS(std::move(Stream)), IconCursorID(1) {
+ ResourceFileWriter(const SearchParams &Params,
+ std::unique_ptr<raw_fd_ostream> Stream)
+ : Params(Params), FS(std::move(Stream)), IconCursorID(1) {
assert(FS && "Output stream needs to be provided to the serializator");
}
@@ -136,6 +143,8 @@ private:
Error writeVersionInfoBlock(const VersionInfoBlock &);
Error writeVersionInfoValue(const VersionInfoValue &);
+ const SearchParams &Params;
+
// Output stream handling.
std::unique_ptr<raw_fd_ostream> FS;
@@ -170,6 +179,8 @@ private:
void padStream(uint64_t Length);
+ Expected<std::unique_ptr<MemoryBuffer>> loadFile(StringRef File) const;
+
// Icon and cursor IDs are allocated starting from 1 and increasing for
// each icon/cursor dumped. This maintains the current ID to be allocated.
uint16_t IconCursorID;
Modified: llvm/trunk/tools/llvm-rc/ResourceScriptParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rc/ResourceScriptParser.cpp?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-rc/ResourceScriptParser.cpp (original)
+++ llvm/trunk/tools/llvm-rc/ResourceScriptParser.cpp Wed Oct 11 13:12:09 2017
@@ -12,6 +12,10 @@
//===---------------------------------------------------------------------===//
#include "ResourceScriptParser.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
// Take an expression returning llvm::Error and forward the error if it exists.
#define RETURN_IF_ERROR(Expr) \
Modified: llvm/trunk/tools/llvm-rc/ResourceScriptParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rc/ResourceScriptParser.h?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-rc/ResourceScriptParser.h (original)
+++ llvm/trunk/tools/llvm-rc/ResourceScriptParser.h Wed Oct 11 13:12:09 2017
@@ -25,6 +25,9 @@
#include <vector>
namespace llvm {
+namespace opt {
+class InputArgList;
+}
namespace rc {
class RCParser {
@@ -51,7 +54,7 @@ public:
LocIter ErrorLoc, FileEnd;
};
- RCParser(std::vector<RCToken> TokenList);
+ explicit RCParser(std::vector<RCToken> TokenList);
// Reads and returns a single resource definition, or error message if any
// occurred.
Modified: llvm/trunk/tools/llvm-rc/llvm-rc.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rc/llvm-rc.cpp?rev=315499&r1=315498&r2=315499&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-rc/llvm-rc.cpp (original)
+++ llvm/trunk/tools/llvm-rc/llvm-rc.cpp Wed Oct 11 13:12:09 2017
@@ -68,7 +68,7 @@ public:
static ExitOnError ExitOnErr;
-LLVM_ATTRIBUTE_NORETURN static void fatalError(Twine Message) {
+LLVM_ATTRIBUTE_NORETURN static void fatalError(const Twine &Message) {
errs() << Message << "\n";
exit(1);
}
@@ -107,10 +107,10 @@ int main(int argc_, const char *argv_[])
}
// Read and tokenize the input file.
- const Twine &Filename = InArgsInfo[0];
- ErrorOr<std::unique_ptr<MemoryBuffer>> File = MemoryBuffer::getFile(Filename);
+ ErrorOr<std::unique_ptr<MemoryBuffer>> File =
+ MemoryBuffer::getFile(InArgsInfo[0]);
if (!File) {
- fatalError("Error opening file '" + Filename +
+ fatalError("Error opening file '" + Twine(InArgsInfo[0]) +
"': " + File.getError().message());
}
@@ -138,6 +138,13 @@ int main(int argc_, const char *argv_[])
}
}
+ SearchParams Params;
+ SmallString<128> InputFile(InArgsInfo[0]);
+ llvm::sys::fs::make_absolute(InputFile);
+ Params.InputFilePath = InputFile;
+ Params.Include = InputArgs.getAllArgValues(OPT_INCLUDE);
+ Params.NoInclude = InputArgs.getAllArgValues(OPT_NOINCLUDE);
+
std::unique_ptr<ResourceFileWriter> Visitor;
bool IsDryRun = InputArgs.hasArg(OPT_DRY_RUN);
@@ -153,7 +160,7 @@ int main(int argc_, const char *argv_[])
if (EC)
fatalError("Error opening output file '" + OutArgsInfo[0] +
"': " + EC.message());
- Visitor = llvm::make_unique<ResourceFileWriter>(std::move(FOut));
+ Visitor = llvm::make_unique<ResourceFileWriter>(Params, std::move(FOut));
Visitor->AppendNull = InputArgs.hasArg(OPT_ADD_NULL);
ExitOnErr(NullResource().visit(Visitor.get()));
More information about the llvm-commits
mailing list