Skip to content

Commit 5de2d18

Browse files
author
Yuanfang Chen
committed
[Diagnose] Unify MCContext and LLVMContext diagnosing
The situation with inline asm/MC error reporting is kind of messy at the moment. The errors from MC layout are not reliably propagated and users have to specify an inlineasm handler separately to get inlineasm diagnose. The latter issue is not a correctness issue but could be improved. * Kill LLVMContext inlineasm diagnose handler and migrate it to use DiagnoseInfo/DiagnoseHandler. * Introduce `DiagnoseInfoSrcMgr` to diagnose SourceMgr backed errors. This covers use cases like inlineasm, MC, and any clients using SourceMgr. * Move AsmPrinter::SrcMgrDiagInfo and its instance to MCContext. The next step is to combine MCContext::SrcMgr and MCContext::InlineSrcMgr because in all use cases, only one of them is used. * If LLVMContext is available, let MCContext uses LLVMContext's diagnose handler; if LLVMContext is not available, MCContext uses its own default diagnose handler which just prints SMDiagnostic. * Change a few clients(Clang, llc, lldb) to use the new way of reporting. Reviewed By: MaskRay Differential Revision: https://reviews.llvm.org/D97449
1 parent 2b5f3f4 commit 5de2d18

20 files changed

+311
-279
lines changed

‎clang/include/clang/Basic/DiagnosticCategories.td

+1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@
77
//===----------------------------------------------------------------------===//
88

99
class CatInlineAsm : DiagCategory<"Inline Assembly Issue">;
10+
class CatSourceMgr : DiagCategory<"SourceMgr Reported Issue">;
1011
class CatBackend : DiagCategory<"Backend Issue">;

‎clang/include/clang/Basic/DiagnosticFrontendKinds.td

+4
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ def err_fe_inline_asm : Error<"%0">, CatInlineAsm;
1919
def warn_fe_inline_asm : Warning<"%0">, CatInlineAsm, InGroup<BackendInlineAsm>;
2020
def note_fe_inline_asm : Note<"%0">, CatInlineAsm;
2121
def note_fe_inline_asm_here : Note<"instantiated into assembly here">;
22+
def err_fe_source_mgr : Error<"%0">, CatSourceMgr;
23+
def warn_fe_source_mgr : Warning<"%0">, CatSourceMgr, InGroup<BackendSourceMgr>;
24+
def note_fe_source_mgr : Note<"%0">, CatSourceMgr;
25+
def remark_fe_source_mgr: Remark<"%0">, CatSourceMgr, InGroup<BackendSourceMgr>;
2226
def err_fe_cannot_link_module : Error<"cannot link module '%0': %1">,
2327
DefaultFatal;
2428

‎clang/include/clang/Basic/DiagnosticGroups.td

+1
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,7 @@ def OpenMP : DiagGroup<"openmp", [
11451145

11461146
// Backend warnings.
11471147
def BackendInlineAsm : DiagGroup<"inline-asm">;
1148+
def BackendSourceMgr : DiagGroup<"source-mgr">;
11481149
def BackendFrameLargerThanEQ : DiagGroup<"frame-larger-than=">;
11491150
def BackendPlugin : DiagGroup<"backend-plugin">;
11501151
def RemarkBackendPlugin : DiagGroup<"remark-backend-plugin">;

‎clang/lib/CodeGen/CodeGenAction.cpp

+64-101
Original file line numberDiff line numberDiff line change
@@ -301,14 +301,7 @@ namespace clang {
301301
if (!getModule())
302302
return;
303303

304-
// Install an inline asm handler so that diagnostics get printed through
305-
// our diagnostics hooks.
306304
LLVMContext &Ctx = getModule()->getContext();
307-
LLVMContext::InlineAsmDiagHandlerTy OldHandler =
308-
Ctx.getInlineAsmDiagnosticHandler();
309-
void *OldContext = Ctx.getInlineAsmDiagnosticContext();
310-
Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
311-
312305
std::unique_ptr<DiagnosticHandler> OldDiagnosticHandler =
313306
Ctx.getDiagnosticHandler();
314307
Ctx.setDiagnosticHandler(std::make_unique<ClangDiagnosticHandler>(
@@ -342,8 +335,6 @@ namespace clang {
342335
LangOpts, C.getTargetInfo().getDataLayout(),
343336
getModule(), Action, std::move(AsmOutStream));
344337

345-
Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
346-
347338
Ctx.setDiagnosticHandler(std::move(OldDiagnosticHandler));
348339

349340
if (OptRecordFile)
@@ -377,27 +368,20 @@ namespace clang {
377368
Gen->HandleVTable(RD);
378369
}
379370

380-
static void InlineAsmDiagHandler(const llvm::SMDiagnostic &SM,void *Context,
381-
unsigned LocCookie) {
382-
SourceLocation Loc = SourceLocation::getFromRawEncoding(LocCookie);
383-
((BackendConsumer*)Context)->InlineAsmDiagHandler2(SM, Loc);
384-
}
385-
386371
/// Get the best possible source location to represent a diagnostic that
387372
/// may have associated debug info.
388373
const FullSourceLoc
389374
getBestLocationFromDebugLoc(const llvm::DiagnosticInfoWithLocationBase &D,
390375
bool &BadDebugInfo, StringRef &Filename,
391376
unsigned &Line, unsigned &Column) const;
392377

393-
void InlineAsmDiagHandler2(const llvm::SMDiagnostic &,
394-
SourceLocation LocCookie);
395-
396378
void DiagnosticHandlerImpl(const llvm::DiagnosticInfo &DI);
397379
/// Specialized handler for InlineAsm diagnostic.
398380
/// \return True if the diagnostic has been successfully reported, false
399381
/// otherwise.
400382
bool InlineAsmDiagHandler(const llvm::DiagnosticInfoInlineAsm &D);
383+
/// Specialized handler for diagnostics reported using SMDiagnostic.
384+
void SrcMgrDiagHandler(const llvm::DiagnosticInfoSrcMgr &D);
401385
/// Specialized handler for StackSize diagnostic.
402386
/// \return True if the diagnostic has been successfully reported, false
403387
/// otherwise.
@@ -456,64 +440,6 @@ static FullSourceLoc ConvertBackendLocation(const llvm::SMDiagnostic &D,
456440
return FullSourceLoc(NewLoc, CSM);
457441
}
458442

459-
460-
/// InlineAsmDiagHandler2 - This function is invoked when the backend hits an
461-
/// error parsing inline asm. The SMDiagnostic indicates the error relative to
462-
/// the temporary memory buffer that the inline asm parser has set up.
463-
void BackendConsumer::InlineAsmDiagHandler2(const llvm::SMDiagnostic &D,
464-
SourceLocation LocCookie) {
465-
// There are a couple of different kinds of errors we could get here. First,
466-
// we re-format the SMDiagnostic in terms of a clang diagnostic.
467-
468-
// Strip "error: " off the start of the message string.
469-
StringRef Message = D.getMessage();
470-
if (Message.startswith("error: "))
471-
Message = Message.substr(7);
472-
473-
// If the SMDiagnostic has an inline asm source location, translate it.
474-
FullSourceLoc Loc;
475-
if (D.getLoc() != SMLoc())
476-
Loc = ConvertBackendLocation(D, Context->getSourceManager());
477-
478-
unsigned DiagID;
479-
switch (D.getKind()) {
480-
case llvm::SourceMgr::DK_Error:
481-
DiagID = diag::err_fe_inline_asm;
482-
break;
483-
case llvm::SourceMgr::DK_Warning:
484-
DiagID = diag::warn_fe_inline_asm;
485-
break;
486-
case llvm::SourceMgr::DK_Note:
487-
DiagID = diag::note_fe_inline_asm;
488-
break;
489-
case llvm::SourceMgr::DK_Remark:
490-
llvm_unreachable("remarks unexpected");
491-
}
492-
// If this problem has clang-level source location information, report the
493-
// issue in the source with a note showing the instantiated
494-
// code.
495-
if (LocCookie.isValid()) {
496-
Diags.Report(LocCookie, DiagID).AddString(Message);
497-
498-
if (D.getLoc().isValid()) {
499-
DiagnosticBuilder B = Diags.Report(Loc, diag::note_fe_inline_asm_here);
500-
// Convert the SMDiagnostic ranges into SourceRange and attach them
501-
// to the diagnostic.
502-
for (const std::pair<unsigned, unsigned> &Range : D.getRanges()) {
503-
unsigned Column = D.getColumnNo();
504-
B << SourceRange(Loc.getLocWithOffset(Range.first - Column),
505-
Loc.getLocWithOffset(Range.second - Column));
506-
}
507-
}
508-
return;
509-
}
510-
511-
// Otherwise, report the backend issue as occurring in the generated .s file.
512-
// If Loc is invalid, we still need to report the issue, it just gets no
513-
// location info.
514-
Diags.Report(Loc, DiagID).AddString(Message);
515-
}
516-
517443
#define ComputeDiagID(Severity, GroupName, DiagID) \
518444
do { \
519445
switch (Severity) { \
@@ -550,6 +476,65 @@ void BackendConsumer::InlineAsmDiagHandler2(const llvm::SMDiagnostic &D,
550476
} \
551477
} while (false)
552478

479+
void BackendConsumer::SrcMgrDiagHandler(const llvm::DiagnosticInfoSrcMgr &DI) {
480+
const llvm::SMDiagnostic &D = DI.getSMDiag();
481+
482+
unsigned DiagID;
483+
if (DI.isInlineAsmDiag())
484+
ComputeDiagID(DI.getSeverity(), inline_asm, DiagID);
485+
else
486+
ComputeDiagID(DI.getSeverity(), source_mgr, DiagID);
487+
488+
// This is for the empty BackendConsumer that uses the clang diagnostic
489+
// handler for IR input files.
490+
if (!Context) {
491+
D.print(nullptr, llvm::errs());
492+
Diags.Report(DiagID).AddString("cannot compile inline asm");
493+
return;
494+
}
495+
496+
// There are a couple of different kinds of errors we could get here.
497+
// First, we re-format the SMDiagnostic in terms of a clang diagnostic.
498+
499+
// Strip "error: " off the start of the message string.
500+
StringRef Message = D.getMessage();
501+
(void)Message.consume_front("error: ");
502+
503+
// If the SMDiagnostic has an inline asm source location, translate it.
504+
FullSourceLoc Loc;
505+
if (D.getLoc() != SMLoc())
506+
Loc = ConvertBackendLocation(D, Context->getSourceManager());
507+
508+
// If this problem has clang-level source location information, report the
509+
// issue in the source with a note showing the instantiated
510+
// code.
511+
if (DI.isInlineAsmDiag()) {
512+
SourceLocation LocCookie =
513+
SourceLocation::getFromRawEncoding(DI.getLocCookie());
514+
if (LocCookie.isValid()) {
515+
Diags.Report(LocCookie, DiagID).AddString(Message);
516+
517+
if (D.getLoc().isValid()) {
518+
DiagnosticBuilder B = Diags.Report(Loc, diag::note_fe_inline_asm_here);
519+
// Convert the SMDiagnostic ranges into SourceRange and attach them
520+
// to the diagnostic.
521+
for (const std::pair<unsigned, unsigned> &Range : D.getRanges()) {
522+
unsigned Column = D.getColumnNo();
523+
B << SourceRange(Loc.getLocWithOffset(Range.first - Column),
524+
Loc.getLocWithOffset(Range.second - Column));
525+
}
526+
}
527+
return;
528+
}
529+
}
530+
531+
// Otherwise, report the backend issue as occurring in the generated .s file.
532+
// If Loc is invalid, we still need to report the issue, it just gets no
533+
// location info.
534+
Diags.Report(Loc, DiagID).AddString(Message);
535+
return;
536+
}
537+
553538
bool
554539
BackendConsumer::InlineAsmDiagHandler(const llvm::DiagnosticInfoInlineAsm &D) {
555540
unsigned DiagID;
@@ -783,6 +768,9 @@ void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) {
783768
return;
784769
ComputeDiagID(Severity, inline_asm, DiagID);
785770
break;
771+
case llvm::DK_SrcMgr:
772+
SrcMgrDiagHandler(cast<DiagnosticInfoSrcMgr>(DI));
773+
return;
786774
case llvm::DK_StackSize:
787775
if (StackSizeDiagHandler(cast<DiagnosticInfoStackSize>(DI)))
788776
return;
@@ -979,30 +967,6 @@ CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
979967
return std::move(Result);
980968
}
981969

982-
static void BitcodeInlineAsmDiagHandler(const llvm::SMDiagnostic &SM,
983-
void *Context,
984-
unsigned LocCookie) {
985-
SM.print(nullptr, llvm::errs());
986-
987-
auto Diags = static_cast<DiagnosticsEngine *>(Context);
988-
unsigned DiagID;
989-
switch (SM.getKind()) {
990-
case llvm::SourceMgr::DK_Error:
991-
DiagID = diag::err_fe_inline_asm;
992-
break;
993-
case llvm::SourceMgr::DK_Warning:
994-
DiagID = diag::warn_fe_inline_asm;
995-
break;
996-
case llvm::SourceMgr::DK_Note:
997-
DiagID = diag::note_fe_inline_asm;
998-
break;
999-
case llvm::SourceMgr::DK_Remark:
1000-
llvm_unreachable("remarks unexpected");
1001-
}
1002-
1003-
Diags->Report(DiagID).AddString("cannot compile inline asm");
1004-
}
1005-
1006970
std::unique_ptr<llvm::Module>
1007971
CodeGenAction::loadModule(MemoryBufferRef MBRef) {
1008972
CompilerInstance &CI = getCompilerInstance();
@@ -1105,7 +1069,6 @@ void CodeGenAction::ExecuteAction() {
11051069
EmbedBitcode(TheModule.get(), CodeGenOpts, *MainFile);
11061070

11071071
LLVMContext &Ctx = TheModule->getContext();
1108-
Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler, &Diagnostics);
11091072

11101073
// Restore any diagnostic handler previously set before returning from this
11111074
// function.

‎lldb/source/Expression/IRExecutionUnit.cpp

+22-10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "llvm/ExecutionEngine/ExecutionEngine.h"
1010
#include "llvm/ExecutionEngine/ObjectCache.h"
1111
#include "llvm/IR/Constants.h"
12+
#include "llvm/IR/DiagnosticHandler.h"
13+
#include "llvm/IR/DiagnosticInfo.h"
1214
#include "llvm/IR/LLVMContext.h"
1315
#include "llvm/IR/Module.h"
1416
#include "llvm/Support/SourceMgr.h"
@@ -200,16 +202,26 @@ Status IRExecutionUnit::DisassembleFunction(Stream &stream,
200202
return ret;
201203
}
202204

203-
static void ReportInlineAsmError(const llvm::SMDiagnostic &diagnostic,
204-
void *Context, unsigned LocCookie) {
205-
Status *err = static_cast<Status *>(Context);
205+
namespace {
206+
struct IRExecDiagnosticHandler : public llvm::DiagnosticHandler {
207+
Status *err;
208+
IRExecDiagnosticHandler(Status *err) : err(err) {}
209+
bool handleDiagnostics(const llvm::DiagnosticInfo &DI) override {
210+
if (DI.getKind() == llvm::DK_SrcMgr) {
211+
const auto &DISM = llvm::cast<llvm::DiagnosticInfoSrcMgr>(DI);
212+
if (err && err->Success()) {
213+
err->SetErrorToGenericError();
214+
err->SetErrorStringWithFormat(
215+
"Inline assembly error: %s",
216+
DISM.getSMDiag().getMessage().str().c_str());
217+
}
218+
return true;
219+
}
206220

207-
if (err && err->Success()) {
208-
err->SetErrorToGenericError();
209-
err->SetErrorStringWithFormat("Inline assembly error: %s",
210-
diagnostic.getMessage().str().c_str());
221+
return false;
211222
}
212-
}
223+
};
224+
} // namespace
213225

214226
void IRExecutionUnit::ReportSymbolLookupError(ConstString name) {
215227
m_failed_lookups.push_back(name);
@@ -257,8 +269,8 @@ void IRExecutionUnit::GetRunnableInfo(Status &error, lldb::addr_t &func_addr,
257269
LLDB_LOGF(log, "Module being sent to JIT: \n%s", s.c_str());
258270
}
259271

260-
m_module_up->getContext().setInlineAsmDiagnosticHandler(ReportInlineAsmError,
261-
&error);
272+
m_module_up->getContext().setDiagnosticHandler(
273+
std::make_unique<IRExecDiagnosticHandler>(&error));
262274

263275
llvm::EngineBuilder builder(std::move(m_module_up));
264276
llvm::Triple triple(m_module->getTargetTriple());

‎llvm/include/llvm/CodeGen/AsmPrinter.h

-12
Original file line numberDiff line numberDiff line change
@@ -185,25 +185,13 @@ class AsmPrinter : public MachineFunctionPass {
185185
std::vector<HandlerInfo> Handlers;
186186
size_t NumUserHandlers = 0;
187187

188-
public:
189-
struct SrcMgrDiagInfo {
190-
SourceMgr SrcMgr;
191-
std::vector<const MDNode *> LocInfos;
192-
LLVMContext::InlineAsmDiagHandlerTy DiagHandler;
193-
void *DiagContext;
194-
};
195-
196188
private:
197189
/// If generated on the fly this own the instance.
198190
std::unique_ptr<MachineDominatorTree> OwnedMDT;
199191

200192
/// If generated on the fly this own the instance.
201193
std::unique_ptr<MachineLoopInfo> OwnedMLI;
202194

203-
/// Structure for generating diagnostics for inline assembly. Only initialised
204-
/// when necessary.
205-
mutable std::unique_ptr<SrcMgrDiagInfo> DiagInfo;
206-
207195
/// If the target supports dwarf debug info, this pointer is non-null.
208196
DwarfDebug *DD = nullptr;
209197

0 commit comments

Comments
 (0)