Skip to content

Commit

Permalink
Correctly set CompilingPCH in PrecompilePreambleAction.
Browse files Browse the repository at this point in the history
This fixes a crash bug in clangd when used with modules. ASTWriter would
end up writing references to submodules into the PCH file, but upon
reading the submodules would not exists and
HeaderFileInfoTrait::ReadData would crash.

Differential Revision: https://reviews.llvm.org/D85532
  • Loading branch information
gislan committed Aug 10, 2020
1 parent f9500cc commit e2d61ae
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 0 deletions.
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/unittests/CMakeLists.txt
Expand Up @@ -63,6 +63,7 @@ add_unittest(ClangdUnitTests ClangdTests
IndexTests.cpp
JSONTransportTests.cpp
LSPClient.cpp
ModulesTests.cpp
ParsedASTTests.cpp
PathMappingTests.cpp
PreambleTests.cpp
Expand Down
44 changes: 44 additions & 0 deletions clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,44 @@
//===-- ModulesTests.cpp ---------------------------------------*- C++ -*-===//
//
// 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 "Annotations.h"
#include "TestFS.h"
#include "TestTU.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

#include <memory>
#include <string>

namespace clang {
namespace clangd {
namespace {

TEST(Modules, TextualIncludeInPreamble) {
TestTU TU = TestTU::withCode(R"cpp(
#include "Textual.h"
void foo() {}
)cpp");
TU.ExtraArgs.push_back("-fmodule-name=M");
TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
TU.AdditionalFiles["Textual.h"] = "void foo();";
TU.AdditionalFiles["m.modulemap"] = R"modulemap(
module M {
module Textual {
textual header "Textual.h"
}
}
)modulemap";
// Test that we do not crash.
TU.index();
}

} // namespace
} // namespace clangd
} // namespace clang
7 changes: 7 additions & 0 deletions clang/lib/Frontend/PrecompiledPreamble.cpp
Expand Up @@ -208,6 +208,11 @@ class PrecompilePreambleAction : public ASTFrontendAction {
Callbacks.AfterPCHEmitted(Writer);
}

bool BeginSourceFileAction(CompilerInstance &CI) override {
assert(CI.getLangOpts().CompilingPCH);
return ASTFrontendAction::BeginSourceFileAction(CI);
}

bool shouldEraseOutputFiles() override { return !hasEmittedPreamblePCH(); }
bool hasCodeCompletionSupport() const override { return false; }
bool hasASTFileSupport() const override { return false; }
Expand Down Expand Up @@ -396,6 +401,8 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
auto PreambleDepCollector = std::make_shared<PreambleDependencyCollector>();
Clang->addDependencyCollector(PreambleDepCollector);

Clang->getLangOpts().CompilingPCH = true;

// Remap the main source file to the preamble buffer.
StringRef MainFilePath = FrontendOpts.Inputs[0].getFile();
auto PreambleInputBuffer = llvm::MemoryBuffer::getMemBufferCopy(
Expand Down
39 changes: 39 additions & 0 deletions clang/unittests/Frontend/ASTUnitTest.cpp
Expand Up @@ -13,6 +13,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Lex/HeaderSearch.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/ToolOutputFile.h"
Expand Down Expand Up @@ -111,4 +112,42 @@ TEST_F(ASTUnitTest, GetBufferForFileMemoryMapping) {
llvm::MemoryBuffer::MemoryBuffer_MMap);
}

TEST_F(ASTUnitTest, ModuleTextualHeader) {
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFs =
new llvm::vfs::InMemoryFileSystem();
InMemoryFs->addFile("test.cpp", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
#include "Textual.h"
void foo() {}
)cpp"));
InMemoryFs->addFile("m.modulemap", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
module M {
module Textual {
textual header "Textual.h"
}
}
)cpp"));
InMemoryFs->addFile("Textual.h", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
void foo();
)cpp"));

const char *Args[] = {"clang", "test.cpp", "-fmodule-map-file=m.modulemap",
"-fmodule-name=M"};
Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
CInvok = createInvocationFromCommandLine(Args, Diags);
ASSERT_TRUE(CInvok);

FileManager *FileMgr = new FileManager(FileSystemOptions(), InMemoryFs);
PCHContainerOps = std::make_shared<PCHContainerOperations>();

auto AU = ASTUnit::LoadFromCompilerInvocation(
CInvok, PCHContainerOps, Diags, FileMgr, false, CaptureDiagsKind::None, 1,
TU_Complete, false, false, false);
ASSERT_TRUE(AU);
auto File = AU->getFileManager().getFileRef("Textual.h", false, false);
ASSERT_TRUE(bool(File));
// Verify that we do not crash here.
EXPECT_TRUE(AU->getPreprocessor().getHeaderSearchInfo().getExistingFileInfo(
&File->getFileEntry()));
}

} // anonymous namespace

0 comments on commit e2d61ae

Please sign in to comment.