[clang][Lexer] Fix crash/assert clang::HeaderSearch::search_dir_nth

[DmitryPolukhin]

Summary

The issue was introduced in D135801. When there are only header maps in the SearchDirs,
the out of bounds value is assigned to FirstNonHeaderMapSearchDirIdx.

clang: /data/users/dpolukhin/llvm-project/clang/include/clang/Lex/HeaderSearch.h:817: ConstSearchDirIterator clang::HeaderSearch::search_dir_nth(size_t) const: Assertion `n < SearchDirs.size()' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: /data/users/dpolukhin/llvm-project/build/bin/clang -cc1 -E /data/users/dpolukhin/llvm-project/clang/test/Preprocessor/header-search-crash.c -I /data/users/dpolukhin/llvm-project/build/tools/clang/test/Preprocessor/Output/header-search-crash.c.tmp/foo.hmap -verify
 #0 0x000055650181745d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /data/users/dpolukhin/llvm-project/llvm/lib/Support/Unix/Signals.inc:602:11
 #1 0x00005565018178eb PrintStackTraceSignalHandler(void*) /data/users/dpolukhin/llvm-project/llvm/lib/Support/Unix/Signals.inc:676:1
 #2 0x0000556501815bb6 llvm::sys::RunSignalHandlers() /data/users/dpolukhin/llvm-project/llvm/lib/Support/Signals.cpp:104:5
 #3 0x00005565018180a5 SignalHandler(int) /data/users/dpolukhin/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #4 0x00007f5a26c12cf0 __restore_rt (/lib64/libpthread.so.0+0x12cf0)
 #5 0x00007f5a24c4eacf raise (/lib64/libc.so.6+0x4eacf)
 #6 0x00007f5a24c21ea5 abort (/lib64/libc.so.6+0x21ea5)
 #7 0x00007f5a24c21d79 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d79)
 #8 0x00007f5a24c47426 (/lib64/libc.so.6+0x47426)
 #9 0x000055650810facd clang::HeaderSearch::search_dir_nth(unsigned long) const /data/users/dpolukhin/llvm-project/clang/include/clang/Lex/HeaderSearch.h:0:5
#10 0x0000556508108932 clang::HeaderSearch::LookupFile(llvm::StringRef, clang::SourceLocation, bool, clang::detail::SearchDirIteratorImpl<true>, clang::detail::SearchDirIteratorImpl<true>*, llvm::ArrayRef<std::pair<clang::FileEntry const*, clang::DirectoryEntry const*>>, llvm::SmallVectorImpl<char>*, llvm::SmallVectorImpl<char>*, clang::Module*, clang::ModuleMap::KnownHeader*, bool*, bool*, bool, bool, bool, bool) /data/users/dpolukhin/llvm-project/clang/lib/Lex/HeaderSearch.cpp:1030:16
#11 0x000055650817521c clang::Preprocessor::LookupFile(clang::SourceLocation, llvm::StringRef, bool, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*, clang::detail::SearchDirIteratorImpl<true>*, llvm::SmallVectorImpl<char>*, llvm::SmallVectorImpl<char>*, clang::ModuleMap::KnownHeader*, bool*, bool*, bool, bool, bool) /data/users/dpolukhin/llvm-project/clang/lib/Lex/PPDirectives.cpp:1014:40
#12 0x000055650817ce68 clang::Preprocessor::LookupHeaderIncludeOrImport(clang::detail::SearchDirIteratorImpl<true>*, llvm::StringRef&, clang::SourceLocation, clang::CharSourceRange, clang::Token const&, bool&, bool, bool&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*, llvm::StringRef&, llvm::SmallVectorImpl<char>&, llvm::SmallVectorImpl<char>&, clang::ModuleMap::KnownHeader&, bool) /data/users/dpolukhin/llvm-project/clang/lib/Lex/PPDirectives.cpp:1995:31
#13 0x000055650817b077 clang::Preprocessor::HandleHeaderIncludeOrImport(clang::SourceLocation, clang::Token&, clang::Token&, clang::SourceLocation, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) /data/users/dpolukhin/llvm-project/clang/lib/Lex/PPDirectives.cpp:2168:31
#14 0x000055650817664c clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) /data/users/dpolukhin/llvm-project/clang/lib/Lex/PPDirectives.cpp:1959:17
#15 0x0000556508176da3 clang::Preprocessor::HandleDirective(clang::Token&) /data/users/dpolukhin/llvm-project/clang/lib/Lex/PPDirectives.cpp:1220:14
#16 0x000055650812f744 clang::Lexer::LexTokenInternal(clang::Token&, bool) /data/users/dpolukhin/llvm-project/clang/lib/Lex/Lexer.cpp:4362:7
#17 0x000055650812b10c clang::Lexer::Lex(clang::Token&) /data/users/dpolukhin/llvm-project/clang/lib/Lex/Lexer.cpp:3576:8
#18 0x00005565081c4b4f clang::Preprocessor::Lex(clang::Token&) /data/users/dpolukhin/llvm-project/clang/lib/Lex/Preprocessor.cpp:891:21
#19 0x0000556502deb1e7 clang::DoPrintPreprocessedInput(clang::Preprocessor&, llvm::raw_ostream*, clang::PreprocessorOutputOptions const&) /data/users/dpolukhin/llvm-project/clang/lib/Frontend/PrintPreprocessedOutput.cpp:1015:13
#20 0x0000556502cadc58 clang::PrintPreprocessedAction::ExecuteAction() /data/users/dpolukhin/llvm-project/clang/lib/Frontend/FrontendActions.cpp:1018:1
#21 0x0000556502c6834c clang::FrontendAction::Execute() /data/users/dpolukhin/llvm-project/clang/lib/Frontend/FrontendAction.cpp:1062:7
#22 0x0000556502b96268 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /data/users/dpolukhin/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:1049:23
#23 0x0000556502e332f7 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /data/users/dpolukhin/llvm-project/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:264:8
#24 0x00005564ffa1d270 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /data/users/dpolukhin/llvm-project/clang/tools/driver/cc1_main.cpp:251:13
#25 0x00005564ffa08d82 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) /data/users/dpolukhin/llvm-project/clang/tools/driver/driver.cpp:363:5
#26 0x00005564ffa07a97 clang_main(int, char**, llvm::ToolContext const&) /data/users/dpolukhin/llvm-project/clang/tools/driver/driver.cpp:441:5
#27 0x00005564ffa3e49d main /data/users/dpolukhin/llvm-project/build/tools/clang/tools/driver/clang-driver.cpp:15:3
#28 0x00007f5a24c3ad85 __libc_start_main (/lib64/libc.so.6+0x3ad85)
#29 0x00005564ffa0722e _start (/data/users/dpolukhin/llvm-project/build/bin/clang+0x52a722e)
/data/users/dpolukhin/llvm-project/build/tools/clang/test/Preprocessor/Output/header-search-crash.c.script: line 3: 2337741 Aborted                 (core dumped) /data/users/dpolukhin/llvm-project/build/bin/clang -cc1 -E /data/users/dpolukhin/llvm-project/clang/test/Preprocessor/header-search-crash.c -I /data/users/dpolukhin/llvm-project/build/tools/clang/test/Preprocessor/Output/header-search-crash.c.tmp/foo.hmap -verify

[shafik]

Is there a way to test this?

[bruno]

Good catch, thanks! LGTM

[DmitryPolukhin]

Test added

[DmitryPolukhin]

In D146156#4197289, @shafik wrote:

Is there a way to test this?

It was easier to reproduce on small test case than I thought, test added.

clang/lib/Lex/HeaderSearch.cpp

Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName,
 
   return Module;
 }
 
 void HeaderSearch::indexInitialHeaderMaps() {
   llvm::StringMap<unsigned, llvm::BumpPtrAllocator> Index(SearchDirs.size());
 
   // Iterate over all filename keys and associate them with the index i.
-  unsigned i = 0;
-  for (; i != SearchDirs.size(); ++i) {
+  for (unsigned i = 0; i != SearchDirs.size(); ++i) {
     auto &Dir = SearchDirs[i];
 
     // We're concerned with only the initial contiguous run of header
     // maps within SearchDirs, which can be 99% of SearchDirs when
     // SearchDirs.size() is ~10000.
-    if (!Dir.isHeaderMap())
+    if (!Dir.isHeaderMap()) {
+      SearchDirHeaderMapIndex = std::move(Index);
+      FirstNonHeaderMapSearchDirIdx = i;
       break;
+    }
 
     // Give earlier keys precedence over identical later keys.
     auto Callback = [&](StringRef Filename) {
       Index.try_emplace(Filename.lower(), i);
     };
     Dir.getHeaderMap()->forEachKey(Callback);
   }
-
-  SearchDirHeaderMapIndex = std::move(Index);
-  FirstNonHeaderMapSearchDirIdx = i;
 }
 
 //===----------------------------------------------------------------------===//
 // File lookup within a DirectoryLookup scope
 //===----------------------------------------------------------------------===//
 
 /// getName - Return the directory or filename corresponding to this lookup
 /// object.
std::string HeaderSearch::suggestPathToFileForDiagnostics(
   return suggestPathToFileForDiagnostics(File->getName(), /*WorkingDir=*/"",
                                          MainFile, IsSystem);
 }
 
 std::string HeaderSearch::suggestPathToFileForDiagnostics(
     llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile,
     bool *IsSystem) {
   using namespace llvm::sys;
-  
   llvm::SmallString<32> FilePath = File;
   // remove_dots switches to backslashes on windows as a side-effect!
   // We always want to suggest forward slashes for includes.
   // (not remove_dots(..., posix) as that misparses windows paths).
   path::remove_dots(FilePath, /*remove_dot_dot=*/true);
   path::native(FilePath, path::Style::posix);
   File = FilePath;
 

clang/test/Preprocessor/Inputs/header-search-crash/foo.hmap.json

+{
+  "mappings" :
+    {
+     "Foo.h" : "Foo/Foo.h"
+    }
+}

clang/test/Preprocessor/header-search-crash.c

+// RUN: rm -rf %t && mkdir %t
+// RUN: %hmaptool write %S/Inputs/header-search-crash/foo.hmap.json %t/foo.hmap
+// RUN: %clang -cc1 -E %s -I %t/foo.hmap -verify
+
+#include "MissingHeader.h" // expected-error {{'MissingHeader.h' file not found}}