[clang] [PS4, PS5][Driver] Detangle --sysroot and -isysroot (PR #107410)

[Paul T Robinson via cfe-commits]

================
@@ -323,46 +323,63 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const llvm::Triple &Triple,
                                    const ArgList &Args, StringRef Platform,
                                    const char *EnvVar)
     : Generic_ELF(D, Triple, Args) {
-  // Determine where to find the PS4/PS5 libraries.
-  // If -isysroot was passed, use that as the SDK base path.
-  // If not, we use the EnvVar if it exists; otherwise use the driver's
-  // installation path, which should be <SDK_DIR>/host_tools/bin.
+  // Determine the baseline SDK directory from the environment, else
+  // the driver's location, which should be <SDK_DIR>/host_tools/bin.
+  SmallString<128> SDKRootDir;
   SmallString<80> Whence;
-  if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
-    SDKRootDir = A->getValue();
-    if (!llvm::sys::fs::exists(SDKRootDir))
-      D.Diag(clang::diag::warn_missing_sysroot) << SDKRootDir;
-    Whence = A->getSpelling();
-  } else if (const char *EnvValue = getenv(EnvVar)) {
+  if (const char *EnvValue = getenv(EnvVar)) {
     SDKRootDir = EnvValue;
-    Whence = { "environment variable '", EnvVar, "'" };
+    Whence = {"environment variable '", EnvVar, "'"};
   } else {
     SDKRootDir = D.Dir + "/../../";
     Whence = "compiler's location";
   }
 
-  SmallString<512> SDKIncludeDir(SDKRootDir);
-  llvm::sys::path::append(SDKIncludeDir, "target/include");
-  if (!Args.hasArg(options::OPT_nostdinc) &&
-      !Args.hasArg(options::OPT_nostdlibinc) &&
-      !Args.hasArg(options::OPT_isysroot) &&
-      !Args.hasArg(options::OPT__sysroot_EQ) &&
-      !llvm::sys::fs::exists(SDKIncludeDir)) {
-    D.Diag(clang::diag::warn_drv_unable_to_find_directory_expected)
-        << Twine(Platform, " system headers").str() << SDKIncludeDir << Whence;
-  }
+  // Allow --sysroot= to override the root directory for header and library
+  // search, and -sysroot to override header search. If both are specified,
+  // -isysroot overrides --sysroot for header search.
+  auto OverrideRoot = [&](const options::ID &Opt, std::string &Root,
+                          StringRef Default) {
+    if (const Arg *A = Args.getLastArg(Opt)) {
+      Root = A->getValue();
+      if (!llvm::sys::fs::exists(Root))
+        D.Diag(clang::diag::warn_missing_sysroot) << Root;
+      return true;
+    }
+    Root = Default.str();
+    return false;
+  };
 
-  SmallString<512> SDKLibDir(SDKRootDir);
-  llvm::sys::path::append(SDKLibDir, "target/lib");
-  if (!Args.hasArg(options::OPT__sysroot_EQ) && !Args.hasArg(options::OPT_E) &&
-      !Args.hasArg(options::OPT_c) && !Args.hasArg(options::OPT_S) &&
-      !Args.hasArg(options::OPT_emit_ast) &&
-      !llvm::sys::fs::exists(SDKLibDir)) {
+  bool CustomSysroot =
+      OverrideRoot(options::OPT__sysroot_EQ, SDKLibraryRootDir, SDKRootDir);
+  bool CustomISysroot =
+      OverrideRoot(options::OPT_isysroot, SDKHeaderRootDir, SDKLibraryRootDir);
+
+  // Emit warnings if parts of the SDK are missing, unless the user has taken
+  // control of header or library search. If we're not linking, don't check
+  // for missing libraries.
+  auto CheckSDKPartExists = [&](StringRef Dir, StringRef Desc) {
+    if (llvm::sys::fs::exists(Dir))
+      return true;
     D.Diag(clang::diag::warn_drv_unable_to_find_directory_expected)
-        << Twine(Platform, " system libraries").str() << SDKLibDir << Whence;
-    return;
+        << (Twine(Platform) + " " + Desc).str() << Dir << Whence;
+    return false;
+  };
+
+  bool Linking = !Args.hasArg(options::OPT_E, options::OPT_c, options::OPT_S,
+                              options::OPT_emit_ast);
+  if (!CustomSysroot && Linking) {
+    SmallString<128> Dir(SDKLibraryRootDir);
+    llvm::sys::path::append(Dir, "target/lib");
+    if (CheckSDKPartExists(Dir, "system libraries"))
+      getFilePaths().push_back(std::string(Dir));
----------------
pogo59 wrote:

This `push_back` doesn't happen if there's a custom sysroot? Offhand I don't recall what the effect of FilePaths is, but I think that's not what we want.

https://github.com/llvm/llvm-project/pull/107410