FEXLoader: Fixes newer wine versions and Fedora

FEXHeaderUtils/FEXHeaderUtils/StringArgumentParser.h

@@ -0,0 +1,39 @@
+#pragma once
+#include <FEXCore/fextl/vector.h>
+#include <FEXCore/fextl/fmt.h>
+
+#include <algorithm>
+#include <string_view>
+
+namespace FHU {
+
+/**
+ * @brief Parses a string of arguments, returning a vector of string_views.
+ *
+ * @param ArgumentString The string of arguments to parse
+ *
+ * @return The array of parsed arguments
+ */
+static inline fextl::vector<std::string_view> ParseArgumentsFromString(const std::string_view ArgumentString) {
+  fextl::vector<std::string_view> Arguments;
+
+  auto Begin = ArgumentString.begin();
+  auto ArgEnd = Begin;
+  const auto End = ArgumentString.end();
+  while (ArgEnd != End && Begin != End) {
+    // The end of an argument ends with a space or the end of the interpreter line.
+    ArgEnd = std::find(Begin, End, ' ');
+
+    if (Begin != ArgEnd) {
+      const auto View = std::string_view(Begin, ArgEnd - Begin);
+      if (!View.empty()) {
+        Arguments.emplace_back(View);
+      }
+    }
+
+    Begin = ArgEnd + 1;
+  }
+
+  return Arguments;
+}
+} // namespace FHU

Source/Common/Config.cpp

@@ -337,7 +337,7 @@ fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInte
   return Program;
 }
 
-ApplicationNames GetApplicationNames(fextl::vector<fextl::string> Args, bool ExecFDInterp, int ProgramFDFromEnv) {
+ApplicationNames GetApplicationNames(const fextl::vector<fextl::string>& Args, bool ExecFDInterp, int ProgramFDFromEnv) {
   if (Args.empty()) {
     // Early exit if we weren't passed an argument
     return {};
@@ -346,8 +346,7 @@ ApplicationNames GetApplicationNames(fextl::vector<fextl::string> Args, bool Exe
   fextl::string Program {};
   fextl::string ProgramName {};
 
-  Args[0] = RecoverGuestProgramFilename(std::move(Args[0]), ExecFDInterp, ProgramFDFromEnv);
-  Program = Args[0];
+  Program = RecoverGuestProgramFilename(Args[0], ExecFDInterp, ProgramFDFromEnv);
 
   bool Wine = false;
   for (size_t CurrentProgramNameIndex = 0; CurrentProgramNameIndex < Args.size(); ++CurrentProgramNameIndex) {

Source/Common/Config.h

@@ -40,7 +40,7 @@ struct PortableInformation {
  *
  * @return The application name and path structure
  */
-ApplicationNames GetApplicationNames(fextl::vector<fextl::string> Args, bool ExecFDInterp, int ProgramFDFromEnv);
+ApplicationNames GetApplicationNames(const fextl::vector<fextl::string>& Args, bool ExecFDInterp, int ProgramFDFromEnv);
 
 /**
  * @brief Loads the FEX and application configurations for the application that is getting ready to run.

Source/Tools/FEXLoader/FEXLoader.cpp

@@ -38,6 +38,7 @@ desc: Glues the ELF loader, FEXCore and LinuxSyscalls to launch an elf under fex
 #include <FEXCore/fextl/string.h>
 #include <FEXCore/fextl/vector.h>
 #include <FEXHeaderUtils/Filesystem.h>
+#include <FEXHeaderUtils/StringArgumentParser.h>
 
 #include <atomic>
 #include <cerrno>
@@ -135,63 +136,44 @@ class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriter {
 };
 } // namespace AOTIR
 
-void InterpreterHandler(fextl::string* Filename, const fextl::string& RootFS, fextl::vector<fextl::string>* args) {
-  // Open the Filename to determine if it is a shebang file.
-  int FD = open(Filename->c_str(), O_RDONLY | O_CLOEXEC);
+bool InterpreterHandler(fextl::string* Filename, const fextl::string& RootFS, fextl::vector<fextl::string>* args) {
+  int FD {-1};
+
+  // Attempt to open the filename from the rootfs first.
+  FD = open(fextl::fmt::format("{}{}", RootFS, *Filename).c_str(), O_RDONLY | O_CLOEXEC);
   if (FD == -1) {
-    return;
+    // Failing that, attempt to open the filename directly.
+    FD = open(Filename->c_str(), O_RDONLY | O_CLOEXEC);
+    if (FD == -1) {
+      return false;
+    }
   }
 
   std::array<char, 257> Header;
   const auto ChunkSize = 257l;
   const auto ReadSize = pread(FD, &Header.at(0), ChunkSize, 0);
+  close(FD);
 
   const auto Data = std::span<char>(Header.data(), ReadSize);
 
   // Is the file large enough for shebang
   if (ReadSize <= 2) {
-    close(FD);
-    return;
+    return false;
   }
 
   // Handle shebang files
   if (Data[0] == '#' && Data[1] == '!') {
-    fextl::string InterpreterLine {Data.begin() + 2, // strip off "#!" prefix
-                                   std::find(Data.begin(), Data.end(), '\n')};
-    fextl::vector<fextl::string> ShebangArguments {};
-
-    // Shebang line can have a single argument
-    fextl::istringstream InterpreterSS(InterpreterLine);
-    fextl::string Argument;
-    while (std::getline(InterpreterSS, Argument, ' ')) {
-      if (Argument.empty()) {
-        continue;
-      }
-      ShebangArguments.push_back(std::move(Argument));
-    }
+    std::string_view InterpreterLine {Data.begin() + 2, // strip off "#!" prefix
+                                      std::find(Data.begin(), Data.end(), '\n')};
+    const auto ShebangArguments = FHU::ParseArgumentsFromString(InterpreterLine);
 
     // Executable argument
-    fextl::string& ShebangProgram = ShebangArguments[0];
-
-    // If the filename is absolute then prepend the rootfs
-    // If it is relative then don't append the rootfs
-    if (ShebangProgram[0] == '/') {
-      ShebangProgram = RootFS + ShebangProgram;
-    }
-    *Filename = ShebangProgram;
+    *Filename = ShebangArguments.at(0);
 
     // Insert all the arguments at the start
     args->insert(args->begin(), ShebangArguments.begin(), ShebangArguments.end());
   }
-  close(FD);
-}
-
-void RootFSRedirect(fextl::string* Filename, const fextl::string& RootFS) {
-  auto RootFSLink = ELFCodeLoader::ResolveRootfsFile(*Filename, RootFS);
-
-  if (FHU::Filesystem::Exists(RootFSLink)) {
-    *Filename = RootFSLink;
-  }
+  return true;
 }
 
 FEX::Config::PortableInformation ReadPortabilityInformation() {
@@ -435,10 +417,21 @@ int main(int argc, char** argv, char** const envp) {
   FEXCore::Profiler::Init();
   FEXCore::Telemetry::Initialize();
 
-  RootFSRedirect(&Program.ProgramPath, LDPath());
-  InterpreterHandler(&Program.ProgramPath, LDPath(), &Args);
+  if (Program.ProgramPath.starts_with(LDPath())) {
+    // From this point on, ProgramPath needs to not have the LDPath prefixed on to it.
+    auto RootFSLength = LDPath().size();
+    if (Program.ProgramPath.at(RootFSLength) != '/') {
+      // Ensure the modified path starts as an absolute path.
+      // This edge case can occur when ROOTFS ends with '/' and passed a path like `<ROOTFS>usr/bin/true`.
+      --RootFSLength;
+    }
+
+    Program.ProgramPath.erase(0, RootFSLength);
+  }
+
+  bool ProgramExists = InterpreterHandler(&Program.ProgramPath, LDPath(), &Args);
 
-  if (!ExecutedWithFD && FEXFD == -1 && !FHU::Filesystem::Exists(Program.ProgramPath)) {
+  if (!ExecutedWithFD && FEXFD == -1 && !ProgramExists) {
     // Early exit if the program passed in doesn't exist
     // Will prevent a crash later
     fextl::fmt::print(stderr, "{}: command not found\n", Program.ProgramPath);

unittests/APITests/ArgumentParser.cpp

@@ -0,0 +1,75 @@
+#include <catch2/catch_test_macros.hpp>
+
+#include <FEXHeaderUtils/StringArgumentParser.h>
+
+TEST_CASE("Basic") {
+  const auto ArgString = "Test a b c";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 4);
+  CHECK(Args.at(0) == "Test");
+  CHECK(Args.at(1) == "a");
+  CHECK(Args.at(2) == "b");
+  CHECK(Args.at(3) == "c");
+}
+
+TEST_CASE("Basic - Empty") {
+  const auto ArgString = "";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 0);
+}
+
+TEST_CASE("Basic - Empty spaces") {
+  const auto ArgString = "                       ";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 0);
+}
+
+TEST_CASE("Basic - Space at start") {
+  const auto ArgString = "      Test a b c";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 4);
+  CHECK(Args.at(0) == "Test");
+  CHECK(Args.at(1) == "a");
+  CHECK(Args.at(2) == "b");
+  CHECK(Args.at(3) == "c");
+}
+
+TEST_CASE("Basic - Bonus spaces between args") {
+  const auto ArgString = "Test       a      b      c";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 4);
+  CHECK(Args.at(0) == "Test");
+  CHECK(Args.at(1) == "a");
+  CHECK(Args.at(2) == "b");
+  CHECK(Args.at(3) == "c");
+}
+
+TEST_CASE("Basic - non printable") {
+  const auto ArgString = "Test a b \x01c";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 4);
+  CHECK(Args.at(0) == "Test");
+  CHECK(Args.at(1) == "a");
+  CHECK(Args.at(2) == "b");
+  CHECK(Args.at(3) == "\x01c");
+}
+
+TEST_CASE("Basic - Emoji") {
+  const auto ArgString = "Test a b 🐸";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 4);
+  CHECK(Args.at(0) == "Test");
+  CHECK(Args.at(1) == "a");
+  CHECK(Args.at(2) == "b");
+  CHECK(Args.at(3) == "🐸");
+}
+
+TEST_CASE("Basic - space at the end") {
+  const auto ArgString = "Test a b 🐸        ";
+  auto Args = FHU::ParseArgumentsFromString(ArgString);
+  REQUIRE(Args.size() == 4);
+  CHECK(Args.at(0) == "Test");
+  CHECK(Args.at(1) == "a");
+  CHECK(Args.at(2) == "b");
+  CHECK(Args.at(3) == "🐸");
+}

unittests/APITests/CMakeLists.txt

@@ -1,5 +1,6 @@
 set (TESTS
   Allocator
+  ArgumentParser
   InterruptableConditionVariable
   Filesystem
   )