Merge pull request #3639 from Sonicadvance1/cleanupFD

FEXLoader: Cleanup FD extraction from environment variables

Source/Common/Config.cpp

@@ -282,15 +282,14 @@ fextl::unique_ptr<FEXCore::Config::Layer> CreateEnvironmentLayer(char* const _en
   return fextl::make_unique<EnvLoader>(_envp);
 }
 
-fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInterp, const std::string_view ProgramFDFromEnv) {
+fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInterp, int ProgramFDFromEnv) {
   // If executed with a FEX FD then the Program argument might be empty.
   // In this case we need to scan the FD node to recover the application binary that exists on disk.
   // Only do this if the Program argument is empty, since we would prefer the application's expectation
   // of application name.
-  if (!ProgramFDFromEnv.empty() && Program.empty()) {
+  if (ProgramFDFromEnv != -1 && Program.empty()) {
     // Get the `dev` node of the execveat fd string.
-    Program = "/dev/fd/";
-    Program += ProgramFDFromEnv;
+    Program = fextl::fmt::format("/dev/fd/{}", ProgramFDFromEnv);
   }
 
   // If we were provided a relative path then we need to canonicalize it to become absolute.
@@ -328,7 +327,7 @@ fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInte
   //  - Regular execveat with FD. FD points to file on disk that has been deleted.
   //    execveat binfmt_misc args layout: `FEXInterpreter /dev/fd/<FD> <user provided argv[0]> <user provided argv[n]>...`
 #ifndef _WIN32
-  if (ExecFDInterp || !ProgramFDFromEnv.empty()) {
+  if (ExecFDInterp || ProgramFDFromEnv != -1) {
     // Only in the case that FEX is executing an FD will the program argument potentially be a symlink.
     // This symlink will be in the style of `/dev/fd/<FD>`.
     //
@@ -349,8 +348,8 @@ fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInte
   return Program;
 }
 
-ApplicationNames LoadConfig(bool NoFEXArguments, bool LoadProgramConfig, int argc, char** argv, char** const envp, bool ExecFDInterp,
-                            const std::string_view ProgramFDFromEnv) {
+ApplicationNames
+LoadConfig(bool NoFEXArguments, bool LoadProgramConfig, int argc, char** argv, char** const envp, bool ExecFDInterp, int ProgramFDFromEnv) {
   FEX::Config::InitializeConfigs();
   FEXCore::Config::Initialize();
   FEXCore::Config::AddLayer(CreateGlobalMainLayer());

Source/Common/Config.h

@@ -38,8 +38,8 @@ struct ApplicationNames {
  *
  * @return The application name and path structure
  */
-ApplicationNames LoadConfig(bool NoFEXArguments, bool LoadProgramConfig, int argc, char** argv, char** const envp, bool ExecFDInterp,
-                            const std::string_view ProgramFDFromEnv);
+ApplicationNames
+LoadConfig(bool NoFEXArguments, bool LoadProgramConfig, int argc, char** argv, char** const envp, bool ExecFDInterp, int ProgramFDFromEnv);
 
 const char* GetHomeDirectory();
 

Source/Tools/FEXLoader/ELFCodeLoader.h

@@ -231,7 +231,7 @@ public:
 
   fextl::vector<LoadedSection> Sections;
 
-  ELFCodeLoader(const fextl::string& Filename, const std::string_view FEXFDString, const fextl::string& RootFS,
+  ELFCodeLoader(const fextl::string& Filename, int ProgramFDFromEnv, const fextl::string& RootFS,
                 [[maybe_unused]] const fextl::vector<fextl::string>& args, const fextl::vector<fextl::string>& ParsedArgs,
                 char** const envp = nullptr, FEXCore::Config::Value<fextl::string>* AdditionalEnvp = nullptr)
     : Args {args} {
@@ -239,16 +239,9 @@ public:
     bool LoadedWithFD = false;
     int FD = getauxval(AT_EXECFD);
 
-    if (!FEXFDString.empty()) {
+    if (ProgramFDFromEnv != -1) {
       // If we passed the execve FD to us then use that.
-      const char* StartPtr = FEXFDString.data();
-      char* EndPtr {};
-      FD = ::strtol(StartPtr, &EndPtr, 10);
-      if (EndPtr == StartPtr) {
-        LogMan::Msg::AFmt("FEXInterpreter passed invalid FD to exececute: {}", FEXFDString);
-        return;
-      }
-      unsetenv("FEX_EXECVEFD");
+      FD = ProgramFDFromEnv;
     }
 
     // If we are provided an EXECFD then attempt to execute that first

Source/Tools/FEXLoader/FEXLoader.cpp

@@ -245,21 +245,38 @@ void SetupTSOEmulation(FEXCore::Context::Context* CTX) {
 }
 } // namespace FEX::TSO
 
+/**
+ * @brief Get an FD from an environment variable and then unset the environment variable.
+ *
+ * @param Env The environment variable to extract the FD from.
+ *
+ * @return -1 if the variable didn't exist.
+ */
+static int StealFEXFDFromEnv(const char* Env) {
+  int FEXFD {-1};
+  const char* FEXFDStr = getenv(Env);
+  if (FEXFDStr) {
+    const std::string_view FEXFDView {FEXFDStr};
+    std::from_chars(FEXFDView.data(), FEXFDView.data() + FEXFDView.size(), FEXFD, 10);
+    unsetenv(Env);
+  }
+  return FEXFD;
+}
+
 int main(int argc, char** argv, char** const envp) {
   auto SBRKPointer = FEXCore::Allocator::DisableSBRKAllocations();
   FEXCore::Allocator::GLIBCScopedFault GLIBFaultScope;
   const bool IsInterpreter = RanAsInterpreter(argv[0]);
 
   ExecutedWithFD = getauxval(AT_EXECFD) != 0;
-  const char* FEXFD = getenv("FEX_EXECVEFD");
-  const std::string_view FEXFDView = FEXFD ? std::string_view {FEXFD} : std::string_view {};
+  int FEXFD {StealFEXFDFromEnv("FEX_EXECVEFD")};
 
   LogMan::Throw::InstallHandler(AssertHandler);
   LogMan::Msg::InstallHandler(MsgHandler);
 
-  auto Program = FEX::Config::LoadConfig(IsInterpreter, true, argc, argv, envp, ExecutedWithFD, FEXFDView);
+  auto Program = FEX::Config::LoadConfig(IsInterpreter, true, argc, argv, envp, ExecutedWithFD, FEXFD);
 
-  if (Program.ProgramPath.empty() && !FEXFD) {
+  if (Program.ProgramPath.empty() && FEXFD == -1) {
     // Early exit if we weren't passed an argument
     return 0;
   }
@@ -345,7 +362,7 @@ int main(int argc, char** argv, char** const envp) {
   RootFSRedirect(&Program.ProgramPath, LDPath());
   InterpreterHandler(&Program.ProgramPath, LDPath(), &Args);
 
-  if (!ExecutedWithFD && !FEXFD && !FHU::Filesystem::Exists(Program.ProgramPath)) {
+  if (!ExecutedWithFD && FEXFD == -1 && !FHU::Filesystem::Exists(Program.ProgramPath)) {
     // 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);
@@ -365,7 +382,7 @@ int main(int argc, char** argv, char** const envp) {
     putenv(HostEnv.data());
   }
 
-  ELFCodeLoader Loader {Program.ProgramPath, FEXFDView, LDPath(), Args, ParsedArgs, envp, &Environment};
+  ELFCodeLoader Loader {Program.ProgramPath, FEXFD, LDPath(), Args, ParsedArgs, envp, &Environment};
 
   if (!Loader.ELFWasLoaded()) {
     // Loader couldn't load this program for some reason
@@ -385,7 +402,7 @@ int main(int argc, char** argv, char** const envp) {
     // Don't need to canonicalize Program.ProgramPath, Config loader will have resolved this already.
     FEXCore::Config::EraseSet(FEXCore::Config::CONFIG_APP_FILENAME, Program.ProgramPath);
     FEXCore::Config::EraseSet(FEXCore::Config::CONFIG_APP_CONFIG_NAME, Program.ProgramName);
-  } else if (FEXFD) {
+  } else if (FEXFD != -1) {
     // Anonymous program.
     FEXCore::Config::EraseSet(FEXCore::Config::CONFIG_APP_FILENAME, "<Anonymous>");
     FEXCore::Config::EraseSet(FEXCore::Config::CONFIG_APP_CONFIG_NAME, "<Anonymous>");