sammccall updated this revision to Diff 232791. sammccall added a comment. revert random meaningless changes
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(); @@ -591,6 +594,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 (!MRI || !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 +616,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