sammccall updated this revision to Diff 232798.
sammccall marked an inline comment as done.
sammccall added a comment.
address comments
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
