fix(cxx_extractor): don't crash when modules are on

kythe/cxx/extractor/BUILD

@@ -117,6 +117,7 @@ cc_library(
         "//kythe/cxx/common:path_utils",
         "//kythe/cxx/common:sha256_hasher",
         "//kythe/cxx/indexer/cxx:clang_utils",
+        "//kythe/cxx/indexer/cxx:stream_adapter",
         "//kythe/proto:analysis_cc_proto",
         "//kythe/proto:buildinfo_cc_proto",
         "//kythe/proto:cxx_cc_proto",

kythe/cxx/extractor/cxx_extractor.cc

@@ -35,6 +35,7 @@
 #include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
 #include "absl/types/optional.h"
+#include "clang/Basic/Module.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Lex/MacroArgs.h"
@@ -50,6 +51,7 @@
 #include "kythe/cxx/extractor/CommandLineUtils.h"
 #include "kythe/cxx/extractor/language.h"
 #include "kythe/cxx/extractor/path_utils.h"
+#include "kythe/cxx/indexer/cxx/stream_adapter.h"
 #include "kythe/proto/analysis.pb.h"
 #include "kythe/proto/buildinfo.pb.h"
 #include "kythe/proto/cxx.pb.h"
@@ -83,6 +85,11 @@ constexpr absl::string_view kStableRootDirectories[] = {
     "/kythe_cxx_extractor_root",
 };
 
+bool IsSpecialBufferName(llvm::StringRef id) {
+  return id == clang::Module::getModuleInputBufferName() ||
+         id == "<built-in>" || id == "<command line>";
+}
+
 absl::string_view GetPathForProto(
     const proto::CxxCompilationUnitDetails::SystemHeaderPrefix& prefix) {
   return prefix.prefix();
@@ -538,12 +545,33 @@ ExtractorPPCallbacks::ExtractorPPCallbacks(ExtractorState state)
 }
 
 void ExtractorPPCallbacks::FileChanged(
-    clang::SourceLocation /*Loc*/, FileChangeReason Reason,
+    clang::SourceLocation Loc, FileChangeReason Reason,
     clang::SrcMgr::CharacteristicKind /*FileType*/, clang::FileID /*PrevFID*/) {
   if (Reason == EnterFile) {
     if (last_inclusion_directive_path_.empty()) {
-      current_files_.push(FileState{std::string(GetMainFile()->getName()),
-                                    ClaimDirective::AlwaysClaim});
+      if (auto* mfile = GetMainFile()) {
+        current_files_.push(FileState{std::string(mfile->getName()),
+                                      ClaimDirective::AlwaysClaim});
+      } else {
+        // For some compilations with modules enabled, there may be no main
+        // source file set. Previously we would segfault
+        // (`GetMainFile()->getName()`) above instead of `mfile`, so CHECK-
+        // failing below is no more unpleasant.
+        LOG(WARNING) << "unusual EnterFile @"
+                     << Loc.printToString(*source_manager_);
+        auto fid = source_manager_->getFileID(Loc);
+        CHECK(fid.isValid());
+        auto buffer = source_manager_->getBufferOrNone(fid);
+        CHECK(buffer.has_value());
+        auto id = buffer->getBufferIdentifier();
+        CHECK(IsSpecialBufferName(id))
+            << "unknown buffer " << StreamAdapter::Stream(id);
+        // TODO(zarko): we need a more appropriate path for the synthesized
+        // <module-includes> buffer.
+        current_files_.push(
+            FileState{std::string(buffer->getBufferIdentifier()),
+                      ClaimDirective::AlwaysClaim});
+      }
     } else {
       CHECK(!current_files_.empty());
       current_files_.top().last_include_offset = last_inclusion_offset_;
@@ -591,8 +619,10 @@ PreprocessorTranscript ExtractorPPCallbacks::PopFile() {
 }
 
 void ExtractorPPCallbacks::EndOfMainFile() {
-  AddFile(GetMainFile(), std::string(GetMainFile()->getName()));
-  *main_source_file_transcript_ = PopFile();
+  if (auto* mfile = GetMainFile()) {
+    AddFile(GetMainFile(), std::string(GetMainFile()->getName()));
+    *main_source_file_transcript_ = PopFile();
+  }
 }
 
 std::string ExtractorPPCallbacks::FixStdinPath(const clang::FileEntry* file,
@@ -846,24 +876,24 @@ std::string ExtractorPPCallbacks::AddFile(const clang::FileEntry* file,
                                           llvm::StringRef file_name,
                                           llvm::StringRef search_path,
                                           llvm::StringRef relative_path) {
-  CHECK(!current_files_.top().file_path.empty());
+  const auto& top_path = current_files_.top().file_path;
+  CHECK(!top_path.empty());
   const auto search_path_entry =
       source_manager_->getFileManager().getDirectory(search_path);
+  llvm::ErrorOr<const clang::FileEntry*> file_or =
+      source_manager_->getFileManager().getFile(top_path);
   const auto current_file_parent_entry =
-      (*source_manager_->getFileManager().getFile(
-           current_files_.top().file_path.c_str()))
-          ->getDir();
+      file_or.getError() ? nullptr : (*file_or)->getDir();
   // If the include file was found relatively to the current file's parent
   // directory or a search path, we need to normalize it. This is necessary
   // because llvm internalizes the path by which an inode was first accessed,
   // and always returns that path afterwards. If we do not normalize this
   // we will get an error when we replay the compilation, as the virtual
   // file system is not aware of inodes.
   llvm::SmallString<1024> out_name;
-  if (*search_path_entry == current_file_parent_entry) {
-    auto parent =
-        llvm::sys::path::parent_path(current_files_.top().file_path.c_str())
-            .str();
+  if (!search_path_entry.getError() &&
+      *search_path_entry == current_file_parent_entry) {
+    auto parent = llvm::sys::path::parent_path(top_path).str();
 
     // If the file is a top level file ("file.cc"), we normalize to a path
     // relative to "./".
@@ -879,7 +909,9 @@ std::string ExtractorPPCallbacks::AddFile(const clang::FileEntry* file,
     out_name = search_path;
     llvm::sys::path::append(out_name, relative_path);
   } else {
-    CHECK(llvm::sys::path::is_absolute(file_name)) << file_name.str();
+    CHECK(IsSpecialBufferName(top_path) ||
+          llvm::sys::path::is_absolute(file_name))
+        << StreamAdapter::Stream(file_name);
     out_name = file_name;
   }
   std::string out_name_string(out_name.str());

kythe/cxx/extractor/testdata/BUILD

@@ -82,6 +82,24 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "header_only_module_test",
+    srcs = ["header_only_module_test.cc"],
+    data = [
+        "header_only_module.cppmap",
+        "header_only_module.h",
+    ],
+    deps = [
+        "//kythe/cxx/extractor:testlib",
+        "//kythe/proto:analysis_cc_proto",
+        "//third_party:gmock_main",
+        "//third_party:gtest",
+        "@com_github_inazarenko_protobuf_matchers//protobuf-matchers",
+        "@com_google_absl//absl/algorithm:container",
+        "@com_google_absl//absl/strings",
+    ],
+)
+
 cc_test(
     name = "extract_transcript_test",
     srcs = ["extract_transcript_test.cc"],

kythe/cxx/extractor/testdata/header_only_module.cppmap

@@ -0,0 +1,3 @@
+module "//kythe/cxx/extractor/testdata:header_only_module" {
+  header "kythe/cxx/extractor/testdata/header_only_module.h"
+}

kythe/cxx/extractor/testdata/header_only_module.h

@@ -0,0 +1 @@
+int f();

kythe/cxx/extractor/testdata/header_only_module_test.cc

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2019 The Kythe Authors. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "absl/algorithm/container.h"
+#include "absl/strings/string_view.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "kythe/cxx/extractor/testlib.h"
+#include "kythe/proto/analysis.pb.h"
+#include "protobuf-matchers/protocol-buffer-matchers.h"
+
+namespace kythe {
+namespace {
+using ::kythe::proto::CompilationUnit;
+using ::protobuf_matchers::EquivToProto;
+
+proto::CompilationUnit ExpectedCompilation() {
+  return ParseTextCompilationUnitOrDie(R"pb(
+    v_name { language: "c++" }
+    required_input {
+      v_name { path: "kythe/cxx/extractor/testdata/header_only_module.h" }
+      info {
+        path: "kythe/cxx/extractor/testdata/header_only_module.h"
+        digest: "1c1216acd9ba7e73f6e66b5174c9445b191ccefa3fc3a3df3c20cbd931755ca7"
+      }
+      details {
+        [type.googleapis.com/kythe.proto.ContextDependentVersion] {
+          row { source_context: "hash1" always_process: false }
+        }
+      }
+    }
+    argument: "/dummy/bin/clang"
+    argument: "-target"
+    argument: "dummy-target"
+    argument: "-DKYTHE_IS_RUNNING=1"
+    argument: "-resource-dir"
+    argument: "/kythe_builtins"
+    argument: "-I./kythe/cxx/extractor/testdata"
+    argument: "-xc++"
+    argument: "-Xclang=-emit-module"
+    argument: "-Xclang=-fmodules-embed-all-files"
+    argument: "-Xclang=-fmodules-local-submodule-visibility"
+    argument: "-fmodules"
+    argument: "-fmodule-file-deps"
+    argument: "-fno-implicit-modules"
+    argument: "-fno-implicit-module-maps"
+    argument: "-fmodules-strict-decluse"
+    argument: "-fmodule-name=//kythe/cxx/extractor/testdata:header_only_module"
+    argument: "-fmodule-map-file=./kythe/cxx/extractor/testdata/header_only_module.cppmap"
+    argument: "-Xclang=-fmodule-map-file-home-is-cwd"
+    argument: "-fPIE"
+    argument: "-fPIC"
+    argument: "./kythe/cxx/extractor/testdata/header_only_module.cppmap"
+    argument: "-o"
+    argument: "header_only_module.pic.pcm"
+    argument: "-fsyntax-only"
+    source_file: "./kythe/cxx/extractor/testdata/header_only_module.cppmap"
+    working_directory: "/root"
+    entry_context: "hash0"
+    output_key: "header_only_module.pic.pcm"
+  )pb");
+}
+
+TEST(CxxExtractorTest, TestHeaderOnlyModule) {
+  CompilationUnit unit = ExtractSingleCompilationOrDie(
+      {{"--with_executable",
+        "/dummy/bin/clang",
+        "-I./kythe/cxx/extractor/testdata",
+        "-xc++",
+        "-Xclang=-emit-module",
+        "-Xclang=-fmodules-embed-all-files",
+        "-Xclang=-fmodules-local-submodule-visibility",
+        "-fmodules",
+        "-fmodule-file-deps",
+        "-fno-implicit-modules",
+        "-fno-implicit-module-maps",
+        "-fmodules-strict-decluse",
+        "-fmodule-name=//kythe/cxx/extractor/testdata:header_only_module",
+        "-fmodule-map-file=./kythe/cxx/extractor/testdata/"
+        "header_only_module.cppmap",
+        "-Xclang=-fmodule-map-file-home-is-cwd",
+        "-fPIE",
+        "-fPIC",
+        "-c",
+        "./kythe/cxx/extractor/testdata/header_only_module.cppmap",
+        "-o",
+        "header_only_module.pic.pcm"}});
+
+  // Fix up things which may legitmately vary between runs.
+  // TODO(shahms): use gMock protobuf matchers when available.
+  CanonicalizeHashes(&unit);
+  unit.set_argument(2, "dummy-target");
+  unit.clear_details();
+
+  EXPECT_THAT(unit, EquivToProto(ExpectedCompilation()));
+}
+
+}  // namespace
+}  // namespace kythe