[llvm-rc] Handle the /I option

[zturner]

Summary

This is not only the directory that include files are searched for, which could be handled by a C/C++ preprocessor, but also controls where resource files are searched for. For example, if you specify /I foo and then have a statement like mybmp BITMAP foo.bmp, then foo.bmp is searched for in the directory foo. A C preprocessor can't handle this, so we still need logic for it. The rules implemented in this patch conform to the algorithm described in MSDN for how rc searches for resources.

[rnk]

lgtm

llvm/tools/llvm-rc/ResourceFileWriter.cpp

1439

Maybe add a similar comment?

// 4. Last, search INCLUDE paths from the environment.

llvm/include/llvm/Support/Process.h

public:
 
   // This function returns the environment variable \arg name's value as a UTF-8
   // string. \arg Name is assumed to be in UTF-8 encoding too.
   static Optional<std::string> GetEnv(StringRef name);
 
   /// 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
   /// the vector passed in from main.
   static std::error_code
   GetArgumentVector(SmallVectorImpl<const char *> &Args,
                     ArrayRef<const char *> ArgsFromMain,
                     SpecificBumpPtrAllocator<char> &ArgAllocator);

llvm/lib/Support/Process.cpp

 //===-- Process.cpp - Implement OS Process Concept --------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements the operating system Process concept.
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 
 using namespace llvm;
 using namespace sys;
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only TRULY operating system
 //===          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);
   if (!OptPath.hasValue())
     return FoundPath;
 
   const char EnvPathSeparatorStr[] = {EnvPathSeparator, '\0'};
   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))) {
       FoundPath = FilePath.str();
       break;
     }
   }
 

llvm/test/tools/llvm-rc/Inputs/deep-include.rc

+// 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

llvm/test/tools/llvm-rc/Inputs/include.rc

+// Found because bitmap.bmp is in same directory
+foo BITMAP "bitmap.bmp"
 No newline at end of file

llvm/test/tools/llvm-rc/include-paths.test

+; 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

llvm/tools/llvm-rc/ResourceFileWriter.h

 #include "ResourceScriptStmt.h"
 #include "ResourceVisitor.h"
 
 #include "llvm/Support/Endian.h"
 
 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");
   }
 
   Error visitNullResource(const RCResource *) override;
   Error visitAcceleratorsResource(const RCResource *) override;
   Error visitCursorResource(const RCResource *) override;
   Error visitDialogResource(const RCResource *) override;
   Error visitHTMLResource(const RCResource *) override;
private:
   // User defined resource
   Error writeUserDefinedBody(const RCResource *);
 
   // VersionInfoResource
   Error writeVersionInfoBody(const RCResource *);
   Error writeVersionInfoBlock(const VersionInfoBlock &);
   Error writeVersionInfoValue(const VersionInfoValue &);
 
+  const SearchParams &Params;
+
   // Output stream handling.
   std::unique_ptr<raw_fd_ostream> FS;
 
   uint64_t tell() const { return FS->tell(); }
 
   uint64_t writeObject(const ArrayRef<uint8_t> Data);
 
   template <typename T> uint64_t writeInt(const T &Value) {
private:
   Error writeIntOrString(const IntOrString &Data);
 
   void writeRCInt(RCInt);
 
   Error appendFile(StringRef Filename);
 
   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;
 };
 
 } // namespace rc
 } // namespace llvm
 
 #endif

llvm/tools/llvm-rc/ResourceFileWriter.cpp

 //===---------------------------------------------------------------------===//
 
 #include "ResourceFileWriter.h"
 
 #include "llvm/Object/WindowsResource.h"
 #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;
 
 // Take an expression returning llvm::Error and forward the error if it exists.
 #define RETURN_IF_ERROR(Expr)                                                  \
   if (auto Err = (Expr))                                                       \
     return Err;
 
 namespace llvm {
 namespace rc {
 
 // Class that employs RAII to save the current FileWriter object state
 // and revert to it as soon as we leave the scope. This is useful if resources
 // declare their own resource-local statements.
 class ContextKeeper {
   ResourceFileWriter *FileWriter;
   ResourceFileWriter::ObjectInfo SavedInfo;
 
 public:
   ContextKeeper(ResourceFileWriter *V)
       : FileWriter(V), SavedInfo(V->ObjectData) {}
   ~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();
   return createError(FieldName + " (" + Twine(Number) + ") does not fit in " +
                          Twine(MaxBits) + " bits.",
                      std::errc::value_too_large);
 }
 
 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() ||
       SignedNum > std::numeric_limits<FitType>::max())
     return createError(FieldName + " (" + Twine(SignedNum) +
                            ") does not fit in " + Twine(sizeof(FitType) * 8) +
                            "-bit signed integer type.",
                        std::errc::value_too_large);
 
   if (!CanBeNegative && SignedNum < 0)
     return createError(FieldName + " (" + Twine(SignedNum) +
                        ") cannot be negative.");
 
   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);
 }
 
 static bool stripQuotes(StringRef &Str, bool &IsLongString) {
   if (!Str.contains('"'))
     return false;
void ResourceFileWriter::writeRCInt(RCInt Value) {
   else
     writeInt<uint16_t>(Value);
 }
 
 Error ResourceFileWriter::appendFile(StringRef Filename) {
   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();
 }
 
 void ResourceFileWriter::padStream(uint64_t Length) {
   assert(Length > 0);
   uint64_t Location = tell();
   Location %= Length;
Error ResourceFileWriter::visitIconOrCursorResource(const RCResource *Base) {
   } else {
     auto *CursorRes = dyn_cast<CursorResource>(Base);
     FileStr = CursorRes->CursorLoc;
     Type = IconCursorGroupType::Cursor;
   }
 
   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);
 
   // Read the file headers.
   //   - At the beginning, ICONDIR/NEWHEADER header.
   //   - Then, a number of RESDIR headers follow. These contain offsets
   //       to data.
   const GroupIconDir *Header;
Error ResourceFileWriter::writeVersionInfoBody(const RCResource *Base) {
   RETURN_IF_ERROR(writeVersionInfoBlock(Res->MainBlock));
 
   // FIXME: check overflow?
   writeObjectAt(ulittle16_t(tell() - LengthLoc), LengthLoc);
 
   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

llvm/tools/llvm-rc/ResourceScriptParser.h

 
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <system_error>
 #include <vector>
 
 namespace llvm {
+namespace opt {
+class InputArgList;
+}
 namespace rc {
 
 class RCParser {
 public:
   using LocIter = std::vector<RCToken>::iterator;
   using ParseType = Expected<std::unique_ptr<RCResource>>;
   using ParseOptionType = Expected<std::unique_ptr<OptionalStmt>>;
 
public:
 
     static char ID; // Keep llvm::Error happy.
 
   private:
     std::string CurMessage;
     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.
   ParseType parseSingleResource();
 
   bool isEof() const;
 
 private:

llvm/tools/llvm-rc/ResourceScriptParser.cpp

 //===-- ResourceScriptParser.cpp --------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 //
 // This implements the parser defined in ResourceScriptParser.h.
 //
 //===---------------------------------------------------------------------===//
 
 #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)                                                  \
   if (auto Err = (Expr))                                                       \
     return std::move(Err);
 
 // Take an expression returning llvm::Expected<T> and assign it to Var or
 // forward the error out of the function.

llvm/tools/llvm-rc/llvm-rc.cpp

 
 class RcOptTable : public opt::OptTable {
 public:
   RcOptTable() : OptTable(InfoTable, /* IgnoreCase = */ true) {}
 };
 
 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);
 }
 
 } // anonymous namespace
 
 int main(int argc_, const char *argv_[]) {
   sys::PrintStackTraceOnErrorSignal(argv_[0]);
int main(int argc_, const char *argv_[]) {
   const bool BeVerbose = InputArgs.hasArg(OPT_VERBOSE);
 
   std::vector<std::string> InArgsInfo = InputArgs.getAllArgValues(OPT_INPUT);
   if (InArgsInfo.size() != 1) {
     fatalError("Exactly one input file should be provided.");
   }
 
   // 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());
   }
 
   std::unique_ptr<MemoryBuffer> FileContents = std::move(*File);
   StringRef Contents = FileContents->getBuffer();
 
   std::vector<RCToken> Tokens = ExitOnErr(tokenizeRC(Contents));
 
for (const RCToken &Token : Tokens) {
              << Token.value();
       if (Token.kind() == RCToken::Kind::Int)
         outs() << "; int value = " << Token.intValue();
 
       outs() << "\n";
     }
   }
 
+  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);
 
   if (!IsDryRun) {
     auto OutArgsInfo = InputArgs.getAllArgValues(OPT_FILEOUT);
     if (OutArgsInfo.size() != 1)
       fatalError(
           "Exactly one output file should be provided (using /FO flag).");
 
     std::error_code EC;
     auto FOut =
         llvm::make_unique<raw_fd_ostream>(OutArgsInfo[0], EC, sys::fs::F_RW);
     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()));
 
     // Set the default language; choose en-US arbitrarily.
     ExitOnErr(LanguageResource(0x09, 0x01).visit(Visitor.get()));
   }