[clang-tools-extra] [clang-tidy] fix incorrect configuration file path resolving when file paths contain `..` (PR #121323)

via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 30 00:39:20 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

<details>
<summary>Changes</summary>

`makeAbsolute` will not normalize path. When getting parent folder, `..` will go into the subfolder instead of the parent folder.


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


6 Files Affected:

- (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.cpp (+21-11) 
- (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.h (+4) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3) 
- (added) clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/code.cpp () 
- (added) clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/error-config/.clang-tidy (+1) 
- (added) clang-tools-extra/test/clang-tidy/infrastructure/normalized-path.test (+3) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 445c7f85c900c6..1ad278e5d9a795 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -12,6 +12,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Support/Path.h"
@@ -298,12 +299,11 @@ ConfigOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   if (ConfigOptions.InheritParentConfig.value_or(false)) {
     LLVM_DEBUG(llvm::dbgs()
                << "Getting options for file " << FileName << "...\n");
-    assert(FS && "FS must be set.");
 
-    llvm::SmallString<128> AbsoluteFilePath(FileName);
-
-    if (!FS->makeAbsolute(AbsoluteFilePath)) {
-      addRawFileOptions(AbsoluteFilePath, RawOptions);
+    llvm::ErrorOr<llvm::SmallString<128>> AbsoluteFilePath =
+        getNormalizedAbsolutePath(FileName);
+    if (AbsoluteFilePath) {
+      addRawFileOptions(AbsoluteFilePath->str(), RawOptions);
     }
   }
   RawOptions.emplace_back(ConfigOptions,
@@ -334,6 +334,17 @@ FileOptionsBaseProvider::FileOptionsBaseProvider(
       OverrideOptions(std::move(OverrideOptions)),
       ConfigHandlers(std::move(ConfigHandlers)) {}
 
+llvm::ErrorOr<llvm::SmallString<128>>
+FileOptionsBaseProvider::getNormalizedAbsolutePath(llvm::StringRef Path) {
+  assert(FS && "FS must be set.");
+  llvm::SmallString<128> NormalizedAbsolutePath = {Path};
+  std::error_code Err = FS->makeAbsolute(NormalizedAbsolutePath);
+  if (Err)
+    return Err;
+  llvm::sys::path::remove_dots(NormalizedAbsolutePath, /*remove_dot_dot=*/true);
+  return NormalizedAbsolutePath;
+}
+
 void FileOptionsBaseProvider::addRawFileOptions(
     llvm::StringRef AbsolutePath, std::vector<OptionsSource> &CurOptions) {
   auto CurSize = CurOptions.size();
@@ -396,16 +407,15 @@ std::vector<OptionsSource>
 FileOptionsProvider::getRawOptions(StringRef FileName) {
   LLVM_DEBUG(llvm::dbgs() << "Getting options for file " << FileName
                           << "...\n");
-  assert(FS && "FS must be set.");
-
-  llvm::SmallString<128> AbsoluteFilePath(FileName);
 
-  if (FS->makeAbsolute(AbsoluteFilePath))
+  llvm::ErrorOr<llvm::SmallString<128>> AbsoluteFilePath =
+      getNormalizedAbsolutePath(FileName);
+  if (!AbsoluteFilePath)
     return {};
 
   std::vector<OptionsSource> RawOptions =
-      DefaultOptionsProvider::getRawOptions(AbsoluteFilePath.str());
-  addRawFileOptions(AbsoluteFilePath, RawOptions);
+      DefaultOptionsProvider::getRawOptions(AbsoluteFilePath->str());
+  addRawFileOptions(AbsoluteFilePath->str(), RawOptions);
   OptionsSource CommandLineOptions(OverrideOptions,
                                    OptionsSourceTypeCheckCommandLineOption);
 
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index 85d5a02ebbc1bc..95abe932d56ccf 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H
 
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorOr.h"
@@ -237,6 +238,9 @@ class FileOptionsBaseProvider : public DefaultOptionsProvider {
   void addRawFileOptions(llvm::StringRef AbsolutePath,
                          std::vector<OptionsSource> &CurOptions);
 
+  llvm::ErrorOr<llvm::SmallString<128>>
+  getNormalizedAbsolutePath(llvm::StringRef AbsolutePath);
+
   /// Try to read configuration files from \p Directory using registered
   /// \c ConfigHandlers.
   std::optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3cab440155250b..2bbe7324b1f4d4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -117,6 +117,9 @@ Improvements to clang-tidy
 
 - Improved :program:`clang-tidy` by accepting parameters file in command line.
 
+- Improved :program:`clang-tidy` by fixing incorrect configuration file path
+  resolving when file paths contain ``..``.
+
 - Removed :program:`clang-tidy`'s global options for most of checks. All options
   are changed to local options except `IncludeStyle`, `StrictMode` and
   `IgnoreMacros`.
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/code.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/code.cpp
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/error-config/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/error-config/.clang-tidy
new file mode 100644
index 00000000000000..83bc4d519722bf
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/normalized-path/error-config/.clang-tidy
@@ -0,0 +1 @@
+InvalidYamlFormat
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/normalized-path.test b/clang-tools-extra/test/clang-tidy/infrastructure/normalized-path.test
new file mode 100644
index 00000000000000..d14ad43bb8b137
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/normalized-path.test
@@ -0,0 +1,3 @@
+// RUN: clang-tidy %S/Inputs/normalized-path/error-config/../code.cpp --verify-config 2>&1 | FileCheck %s
+
+// CHECK-NOT: Error parsing

``````````

</details>


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


More information about the cfe-commits mailing list