Skip to content

[Clang][Driver] Improve config file handling on Darwin #111306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

carlocab
Copy link
Member

@carlocab carlocab commented Oct 6, 2024

On Darwin, the default target triple contains the version of the kernel:

❯ clang --print-target-triple
arm64-apple-darwin24.0.0
❯ uname -r
24.0.0

This makes writing config files for the target triple rather cumbersome,
because they require including the full version of the kernel in the
filename when one generally cares only about the major version if at
all.

Let's improve this by also checking for major-versioned and unversioned
.cfg files if we weren't able to find a .cfg file for the full
triple when targetting Darwin.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Carlo Cabrera (carlocab)

Changes

On Darwin, the default target triple contains the version of the kernel:

❯ clang --print-target-triple
arm64-apple-darwin24.0.0
❯ uname -r
24.0.0

This makes writing config files for the target triple rather cumbersome,
because they require including the full version of the kernel in the
filename when one generally cares only about the major version if at
all.

Let's improve this by also checking for major-versioned and unversioned
.cfg files if we weren't able to find a .cfg file for the full
triple when targetting Darwin.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+19)
  • (added) clang/test/Driver/config-file-darwin.c (+24)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 2aaa52072b03d2..e9f241228d4aab 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -95,6 +95,7 @@
 #include "llvm/TargetParser/Host.h"
 #include "llvm/TargetParser/RISCVISAInfo.h"
 #include <cstdlib> // ::getenv
+#include <cstring>
 #include <map>
 #include <memory>
 #include <optional>
@@ -1222,6 +1223,24 @@ bool Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
   if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
     return readConfigFile(CfgFilePath, ExpCtx);
 
+  // On Darwin, try a major-versioned triple then an unversioned triple.
+  auto pos = Triple.find("-apple-darwin");
+  auto kernelVersionStart = pos + std::strlen("-apple-darwin");
+  if (pos != Triple.npos && Triple.length() > kernelVersionStart) {
+    // First, find the major-versioned triple (e.g. arm64-apple-darwin24).
+    auto T = Triple.substr(0, Triple.find(".", kernelVersionStart));
+    CfgFileName = T + ".cfg";
+    if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+      return readConfigFile(CfgFilePath, ExpCtx);
+
+    // If that is not available, try an unversioned triple
+    // (e.g. arm64-apple-darwin).
+    T = Triple.substr(0, kernelVersionStart);
+    CfgFileName = T + ".cfg";
+    if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+      return readConfigFile(CfgFilePath, ExpCtx);
+  }
+
   // If we were unable to find a config file deduced from executable name,
   // that is not an error.
   return false;
diff --git a/clang/test/Driver/config-file-darwin.c b/clang/test/Driver/config-file-darwin.c
new file mode 100644
index 00000000000000..c24eb7ad19fe67
--- /dev/null
+++ b/clang/test/Driver/config-file-darwin.c
@@ -0,0 +1,24 @@
+// REQUIRES: shell
+
+// RUN: unset CLANG_NO_DEFAULT_CONFIG
+// RUN: rm -rf %t && mkdir %t
+
+//--- Major-versioned config files are used when targetting *-apple-darwin*
+//
+// RUN: mkdir -p %t/testbin
+// RUN: ln -s %clang %t/testbin/x86_64-apple-darwin24.0.0-clang
+// RUN: echo "-Werror" > %t/testbin/x86_64-apple-darwin24.cfg
+// RUN: %t/testbin/x86_64-apple-darwin24.0.0-clang --config-system-dir= --config-user-dir= -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-MAJOR-VERSIONED
+//
+// CHECK-MAJOR-VERSIONED: Configuration file: {{.*}}/testbin/x86_64-apple-darwin24.cfg
+// CHECK-MAJOR-VERSIONED: -Werror
+
+//--- Unversioned config files are used when targetting *-apple-darwin*
+//
+// RUN: mkdir -p %t/testbin
+// RUN: ln -s %clang %t/testbin/arm64-apple-darwin23.1.2-clang
+// RUN: echo "-Werror" > %t/testbin/arm64-apple-darwin.cfg
+// RUN: %t/testbin/arm64-apple-darwin23.1.2-clang --config-system-dir= --config-user-dir= -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-UNVERSIONED
+//
+// CHECK-UNVERSIONED: Configuration file: {{.*}}/testbin/arm64-apple-darwin.cfg
+// CHECK-UNVERSIONED: -Werror

Copy link

github-actions bot commented Oct 6, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

On Darwin, the default target triple contains the version of the kernel:

    ❯ clang --print-target-triple
    arm64-apple-darwin24.0.0
    ❯ uname -r
    24.0.0

This makes writing config files for the target triple rather cumbersome,
because they require including the full version of the kernel in the
filename when one generally cares only about the major version if at
all.

Let's improve this by also checking for major-versioned and unversioned
`.cfg` files if we weren't able to find a `.cfg` file for the full
triple when targetting Darwin.
@carlocab
Copy link
Member Author

carlocab commented Oct 6, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off Keep my email addresses private setting in your account. See LLVM Discourse for more information.

This seems to be based on the commit author rather than my actual GitHub configuration. See also #110962 (comment)

Fixed now.

@carlocab
Copy link
Member Author

carlocab commented Oct 7, 2024

#111387 is a better approach to this.

@carlocab carlocab closed this Oct 7, 2024
@carlocab carlocab deleted the darwin-cfg-file branch October 7, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants