https://github.com/arichardson created https://github.com/llvm/llvm-project/pull/227569
Add a `-verify` (and `-verify=<prefix1,prefix2,...>`) option to `llvm-mc`, backed by `llvm::SourceMgrDiagnosticVerifier`, to check assembly diagnostics inline via `expected-(error|warning|remark|note)` comments without needing an external `not llvm-mc ... 2>&1 | FileCheck` pipeline. In `-verify` mode, assembly output is suppressed (`MCNullStreamer`), all buffers registered with `SourceMgr` (including `.include`s) are scanned for directives scoped to the target's `MCAsmInfo::getCommentString()` plus `//` and `#`, and `llvm-mc` exits zero if and only if all expected diagnostics match and no unexpected diagnostics are emitted. This commit was created with the help of AI tools >From ff6ac85ce259ba8c90ab88b9d6f45046d91e8deb Mon Sep 17 00:00:00 2001 From: Alex Richardson <[email protected]> Date: Tue, 29 Sep 2026 22:49:13 -0700 Subject: [PATCH] [llvm-mc] Add -verify=<prefixes> diagnostic-verification mode Add a `-verify` (and `-verify=<prefix1,prefix2,...>`) option to `llvm-mc`, backed by `llvm::SourceMgrDiagnosticVerifier`, to check assembly diagnostics inline via `expected-(error|warning|remark|note)` comments without needing an external `not llvm-mc ... 2>&1 | FileCheck` pipeline. In `-verify` mode, assembly output is suppressed (`MCNullStreamer`), all buffers registered with `SourceMgr` (including `.include`s) are scanned for directives scoped to the target's `MCAsmInfo::getCommentString()` plus `//` and `#`, and `llvm-mc` exits zero if and only if all expected diagnostics match and no unexpected diagnostics are emitted. This commit was created with the help of AI tools --- llvm/docs/CommandGuide/llvm-mc.md | 49 +++++++++++ llvm/docs/TestingGuide.md | 11 +++ llvm/test/tools/llvm-mc/verify/basic.s | 112 +++++++++++++++++++++++++ llvm/tools/llvm-mc/CMakeLists.txt | 1 + llvm/tools/llvm-mc/Verifier.cpp | 97 +++++++++++++++++++++ llvm/tools/llvm-mc/Verifier.h | 64 ++++++++++++++ llvm/tools/llvm-mc/llvm-mc.cpp | 58 +++++++++++++ 7 files changed, 392 insertions(+) create mode 100644 llvm/test/tools/llvm-mc/verify/basic.s create mode 100644 llvm/tools/llvm-mc/Verifier.cpp create mode 100644 llvm/tools/llvm-mc/Verifier.h diff --git a/llvm/docs/CommandGuide/llvm-mc.md b/llvm/docs/CommandGuide/llvm-mc.md index 4fdde854cdecc..7ca33b7d7ed99 100644 --- a/llvm/docs/CommandGuide/llvm-mc.md +++ b/llvm/docs/CommandGuide/llvm-mc.md @@ -157,6 +157,55 @@ prints in Intel/MASM syntax. Choose DWARF debug sections compression. ::: +:::{option} --verify=<prefixes> + +Instead of just printing diagnostics, check them against `<prefix>-error`, +`<prefix>-warning`, `<prefix>-note` and `<prefix>-remark` comments in +the input, similar to Clang's `-verify` flag. `<prefixes>` is a +comma-separated list of prefixes to recognize; a bare `--verify` (with no +`=<prefixes>`) is equivalent to `--verify=expected`. + +A directive has the form: +```asm +<prefix>-<kind>[-re][@<offset>] {{<text>}} +``` + +`<kind>` is one of `error`, `warning`, `note` or `remark`. Without +an `@<offset>`, the directive applies to its own line; `@+N`/`@-N` +apply to the line `N` lines below/above, and `@above`/`@below` apply to +the nearest non-directive line above/below. By default `<text>` is matched +as a substring of the diagnostic's message; with the `-re` suffix, any +`{{...}}` block nested inside `<text>` is matched as a regular +expression instead (the rest of `<text>` is matched literally), for +example: + +```asm +## Matches only diagnostics containing this exact text. +.foo +# expected-error@-1 {{unknown directive}} + +## Matches any diagnostic starting with "unknown" and ending with +## "directive"; only the nested {{.*}} is a regex, the surrounding text +## is literal. +.foo +# expected-error-re@-1 {{unknown {{.*}} directive}} +``` + +A directive must appear within a comment recognized by the target being +assembled for (its own comment string, plus `//` and `#` if the target +allows additional comment forms); text that merely resembles a directive +elsewhere on the line is ignored. Directives are matched line-based, so they +are not recognized inside multi-line `/* ... */` block comments. Use a +single-line comment form instead. + +Under `--verify`, the process exit code reflects whether the diagnostics +matched what was expected, not whether assembling/disassembling itself +produced an error: a fully expected error is a successful (exit code 0) run. +`--verify` is not supported together with `--as-lex`, since `--as-lex` +does not report its errors as diagnostics with a message or location. + +::: + ## EXIT STATUS If {program}`llvm-mc` succeeds, it will exit with 0. Otherwise, if an error diff --git a/llvm/docs/TestingGuide.md b/llvm/docs/TestingGuide.md index 77803c4443da2..b344a688b17a2 100644 --- a/llvm/docs/TestingGuide.md +++ b/llvm/docs/TestingGuide.md @@ -561,6 +561,17 @@ This test will fail if placed into a `download` directory. To make your tests robust, always use `opt ... < %s` in the `RUN` line. {program}`opt` does not output a `ModuleID` when input comes from stdin. +Another common source of fragile tests is checking assembler diagnostics with +plain `FileCheck`, e.g. `RUN: not llvm-mc ... | FileCheck %s` with unanchored +`CHECK: error:` lines. `FileCheck` only asserts that the listed diagnostics +appear somewhere, in order; it does not notice an unrelated *extra* +diagnostic that a later change starts emitting next to the one under test. +For `llvm-mc` tests, prefer `--verify` (see the +{doc}`llvm-mc <CommandGuide/llvm-mc>` documentation), which checks that every +diagnostic produced matches an `expected-error`/`expected-warning`/ +`expected-note`/`expected-remark` comment in the input and vice versa, +similar to Clang's `-verify` flag. + ### Platform-Specific Tests Whenever adding tests that require the knowledge of a specific platform, diff --git a/llvm/test/tools/llvm-mc/verify/basic.s b/llvm/test/tools/llvm-mc/verify/basic.s new file mode 100644 index 0000000000000..ce4b717c91132 --- /dev/null +++ b/llvm/test/tools/llvm-mc/verify/basic.s @@ -0,0 +1,112 @@ +# Tests for llvm-mc's -verify=<prefixes> diagnostic-verification mode. + +# REQUIRES: riscv-registered-target +# RUN: rm -rf %t && split-file %s %t + +## Matching expected-error directives: a plain substring match, a '-re' +## regex match (note the nested {{...}} for the regex portion itself), and +## an '@+1' forward offset all pass. +# RUN: llvm-mc -triple riscv32 -verify %t/match.s + +## A custom '-verify=<prefix>' prefix is recognized instead of 'expected'. +# RUN: llvm-mc -triple riscv32 -verify=check %t/match-custom-prefix.s + +## A diagnostic whose text matches an expected-* comment but whose kind +## doesn't (e.g. expected-warning for an actual error) is a "near miss": it +## still counts as not produced, since it wasn't a full match. +# RUN: not llvm-mc -triple riscv32 -verify %t/near-miss.s 2>&1 \ +# RUN: | FileCheck %t/near-miss.s --check-prefix=NEAR-MISS + +## A diagnostic with no matching expected-* comment fails verification. +# RUN: not llvm-mc -triple riscv32 -verify %t/unexpected.s 2>&1 \ +# RUN: | FileCheck %t/unexpected.s --check-prefix=UNEXPECTED + +## An expected-error that is never produced fails verification. +# RUN: not llvm-mc -triple riscv32 -verify %t/missing.s 2>&1 \ +# RUN: | FileCheck %t/missing.s --check-prefix=MISSING + +## An expected-error in a '.include'd file that's never produced also fails +## verification: the included file's buffer isn't known to SourceMgr until +## it's actually .include'd during parsing, so process() never has an actual +## diagnostic in it to trigger scanning it. This must be caught by verify() +## re-scanning all of SourceMgr's buffers, not just the ones process() saw. +# RUN: not llvm-mc -triple riscv32 -verify -I %t %t/main-include.s 2>&1 \ +# RUN: | FileCheck %t/included-missing.s --check-prefix=INCLUDE-MISSING + +## A directive-looking magic string outside of a comment is not treated as a +## directive, so the resulting diagnostic is still unexpected. +# RUN: not llvm-mc -triple riscv32 -verify %t/outside-comment.s 2>&1 \ +# RUN: | FileCheck %t/outside-comment.s --check-prefix=UNEXPECTED + +## A file with no diagnostics and no expected-* comments passes. Unlike +## clang's -verify, there is no dedicated 'expected-no-diagnostics' marker: +## none is needed, since if nothing is expected and nothing happens, +## verification trivially succeeds, and any diagnostic that *does* occur is +## still caught as unexpected (see the 'unexpected' case above). +# RUN: llvm-mc -triple riscv32 -verify %t/no-diagnostics.s + +## Diagnostics reported through MCContext (e.g. by the DWARF CFI checker, +## which doesn't go through SourceMgr's diagnostic handler) are also +## checked, not just diagnostics from the assembler/parser. +# RUN: llvm-mc -triple riscv32 -verify -validate-cfi -filetype=null %t/cfi.s + +## -verify is rejected together with -as-lex, since -as-lex doesn't report +## its errors as diagnostics with a message/location at all. +# RUN: not llvm-mc -triple riscv32 -verify -as-lex %t/no-diagnostics.s 2>&1 \ +# RUN: | FileCheck %s --check-prefix=AS-LEX-ERR +# AS-LEX-ERR: -verify is not supported with -as-lex + +#--- match.s +.foo_directive +# expected-error@-1 {{unknown directive}} + +.bar_directive +# expected-error-re@-1 {{unknown {{.*}}directive}} + +# expected-error@+1 {{unknown directive}} +.baz_directive + +#--- match-custom-prefix.s +.foo_directive +# check-error@-1 {{unknown directive}} + +#--- near-miss.s +.foo_directive +# expected-warning@-1 {{unknown directive}} +# NEAR-MISS: 'error' diagnostic emitted when expecting a 'warning' +# NEAR-MISS: expected warning "unknown directive" was not produced + +#--- unexpected.s +.foo_directive +# UNEXPECTED: error: unknown directive + +#--- missing.s +addi a0, a0, 1 +# expected-error@-1 {{this will never happen}} +# MISSING: expected error "this will never happen" was not produced + +#--- main-include.s +.include "included-missing.s" + +#--- included-missing.s +addi a0, a0, 1 +# expected-error@-1 {{this will never happen}} +# INCLUDE-MISSING: expected error "this will never happen" was not produced + +#--- outside-comment.s +.foo_directive expected-error {{unknown directive}} +# UNEXPECTED: error: unknown directive + +#--- no-diagnostics.s +addi a0, a0, 1 + +#--- cfi.s + .text + .globl f + .type f,@function +f: + .cfi_startproc + .cfi_same_value ra + li a0, 10 + # expected-error@-1 {{changed register X10, that register X10's unwinding rule uses, but there is no CFI directives about it}} + .cfi_endproc diff --git a/llvm/tools/llvm-mc/CMakeLists.txt b/llvm/tools/llvm-mc/CMakeLists.txt index 67ac890966fa6..bf7f217bc13c1 100644 --- a/llvm/tools/llvm-mc/CMakeLists.txt +++ b/llvm/tools/llvm-mc/CMakeLists.txt @@ -13,5 +13,6 @@ set(LLVM_LINK_COMPONENTS add_llvm_tool(llvm-mc llvm-mc.cpp Disassembler.cpp + Verifier.cpp ) diff --git a/llvm/tools/llvm-mc/Verifier.cpp b/llvm/tools/llvm-mc/Verifier.cpp new file mode 100644 index 0000000000000..19de8d7f785cd --- /dev/null +++ b/llvm/tools/llvm-mc/Verifier.cpp @@ -0,0 +1,97 @@ +//===- Verifier.cpp - Diagnostic Verifier for llvm-mc ---------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Verifier.h" +#include "llvm/MC/MCContext.h" +#include "llvm/Support/SourceMgr.h" +#include "llvm/Support/raw_ostream.h" + +using namespace llvm; + +namespace { +/// SourceMgr::PrintMessage unconditionally re-invokes SrcMgr's installed +/// DiagHandler if one is set, rather than only doing so for diagnostics that +/// didn't originate from the handler itself. Since MCVerifier is installed as +/// that handler, and its own printing (near-miss / not-produced messages) +/// goes through SourceMgr::PrintMessage, calling into it from inside the +/// handler would otherwise re-enter the handler and recurse indefinitely. +/// This temporarily clears the handler for the duration of the guard so that +/// PrintMessage prints directly instead. +class ScopedHandlerDisable { + SourceMgr &SrcMgr; + SourceMgr::DiagHandlerTy SavedHandler; + void *SavedContext; + +public: + explicit ScopedHandlerDisable(SourceMgr &SrcMgr) + : SrcMgr(SrcMgr), SavedHandler(SrcMgr.getDiagHandler()), + SavedContext(SrcMgr.getDiagContext()) { + SrcMgr.setDiagHandler(nullptr, nullptr); + } + ~ScopedHandlerDisable() { SrcMgr.setDiagHandler(SavedHandler, SavedContext); } +}; +} // namespace + +MCVerifier::MCVerifier(SourceMgr &SrcMgr, ArrayRef<std::string> Prefixes, + ArrayRef<std::string> CommentPrefixes) + : SrcMgr(SrcMgr), Verifier(Prefixes, CommentPrefixes) { + // Scan every buffer already known to SrcMgr up front, so that an expected + // diagnostic that's never actually produced (e.g. because the run never + // reports any diagnostic at all) is still recorded and reported as missing + // by verify(), rather than never being noticed since process() was never + // called for that buffer. Guarded like every other call into Verifier in + // this file: a malformed directive (e.g. an invalid '-re' regex) makes + // computeExpectedDiags print through SrcMgr, which would otherwise re-enter + // installHandlers's handler if the constructor ever ran after it (it + // currently never does, at the sole call site in llvm-mc.cpp). + ScopedHandlerDisable Guard(SrcMgr); + for (unsigned I = 0, E = SrcMgr.getNumBuffers(); I != E; ++I) + (void)Verifier.computeExpectedDiags(errs(), SrcMgr, + SrcMgr.getMemoryBuffer(I + 1)); +} + +MCVerifier::~MCVerifier() { + // SrcMgr may outlive this MCVerifier; don't leave its handler pointing at + // freed memory. + if (SrcMgr.getDiagHandler() == &MCVerifier::handleSourceMgrDiag) + SrcMgr.setDiagHandler(nullptr, nullptr); +} + +void MCVerifier::installHandlers(MCContext &Ctx) { + SrcMgr.setDiagHandler(&MCVerifier::handleSourceMgrDiag, this); + Ctx.setDiagnosticHandler( + [this](const SMDiagnostic &Diag, bool /*InlineAsm*/, + const SourceMgr & /*SrcMgr*/, + std::vector<const MDNode *> & /*LocInfos*/) { process(Diag); }); +} + +bool MCVerifier::verify() { + ScopedHandlerDisable Guard(SrcMgr); + return Verifier.verify(errs(), SrcMgr); +} + +void MCVerifier::handleSourceMgrDiag(const SMDiagnostic &Diag, void *Context) { + static_cast<MCVerifier *>(Context)->process(Diag); +} + +void MCVerifier::process(const SMDiagnostic &Diag) { + ScopedHandlerDisable Guard(SrcMgr); + + bool HasLoc = Diag.getLoc().isValid(); + const MemoryBuffer *Buf = nullptr; + if (HasLoc) { + if (unsigned ID = SrcMgr.FindBufferContainingLoc(Diag.getLoc())) + Buf = SrcMgr.getMemoryBuffer(ID); + } + + auto Result = Verifier.process( + errs(), SrcMgr, Diag.getKind(), HasLoc, Buf, + HasLoc ? static_cast<unsigned>(Diag.getLineNo()) : 0, Diag.getMessage()); + if (Result == SourceMgrDiagnosticVerifier::MatchResult::Unexpected) + SrcMgr.PrintMessage(errs(), Diag); +} diff --git a/llvm/tools/llvm-mc/Verifier.h b/llvm/tools/llvm-mc/Verifier.h new file mode 100644 index 0000000000000..d567141ef0a0c --- /dev/null +++ b/llvm/tools/llvm-mc/Verifier.h @@ -0,0 +1,64 @@ +//===- Verifier.h - Diagnostic Verifier for llvm-mc -----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This implements '-verify=<prefixes>', a mode that checks emitted +// diagnostics against '<prefix>-error'/'-warning'/'-note'/'-remark' comments +// in the input, similar to clang's '-verify' flag. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_TOOLS_LLVM_MC_VERIFIER_H +#define LLVM_TOOLS_LLVM_MC_VERIFIER_H + +#include "llvm/ADT/ArrayRef.h" +#include "llvm/Support/SourceMgrDiagnosticVerifier.h" +#include <string> + +namespace llvm { + +class MCContext; +class SourceMgr; +class SMDiagnostic; + +/// Checks the diagnostics produced while assembling/disassembling against +/// '<prefix>-<kind>' comments in the input. +class MCVerifier { +public: + /// \p Prefixes are the accepted directive prefixes (from '-verify=...'). + /// \p CommentPrefixes are the comment-start strings recognized by the + /// current target, so a directive outside of a comment is ignored. + MCVerifier(SourceMgr &SrcMgr, ArrayRef<std::string> Prefixes, + ArrayRef<std::string> CommentPrefixes); + + /// Clears SrcMgr's diagnostic handler if it's still this object's, so a + /// SourceMgr that outlives this MCVerifier is never left with a dangling + /// handler pointing at freed memory. + ~MCVerifier(); + + /// Installs this as the diagnostic handler for both the SourceMgr passed to + /// the constructor and \p Ctx. Both are needed: most parser/disassembler + /// diagnostics go through SourceMgr, but some (e.g. the DWARF CFI checker's, + /// via MCContext::reportError/reportWarning) are reported directly through + /// MCContext and never reach SourceMgr's handler at all. + void installHandlers(MCContext &Ctx); + + /// Reports any expected diagnostic that was never produced, and returns + /// whether verification succeeded overall. + bool verify(); + +private: + static void handleSourceMgrDiag(const SMDiagnostic &Diag, void *Context); + void process(const SMDiagnostic &Diag); + + SourceMgr &SrcMgr; + SourceMgrDiagnosticVerifier Verifier; +}; + +} // namespace llvm + +#endif // LLVM_TOOLS_LLVM_MC_VERIFIER_H diff --git a/llvm/tools/llvm-mc/llvm-mc.cpp b/llvm/tools/llvm-mc/llvm-mc.cpp index 01d42f07e337d..0d9ded02cd159 100644 --- a/llvm/tools/llvm-mc/llvm-mc.cpp +++ b/llvm/tools/llvm-mc/llvm-mc.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "Disassembler.h" +#include "Verifier.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/DWARFCFIChecker/DWARFCFIFunctionFrameAnalyzer.h" #include "llvm/DWARFCFIChecker/DWARFCFIFunctionFrameStreamer.h" @@ -223,6 +224,15 @@ static cl::opt<bool> ValidateCFI("validate-cfi", cl::desc("Validate the CFI directives"), cl::cat(MCCategory)); +static cl::list<std::string> Verify( + "verify", cl::CommaSeparated, cl::ValueOptional, + cl::desc( + "Check the diagnostics produced against '<prefix>-error'/" + "'-warning'/'-note'/'-remark' comments in the input instead of just " + "printing them, using a comma-separated list of <prefixes> " + "(default: 'expected') as the recognized directive prefixes"), + cl::value_desc("prefixes"), cl::cat(MCCategory)); + enum ActionType { AC_AsLex, AC_Assemble, @@ -420,6 +430,14 @@ int main(int argc, char **argv) { setDwarfDebugProducer(); const char *ProgName = argv[0]; + + bool VerifyEnabled = Verify.getNumOccurrences() > 0; + if (VerifyEnabled && Action == AC_AsLex) { + WithColor::error(errs(), ProgName) + << "-verify is not supported with -as-lex\n"; + return 1; + } + const Target *TheTarget = GetTarget(ProgName); if (!TheTarget) return 1; @@ -464,6 +482,37 @@ int main(int argc, char **argv) { MAI->setPreserveAsmComments(PreserveComments); MAI->setCommentColumn(CommentColumn); + std::unique_ptr<MCVerifier> Verifier; + if (VerifyEnabled) { + std::vector<std::string> Prefixes; + for (StringRef Prefix : Verify) + if (!Prefix.empty()) + Prefixes.push_back(Prefix.str()); + // A bare '-verify' (no '=value'), an explicit empty prefix list, and a + // list that parses to only empty strings (e.g. a stray comma) all mean + // "use the default prefix". + if (Prefixes.empty()) + Prefixes = {"expected"}; + + // Only text at or after a recognized comment marker is scanned for + // directives, since the comment character varies by target and .s + // source is more likely than e.g. MLIR IR to contain incidental text + // that happens to match a directive. Deliberately excludes '/*': the + // scan is line-based, so it cannot correctly recognize a directive on a + // continuation line of a multi-line block comment (nor correctly stop + // treating text as commented once a block comment closes with '*/' on + // the same line). Directives must use a single-line comment form. + std::vector<std::string> CommentPrefixes = { + std::string(MAI->getCommentString())}; + if (MAI->shouldAllowAdditionalComments()) { + CommentPrefixes.push_back("//"); + CommentPrefixes.push_back("#"); + } + + Verifier = + std::make_unique<MCVerifier>(SrcMgr, Prefixes, CommentPrefixes); + } + // Package up features to be passed to target/subtarget SubtargetFeatures Features; std::string FeaturesStr; @@ -496,6 +545,9 @@ int main(int argc, char **argv) { TheTarget->createMCObjectFileInfo(Ctx, PIC, LargeCodeModel)); Ctx.setObjectFileInfo(MOFI.get()); + if (Verifier) + Verifier->installHandlers(Ctx); + Ctx.setGenDwarfForAssembly(GenDwarfForAssembly); // Default to 4 for dwarf version. unsigned DwarfVersion = MCOptions.DwarfVersion ? MCOptions.DwarfVersion : 4; @@ -674,6 +726,12 @@ int main(int argc, char **argv) { Res = Disassembler::disassemble(*TheTarget, *STI, *Str, *Buffer, SrcMgr, Ctx, HexBytes, NumBenchmarkRuns); + // Under -verify, the exit code reflects whether the diagnostics matched + // what was expected, not whether assembling/disassembling itself hit an + // error: an expected error is a successful run. + if (Verifier) + Res = Verifier->verify() ? 0 : 1; + // Keep output if no errors. if (Res == 0) { Out->keep(); _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
