Skip to content

Commit 96aca7c

Browse files
authored
[LTO] Improve diagnostics handling when parsing module-level inline assembly (#75726)
Non-LTO compiles set the buffer name to "<inline asm>" (`AsmPrinter::addInlineAsmDiagBuffer`) and pass diagnostics to `ClangDiagnosticHandler` (through the `MCContext` handler in `MachineModuleInfoWrapperPass::doInitialization`) to ensure that the exit code is 1 in the presence of errors. In contrast, LTO compiles spuriously succeed even if error messages are printed. ``` % cat a.c void _start() {} asm("unknown instruction"); % clang -c a.c <inline asm>:1:1: error: invalid instruction mnemonic 'unknown' 1 | unknown instruction | ^ 1 error generated. % clang -c -flto a.c; echo $? # -flto=thin is the same error: invalid instruction mnemonic 'unknown' unknown instruction ^~~~~~~ error: invalid instruction mnemonic 'unknown' unknown instruction ^~~~~~~ 0 ``` `CollectAsmSymbols` parses inline assembly and is transitively called by both `ModuleSummaryIndexAnalysis::run` and `WriteBitcodeToFile`, leading to duplicate diagnostics. This patch updates `CollectAsmSymbols` to be similar to non-LTO compiles. ``` % clang -c -flto=thin a.c; echo $? <inline asm>:1:1: error: invalid instruction mnemonic 'unknown' 1 | unknown instruction | ^ 1 errors generated. 1 ``` The `HasErrors` check does not prevent duplicate warnings but assembler warnings are very uncommon.
1 parent 672f1a0 commit 96aca7c

File tree

5 files changed

+26
-6
lines changed

5 files changed

+26
-6
lines changed

clang/lib/CodeGen/CodeGenAction.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,8 @@ void BackendConsumer::anchor() { }
418418
} // namespace clang
419419

420420
bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo &DI) {
421+
if (DI.getSeverity() == DS_Error)
422+
HasErrors = true;
421423
BackendCon->DiagnosticHandlerImpl(DI);
422424
return true;
423425
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
// REQUIRES: arm-registered-target
22
// RUN: not %clang_cc1 -emit-obj -triple armv6-unknown-unknown -o %t %s 2>&1 | FileCheck %s
3+
// RUN: not %clang_cc1 -emit-obj -triple armv6-unknown-unknown -flto -o %t %s 2>&1 | FileCheck %s
4+
5+
/// Test the diagnostic behavior considering the whole system including the driver.
6+
// RUN: not %clang --target=armv6-unknown-unknown -c -flto=thin -o %t %s 2>&1 | FileCheck %s
37
#pragma clang diagnostic ignored "-Wmissing-noreturn"
48
__asm__(".Lfoo: movw r2, #:lower16:.Lbar - .Lfoo");
59
// CHECK: <inline asm>:1:8: error: instruction requires: armv6t2
10+
// CHECK-NOT: error:

lld/test/MachO/lto-module-asm-err.ll

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@
33
; RUN: not %lld %t.bc -o /dev/null 2>&1 | FileCheck %s --check-prefix=REGULAR
44

55
;; For regular LTO, the original module name is lost.
6-
;; TODO Fix the line number
7-
; REGULAR: error: ld-temp.o <inline asm>:3:1: invalid instruction mnemonic 'invalid'
6+
; REGULAR: error: <inline asm>:2:1: invalid instruction mnemonic 'invalid'
87

9-
; RUN: opt -module-summary %s -o %t.bc
10-
; RUN: not %lld %t.bc -o /dev/null 2>&1 | FileCheck %s --check-prefix=THIN
8+
; RUN: not opt -module-summary %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=THIN
119

12-
; THIN: error: {{.*}}.bc <inline asm>:2:1: invalid instruction mnemonic 'invalid'
10+
; THIN: error: <inline asm>:2:1: invalid instruction mnemonic 'invalid'
1311

1412
target triple = "x86_64-apple-macosx10.15.0"
1513
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

llvm/include/llvm/IR/DiagnosticHandler.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class DiagnosticInfo;
2323
/// which remarks are enabled.
2424
struct DiagnosticHandler {
2525
void *DiagnosticContext = nullptr;
26+
bool HasErrors = false;
2627
DiagnosticHandler(void *DiagContext = nullptr)
2728
: DiagnosticContext(DiagContext) {}
2829
virtual ~DiagnosticHandler() = default;

llvm/lib/Object/ModuleSymbolTable.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "RecordStreamer.h"
1717
#include "llvm/ADT/STLExtras.h"
1818
#include "llvm/ADT/StringRef.h"
19+
#include "llvm/IR/DiagnosticInfo.h"
1920
#include "llvm/IR/Function.h"
2021
#include "llvm/IR/GlobalAlias.h"
2122
#include "llvm/IR/GlobalValue.h"
@@ -68,6 +69,11 @@ void ModuleSymbolTable::addModule(Module *M) {
6869
static void
6970
initializeRecordStreamer(const Module &M,
7071
function_ref<void(RecordStreamer &)> Init) {
72+
// This function may be called twice, once for ModuleSummaryIndexAnalysis and
73+
// the other when writing the IR symbol table. If parsing inline assembly has
74+
// caused errors in the first run, suppress the second run.
75+
if (M.getContext().getDiagHandlerPtr()->HasErrors)
76+
return;
7177
StringRef InlineAsm = M.getModuleInlineAsm();
7278
if (InlineAsm.empty())
7379
return;
@@ -95,7 +101,8 @@ initializeRecordStreamer(const Module &M,
95101
if (!MCII)
96102
return;
97103

98-
std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(InlineAsm));
104+
std::unique_ptr<MemoryBuffer> Buffer(
105+
MemoryBuffer::getMemBuffer(InlineAsm, "<inline asm>"));
99106
SourceMgr SrcMgr;
100107
SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
101108

@@ -115,6 +122,13 @@ initializeRecordStreamer(const Module &M,
115122
if (!TAP)
116123
return;
117124

125+
MCCtx.setDiagnosticHandler([&](const SMDiagnostic &SMD, bool IsInlineAsm,
126+
const SourceMgr &SrcMgr,
127+
std::vector<const MDNode *> &LocInfos) {
128+
M.getContext().diagnose(
129+
DiagnosticInfoSrcMgr(SMD, M.getName(), IsInlineAsm, /*LocCookie=*/0));
130+
});
131+
118132
// Module-level inline asm is assumed to use At&t syntax (see
119133
// AsmPrinter::doInitialization()).
120134
Parser->setAssemblerDialect(InlineAsm::AD_ATT);

0 commit comments

Comments
 (0)