[Clang][Driver] Ensure default sysroot is computed and passed to cc1

[fanghuaqi]

Greetings, everyone.

I found an issue in #82084, and discussed my concerns in #82084 (comment) and I fixed it in my local branch, and @jansvoboda11 recommended me to submit a PR to fix it. Here are the descriptions for it.

Previously, the driver would only add the -isysroot flag to the cc1 command if an initial sysroot (e.g., from the --sysroot argument) was non-empty.

This logic was flawed because it ignored toolchains that can compute a default sysroot (e.g., for bare-metal targets). When no sysroot was provided on the command line, the computeSysRoot() method was never called, and cc1 was invoked without the necessary -isysroot flag. This could lead to failures in locating system headers, especially those added via sysroot-relative paths, such as -isystem=/include/libncrt, were incorrectly resolved instead of the intended target's sysroot.

This commit refactors the logic to:

1. First, check if the initial sysroot is empty.
2. If it is, call the toolchain's computeSysRoot() method to get the default computed sysroot.
3. Finally, pass the determined sysroot to cc1 via -isysroot if one wasn't already specified by the user.

See discussion at: https://github.com/llvm/llvm-project/pull/82084/files#r2387373311

Thanks
Huaqi

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Huaqi Fang (fanghuaqi)

Changes

Greetings, everyone.

I found an issue in #82084, and discussed my concerns in #82084 (comment) and I fixed it in my local branch, and @jansvoboda11 recommended me to submit a PR to fix it. Here are the descriptions for it.

Previously, the driver would only add the -isysroot flag to the cc1 command if an initial sysroot (e.g., from the --sysroot argument) was non-empty.

This logic was flawed because it ignored toolchains that can compute a default sysroot (e.g., for bare-metal targets). When no sysroot was provided on the command line, the computeSysRoot() method was never called, and cc1 was invoked without the necessary -isysroot flag. This could lead to failures in locating system headers, especially those added via sysroot-relative paths, such as -isystem=/include/libncrt, were incorrectly resolved instead of the intended target's sysroot.

This commit refactors the logic to:

1. First, check if the initial sysroot is empty.
2. If it is, call the toolchain's computeSysRoot() method to get the default computed sysroot.
3. Finally, pass the determined sysroot to cc1 via -isysroot if one wasn't already specified by the user.

See discussion at: https://github.com/llvm/llvm-project/pull/82084/files#r2387373311

Thanks
Huaqi

---

Full diff: https://github.com/llvm/llvm-project/pull/163139.diff

1 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+7-6)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index bf755739760c0..a16c94ac9670a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1083,12 +1083,13 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
 
   // If we have a --sysroot, and don't have an explicit -isysroot flag, add an
   // -isysroot to the CC1 invocation.
-  StringRef sysroot = C.getSysRoot();
-  if (sysroot != "") {
-    if (!Args.hasArg(options::OPT_isysroot)) {
-      CmdArgs.push_back("-isysroot");
-      CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
-    }
+  std::string sysroot = std::string(C.getSysRoot());
+  if (sysroot.empty()) {
+    sysroot = getToolChain().computeSysRoot();
+  }
+  if (!Args.hasArg(options::OPT_isysroot)) {
+    CmdArgs.push_back("-isysroot");
+    CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
   }
 
   // Parse additional include paths from environment variables.

[jansvoboda11]

Can you please add a test for this?

[fanghuaqi]

Can you please add a test for this?
I have no idea about how to add test case for it, I checked this test code https://github.com/etcwilde/llvm-project/blob/main/clang/test/Preprocessor/sysroot-prefix.c, since there is no sysroot such as newlib sysroot built during clang ci, so I think this sysroot will be empty, it will not able to test this patch, but in my local site, I have tested, it works as expected for me.