diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index f181c7befec15..d5818e0ca309b 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -700,6 +700,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI, Result->Marks = CapturedInfo.takeMarks(); Result->StatCache = StatCache; Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); + Result->TargetOpts = CI.TargetOpts; if (PreambleCallback) { trace::Span Tracer("Running PreambleCallback"); auto Ctx = CapturedInfo.takeLife(); @@ -913,6 +914,12 @@ PreamblePatch PreamblePatch::createMacroPatch(llvm::StringRef FileName, } void PreamblePatch::apply(CompilerInvocation &CI) const { + // Make sure the compilation uses same target opts as the preamble. Clang has + // no guarantees around using arbitrary options when reusing PCHs, and + // different target opts can result in crashes, see + // ParsedASTTest.PreambleWithDifferentTarget. + CI.TargetOpts = Baseline->TargetOpts; + // No need to map an empty file. if (PatchContents.empty()) return; diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index 37da3833748a9..160b884beb56b 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -30,6 +30,7 @@ #include "clang-include-cleaner/Record.h" #include "support/Path.h" #include "clang/Basic/SourceManager.h" +#include "clang/Basic/TargetOptions.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/Lexer.h" @@ -97,6 +98,10 @@ struct PreambleData { // Version of the ParseInputs this preamble was built from. std::string Version; tooling::CompileCommand CompileCommand; + // Target options used when building the preamble. Changes in target can cause + // crashes when deserializing preamble, this enables consumers to use the + // same target (without reparsing CompileCommand). + std::shared_ptr TargetOpts = nullptr; PrecompiledPreamble Preamble; std::vector Diags; // Processes like code completions and go-to-definitions will need #include diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 8fbac73cb653b..96d1ee1f0add7 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -4160,7 +4160,32 @@ TEST(CompletionTest, DoNotCrash) { auto Completions = completions(Case); } } +TEST(CompletionTest, PreambleFromDifferentTarget) { + constexpr std::string_view PreambleTarget = "x86_64"; + constexpr std::string_view Contents = + "int foo(int); int num; int num2 = foo(n^"; + Annotations Test(Contents); + auto TU = TestTU::withCode(Test.code()); + TU.ExtraArgs.emplace_back("-target"); + TU.ExtraArgs.emplace_back(PreambleTarget); + auto Preamble = TU.preamble(); + ASSERT_TRUE(Preamble); + // Switch target to wasm. + TU.ExtraArgs.pop_back(); + TU.ExtraArgs.emplace_back("wasm32"); + + MockFS FS; + auto Inputs = TU.inputs(FS); + auto Result = codeComplete(testPath(TU.Filename), Test.point(), + Preamble.get(), Inputs, {}); + auto Signatures = + signatureHelp(testPath(TU.Filename), Test.point(), *Preamble, Inputs, {}); + + // Make sure we don't crash. + EXPECT_THAT(Result.Completions, Not(testing::IsEmpty())); + EXPECT_THAT(Signatures.signatures, Not(testing::IsEmpty())); +} } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index 500b72b9b327a..4bb76cd6ab830 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -12,10 +12,7 @@ //===----------------------------------------------------------------------===// #include "../../clang-tidy/ClangTidyCheck.h" -#include "../../clang-tidy/ClangTidyModule.h" -#include "../../clang-tidy/ClangTidyModuleRegistry.h" #include "AST.h" -#include "CompileCommands.h" #include "Compiler.h" #include "Config.h" #include "Diagnostics.h" @@ -32,7 +29,6 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" -#include "clang/Lex/PPCallbacks.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/StringRef.h" #include "llvm/Testing/Annotations/Annotations.h" @@ -41,6 +37,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include #include #include @@ -347,9 +344,8 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) { } for (const auto &R : AST.getMacros().UnknownMacros) MacroExpansionPositions.push_back(R.StartOffset); - EXPECT_THAT( - MacroExpansionPositions, - testing::UnorderedElementsAreArray(TestCase.points())); + EXPECT_THAT(MacroExpansionPositions, + testing::UnorderedElementsAreArray(TestCase.points())); } MATCHER_P(withFileName, Inc, "") { return arg.FileName == Inc; } @@ -768,6 +764,35 @@ TEST(ParsedASTTest, GracefulFailureOnAssemblyFile) { << "Should not try to build AST for assembly source file"; } +TEST(ParsedASTTest, PreambleWithDifferentTarget) { + constexpr std::string_view kPreambleTarget = "x86_64"; + // Specifically picking __builtin_va_list as it triggers crashes when + // switching to wasm. + // It's due to different predefined types in different targets. + auto TU = TestTU::withHeaderCode("void foo(__builtin_va_list);"); + TU.Code = "void bar() { foo(2); }"; + TU.ExtraArgs.emplace_back("-target"); + TU.ExtraArgs.emplace_back(kPreambleTarget); + const auto Preamble = TU.preamble(); + + // Switch target to wasm. + TU.ExtraArgs.pop_back(); + TU.ExtraArgs.emplace_back("wasm32"); + + IgnoreDiagnostics Diags; + MockFS FS; + auto Inputs = TU.inputs(FS); + auto CI = buildCompilerInvocation(Inputs, Diags); + ASSERT_TRUE(CI) << "Failed to build compiler invocation"; + + auto AST = ParsedAST::build(testPath(TU.Filename), std::move(Inputs), + std::move(CI), {}, Preamble); + + ASSERT_TRUE(AST); + // We use the target from preamble, not with the most-recent flags. + EXPECT_EQ(AST->getASTContext().getTargetInfo().getTriple().getArchName(), + llvm::StringRef(kPreambleTarget)); +} } // namespace } // namespace clangd } // namespace clang