kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov, mgorny.
Herald added a project: clang.

PCH format is unstable, hence using a preamble built with a different
version of clang (or even worse, a different compiler) might result in
unexpected behaviour.

PCH creation on the other hand is something clangd wouldn't want to perform, as
it doesn't generate any output files.

This patch makes sure clangd drops any PCH related compile commands after
parsing the command line args.

Fixes https://github.com/clangd/clangd/issues/248


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79669

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CompilerTests.cpp

Index: clang-tools-extra/clangd/unittests/CompilerTests.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CompilerTests.cpp
@@ -0,0 +1,55 @@
+//===-- CompilerTests.cpp -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Compiler.h"
+#include "TestTU.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::IsEmpty;
+
+TEST(BuildCompilerInvocation, DropsPCH) {
+  IgnoreDiagnostics Diags;
+  TestTU TU;
+  TU.AdditionalFiles["test.h.pch"] = "";
+
+  TU.ExtraArgs = {"-include-pch", "test.h.pch"};
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+                  ->getPreprocessorOpts()
+                  .ImplicitPCHInclude,
+              IsEmpty());
+
+  // Transparent include translation
+  TU.ExtraArgs = {"-include", "test.h"};
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+                  ->getPreprocessorOpts()
+                  .ImplicitPCHInclude,
+              IsEmpty());
+
+  // CL mode parsing.
+  TU.AdditionalFiles["test.pch"] = "";
+  TU.ExtraArgs = {"--driver-mode=cl"};
+  TU.ExtraArgs.push_back("/Yutest.h");
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+                  ->getPreprocessorOpts()
+                  .ImplicitPCHInclude,
+              IsEmpty());
+  EXPECT_THAT(buildCompilerInvocation(TU.inputs(), Diags)
+                  ->getPreprocessorOpts()
+                  .PCHThroughHeader,
+              IsEmpty());
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -34,6 +34,7 @@
   CodeCompletionStringsTests.cpp
   CollectMacrosTests.cpp
   CompileCommandsTests.cpp
+  CompilerTests.cpp
   DexTests.cpp
   DiagnosticsTests.cpp
   DraftStoreTests.cpp
Index: clang-tools-extra/clangd/Compiler.cpp
===================================================================
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -41,8 +41,7 @@
 }
 
 std::unique_ptr<CompilerInvocation>
-buildCompilerInvocation(const ParseInputs &Inputs,
-                        clang::DiagnosticConsumer &D,
+buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
                         std::vector<std::string> *CC1Args) {
   std::vector<const char *> ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
@@ -74,6 +73,15 @@
   CI->getDependencyOutputOpts().HeaderIncludeOutputFile.clear();
   CI->getDependencyOutputOpts().DOTOutputFile.clear();
   CI->getDependencyOutputOpts().ModuleDependencyOutputDir.clear();
+
+  // Disable any pch generation/usage operations. Since serialized preamble
+  // format is unstable, using an incompatible one might result in unexpected
+  // behaviours, including crashes.
+  CI->getPreprocessorOpts().ImplicitPCHInclude.clear();
+  CI->getPreprocessorOpts().PrecompiledPreambleBytes = {0, false};
+  CI->getPreprocessorOpts().PCHThroughHeader.clear();
+  CI->getPreprocessorOpts().PCHWithHdrStop = false;
+  CI->getPreprocessorOpts().PCHWithHdrStopCreate = false;
   return CI;
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to