[clang] 94603ec - [Parser] Don't crash on MS assembly if target desc/asm parser isn't linked in.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 9 05:34:40 PST 2019
Author: Sam McCall
Date: 2019-12-09T14:34:31+01:00
New Revision: 94603ec11b55ca22b5dbebcfca5e83f313b632e3
URL: https://github.com/llvm/llvm-project/commit/94603ec11b55ca22b5dbebcfca5e83f313b632e3
DIFF: https://github.com/llvm/llvm-project/commit/94603ec11b55ca22b5dbebcfca5e83f313b632e3.diff
LOG: [Parser] Don't crash on MS assembly if target desc/asm parser isn't linked in.
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
Reviewers: kadircet
Subscribers: mgorny, ilya-biryukov, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D71189
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index cd95807162bc..f97191196e7b 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -73,6 +73,13 @@ bool mentionsMainFile(const Diag &D) {
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::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
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)) {
diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index d2a689056ecd..f8b24c606962 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -1,5 +1,6 @@
set(LLVM_LINK_COMPONENTS
support
+ AllTargetsInfos
)
get_filename_component(CLANGD_SOURCE_DIR
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 3c0257849021..0941af25213c 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/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 @@ TEST(DiagnosticsTest, NoFixItInMacro) {
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"), "");
diff --git a/clang/lib/Parse/ParseStmtAsm.cpp b/clang/lib/Parse/ParseStmtAsm.cpp
index 6b301667d3f2..98133aaf67af 100644
--- a/clang/lib/Parse/ParseStmtAsm.cpp
+++ b/clang/lib/Parse/ParseStmtAsm.cpp
@@ -563,16 +563,19 @@ StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
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 @@ StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
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 @@ StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
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 @@ StmtResult Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
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));
More information about the cfe-commits
mailing list