[Parser] Don't crash on MS assembly if target desc/asm parser isn't linked in.

[sammccall]

Summary

Instead, emit a diagnostic and return an empty ASM node, as we do if the target
is missing.

Filter this diagnostic out in clangd, where it's not meaningful.

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

[sammccall]

revert random meaningless changes

[kadircet]

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

1017

nit: empty line

clang/lib/Parse/ParseStmtAsm.cpp

596

is it ok for MRI to be dereferenced here?

604

the original bail out(no target or no tokens) doesn't seem to be emitting diags, are we sure we want to emit diags here? (same for the one below)

[merge_guards_bot]

Build result: pass - 60607 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[merge_guards_bot]

Build result: pass - 60607 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[sammccall]

clang/lib/Parse/ParseStmtAsm.cpp

596

No :-( Fixed with yet more code
(In practice it didn't cause a problem because all these are null or not-null, and the function doesn't do anything if they're null)

604

For no tokens there's no diagnostic required, the empty statement is correct.

Missing target does have a diagnostic: the if statement on line 555 will either set Target to non-null or emit a diagnostic.

[sammccall]

address comments

[merge_guards_bot]

Build result: pass - 60607 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[kadircet]

LGTM, thanks!

clang-tools-extra/clangd/Diagnostics.cpp

if (!D.Fixes.empty())
     return true;
   for (auto &N : D.Notes) {
     if (N.InsideMainFile)
       return true;
   }
   return false;
 }
 
+bool isBlacklisted(const Diag &D) {
+  // clang will always fail to MS ASM as we don't link in desc + asm parser.
+  if (D.ID == clang::diag::err_msasm_unable_to_create_target)
+    return true;
+  return false;
+}
+
 // Checks whether a location is within a half-open range.
 // Note that clang also uses closed source ranges, which this can't handle!
 bool locationInRange(SourceLocation L, CharSourceRange R,
                      const SourceManager &M) {
   assert(R.isCharRange());
   if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) ||
       M.getFileID(R.getBegin()) != M.getFileID(L))
     return false;
if (!Info.getFixItHints().empty()) {
       LastDiag->Notes.push_back(std::move(N));
     }
   }
 }
 
 void StoreDiags::flushLastDiag() {
   if (!LastDiag)
     return;
-  if (mentionsMainFile(*LastDiag) &&
+  if (!isBlacklisted(*LastDiag) && mentionsMainFile(*LastDiag) &&
       (!LastDiagWasAdjusted ||
        // Only report the first diagnostic coming from each particular header.
        IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
     Output.push_back(std::move(*LastDiag));
   } else {
     vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
   }
   LastDiag.reset();
   LastDiagWasAdjusted = false;
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   support
+  AllTargetsInfos
   )
 
 get_filename_component(CLANGD_SOURCE_DIR
   ${CMAKE_CURRENT_SOURCE_DIR}/../../clangd REALPATH)
 get_filename_component(CLANGD_BINARY_DIR
   ${CMAKE_CURRENT_BINARY_DIR}/../../clangd REALPATH)
 include_directories(
   ${CLANGD_SOURCE_DIR}

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/TargetSelect.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <algorithm>
 
 namespace clang {
 namespace clangd {
 namespace {
 
Annotations Test(R"cpp(
     [[Define]](main)
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   EXPECT_THAT(TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
                                 Not(WithFix(_)))));
 }
 
+TEST(ClangdTest, MSAsm) {
+  // Parsing MS assembly tries to use the target MCAsmInfo, which we don't link.
+  // We used to crash here. Now clang emits a diagnostic, which we filter out.
+  llvm::InitializeAllTargetInfos(); // As in ClangdMain
+  auto TU = TestTU::withCode("void fn() { __asm { cmp cl,64 } }");
+  TU.ExtraArgs = {"-fms-extensions"};
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+}
+
 TEST(DiagnosticsTest, ToLSP) {
   URIForFile MainFile =
       URIForFile::canonicalize(testPath("foo/bar/main.cpp"), "");
   URIForFile HeaderFile =
       URIForFile::canonicalize(testPath("foo/bar/header.h"), "");
 
   clangd::Diag D;
   D.ID = clang::diag::err_enum_class_reference;
TEST(IgnoreDiags, FromNonWrittenInclude) {
   TestTU TU;
   TU.ExtraArgs.push_back("--include=a.h");
   TU.AdditionalFiles = {{"a.h", "void main();"}};
   // The diagnostic "main must return int" is from the header, we don't attempt
   // to render it in the main file as there is no written location there.
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang/lib/Parse/ParseStmtAsm.cpp

if (UnsupportedArch) {
     std::string Error;
     TheTarget = llvm::TargetRegistry::lookupTarget(TT, Error);
     if (!TheTarget)
       Diag(AsmLoc, diag::err_msasm_unable_to_create_target) << Error;
   }
 
   assert(!LBraceLocs.empty() && "Should have at least one location here");
 
+  SmallString<512> AsmString;
+  auto EmptyStmt = [&] {
+    return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLocs[0], AsmToks, AsmString,
+                                  /*NumOutputs*/ 0, /*NumInputs*/ 0,
+                                  ConstraintRefs, ClobberRefs, Exprs, EndLoc);
+  };
   // If we don't support assembly, or the assembly is empty, we don't
   // need to instantiate the AsmParser, etc.
   if (!TheTarget || AsmToks.empty()) {
-    return Actions.ActOnMSAsmStmt(AsmLoc, LBraceLocs[0], AsmToks, StringRef(),
-                                  /*NumOutputs*/ 0, /*NumInputs*/ 0,
-                                  ConstraintRefs, ClobberRefs, Exprs, EndLoc);
+    return EmptyStmt();
   }
 
   // Expand the tokens into a string buffer.
-  SmallString<512> AsmString;
   SmallVector<unsigned, 8> TokOffsets;
   if (buildMSAsmString(PP, AsmLoc, AsmToks, TokOffsets, AsmString))
     return StmtError();
 
   const TargetOptions &TO = Actions.Context.getTargetInfo().getTargetOpts();
   std::string FeaturesStr =
       llvm::join(TO.Features.begin(), TO.Features.end(), ",");
 
   std::unique_ptr<llvm::MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TT));
+  if (!MRI) {
+    Diag(AsmLoc, diag::err_msasm_unable_to_create_target)
+        << "target MC unavailable";
+    return EmptyStmt();
+  }
   // FIXME: init MCOptions from sanitizer flags here.
   llvm::MCTargetOptions MCOptions;
   std::unique_ptr<llvm::MCAsmInfo> MAI(
       TheTarget->createMCAsmInfo(*MRI, TT, MCOptions));
   // Get the instruction descriptor.
   std::unique_ptr<llvm::MCInstrInfo> MII(TheTarget->createMCInstrInfo());
   std::unique_ptr<llvm::MCObjectFileInfo> MOFI(new llvm::MCObjectFileInfo());
   std::unique_ptr<llvm::MCSubtargetInfo> STI(
       TheTarget->createMCSubtargetInfo(TT, TO.CPU, FeaturesStr));
+  // Target MCTargetDesc may not be linked in clang-based tools.
+  if (!MAI || !MII | !MOFI || !STI) {
+    Diag(AsmLoc, diag::err_msasm_unable_to_create_target)
+        << "target MC unavailable";
+    return EmptyStmt();
+  }
 
   llvm::SourceMgr TempSrcMgr;
   llvm::MCContext Ctx(MAI.get(), MRI.get(), MOFI.get(), &TempSrcMgr);
   MOFI->InitMCObjectFileInfo(TheTriple, /*PIC*/ false, Ctx);
   std::unique_ptr<llvm::MemoryBuffer> Buffer =
       llvm::MemoryBuffer::getMemBuffer(AsmString, "<MS inline asm>");
 
   // Tell SrcMgr about this buffer, which is what the parser will pick up.
   TempSrcMgr.AddNewSourceBuffer(std::move(Buffer), llvm::SMLoc());
 
   std::unique_ptr<llvm::MCStreamer> Str(createNullStreamer(Ctx));
   std::unique_ptr<llvm::MCAsmParser> Parser(
       createMCAsmParser(TempSrcMgr, Ctx, *Str.get(), *MAI));
 
   std::unique_ptr<llvm::MCTargetAsmParser> TargetParser(
       TheTarget->createMCAsmParser(*STI, *Parser, *MII, MCOptions));
+  // Target AsmParser may not be linked in clang-based tools.
+  if (!TargetParser) {
+    Diag(AsmLoc, diag::err_msasm_unable_to_create_target)
+        << "target ASM parser unavailable";
+    return EmptyStmt();
+  }
 
   std::unique_ptr<llvm::MCInstPrinter> IP(
       TheTarget->createMCInstPrinter(llvm::Triple(TT), 1, *MAI, *MII, *MRI));
 
   // Change to the Intel dialect.
   Parser->setAssemblerDialect(1);
   Parser->setTargetParser(*TargetParser.get());
   Parser->setParsingInlineAsm(true);