Fix use of dangling stack allocated string in IncludeFixer

[dgoldman]

Summary

IncludeFixer uses this BuildDir string later on if given relative paths.

[dgoldman]

LMK of the best way to add a test for this, maybe I can somehow make a test with relative path arguments?

[kadircet]

yeah unfortunately testing this is hard, but bug&fix are obvious so it's fine to land without a test.

but i think we should rather fix the broken call site at clang-tools-extra/clangd/ParsedAST.cpp and move BuildDir to outer scope where it'll outlive IncludeFixer (~line 468).

[dgoldman]

Fix in ParsedAST.cpp

[dgoldman]

In D138047#3930211, @kadircet wrote:

yeah unfortunately testing this is hard, but bug&fix are obvious so it's fine to land without a test.

but i think we should rather fix the broken call site at clang-tools-extra/clangd/ParsedAST.cpp and move BuildDir to outer scope where it'll outlive IncludeFixer (~line 468).

Done.

[kadircet]

thanks!

clang-tools-extra/clangd/ParsedAST.cpp

ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
   // Clang-tidy has some limitations to ensure reasonable performance:
   //  - checks don't see all preprocessor events in the preamble
   //  - matchers run only over the main-file top-level decls (and can't see
   //    ancestors outside this scope).
   // In practice almost all checks work well without modifications.
   std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
   ast_matchers::MatchFinder CTFinder;
   llvm::Optional<tidy::ClangTidyContext> CTContext;
+  // Must outlive FixIncludes.
+  auto BuildDir = VFS->getCurrentWorkingDirectory();
   llvm::Optional<IncludeFixer> FixIncludes;
   llvm::DenseMap<diag::kind, DiagnosticsEngine::Level> OverriddenSeverity;
   // No need to run clang-tidy or IncludeFixerif we are not going to surface
   // diagnostics.
   if (PreserveDiags) {
     trace::Span Tracer("ClangTidyInit");
     tidy::ClangTidyCheckFactories CTFactories;
     for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
           }
         }
       }
       return DiagLevel;
     });
 
     // Add IncludeFixer which can recover diagnostics caused by missing includes
     // (e.g. incomplete type) and attach include insertion fixes to diagnostics.
-    auto BuildDir = VFS->getCurrentWorkingDirectory();
     if (Inputs.Index && !BuildDir.getError()) {
       auto Style =
           getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
       auto Inserter = std::make_shared<IncludeInserter>(
           Filename, Inputs.Contents, Style, BuildDir.get(),
           &Clang->getPreprocessor().getHeaderSearchInfo());
       if (Preamble) {
         for (const auto &Inc : Preamble->Includes.MainFileIncludes)