[llvm] [lld] [clang] [LTO] Improve diagnostics handling when parsing module-level inline assembly (PR #75726)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 17 19:17:34 PST 2023
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/75726
>From 4e5fb8b8c7f39935dce2a11bd90fc27f65cd2d76 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 10 Dec 2023 10:13:59 -0800
Subject: [PATCH 1/2] [LTO] Improve diagnostics handling when parsing
module-level inline assembly
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.
---
clang/lib/CodeGen/CodeGenAction.cpp | 2 ++
clang/test/CodeGen/invalid_global_asm.c | 5 +++++
llvm/include/llvm/IR/DiagnosticHandler.h | 1 +
llvm/lib/Object/ModuleSymbolTable.cpp | 16 +++++++++++++++-
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 753a8fd74fa696..4121a3709bc3af 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -418,6 +418,8 @@ void BackendConsumer::anchor() { }
} // namespace clang
bool ClangDiagnosticHandler::handleDiagnostics(const DiagnosticInfo &DI) {
+ if (DI.getSeverity() == DS_Error)
+ HasErrors = true;
BackendCon->DiagnosticHandlerImpl(DI);
return true;
}
diff --git a/clang/test/CodeGen/invalid_global_asm.c b/clang/test/CodeGen/invalid_global_asm.c
index 5b7e8b43d752d8..d5645f7fc92bf4 100644
--- a/clang/test/CodeGen/invalid_global_asm.c
+++ b/clang/test/CodeGen/invalid_global_asm.c
@@ -1,5 +1,10 @@
// REQUIRES: arm-registered-target
// RUN: not %clang_cc1 -emit-obj -triple armv6-unknown-unknown -o %t %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -emit-obj -triple armv6-unknown-unknown -flto -o %t %s 2>&1 | FileCheck %s
+
+/// Test the diagnostic behavior considering the whole system including the driver.
+// RUN: not %clang --target=armv6-unknown-unknown -c -flto=thin -o %t %s 2>&1 | FileCheck %s
#pragma clang diagnostic ignored "-Wmissing-noreturn"
__asm__(".Lfoo: movw r2, #:lower16:.Lbar - .Lfoo");
// CHECK: <inline asm>:1:8: error: instruction requires: armv6t2
+// CHECK-NOT: error:
diff --git a/llvm/include/llvm/IR/DiagnosticHandler.h b/llvm/include/llvm/IR/DiagnosticHandler.h
index 55e5e5975808d9..db7d7444f75f05 100644
--- a/llvm/include/llvm/IR/DiagnosticHandler.h
+++ b/llvm/include/llvm/IR/DiagnosticHandler.h
@@ -23,6 +23,7 @@ class DiagnosticInfo;
/// which remarks are enabled.
struct DiagnosticHandler {
void *DiagnosticContext = nullptr;
+ bool HasErrors = false;
DiagnosticHandler(void *DiagContext = nullptr)
: DiagnosticContext(DiagContext) {}
virtual ~DiagnosticHandler() = default;
diff --git a/llvm/lib/Object/ModuleSymbolTable.cpp b/llvm/lib/Object/ModuleSymbolTable.cpp
index ab073e18cb4668..07f76688fa43e7 100644
--- a/llvm/lib/Object/ModuleSymbolTable.cpp
+++ b/llvm/lib/Object/ModuleSymbolTable.cpp
@@ -16,6 +16,7 @@
#include "RecordStreamer.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalValue.h"
@@ -68,6 +69,11 @@ void ModuleSymbolTable::addModule(Module *M) {
static void
initializeRecordStreamer(const Module &M,
function_ref<void(RecordStreamer &)> Init) {
+ // This function may be called twice, once for ModuleSummaryIndexAnalysis and
+ // the other when writing the IR symbol table. If parsing inline assembly has
+ // caused errors in the first run, suppress the second run.
+ if (M.getContext().getDiagHandlerPtr()->HasErrors)
+ return;
StringRef InlineAsm = M.getModuleInlineAsm();
if (InlineAsm.empty())
return;
@@ -95,7 +101,8 @@ initializeRecordStreamer(const Module &M,
if (!MCII)
return;
- std::unique_ptr<MemoryBuffer> Buffer(MemoryBuffer::getMemBuffer(InlineAsm));
+ std::unique_ptr<MemoryBuffer> Buffer(
+ MemoryBuffer::getMemBuffer(InlineAsm, "<inline asm>"));
SourceMgr SrcMgr;
SrcMgr.AddNewSourceBuffer(std::move(Buffer), SMLoc());
@@ -115,6 +122,13 @@ initializeRecordStreamer(const Module &M,
if (!TAP)
return;
+ MCCtx.setDiagnosticHandler([&](const SMDiagnostic &SMD, bool IsInlineAsm,
+ const SourceMgr &SrcMgr,
+ std::vector<const MDNode *> &LocInfos) {
+ M.getContext().diagnose(
+ DiagnosticInfoSrcMgr(SMD, M.getName(), IsInlineAsm, /*LocCookie=*/0));
+ });
+
// Module-level inline asm is assumed to use At&t syntax (see
// AsmPrinter::doInitialization()).
Parser->setAssemblerDialect(InlineAsm::AD_ATT);
>From 09f2f27972d75886085bdd790533d154c01984d4 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sun, 17 Dec 2023 19:16:47 -0800
Subject: [PATCH 2/2] Update lto-module-asm-err.ll
---
lld/test/MachO/lto-module-asm-err.ll | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/lld/test/MachO/lto-module-asm-err.ll b/lld/test/MachO/lto-module-asm-err.ll
index 45cd75967dbe74..45dff241a653b1 100644
--- a/lld/test/MachO/lto-module-asm-err.ll
+++ b/lld/test/MachO/lto-module-asm-err.ll
@@ -3,13 +3,11 @@
; RUN: not %lld %t.bc -o /dev/null 2>&1 | FileCheck %s --check-prefix=REGULAR
;; For regular LTO, the original module name is lost.
-;; TODO Fix the line number
-; REGULAR: error: ld-temp.o <inline asm>:3:1: invalid instruction mnemonic 'invalid'
+; REGULAR: error: <inline asm>:2:1: invalid instruction mnemonic 'invalid'
-; RUN: opt -module-summary %s -o %t.bc
-; RUN: not %lld %t.bc -o /dev/null 2>&1 | FileCheck %s --check-prefix=THIN
+; RUN: not opt -module-summary %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=THIN
-; THIN: error: {{.*}}.bc <inline asm>:2:1: invalid instruction mnemonic 'invalid'
+; THIN: error: <inline asm>:2:1: invalid instruction mnemonic 'invalid'
target triple = "x86_64-apple-macosx10.15.0"
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
More information about the llvm-commits
mailing list