Skip to content

Commit

Permalink
[clang][lex] Fix non-portability diagnostics with absolute path (#74782)
Browse files Browse the repository at this point in the history
The existing code incorrectly assumes that `Path` can be empty. It
can't, it always contains at least `<` or `"`. On Unix, this patch fixes
an incorrect diagnostics that instead of `"/Users/blah"` suggested
`"Userss/blah"`. In assert builds, this would outright crash.

This patch also fixes a bug on Windows that would prevent the diagnostic
being triggered due to separator mismatch.

rdar://91172342
  • Loading branch information
jansvoboda11 committed Dec 18, 2023
1 parent 2a8f40d commit f0691bc
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
22 changes: 15 additions & 7 deletions clang/lib/Lex/PPDirectives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1858,11 +1858,18 @@ static void diagnoseAutoModuleImport(
// path to the file, build a properly-cased replacement in the vector,
// and return true if the replacement should be suggested.
static bool trySimplifyPath(SmallVectorImpl<StringRef> &Components,
StringRef RealPathName) {
StringRef RealPathName,
llvm::sys::path::Style Separator) {
auto RealPathComponentIter = llvm::sys::path::rbegin(RealPathName);
auto RealPathComponentEnd = llvm::sys::path::rend(RealPathName);
int Cnt = 0;
bool SuggestReplacement = false;

auto IsSep = [Separator](StringRef Component) {
return Component.size() == 1 &&
llvm::sys::path::is_separator(Component[0], Separator);
};

// Below is a best-effort to handle ".." in paths. It is admittedly
// not 100% correct in the presence of symlinks.
for (auto &Component : llvm::reverse(Components)) {
Expand All @@ -1872,10 +1879,11 @@ static bool trySimplifyPath(SmallVectorImpl<StringRef> &Components,
} else if (Cnt) {
--Cnt;
} else if (RealPathComponentIter != RealPathComponentEnd) {
if (Component != *RealPathComponentIter) {
// If these path components differ by more than just case, then we
// may be looking at symlinked paths. Bail on this diagnostic to avoid
// noisy false positives.
if (!IsSep(Component) && !IsSep(*RealPathComponentIter) &&
Component != *RealPathComponentIter) {
// If these non-separator path components differ by more than just case,
// then we may be looking at symlinked paths. Bail on this diagnostic to
// avoid noisy false positives.
SuggestReplacement =
RealPathComponentIter->equals_insensitive(Component);
if (!SuggestReplacement)
Expand Down Expand Up @@ -2451,7 +2459,7 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
}
#endif

if (trySimplifyPath(Components, RealPathName)) {
if (trySimplifyPath(Components, RealPathName, BackslashStyle)) {
SmallString<128> Path;
Path.reserve(Name.size()+2);
Path.push_back(isAngled ? '<' : '"');
Expand All @@ -2474,7 +2482,7 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
// got copied when the C: was processed and we want to skip that entry.
if (!(Component.size() == 1 && IsSep(Component[0])))
Path.append(Component);
else if (!Path.empty())
else if (Path.size() != 1)
continue;

// Append the separator(s) the user used, or the close quote
Expand Down
13 changes: 13 additions & 0 deletions clang/test/Lexer/case-insensitive-include-absolute.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// REQUIRES: case-insensitive-filesystem

// RUN: rm -rf %t && split-file %s %t
// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c
// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%/t

//--- header.h
//--- tu.c.in
#import "DIR/Header.h"
// CHECK: tu.c:1:9: warning: non-portable path to file '"[[DIR]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path]
// CHECK-NEXT: 1 | #import "[[DIR]]/Header.h"
// CHECK-NEXT: | ^~~~~~~~~~~~~~~~~~
// CHECK-NEXT: | "[[DIR]]/header.h"

0 comments on commit f0691bc

Please sign in to comment.