Index: include/clang/Basic/DiagnosticLexKinds.td =================================================================== --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -274,6 +274,13 @@ "whitespace required after macro name">; def warn_missing_whitespace_after_macro_name : Warning< "whitespace recommended after macro name">; + +class NonportablePath : Warning< + "non-portable path to file '%0'; specified path differs in case from file" + " name on disk">; +def pp_nonportable_path : NonportablePath, InGroup>; +def pp_nonportable_system_path : NonportablePath, DefaultIgnore, + InGroup>; def pp_pragma_once_in_main_file : Warning<"#pragma once in main file">, InGroup>; Index: include/clang/Basic/FileManager.h =================================================================== --- include/clang/Basic/FileManager.h +++ include/clang/Basic/FileManager.h @@ -52,6 +52,7 @@ /// descriptor for the file. class FileEntry { const char *Name; // Name of the file. + std::string RealPathName; // Real path to the file; could be empty. off_t Size; // File size in bytes. time_t ModTime; // Modification time of file. const DirectoryEntry *Dir; // Directory file lives in. @@ -82,6 +83,7 @@ } const char *getName() const { return Name; } + StringRef tryGetRealPathName() const { return RealPathName; } bool isValid() const { return IsValid; } off_t getSize() const { return Size; } unsigned getUID() const { return UID; } Index: include/clang/Basic/VirtualFileSystem.h =================================================================== --- include/clang/Basic/VirtualFileSystem.h +++ include/clang/Basic/VirtualFileSystem.h @@ -91,6 +91,13 @@ virtual ~File(); /// \brief Get the status of the file. virtual llvm::ErrorOr status() = 0; + /// \brief Get the name of the file + virtual llvm::ErrorOr getName() { + if (auto Status = status()) + return Status->getName().str(); + else + return Status.getError(); + } /// \brief Get the contents of the file as a \p MemoryBuffer. virtual llvm::ErrorOr> getBuffer(const Twine &Name, int64_t FileSize = -1, Index: include/clang/Lex/DirectoryLookup.h =================================================================== --- include/clang/Lex/DirectoryLookup.h +++ include/clang/Lex/DirectoryLookup.h @@ -151,6 +151,9 @@ /// /// \param HS The header search instance to search with. /// + /// \param IncludeLoc the source location of the #include or #import + /// directive. + /// /// \param SearchPath If not NULL, will be set to the search path relative /// to which the file was found. /// @@ -172,6 +175,7 @@ /// a framework include ("Foo.h" -> "Foo/Foo.h"), set the new name to this /// vector and point Filename to it. const FileEntry *LookupFile(StringRef &Filename, HeaderSearch &HS, + SourceLocation IncludeLoc, SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath, Module *RequestingModule, Index: include/clang/Lex/HeaderSearch.h =================================================================== --- include/clang/Lex/HeaderSearch.h +++ include/clang/Lex/HeaderSearch.h @@ -580,8 +580,9 @@ /// \brief Look up the file with the specified name and determine its owning /// module. const FileEntry * - getFileAndSuggestModule(StringRef FileName, const DirectoryEntry *Dir, - bool IsSystemHeaderDir, Module *RequestingModule, + getFileAndSuggestModule(StringRef FileName, SourceLocation IncludeLoc, + const DirectoryEntry *Dir, bool IsSystemHeaderDir, + Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule); public: Index: lib/Basic/FileManager.cpp =================================================================== --- lib/Basic/FileManager.cpp +++ lib/Basic/FileManager.cpp @@ -312,6 +312,9 @@ UFE.InPCH = Data.InPCH; UFE.File = std::move(F); UFE.IsValid = true; + if (UFE.File) + if (auto RealPathName = UFE.File->getName()) + UFE.RealPathName = *RealPathName; return &UFE; } Index: lib/Basic/VirtualFileSystem.cpp =================================================================== --- lib/Basic/VirtualFileSystem.cpp +++ lib/Basic/VirtualFileSystem.cpp @@ -140,10 +140,12 @@ class RealFile : public File { int FD; Status S; + std::string RealName; friend class RealFileSystem; - RealFile(int FD, StringRef NewName) + RealFile(int FD, StringRef NewName, StringRef NewRealPathName) : FD(FD), S(NewName, {}, {}, {}, {}, {}, - llvm::sys::fs::file_type::status_error, {}) { + llvm::sys::fs::file_type::status_error, {}), + RealName(NewRealPathName.str()) { assert(FD >= 0 && "Invalid or inactive file descriptor"); } @@ -150,6 +152,7 @@ public: ~RealFile() override; ErrorOr status() override; + ErrorOr getName() override; ErrorOr> getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator, @@ -170,6 +173,10 @@ return S; } +ErrorOr RealFile::getName() { + return RealName.empty() ? S.getName().str() : RealName; +} + ErrorOr> RealFile::getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator, bool IsVolatile) { @@ -207,9 +214,10 @@ ErrorOr> RealFileSystem::openFileForRead(const Twine &Name) { int FD; - if (std::error_code EC = sys::fs::openFileForRead(Name, FD)) + SmallString<256> RealName; + if (std::error_code EC = sys::fs::openFileForRead(Name, FD, &RealName)) return EC; - return std::unique_ptr(new RealFile(FD, Name.str())); + return std::unique_ptr(new RealFile(FD, Name.str(), RealName.str())); } llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const { Index: lib/Lex/HeaderSearch.cpp =================================================================== --- lib/Lex/HeaderSearch.cpp +++ lib/Lex/HeaderSearch.cpp @@ -250,8 +250,9 @@ } const FileEntry *HeaderSearch::getFileAndSuggestModule( - StringRef FileName, const DirectoryEntry *Dir, bool IsSystemHeaderDir, - Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule) { + StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir, + bool IsSystemHeaderDir, Module *RequestingModule, + ModuleMap::KnownHeader *SuggestedModule) { // If we have a module map that might map this header, load it and // check whether we'll have a suggestion for a module. const FileEntry *File = getFileMgr().getFile(FileName, /*OpenFile=*/true); @@ -272,6 +273,7 @@ const FileEntry *DirectoryLookup::LookupFile( StringRef &Filename, HeaderSearch &HS, + SourceLocation IncludeLoc, SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath, Module *RequestingModule, @@ -297,7 +299,7 @@ RelativePath->append(Filename.begin(), Filename.end()); } - return HS.getFileAndSuggestModule(TmpDir, getDir(), + return HS.getFileAndSuggestModule(TmpDir, IncludeLoc, getDir(), isSystemHeaderDirectory(), RequestingModule, SuggestedModule); } @@ -585,7 +587,7 @@ RelativePath->append(Filename.begin(), Filename.end()); } // Otherwise, just return the file. - return getFileAndSuggestModule(Filename, nullptr, + return getFileAndSuggestModule(Filename, IncludeLoc, nullptr, /*IsSystemHeaderDir*/false, RequestingModule, SuggestedModule); } @@ -622,7 +624,7 @@ Includer ? getFileInfo(Includer).DirInfo != SrcMgr::C_User : BuildSystemModule; if (const FileEntry *FE = getFileAndSuggestModule( - TmpDir, IncluderAndDir.second, IncluderIsSystemHeader, + TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader, RequestingModule, SuggestedModule)) { if (!Includer) { assert(First && "only first includer can have no file"); @@ -713,7 +715,7 @@ bool InUserSpecifiedSystemFramework = false; bool HasBeenMapped = false; const FileEntry *FE = SearchDirs[i].LookupFile( - Filename, *this, SearchPath, RelativePath, RequestingModule, + Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule, SuggestedModule, InUserSpecifiedSystemFramework, HasBeenMapped, MappedName); if (HasBeenMapped) { Index: lib/Lex/PPDirectives.cpp =================================================================== --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -24,6 +24,10 @@ #include "clang/Lex/ModuleLoader.h" #include "clang/Lex/Pragma.h" #include "llvm/ADT/APInt.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringSet.h" +#include "llvm/ADT/iterator_range.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Path.h" #include "llvm/Support/SaveAndRestore.h" @@ -137,6 +141,97 @@ return MD_NoWarn; } +// Return true if we want to issue a diagnostic by default if we +// encounter this name in a #include with the wrong case. For now, +// this includes the standard C and C++ headers, Posix headers, +// and Boost headers. Improper case for these #includes is a +// potential portability issue. +static bool warnByDefaultOnWrongCase(StringRef Include) { + // The standard C/C++ and Posix headers + static const std::initializer_list StandardHeadersList{ + // C library headers + "assert.h", "complex.h", "ctype.h", "errno.h", "fenv.h", + "float.h", "inttypes.h", "iso646.h", "limits.h", "locale.h", + "math.h", "setjmp.h", "signal.h", "stdalign.h", "stdarg.h", + "stdatomic.h", "stdbool.h", "stddef.h", "stdint.h", "stdio.h", + "stdlib.h", "stdnoreturn.h", "string.h", "tgmath.h", "threads.h", + "time.h", "uchar.h", "wchar.h", "wctype.h", + + // C++ headers for C library facilities + "cassert", "ccomplex", "cctype", "cerrno", "cfenv", + "cfloat", "cinttypes", "ciso646", "climits", "clocale", + "cmath", "csetjmp", "csignal", "cstdalign", "cstdarg", + "cstdbool", "cstddef", "cstdint", "cstdio", "cstdlib", + "cstring", "ctgmath", "ctime", "cuchar", "cwchar", + "cwctype", + + // C++ library headers + "algorithm", "fstream", "list", "regex", "thread", + "array", "functional", "locale", "scoped_allocator", "tuple", + "atomic", "future", "map", "set", "type_traits", + "bitset", "initializer_list", "memory", "shared_mutex", "typeindex", + "chrono", "iomanip", "mutex", "sstream", "typeinfo", + "codecvt", "ios", "new", "stack", "unordered_map", + "complex", "iosfwd", "numeric", "stdexcept", "unordered_set", + "condition_variable", "iostream", "ostream", "streambuf", "utility", + "deque", "istream", "queue", "string", "valarray", + "exception", "iterator", "random", "strstream", "vector", + "forward_list", "limits", "ratio", "system_error", + + // POSIX headers + "aio.h", "arpa/inet.h", /*"assert.h",*/ /*"complex.h",*/ "cpio.h", + /*"ctype.h",*/ "dirent.h", "dlfcn.h", /*"errno.h",*/ "fcntl.h", /*"fenv.h",*/ + /*"float.h",*/ "fmtmsg.h", "fnmatch.h", "ftw.h", "glob.h", "grp.h", + "iconv.h", /*"inttypes.h",*/ /*"iso646.h",*/ "langinfo.h", "libgen.h", + /*"limits.h",*/ /*"locale.h",*/ /*"math.h",*/ "monetary.h", "mqueue.h", + "ndbm.h", "net/if.h", "netdb.h", "netinet/in.h", "netinet/tcp.h", + "nl_types.h", "poll.h", "pthread.h", "pwd.h", "regex.h", "sched.h", + "search.h", "semaphore.h", /*"setjmp.h",*/ /*"signal.h",*/ "spawn.h", + /*"stdarg.h",*/ /*"stdbool.h",*/ /*"stddef.h",*/ /*"stdint.h",*/ /*"stdio.h",*/ + /*"stdlib.h",*/ /*"string.h",*/ "strings.h", "stropts.h", "sys/ipc.h", + "sys/mman.h", "sys/msg.h", "sys/resource.h", "sys/select.h", + "sys/sem.h", "sys/shm.h", "sys/socket.h", "sys/stat.h", + "sys/statvfs.h", "sys/time.h", "sys/times.h", "sys/types.h", + "sys/uio.h", "sys/un.h", "sys/utsname.h", "sys/wait.h", "syslog.h", + "tar.h", "termios.h", /*"tgmath.h",*/ /*"time.h",*/ "trace.h", "ulimit.h", + "unistd.h", "utime.h", "utmpx.h", /*"wchar.h",*/ /*"wctype.h",*/ "wordexp.h" + }; + static const llvm::StringSet<> StandardHeaders{StandardHeadersList}; + static constexpr std::size_t MaxStdHeaderNameLen = 18u; + // Assert that there is no standard header whose name is longer than MaxStdHeaderNameLen + assert(std::find_if(StandardHeadersList.begin(), StandardHeadersList.end(), + [](StringRef Header) { return Header.size() > MaxStdHeaderNameLen; }) == + StandardHeadersList.end()); + + // If the first component of the path is "boost", treat this like a standard header + // for the purposes of diagnostics. + if (::llvm::sys::path::begin(Include)->equals_lower("boost")) + return true; + + // "condition_variable" is the longest standard header name at 18 characters. + // If the include file name is longer than that, it can't be a standard header. + if (Include.size() > MaxStdHeaderNameLen) + return false; + + // Lowercase and normalize the search string. + SmallString<32> LowerInclude{Include}; + for (char& Ch : LowerInclude) { + // In the ASCII range? + if (Ch < 0 || Ch > 0xff) + return false; // Can't be a standard header + // ASCII lowercase: + if (Ch >= 'A' && Ch <= 'Z') + Ch += 'a' - 'A'; +#ifdef LLVM_ON_WIN32 + // Normalize path separators for comparison purposes. + else if (Ch == '\\') + Ch = '/'; +#endif + } + + return StandardHeaders.find(LowerInclude.str()) != StandardHeaders.end(); +} + bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef, bool *ShadowFlag) { // Missing macro name? @@ -1556,6 +1651,41 @@ ("@import " + PathString + ";").str()); } +namespace { + // Given a vector of path components and a string containing the real + // path to the file, build a properly-cased replacement in the vector, + // and return true if the replacement should be suggested. + bool trySimplifyPath(SmallVectorImpl &Components, + StringRef RealPathName) { + auto RealPathComponentIter = llvm::sys::path::rbegin(RealPathName); + auto RealPathComponentEnd = llvm::sys::path::rend(RealPathName); + int Cnt = 0; + bool SuggestReplacement = false; + // 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)) { + if ("." == Component) { + } else if (".." == Component) { + ++Cnt; + } 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. + SuggestReplacement = RealPathComponentIter->equals_lower(Component); + if (!SuggestReplacement) + break; + Component = *RealPathComponentIter; + } + ++RealPathComponentIter; + } + } + return SuggestReplacement; + } +} + /// HandleIncludeDirective - The "\#include" tokens have just been read, read /// the file to be included from the lexer, then include it! This is a common /// routine with functionality shared between \#include, \#include_next and @@ -1831,6 +1961,39 @@ // FIXME: If we have a suggested module, and we've already visited this file, // don't bother entering it again. We know it has no further effect. + // Issue a diagnostic if the name of the file on disk has a different case + // than the one we're about to open. + const bool CheckIncludePathPortability = + File && !File->tryGetRealPathName().empty(); + + if (CheckIncludePathPortability) { + StringRef Name = LangOpts.MSVCCompat ? NormalizedPath.str() : Filename; + StringRef RealPathName = File->tryGetRealPathName(); + SmallVector Components(llvm::sys::path::begin(Name), + llvm::sys::path::end(Name)); + + if (trySimplifyPath(Components, RealPathName)) { + SmallString<128> Path; + Path.reserve(Name.size()+2); + Path.push_back(isAngled ? '<' : '"'); + for (auto Component : Components) { + Path.append(Component); + // Append the separator the user used, or the close quote + Path.push_back( + Path.size() <= Filename.size() ? Filename[Path.size()-1] : + (isAngled ? '>' : '"')); + } + auto Replacement = Path.str().str(); + // For user files and known standard headers, by default we issue a diagnostic. + // For other system headers, we don't. They can be controlled separately. + auto DiagId = (FileCharacter == SrcMgr::C_User || warnByDefaultOnWrongCase(Name)) ? + diag::pp_nonportable_path : diag::pp_nonportable_system_path; + SourceRange Range(FilenameTok.getLocation(), CharEnd); + Diag(FilenameTok, DiagId) << Replacement << + FixItHint::CreateReplacement(Range, Replacement); + } + } + // Ask HeaderInfo if we should enter this #include file. If not, #including // this file will have no effect. if (ShouldEnter && Index: test/Lexer/Inputs/case-insensitive-include.h =================================================================== --- test/Lexer/Inputs/case-insensitive-include.h +++ test/Lexer/Inputs/case-insensitive-include.h @@ -0,0 +1,8 @@ +#ifndef CASE_INSENSITIVE_INCLUDE_H +#define CASE_INSENSITIVE_INCLUDE_H + +struct S { + int x; +}; + +#endif Index: test/Lexer/case-insensitive-include-ms.c =================================================================== --- test/Lexer/case-insensitive-include-ms.c +++ test/Lexer/case-insensitive-include-ms.c @@ -0,0 +1,18 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: mkdir -p %T/apath +// RUN: cp %S/Inputs/case-insensitive-include.h %T +// RUN: cd %T +// RUN: %clang_cc1 -fsyntax-only -fms-compatibility %s -include %s -I %T -verify +// RUN: %clang_cc1 -fsyntax-only -fms-compatibility -fdiagnostics-parseable-fixits %s -include %s -I %T 2>&1 | FileCheck %s + +#include "..\Output\.\case-insensitive-include.h" +#include "..\Output\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\"" +#include "..\output\.\case-insensitive-include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\"" + +#include "apath\..\.\case-insensitive-include.h" +#include "apath\..\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath\\..\\.\\case-insensitive-include.h\"" +#include "APath\..\.\case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-( Index: test/Lexer/case-insensitive-include.c =================================================================== --- test/Lexer/case-insensitive-include.c +++ test/Lexer/case-insensitive-include.c @@ -0,0 +1,35 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: mkdir -p %T/apath +// RUN: mkdir -p %T/asystempath +// RUN: cp %S/Inputs/case-insensitive-include.h %T +// RUN: cp %S/Inputs/case-insensitive-include.h %T/asystempath/case-insensitive-include2.h +// RUN: cd %T +// RUN: %clang_cc1 -fsyntax-only %s -include %s -I %T -isystem %T/asystempath -verify +// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -include %s -I %T -isystem %T/asystempath 2>&1 | FileCheck %s + +// Known standard header, so warn: +#include // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:20}:"" + +#include "case-insensitive-include.h" +#include "Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:38}:"\"case-insensitive-include.h\"" + +#include "../Output/./case-insensitive-include.h" +#include "../Output/./Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"../Output/./case-insensitive-include.h\"" +#include "../output/./case-insensitive-include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"../Output/./case-insensitive-include.h\"" + +#include "apath/.././case-insensitive-include.h" +#include "apath/.././Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath/.././case-insensitive-include.h\"" +#include "APath/.././case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-( + +#include "../Output/./apath/.././case-insensitive-include.h" +#include "../Output/./APath/.././case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-( +#include "../output/./apath/.././case-insensitive-include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:61}:"\"../Output/./apath/.././case-insensitive-include.h\"" + +#include "CASE-INSENSITIVE-INCLUDE2.H" // Found in an -isystem directory. No warning. Index: test/Lexer/case-insensitive-system-include.c =================================================================== --- test/Lexer/case-insensitive-system-include.c +++ test/Lexer/case-insensitive-system-include.c @@ -0,0 +1,10 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: mkdir -p %T/asystempath +// RUN: cp %S/Inputs/case-insensitive-include.h %T/asystempath/ +// RUN: cd %T +// RUN: %clang_cc1 -fsyntax-only %s -include %s -isystem %T/asystempath -verify -Wnonportable-system-include-path +// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -include %s -isystem %T/asystempath -Wnonportable-system-include-path 2>&1 | FileCheck %s + +#include "CASE-INSENSITIVE-INCLUDE.H" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:38}:"\"case-insensitive-include.h\"" Index: test/PCH/case-insensitive-include.c =================================================================== --- test/PCH/case-insensitive-include.c +++ test/PCH/case-insensitive-include.c @@ -2,7 +2,7 @@ // Test this without pch. // RUN: cp %S/Inputs/case-insensitive-include.h %T -// RUN: %clang_cc1 -fsyntax-only %s -include %s -I %T -verify +// RUN: %clang_cc1 -Wno-nonportable-include-path -fsyntax-only %s -include %s -I %T -verify // Test with pch. // RUN: %clang_cc1 -emit-pch -o %t.pch %s -I %T