This revision was automatically updated to reflect the committed changes.
Closed by commit rG94603ec11b55: [Parser] Don't crash on MS assembly if 
target desc/asm parser isn't linked in. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71189/new/

https://reviews.llvm.org/D71189

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/lib/Parse/ParseStmtAsm.cpp

Index: clang/lib/Parse/ParseStmtAsm.cpp
===================================================================
--- clang/lib/Parse/ParseStmtAsm.cpp
+++ clang/lib/Parse/ParseStmtAsm.cpp
@@ -563,16 +563,19 @@
 
   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();
@@ -582,6 +585,11 @@
       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(
@@ -591,6 +599,12 @@
   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);
@@ -607,6 +621,12 @@
 
   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));
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -19,6 +19,7 @@
 #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>
@@ -405,6 +406,15 @@
                                 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"), "");
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   support
+  AllTargetsInfos
   )
 
 get_filename_component(CLANGD_SOURCE_DIR
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -73,6 +73,13 @@
   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,
@@ -642,7 +649,7 @@
 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)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to