From 5e51efd320d3c3f7952126c8609ea52c495bcb32 Mon Sep 17 00:00:00 2001 From: huangjinjie Date: Fri, 10 Oct 2025 16:12:11 +0800 Subject: [PATCH] support header shadowing detection --- clang/include/clang/Basic/DiagnosticGroups.td | 2 + .../include/clang/Basic/DiagnosticLexKinds.td | 4 + clang/include/clang/Lex/DirectoryLookup.h | 2 +- clang/include/clang/Lex/HeaderSearch.h | 11 +- clang/lib/Lex/HeaderSearch.cpp | 183 ++++++++++++------ clang/test/Preprocessor/header-shadowing.c | 14 ++ 6 files changed, 147 insertions(+), 69 deletions(-) create mode 100644 clang/test/Preprocessor/header-shadowing.c diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 0c994e0b5ca4d..b08adc59c9a8a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -789,6 +789,8 @@ def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor", def ShadowIvar : DiagGroup<"shadow-ivar">; def ShadowUncapturedLocal : DiagGroup<"shadow-uncaptured-local">; +def HeaderShadowing : DiagGroup<"header-shadowing">; + // -Wshadow-all is a catch-all for all shadowing. -Wshadow is just the // shadowing that we think is unsafe. def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified, diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index c7fe6e1db6d1f..7b732b00c5499 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -557,6 +557,10 @@ def warn_pp_hdrstop_filename_ignored : Warning< def remark_pp_search_path_usage : Remark< "search path used: '%0'">, InGroup; +def warn_header_shadowed + : Warning<"multiple candidates for header '%0' found; " + "using the one from '%1', shadowed by '%2'">, + InGroup, DefaultIgnore; def err_pp_file_not_found_angled_include_not_fatal : Error< "'%0' file not found with %select{include|import}1; " "use \"quotes\" instead">; diff --git a/clang/include/clang/Lex/DirectoryLookup.h b/clang/include/clang/Lex/DirectoryLookup.h index bb703dfad2b28..45c7360385a73 100644 --- a/clang/include/clang/Lex/DirectoryLookup.h +++ b/clang/include/clang/Lex/DirectoryLookup.h @@ -181,7 +181,7 @@ class DirectoryLookup { ModuleMap::KnownHeader *SuggestedModule, bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound, bool &IsInHeaderMap, SmallVectorImpl &MappedName, - bool OpenFile = true) const; + bool NeedSuggest, bool OpenFile = true) const; private: OptionalFileEntryRef DoFrameworkLookup( diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 850aea41c4c3b..83f6e11cd6ddc 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -809,12 +809,11 @@ class HeaderSearch { /// Look up the file with the specified name and determine its owning /// module. - OptionalFileEntryRef - getFileAndSuggestModule(StringRef FileName, SourceLocation IncludeLoc, - const DirectoryEntry *Dir, bool IsSystemHeaderDir, - Module *RequestingModule, - ModuleMap::KnownHeader *SuggestedModule, - bool OpenFile = true, bool CacheFailures = true); + OptionalFileEntryRef getFileAndSuggestModule( + StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir, + bool IsSystemHeaderDir, Module *RequestingModule, + ModuleMap::KnownHeader *SuggestedModule, bool NeedSuggest, + bool OpenFile = true, bool CacheFailures = true); /// Cache the result of a successful lookup at the given include location /// using the search path at \c HitIt. diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 238c5e2f2d9a5..bf9686be417f6 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -444,8 +444,8 @@ StringRef DirectoryLookup::getName() const { OptionalFileEntryRef HeaderSearch::getFileAndSuggestModule( StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir, bool IsSystemHeaderDir, Module *RequestingModule, - ModuleMap::KnownHeader *SuggestedModule, bool OpenFile /*=true*/, - bool CacheFailures /*=true*/) { + ModuleMap::KnownHeader *SuggestedModule, bool NeedSuggest, + bool OpenFile /*=true*/, bool CacheFailures /*=true*/) { // If we have a module map that might map this header, load it and // check whether we'll have a suggestion for a module. auto File = getFileMgr().getFileRef(FileName, OpenFile, CacheFailures); @@ -462,6 +462,9 @@ OptionalFileEntryRef HeaderSearch::getFileAndSuggestModule( return std::nullopt; } + if (!NeedSuggest) + return *File; + // If there is a module that corresponds to this header, suggest it. if (!findUsableModuleForHeader( *File, Dir ? Dir : File->getFileEntry().getDir(), RequestingModule, @@ -478,7 +481,7 @@ OptionalFileEntryRef DirectoryLookup::LookupFile( SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath, Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule, bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound, - bool &IsInHeaderMap, SmallVectorImpl &MappedName, + bool &IsInHeaderMap, SmallVectorImpl &MappedName, bool NeedSuggest, bool OpenFile) const { InUserSpecifiedSystemFramework = false; IsInHeaderMap = false; @@ -489,19 +492,21 @@ OptionalFileEntryRef DirectoryLookup::LookupFile( // Concatenate the requested file onto the directory. TmpDir = getDirRef()->getName(); llvm::sys::path::append(TmpDir, Filename); - if (SearchPath) { - StringRef SearchPathRef(getDirRef()->getName()); - SearchPath->clear(); - SearchPath->append(SearchPathRef.begin(), SearchPathRef.end()); - } - if (RelativePath) { - RelativePath->clear(); - RelativePath->append(Filename.begin(), Filename.end()); + if (NeedSuggest) { + if (SearchPath) { + StringRef SearchPathRef(getDirRef()->getName()); + SearchPath->clear(); + SearchPath->append(SearchPathRef.begin(), SearchPathRef.end()); + } + if (RelativePath) { + RelativePath->clear(); + RelativePath->append(Filename.begin(), Filename.end()); + } } return HS.getFileAndSuggestModule( TmpDir, IncludeLoc, getDir(), isSystemHeaderDirectory(), - RequestingModule, SuggestedModule, OpenFile); + RequestingModule, SuggestedModule, NeedSuggest, OpenFile); } if (isFramework()) @@ -881,6 +886,35 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc, << IncludeFilename; } +/// Return true if a shadow has been detected and the caller should +/// stop and return the first-found file and module, false otherwise. +static bool checkAndStoreCandidate( + ModuleMap::KnownHeader *SuggestedModule, OptionalFileEntryRef CandidateFile, + StringRef CandidateDir, DiagnosticsEngine &Diags, StringRef Filename, + SourceLocation IncludeLoc, ModuleMap::KnownHeader &FirstModule, + OptionalFileEntryRef &FirstHeader, SmallString<1024> &FirstDir) { + if (!FirstHeader) { + // Found the first candidate + FirstHeader = CandidateFile; + FirstDir = CandidateDir; + if (SuggestedModule) + FirstModule = *SuggestedModule; + return false; + } + + if (FirstDir != CandidateDir) { + // Found a second candidate from a different directory + Diags.Report(IncludeLoc, diag::warn_header_shadowed) + << Filename << FirstDir << CandidateDir; + if (SuggestedModule) + *SuggestedModule = FirstModule; + return true; + } + + // Found a candidate from the same directory as the first one + return false; +} + /// LookupFile - Given a "foo" or \ reference, look up the indicated file, /// return null on failure. isAngled indicates whether the file reference is /// for system \#include's or not (i.e. using <> instead of ""). Includers, if @@ -923,13 +957,18 @@ OptionalFileEntryRef HeaderSearch::LookupFile( // Otherwise, just return the file. return getFileAndSuggestModule(Filename, IncludeLoc, nullptr, /*IsSystemHeaderDir*/ false, - RequestingModule, SuggestedModule, OpenFile, - CacheFailures); + RequestingModule, SuggestedModule, true, + OpenFile, CacheFailures); } // This is the header that MSVC's header search would have found. + bool First = true; + bool NeedSuggest = true; ModuleMap::KnownHeader MSSuggestedModule; OptionalFileEntryRef MSFE; + ModuleMap::KnownHeader FirstModule; + OptionalFileEntryRef FirstHeader; + SmallString<1024> FirstDir; // Check to see if the file is in the #includer's directory. This cannot be // based on CurDir, because each includer could be a #include of a @@ -938,7 +977,6 @@ OptionalFileEntryRef HeaderSearch::LookupFile( // headers. if (!Includers.empty() && !isAngled) { SmallString<1024> TmpDir; - bool First = true; for (const auto &IncluderAndDir : Includers) { OptionalFileEntryRef Includer = IncluderAndDir.first; @@ -962,10 +1000,15 @@ OptionalFileEntryRef HeaderSearch::LookupFile( }(); if (OptionalFileEntryRef FE = getFileAndSuggestModule( TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader, - RequestingModule, SuggestedModule)) { + RequestingModule, SuggestedModule, NeedSuggest)) { + NeedSuggest = false; + if (!Includer) { assert(First && "only first includer can have no file"); - return FE; + checkAndStoreCandidate( + SuggestedModule, FE, IncluderAndDir.second.getName(), Diags, + Filename, IncludeLoc, FirstModule, FirstHeader, FirstDir); + break; } // Leave CurDir unset. @@ -994,22 +1037,28 @@ OptionalFileEntryRef HeaderSearch::LookupFile( diagnoseFrameworkInclude(Diags, IncludeLoc, IncluderAndDir.second.getName(), Filename, *FE); - return FE; + checkAndStoreCandidate( + SuggestedModule, FE, IncluderAndDir.second.getName(), Diags, + Filename, IncludeLoc, FirstModule, FirstHeader, FirstDir); + break; } // Otherwise, we found the path via MSVC header search rules. If // -Wmsvc-include is enabled, we have to keep searching to see if we // would've found this header in -I or -isystem directories. - if (Diags.isIgnored(diag::ext_pp_include_search_ms, IncludeLoc)) { - return FE; - } else { - MSFE = FE; - if (SuggestedModule) { - MSSuggestedModule = *SuggestedModule; - *SuggestedModule = ModuleMap::KnownHeader(); - } + if (checkAndStoreCandidate( + SuggestedModule, FE, IncluderAndDir.second.getName(), Diags, + Filename, IncludeLoc, FirstModule, FirstHeader, FirstDir)) { + // Found mutiple candidates via MSVC rules + if (Diags.isIgnored(diag::ext_pp_include_search_ms, IncludeLoc)) + return FirstHeader; break; } + MSFE = FE; + if (SuggestedModule) { + MSSuggestedModule = *SuggestedModule; + *SuggestedModule = ModuleMap::KnownHeader(); + } } First = false; } @@ -1069,6 +1118,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile( } SmallString<64> MappedName; + First = true; // Check each directory in sequence to see if it contains this file. for (; It != search_dir_end(); ++It) { @@ -1078,7 +1128,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile( OptionalFileEntryRef File = It->LookupFile( Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule, SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir, - IsInHeaderMap, MappedName, OpenFile); + IsInHeaderMap, MappedName, NeedSuggest, OpenFile); if (!MappedName.empty()) { assert(IsInHeaderMap && "MappedName should come from a header map"); CacheLookup.MappedName = @@ -1097,53 +1147,62 @@ OptionalFileEntryRef HeaderSearch::LookupFile( if (!File) continue; - CurDir = It; - - IncludeNames[*File] = Filename; - - // This file is a system header or C++ unfriendly if the dir is. - HeaderFileInfo &HFI = getFileInfo(*File); - HFI.DirInfo = CurDir->getDirCharacteristic(); - - // If the directory characteristic is User but this framework was - // user-specified to be treated as a system framework, promote the - // characteristic. - if (HFI.DirInfo == SrcMgr::C_User && InUserSpecifiedSystemFramework) - HFI.DirInfo = SrcMgr::C_System; - - // If the filename matches a known system header prefix, override - // whether the file is a system header. - for (unsigned j = SystemHeaderPrefixes.size(); j; --j) { - if (Filename.starts_with(SystemHeaderPrefixes[j - 1].first)) { - HFI.DirInfo = SystemHeaderPrefixes[j-1].second ? SrcMgr::C_System - : SrcMgr::C_User; - break; + if (First) { + First = false; + NeedSuggest = false; + CurDir = It; + IncludeNames[*File] = Filename; + + // This file is a system header or C++ unfriendly if the dir is. + HeaderFileInfo &HFI = getFileInfo(*File); + HFI.DirInfo = CurDir->getDirCharacteristic(); + + // If the directory characteristic is User but this framework was + // user-specified to be treated as a system framework, promote the + // characteristic. + if (HFI.DirInfo == SrcMgr::C_User && InUserSpecifiedSystemFramework) + HFI.DirInfo = SrcMgr::C_System; + + // If the filename matches a known system header prefix, override + // whether the file is a system header. + for (unsigned j = SystemHeaderPrefixes.size(); j; --j) { + if (Filename.starts_with(SystemHeaderPrefixes[j - 1].first)) { + HFI.DirInfo = SystemHeaderPrefixes[j - 1].second ? SrcMgr::C_System + : SrcMgr::C_User; + break; + } } - } - if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), IncludeLoc)) { - if (SuggestedModule) + if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), + IncludeLoc) && + SuggestedModule) *SuggestedModule = MSSuggestedModule; - return MSFE; - } - bool FoundByHeaderMap = !IsMapped ? false : *IsMapped; - if (!Includers.empty()) - diagnoseFrameworkInclude(Diags, IncludeLoc, - Includers.front().second.getName(), Filename, - *File, isAngled, FoundByHeaderMap); + bool FoundByHeaderMap = !IsMapped ? false : *IsMapped; + if (!Includers.empty()) + diagnoseFrameworkInclude(Diags, IncludeLoc, + Includers.front().second.getName(), Filename, + *File, isAngled, FoundByHeaderMap); - // Remember this location for the next lookup we do. - cacheLookupSuccess(CacheLookup, It, IncludeLoc); - return File; + // Remember this location for the next lookup we do. + cacheLookupSuccess(CacheLookup, It, IncludeLoc); + } + + if (checkAndStoreCandidate(SuggestedModule, File, It->getName(), Diags, + Filename, IncludeLoc, FirstModule, FirstHeader, + FirstDir)) + return FirstHeader; } - if (checkMSVCHeaderSearch(Diags, MSFE, nullptr, IncludeLoc)) { + if (First && checkMSVCHeaderSearch(Diags, MSFE, nullptr, IncludeLoc)) { if (SuggestedModule) *SuggestedModule = MSSuggestedModule; return MSFE; } + if (FirstHeader) + return FirstHeader; + // Otherwise, didn't find it. Remember we didn't find this. CacheLookup.HitIt = search_dir_end(); return std::nullopt; diff --git a/clang/test/Preprocessor/header-shadowing.c b/clang/test/Preprocessor/header-shadowing.c new file mode 100644 index 0000000000000..08b3c3e3f8db7 --- /dev/null +++ b/clang/test/Preprocessor/header-shadowing.c @@ -0,0 +1,14 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -Wheader-shadowing -Eonly %t/main.c -I %t/include 2>&1 | FileCheck %s --check-prefix=SHADOWING +// SHADOWING: {{.*}} warning: multiple candidates for header 'header.h' found; + +//--- main.c +#include "header.h" + +//--- include/header.h +#pragma once + +//--- header.h +#pragma once