[clang-tools-extra] 35d867a - [clangd] Filter pch related flags coming from the user

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-05-10T13:03:59+02:00
New Revision: 35d867a790c2bcf2008b2ee1895ae8af2793b797

URL: https://github.com/llvm/llvm-project/commit/35d867a790c2bcf2008b2ee1895ae8af2793b797
DIFF: https://github.com/llvm/llvm-project/commit/35d867a790c2bcf2008b2ee1895ae8af2793b797.diff

LOG: [clangd] Filter pch related flags coming from the user

Summary:
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

Reviewers: sammccall

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D79669

Added: 
    clang-tools-extra/clangd/unittests/CompilerTests.cpp

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp
index 957d7c382125..04d48b08865d 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -41,8 +41,7 @@ void IgnoreDiagnostics::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 }
 
 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 @@ buildCompilerInvocation(const ParseInputs &Inputs,
   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;
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index cccb3ce7308a..a1f6af6747f4 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -34,6 +34,7 @@ add_unittest(ClangdUnitTests ClangdTests
   CodeCompletionStringsTests.cpp
   CollectMacrosTests.cpp
   CompileCommandsTests.cpp
+  CompilerTests.cpp
   DexTests.cpp
   DiagnosticsTests.cpp
   DraftStoreTests.cpp

diff  --git a/clang-tools-extra/clangd/unittests/CompilerTests.cpp b/clang-tools-extra/clangd/unittests/CompilerTests.cpp
new file mode 100644
index 000000000000..a12a7b9ce9af
--- /dev/null
+++ b/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