Skip to content

[analyzer] Use getFileName and do not use realpath names #126039

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

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

usx95
Copy link
Contributor

@usx95 usx95 commented Feb 6, 2025

The real paths resolves symlinks and makes the tests fail when the filesystem is a symlink tree over a content-addressable storage (our internal environment).

@usx95 usx95 requested review from kadircet and hokein February 6, 2025 09:49
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Feb 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Utkarsh Saxena (usx95)

Changes

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

1 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Core/BugReporter.cpp (+6-6)
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 4100812c4623e90..13677ed341d0c4c 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -299,7 +299,8 @@ std::string timeTraceName(const BugReportEquivClass &EQ) {
   return ("Flushing EQC " + BT.getDescription()).str();
 }
 
-llvm::TimeTraceMetadata timeTraceMetadata(const BugReportEquivClass &EQ) {
+llvm::TimeTraceMetadata timeTraceMetadata(const BugReportEquivClass &EQ,
+                                          const SourceManager &SM) {
   // Must be called only when constructing non-bogus TimeTraceScope
   assert(llvm::timeTraceProfilerEnabled());
 
@@ -309,9 +310,7 @@ llvm::TimeTraceMetadata timeTraceMetadata(const BugReportEquivClass &EQ) {
   const BugReport *R = BugReports.front().get();
   const auto &BT = R->getBugType();
   auto Loc = R->getLocation().asLocation();
-  std::string File = "";
-  if (const auto *Entry = Loc.getFileEntry())
-    File = Entry->tryGetRealPathName().str();
+  std::string File = SM.getFilename(Loc).str();
   return {BT.getCheckerName().str(), std::move(File),
           static_cast<int>(Loc.getLineNumber())};
 }
@@ -3150,8 +3149,9 @@ BugReport *PathSensitiveBugReporter::findReportInEquivalenceClass(
 }
 
 void BugReporter::FlushReport(BugReportEquivClass &EQ) {
-  llvm::TimeTraceScope TCS{timeTraceName(EQ),
-                           [&EQ]() { return timeTraceMetadata(EQ); }};
+  llvm::TimeTraceScope TCS{timeTraceName(EQ), [&]() {
+                             return timeTraceMetadata(EQ, getSourceManager());
+                           }};
   SmallVector<BugReport*, 10> bugReports;
   BugReport *report = findReportInEquivalenceClass(EQ, bugReports);
   if (!report)

Copy link
Contributor

@ilya-biryukov ilya-biryukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a description mentioning that real paths resolves symlinks and makes the tests fail when some forms of content-addressable filesystem is used (our internal environment)?

Otherwise LG!

@usx95 usx95 merged commit 112490c into llvm:main Feb 6, 2025
8 of 10 checks passed
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
The real paths resolves symlinks and makes the tests fail when the
filesystem is a symlink tree over a content-addressable storage (our
internal environment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants