https://github.com/delcypher updated https://github.com/llvm/llvm-project/pull/154618
>From db45694078e74e2c080e40dd8ba635d2c9f313c6 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Tue, 19 Aug 2025 15:08:44 -0700 Subject: [PATCH 01/11] [UBSan][BoundsSafety] Implement support for more expressive "trap reasons" In 29992cfd628ed5b968ccb73b17ed0521382ba317 (#145967) support was added for "trap reasons" on traps emitted in UBSan in trapping mode (e.g. `-fsanitize-trap=undefined`). This improved the debugging experience by attaching the reason for trapping as a string on the debug info on trap instructions. Consumers such as LLDB can display this trap reason string when the trap is reached. A limitation of that patch is that the trap reason string is hard-coded for each `SanitizerKind` even though the compiler actually has much more information about the trap available at compile time that could be shown to the user. This patch is an incremental step in fixing that. It consists of two main steps. **1. Introduce infrastructure for building trap reason strings ** To make it convenient to construct trap reason strings this patch re-uses Clang's powerful diagnostic infrastructure to provide a convenient API for constructing trap reason strings. This is achieved by: * Introducing a new `Trap` diagnostic kind to represent trap diagnostics in TableGen files. * Adding a new `CodeGen` diagnostic component. While this part probably isn't technically necessary it seemed like I should follow the existing convention used by the diagnostic system. * Adding `DiagnosticCodeGenKinds.td` to describe the different trap reasons. * Add the `RuntimeTrapDiagnosticBuilder` class to provide an interface for constructing trap reason strings and the trap category. Note this API while similar to `DiagnosticBuilder` has different semantics which are described in the code comments. In particular the behavior when the destructor is called is very different. * Adding `CodeGenModule::RuntimeDiag()` as a convenient constructor for the `RuntimeTrapDiagnosticBuilder`. This use of the diagnostic system is a little unusual in that the emitted trap diagnostics aren't actually consumed by normal diagnostic consumers (e.g. the console). Instead the `RuntimeTrapDiagnosticBuilder` is just used to format a string, so in effect the builder is somewhat analagous to "printf". However, re-using the diagnostics system in this way brings a several benefits: * The powerful diagnostic templating languge (e.g. `%select`) can be used. * Formatting Clang data types (e.g. `Type`, `Expr`, etc.) just work out-of-the-box. * Describing trap reasons in tablegen files opens the door for translation to different languages in the future. * The `RuntimeTrapDiagnosticBuilder` API is very similar to `DiagnosticBuilder` which makes it easy to use by anyone already familiar with Clang's diagnostic system. While UBSan is the first consumer of this new infrastructure the intent is to use this to overhaul how trap reasons are implemented in the `-fbounds-safety` implementation (currently exists downstream). **2. Apply the new infrastructure to UBSan checks for arithmetic overflow** To demonstrate using `RuntimeTrapDiagnosticBuilder` this patch applies it to UBSan traps for arithmetic overflow. The intention is that we would iteratively switch to using the `RuntimeTrapDiagnosticBuilder` for all UBSan traps where it makes sense in future patches. Previously for code like ``` int test(int a, int b) { return a + b; } ``` The trap reason string looked like ``` Undefined Behavior Sanitizer: Integer addition overflowed ``` now the trap message looks like: ``` Undefined Behavior Sanitizer: signed integer addition overflow in 'a + b' ``` This string is much more specific because * It explains if signed or unsigned overflow occurred * It actually shows the expression that overflowed This seems a lot more helpful. One possible downside of this approach is it may blow up Debug info size because now there can be many more distinct trap reason strings. If this is a concern we may want to add a flag to make it possible to continue to use the original hard-coded trap messages to avoid increasing the size of Debug info. rdar://158612755 --- .../clang/Basic/AllDiagnosticKinds.inc | 1 + clang/include/clang/Basic/AllDiagnostics.h | 1 + clang/include/clang/Basic/CMakeLists.txt | 1 + clang/include/clang/Basic/Diagnostic.h | 67 +++++++++++++++++++ clang/include/clang/Basic/Diagnostic.td | 5 +- clang/include/clang/Basic/DiagnosticIDs.h | 11 ++- clang/include/clang/Basic/DiagnosticTrap.h | 15 +++++ .../clang/Basic/DiagnosticTrapKinds.td | 26 +++++++ clang/lib/Basic/Diagnostic.cpp | 31 +++++++++ clang/lib/Basic/DiagnosticIDs.cpp | 4 ++ clang/lib/CodeGen/CGExpr.cpp | 18 +++-- clang/lib/CodeGen/CGExprScalar.cpp | 35 ++++++++-- clang/lib/CodeGen/CodeGenFunction.h | 6 +- clang/lib/CodeGen/CodeGenModule.h | 7 ++ .../Generic/ubsan-trap-reason-add-overflow.c | 18 ++++- .../Generic/ubsan-trap-reason-flag.c | 2 +- .../Generic/ubsan-trap-reason-mul-overflow.c | 17 ++++- .../Generic/ubsan-trap-reason-sub-overflow.c | 17 ++++- .../Shell/Recognizer/ubsan_add_overflow.test | 4 +- 19 files changed, 259 insertions(+), 27 deletions(-) create mode 100644 clang/include/clang/Basic/DiagnosticTrap.h create mode 100644 clang/include/clang/Basic/DiagnosticTrapKinds.td diff --git a/clang/include/clang/Basic/AllDiagnosticKinds.inc b/clang/include/clang/Basic/AllDiagnosticKinds.inc index a946b4a640ac6..2d08bb0525970 100644 --- a/clang/include/clang/Basic/AllDiagnosticKinds.inc +++ b/clang/include/clang/Basic/AllDiagnosticKinds.inc @@ -30,4 +30,5 @@ #include "clang/Basic/DiagnosticAnalysisKinds.inc" #include "clang/Basic/DiagnosticRefactoringKinds.inc" #include "clang/Basic/DiagnosticInstallAPIKinds.inc" +#include "clang/Basic/DiagnosticTrapKinds.inc" // clang-format on diff --git a/clang/include/clang/Basic/AllDiagnostics.h b/clang/include/clang/Basic/AllDiagnostics.h index 3b782732c1507..78e5428ddbfff 100644 --- a/clang/include/clang/Basic/AllDiagnostics.h +++ b/clang/include/clang/Basic/AllDiagnostics.h @@ -26,6 +26,7 @@ #include "clang/Basic/DiagnosticRefactoring.h" #include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/DiagnosticSerialization.h" +#include "clang/Basic/DiagnosticTrap.h" namespace clang { template <size_t SizeOfStr, typename FieldType> class StringSizerHelper { diff --git a/clang/include/clang/Basic/CMakeLists.txt b/clang/include/clang/Basic/CMakeLists.txt index 0cf661a57dfa8..81736006a21a0 100644 --- a/clang/include/clang/Basic/CMakeLists.txt +++ b/clang/include/clang/Basic/CMakeLists.txt @@ -33,6 +33,7 @@ clang_diag_gen(Parse) clang_diag_gen(Refactoring) clang_diag_gen(Sema) clang_diag_gen(Serialization) +clang_diag_gen(Trap) clang_tablegen(DiagnosticGroups.inc -gen-clang-diag-groups SOURCE Diagnostic.td TARGET ClangDiagnosticGroups) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index cee5bed665d0a..92e8e9101e9db 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -1233,6 +1233,7 @@ class DiagnosticBuilder : public StreamingDiagnostic { friend class DiagnosticsEngine; friend class PartialDiagnostic; friend class Diagnostic; + friend class RuntimeTrapDiagnosticBuilder; mutable DiagnosticsEngine *DiagObj = nullptr; @@ -1534,6 +1535,72 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) { return Report(SourceLocation(), DiagID); } +//===----------------------------------------------------------------------===// +// RuntimeTrapDiagnosticBuilder +//===----------------------------------------------------------------------===// +/// Class to make it convenient to construct "trap reasons" to attach to trap +/// instructions. +/// +/// Although this class inherits from `DiagnosticBuilder` it has very different +/// semantics. +/// +/// * This class should only be used with trap diagnostics (declared in +/// `DiagnosticTrapKinds.td`). +/// * The `RuntimeTrapDiagnosticBuilder` does not emit diagnostics to the normal +/// diagnostics consumers on destruction like normal Diagnostic builders. +/// Instead it does nothing on destruction. +/// * Users of this class that want to retrieve the "trap reason" should call +/// call the `getMessage()` and `getCategory()` and use those results before +/// the builder is destroyed. +/// * Unlike the `DiagnosticBuilder` the `RuntimeDiagnosticBuilder` should never +/// be created as a temporary (i.e. rvalue) and instead should be stored. This +/// is because the class is only useful if `getMessage()` and `getCategory()` +/// can be called. +/// +/// Given that this class inherits from `DiagnosticBuilder` it inherits all of +/// its abilities to format diagnostic messages and consume various types in +/// class (e.g. Type, Exprs, etc.). This makes it particularly suited to +/// printing types and expressions from the AST while codegen-ing runtime +/// checks. +/// +/// +/// Example use via the `CodeGenModule::RuntimeDiag` helper. +/// +/// \code +/// { +/// auto* RTDB = CGM.RuntimeDiag(diag::trap_diagnostic); +/// *RTDB << 0 << SomeExpr << SomeType; +/// consume(RTDB->getCategory(), RTDB->getMessage()); +/// } +/// \endcode +/// +/// +class RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { +public: + RuntimeTrapDiagnosticBuilder(DiagnosticsEngine *DiagObj, unsigned DiagID); + ~RuntimeTrapDiagnosticBuilder(); + + // Prevent accidentally copying or assigning + RuntimeTrapDiagnosticBuilder & + operator=(const RuntimeTrapDiagnosticBuilder &) = delete; + RuntimeTrapDiagnosticBuilder & + operator=(const RuntimeTrapDiagnosticBuilder &&) = delete; + RuntimeTrapDiagnosticBuilder(const RuntimeTrapDiagnosticBuilder &) = delete; + RuntimeTrapDiagnosticBuilder(const RuntimeTrapDiagnosticBuilder &&) = delete; + + /// \return Format the trap message and return it. Note the lifetime of + /// the underlying storage pointed to by the returned StringRef is the same + /// as the lifetime of this class. This means it is likely unsafe to store + /// the returned StringRef. + StringRef getMessage(); + /// \return Return the trap category. These are the `CategoryName` property + /// of `trap` diagnostics declared in `DiagnosticTrapKinds.td`. + StringRef getCategory(); + +private: + llvm::SmallVector<char, 64> MessageStorage; +}; + //===----------------------------------------------------------------------===// // Diagnostic //===----------------------------------------------------------------------===// diff --git a/clang/include/clang/Basic/Diagnostic.td b/clang/include/clang/Basic/Diagnostic.td index 65b19f3feea4f..d190c70a401e3 100644 --- a/clang/include/clang/Basic/Diagnostic.td +++ b/clang/include/clang/Basic/Diagnostic.td @@ -30,6 +30,7 @@ def CLASS_REMARK : DiagClass; def CLASS_WARNING : DiagClass; def CLASS_EXTENSION : DiagClass; def CLASS_ERROR : DiagClass; +def CLASS_TRAP : DiagClass; // Responses to a diagnostic in a SFINAE context. class SFINAEResponse; @@ -144,7 +145,8 @@ class Extension<string str> : Diagnostic<str, CLASS_EXTENSION, SEV_Ignored>; class ExtWarn<string str> : Diagnostic<str, CLASS_EXTENSION, SEV_Warning>; // Notes can provide supplementary information on errors, warnings, and remarks. class Note<string str> : Diagnostic<str, CLASS_NOTE, SEV_Fatal/*ignored*/>; - +// Traps messages attached to traps in debug info +class Trap<string str> : Diagnostic<str, CLASS_TRAP, SEV_Fatal/*ignored*/>; class DefaultIgnore { Severity DefaultSeverity = SEV_Ignored; } class DefaultWarn { Severity DefaultSeverity = SEV_Warning; } @@ -235,3 +237,4 @@ include "DiagnosticParseKinds.td" include "DiagnosticRefactoringKinds.td" include "DiagnosticSemaKinds.td" include "DiagnosticSerializationKinds.td" +include "DiagnosticTrapKinds.td" diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h index 17fecd346f03e..a8c00fff0d9c5 100644 --- a/clang/include/clang/Basic/DiagnosticIDs.h +++ b/clang/include/clang/Basic/DiagnosticIDs.h @@ -47,6 +47,7 @@ enum { DIAG_SIZE_ANALYSIS = 100, DIAG_SIZE_REFACTORING = 1000, DIAG_SIZE_INSTALLAPI = 100, + DIAG_SIZE_TRAP = 100, }; // Start position for diagnostics. // clang-format off @@ -64,7 +65,8 @@ enum { DIAG_START_ANALYSIS = DIAG_START_SEMA + static_cast<int>(DIAG_SIZE_SEMA), DIAG_START_REFACTORING = DIAG_START_ANALYSIS + static_cast<int>(DIAG_SIZE_ANALYSIS), DIAG_START_INSTALLAPI = DIAG_START_REFACTORING + static_cast<int>(DIAG_SIZE_REFACTORING), - DIAG_UPPER_LIMIT = DIAG_START_INSTALLAPI + static_cast<int>(DIAG_SIZE_INSTALLAPI) + DIAG_START_TRAP = DIAG_START_INSTALLAPI + static_cast<int>(DIAG_SIZE_INSTALLAPI), + DIAG_UPPER_LIMIT = DIAG_START_TRAP + static_cast<int>(DIAG_SIZE_TRAP) }; // clang-format on @@ -189,7 +191,8 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> { CLASS_REMARK = 0x02, CLASS_WARNING = 0x03, CLASS_EXTENSION = 0x04, - CLASS_ERROR = 0x05 + CLASS_ERROR = 0x05, + CLASS_TRAP = 0x06 }; static bool IsCustomDiag(diag::kind Diag) { @@ -363,6 +366,10 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> { /// bool isExtensionDiag(unsigned DiagID, bool &EnabledByDefault) const; + bool isTrapDiag(unsigned DiagID) const { + return getDiagClass(DiagID) == CLASS_TRAP; + } + /// Given a group ID, returns the flag that toggles the group. /// For example, for Group::DeprecatedDeclarations, returns /// "deprecated-declarations". diff --git a/clang/include/clang/Basic/DiagnosticTrap.h b/clang/include/clang/Basic/DiagnosticTrap.h new file mode 100644 index 0000000000000..7c6a5dc5d40d7 --- /dev/null +++ b/clang/include/clang/Basic/DiagnosticTrap.h @@ -0,0 +1,15 @@ +//===--- DiagnosticTrap.h - Diagnostics for trap instructions ---*- 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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_BASIC_TRAP_H +#define LLVM_CLANG_BASIC_TRAP_H + +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticTrapInterface.inc" + +#endif diff --git a/clang/include/clang/Basic/DiagnosticTrapKinds.td b/clang/include/clang/Basic/DiagnosticTrapKinds.td new file mode 100644 index 0000000000000..ce5b16747071e --- /dev/null +++ b/clang/include/clang/Basic/DiagnosticTrapKinds.td @@ -0,0 +1,26 @@ +//==--- DiagnosticTrapKinds.td - CodeGen Diagnostics -------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +//===----------------------------------------------------------------------===// +// Trap Diagnostics +// +// These are diagnostics that are emitted into Debug Info, rather than to the +// traditional consumers like the terminal. Their primary purpose is to make +// debugging traps (e.g. `-fsanitize-trap=undefined`) easier by attaching +// a trap category and reason to the trap instruction that tools like a debugger +// can show. +//===----------------------------------------------------------------------===// +let Component = "Trap" in { +let CategoryName = "Undefined Behavior Sanitizer" in { + +def trap_ubsan_arith_overflow : Trap< + "%select{unsigned|signed}0 integer " + "%select{addition|subtraction|multiplication}1 overflow in %2">; + +} +} diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index e33e843db6a44..75729d5c8fd9e 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -664,6 +664,8 @@ void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) { void DiagnosticsEngine::Report(Level DiagLevel, const Diagnostic &Info) { assert(DiagLevel != Ignored && "Cannot emit ignored diagnostics!"); + assert(!getDiagnosticIDs()->isTrapDiag(Info.getID()) && + "Trap diagnostics should not be consumed by the DiagnosticsEngine"); Client->HandleDiagnostic(DiagLevel, Info); if (Client->IncludeInDiagnosticCounts()) { if (DiagLevel == Warning) @@ -793,6 +795,35 @@ DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D) D.Clear(); } +RuntimeTrapDiagnosticBuilder::RuntimeTrapDiagnosticBuilder( + DiagnosticsEngine *DiagObj, unsigned DiagID) + : DiagnosticBuilder(DiagObj, SourceLocation(), DiagID) { + assert(DiagObj->getDiagnosticIDs()->isTrapDiag(DiagID)); +} + +RuntimeTrapDiagnosticBuilder::~RuntimeTrapDiagnosticBuilder() { + // Make sure that when `DiagnosticBuilder::~DiagnosticBuilder()` + // calls `Emit()` that it does nothing. + Clear(); +} + +StringRef RuntimeTrapDiagnosticBuilder::getMessage() { + if (MessageStorage.size() == 0) { + // Render the Diagnostic + Diagnostic Info(DiagObj, *this); + Info.FormatDiagnostic(MessageStorage); + } + return StringRef(MessageStorage.data(), MessageStorage.size()); +} + +StringRef RuntimeTrapDiagnosticBuilder::getCategory() { + auto CategoryID = + DiagObj->getDiagnosticIDs()->getCategoryNumberForDiag(DiagID); + if (CategoryID == 0) + return ""; + return DiagObj->getDiagnosticIDs()->getCategoryNameFromID(CategoryID); +} + Diagnostic::Diagnostic(const DiagnosticsEngine *DO, const DiagnosticBuilder &DiagBuilder) : DiagObj(DO), DiagLoc(DiagBuilder.DiagLoc), DiagID(DiagBuilder.DiagID), diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index 73f24a82d4c75..a1d9d0f34d20d 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -69,6 +69,7 @@ enum DiagnosticClass { CLASS_WARNING = DiagnosticIDs::CLASS_WARNING, CLASS_EXTENSION = DiagnosticIDs::CLASS_EXTENSION, CLASS_ERROR = DiagnosticIDs::CLASS_ERROR, + CLASS_TRAP = DiagnosticIDs::CLASS_TRAP, }; struct StaticDiagInfoRec { @@ -139,6 +140,7 @@ VALIDATE_DIAG_SIZE(SEMA) VALIDATE_DIAG_SIZE(ANALYSIS) VALIDATE_DIAG_SIZE(REFACTORING) VALIDATE_DIAG_SIZE(INSTALLAPI) +VALIDATE_DIAG_SIZE(TRAP) #undef VALIDATE_DIAG_SIZE #undef STRINGIFY_NAME @@ -171,6 +173,7 @@ const StaticDiagInfoRec StaticDiagInfo[] = { #include "clang/Basic/DiagnosticAnalysisKinds.inc" #include "clang/Basic/DiagnosticRefactoringKinds.inc" #include "clang/Basic/DiagnosticInstallAPIKinds.inc" +#include "clang/Basic/DiagnosticTrapKinds.inc" // clang-format on #undef DIAG }; @@ -214,6 +217,7 @@ CATEGORY(SEMA, CROSSTU) CATEGORY(ANALYSIS, SEMA) CATEGORY(REFACTORING, ANALYSIS) CATEGORY(INSTALLAPI, REFACTORING) +CATEGORY(TRAP, INSTALLAPI) #undef CATEGORY // Avoid out of bounds reads. diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 2329fa20a2530..1e8c321c88d2d 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -33,6 +33,7 @@ #include "clang/AST/StmtVisitor.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/DiagnosticParse.h" #include "clang/Basic/Module.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/STLExtras.h" @@ -3729,7 +3730,8 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF, void CodeGenFunction::EmitCheck( ArrayRef<std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>> Checked, SanitizerHandler CheckHandler, ArrayRef<llvm::Constant *> StaticArgs, - ArrayRef<llvm::Value *> DynamicArgs) { + ArrayRef<llvm::Value *> DynamicArgs, + std::unique_ptr<RuntimeTrapDiagnosticBuilder> RTDB) { assert(IsSanitizerScope); assert(Checked.size() > 0); assert(CheckHandler >= 0 && @@ -3768,7 +3770,9 @@ void CodeGenFunction::EmitCheck( } if (TrapCond) - EmitTrapCheck(TrapCond, CheckHandler, NoMerge); + EmitTrapCheck(TrapCond, CheckHandler, NoMerge, + RTDB ? RTDB->getMessage() : "", + RTDB ? RTDB->getCategory() : ""); if (!FatalCond && !RecoverableCond) return; @@ -4080,7 +4084,8 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) { void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID, - bool NoMerge) { + bool NoMerge, StringRef Message, + StringRef Category) { llvm::BasicBlock *Cont = createBasicBlock("cont"); // If we're optimizing, collapse all calls to trap down to just one per @@ -4091,12 +4096,15 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, llvm::BasicBlock *&TrapBB = TrapBBs[CheckHandlerID]; llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation(); - llvm::StringRef TrapMessage = GetUBSanTrapForHandler(CheckHandlerID); + llvm::StringRef TrapMessage = + Message.size() > 0 ? Message : GetUBSanTrapForHandler(CheckHandlerID); + llvm::StringRef TrapCategory = + Category.size() > 0 ? Category : "Undefined Behavior Sanitizer"; if (getDebugInfo() && !TrapMessage.empty() && CGM.getCodeGenOpts().SanitizeDebugTrapReasons && TrapLocation) { TrapLocation = getDebugInfo()->CreateTrapFailureMessageFor( - TrapLocation, "Undefined Behavior Sanitizer", TrapMessage); + TrapLocation, TrapCategory, TrapMessage); } NoMerge = NoMerge || !CGM.getCodeGenOpts().OptimizationLevel || diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index fcbfa5e31d149..0264c850534fc 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1813,6 +1813,7 @@ void ScalarExprEmitter::EmitBinOpCheck( SanitizerHandler Check; SmallVector<llvm::Constant *, 4> StaticData; SmallVector<llvm::Value *, 2> DynamicData; + std::unique_ptr<RuntimeTrapDiagnosticBuilder> RTDB = nullptr; BinaryOperatorKind Opcode = Info.Opcode; if (BinaryOperator::isCompoundAssignmentOp(Opcode)) @@ -1839,19 +1840,43 @@ void ScalarExprEmitter::EmitBinOpCheck( StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty)); } else { // Arithmetic overflow (+, -, *). + unsigned ArithOverflowKind = 0; switch (Opcode) { - case BO_Add: Check = SanitizerHandler::AddOverflow; break; - case BO_Sub: Check = SanitizerHandler::SubOverflow; break; - case BO_Mul: Check = SanitizerHandler::MulOverflow; break; - default: llvm_unreachable("unexpected opcode for bin op check"); + case BO_Add: { + ArithOverflowKind = 0; + Check = SanitizerHandler::AddOverflow; + break; + } + case BO_Sub: { + Check = SanitizerHandler::SubOverflow; + ArithOverflowKind = 1; + break; + } + case BO_Mul: { + Check = SanitizerHandler::MulOverflow; + ArithOverflowKind = 2; + break; + } + default: + llvm_unreachable("unexpected opcode for bin op check"); } StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty)); + if (CGF.CGM.getCodeGenOpts().SanitizeTrap.has( + SanitizerKind::UnsignedIntegerOverflow) || + CGF.CGM.getCodeGenOpts().SanitizeTrap.has( + SanitizerKind::SignedIntegerOverflow)) { + // Only pay the cost for constructing the trap diagnostic if they are + // going to be used. + RTDB = CGF.CGM.RuntimeDiag(diag::trap_ubsan_arith_overflow); + *RTDB << Info.Ty->isSignedIntegerOrEnumerationType() + << ArithOverflowKind << Info.E; + } } DynamicData.push_back(Info.LHS); DynamicData.push_back(Info.RHS); } - CGF.EmitCheck(Checks, Check, StaticData, DynamicData); + CGF.EmitCheck(Checks, Check, StaticData, DynamicData, std::move(RTDB)); } //===----------------------------------------------------------------------===// diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index fc65199a0f154..20b84c272ce31 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -5280,7 +5280,8 @@ class CodeGenFunction : public CodeGenTypeCache { EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>> Checked, SanitizerHandler Check, ArrayRef<llvm::Constant *> StaticArgs, - ArrayRef<llvm::Value *> DynamicArgs); + ArrayRef<llvm::Value *> DynamicArgs, + std::unique_ptr<RuntimeTrapDiagnosticBuilder> RTDB = nullptr); /// Emit a slow path cross-DSO CFI check which calls __cfi_slowpath /// if Cond if false. @@ -5296,7 +5297,8 @@ class CodeGenFunction : public CodeGenTypeCache { /// Create a basic block that will call the trap intrinsic, and emit a /// conditional branch to it, for the -ftrapv checks. void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID, - bool NoMerge = false); + bool NoMerge = false, StringRef Message = "", + StringRef Category = ""); /// Emit a call to trap or debugtrap and attach function attribute /// "trap-func-name" if specified. diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 705d9a3cb9de3..535dc968f648f 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -23,6 +23,7 @@ #include "clang/AST/GlobalDecl.h" #include "clang/AST/Mangle.h" #include "clang/Basic/ABI.h" +#include "clang/Basic/DiagnosticTrap.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/NoSanitizeList.h" #include "clang/Basic/ProfileList.h" @@ -1824,6 +1825,12 @@ class CodeGenModule : public CodeGenTypeCache { return PAlign; } + /// Helper function to construct a RuntimeTrapDiagnosticBuilder + [[nodiscard]] std::unique_ptr<RuntimeTrapDiagnosticBuilder> + RuntimeDiag(unsigned DiagID) { + return std::make_unique<RuntimeTrapDiagnosticBuilder>(&getDiags(), DiagID); + } + private: bool shouldDropDLLAttribute(const Decl *D, const llvm::GlobalValue *GV) const; diff --git a/clang/test/DebugInfo/Generic/ubsan-trap-reason-add-overflow.c b/clang/test/DebugInfo/Generic/ubsan-trap-reason-add-overflow.c index 225778d68833d..aefb92819d215 100644 --- a/clang/test/DebugInfo/Generic/ubsan-trap-reason-add-overflow.c +++ b/clang/test/DebugInfo/Generic/ubsan-trap-reason-add-overflow.c @@ -1,9 +1,21 @@ // RUN: %clang_cc1 -triple arm64-apple-macosx14.0.0 -O0 -debug-info-kind=standalone -dwarf-version=5 \ -// RUN: -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -emit-llvm %s -o - | FileCheck %s +// RUN: -fsanitize=signed-integer-overflow,unsigned-integer-overflow \ +// RUN: -fsanitize-trap=signed-integer-overflow,unsigned-integer-overflow \ +// RUN: -emit-llvm %s -o - | FileCheck %s -int add_overflow(int a, int b) { return a + b; } +int sadd_overflow(int a, int b) { return a + b; } + +unsigned add_overflow(unsigned c, unsigned d) { return c + d; } + +// CHECK-LABEL: @sadd_overflow +// CHECK: call void @llvm.ubsantrap(i8 0) {{.*}}!dbg [[SLOC:![0-9]+]] // CHECK-LABEL: @add_overflow // CHECK: call void @llvm.ubsantrap(i8 0) {{.*}}!dbg [[LOC:![0-9]+]] + + +// CHECK: [[SLOC]] = !DILocation(line: 0, scope: [[SMSG:![0-9]+]], {{.+}}) +// CHECK: [[SMSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$signed integer addition overflow in 'a + b'" + // CHECK: [[LOC]] = !DILocation(line: 0, scope: [[MSG:![0-9]+]], {{.+}}) -// CHECK: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$Integer addition overflowed" +// CHECK: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$unsigned integer addition overflow in 'c + d'" diff --git a/clang/test/DebugInfo/Generic/ubsan-trap-reason-flag.c b/clang/test/DebugInfo/Generic/ubsan-trap-reason-flag.c index 5cc16d154bf68..f0e707e5804be 100644 --- a/clang/test/DebugInfo/Generic/ubsan-trap-reason-flag.c +++ b/clang/test/DebugInfo/Generic/ubsan-trap-reason-flag.c @@ -15,7 +15,7 @@ int add_overflow(int a, int b) { return a + b; } // ANNOTATE-LABEL: @add_overflow // ANNOTATE: call void @llvm.ubsantrap(i8 0) {{.*}}!dbg [[LOC:![0-9]+]] // ANNOTATE: [[LOC]] = !DILocation(line: 0, scope: [[MSG:![0-9]+]], {{.+}}) -// ANNOTATE: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$Integer addition overflowed" +// ANNOTATE: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$signed integer addition overflow in 'a + b'" // NO-ANNOTATE-LABEL: @add_overflow // NO-ANNOTATE: call void @llvm.ubsantrap(i8 0) {{.*}}!dbg [[LOC:![0-9]+]] diff --git a/clang/test/DebugInfo/Generic/ubsan-trap-reason-mul-overflow.c b/clang/test/DebugInfo/Generic/ubsan-trap-reason-mul-overflow.c index cf9a0b4e7439c..b1a79b8a6cb24 100644 --- a/clang/test/DebugInfo/Generic/ubsan-trap-reason-mul-overflow.c +++ b/clang/test/DebugInfo/Generic/ubsan-trap-reason-mul-overflow.c @@ -1,9 +1,20 @@ // RUN: %clang_cc1 -triple arm64-apple-macosx14.0.0 -O0 -debug-info-kind=standalone -dwarf-version=5 \ -// RUN: -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -emit-llvm %s -o - | FileCheck %s +// RUN: -fsanitize=signed-integer-overflow,unsigned-integer-overflow \ +// RUN: -fsanitize-trap=signed-integer-overflow,unsigned-integer-overflow \ +// RUN: -emit-llvm %s -o - | FileCheck %s -int mul_overflow(int a, int b) { return a * b; } +int smul_overflow(int a, int b) { return a * b; } + +unsigned mul_overflow(unsigned c, unsigned d) { return c * d; } + +// CHECK-LABEL: @smul_overflow +// CHECK: call void @llvm.ubsantrap(i8 12) {{.*}}!dbg [[SLOC:![0-9]+]] // CHECK-LABEL: @mul_overflow // CHECK: call void @llvm.ubsantrap(i8 12) {{.*}}!dbg [[LOC:![0-9]+]] + +// CHECK: [[SLOC]] = !DILocation(line: 0, scope: [[SMSG:![0-9]+]], {{.+}}) +// CHECK: [[SMSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$signed integer multiplication overflow in 'a * b'" + // CHECK: [[LOC]] = !DILocation(line: 0, scope: [[MSG:![0-9]+]], {{.+}}) -// CHECK: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$Integer multiplication overflowed" +// CHECK: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$unsigned integer multiplication overflow in 'c * d'" diff --git a/clang/test/DebugInfo/Generic/ubsan-trap-reason-sub-overflow.c b/clang/test/DebugInfo/Generic/ubsan-trap-reason-sub-overflow.c index 62aa7fc953dad..f38a5b47e6a00 100644 --- a/clang/test/DebugInfo/Generic/ubsan-trap-reason-sub-overflow.c +++ b/clang/test/DebugInfo/Generic/ubsan-trap-reason-sub-overflow.c @@ -1,9 +1,20 @@ // RUN: %clang_cc1 -triple arm64-apple-macosx14.0.0 -O0 -debug-info-kind=standalone -dwarf-version=5 \ -// RUN: -fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -emit-llvm %s -o - | FileCheck %s +// RUN: -fsanitize=signed-integer-overflow,unsigned-integer-overflow \ +// RUN: -fsanitize-trap=signed-integer-overflow,unsigned-integer-overflow \ +// RUN: -emit-llvm %s -o - | FileCheck %s -int sub_overflow(int a, int b) { return a - b; } +int ssub_overflow(int a, int b) { return a - b; } + +unsigned sub_overflow(unsigned c, unsigned d) { return c - d; } + +// CHECK-LABEL: @ssub_overflow +// CHECK: call void @llvm.ubsantrap(i8 21) {{.*}}!dbg [[SLOC:![0-9]+]] // CHECK-LABEL: @sub_overflow // CHECK: call void @llvm.ubsantrap(i8 21) {{.*}}!dbg [[LOC:![0-9]+]] + +// CHECK: [[SLOC]] = !DILocation(line: 0, scope: [[SMSG:![0-9]+]], {{.+}}) +// CHECK: [[SMSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$signed integer subtraction overflow in 'a - b'" + // CHECK: [[LOC]] = !DILocation(line: 0, scope: [[MSG:![0-9]+]], {{.+}}) -// CHECK: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$Integer subtraction overflowed" +// CHECK: [[MSG]] = distinct !DISubprogram(name: "__clang_trap_msg$Undefined Behavior Sanitizer$unsigned integer subtraction overflow in 'c - d'" diff --git a/lldb/test/Shell/Recognizer/ubsan_add_overflow.test b/lldb/test/Shell/Recognizer/ubsan_add_overflow.test index a5e95cf5a898f..872b5a7a4d585 100644 --- a/lldb/test/Shell/Recognizer/ubsan_add_overflow.test +++ b/lldb/test/Shell/Recognizer/ubsan_add_overflow.test @@ -6,11 +6,11 @@ # RUN: %lldb -b -s %s %t.out | FileCheck %s run -# CHECK: thread #{{.*}} stop reason = Undefined Behavior Sanitizer: Integer addition overflowed +# CHECK: thread #{{.*}} stop reason = Undefined Behavior Sanitizer: signed integer addition overflow in '2147483647 + 1' # CHECK-NEXT: frame #1: {{.*}}`main at ubsan_add_overflow.c bt -# CHECK: frame #0: {{.*}}`__clang_trap_msg$Undefined Behavior Sanitizer$Integer addition overflowed{{.*}} +# CHECK: frame #0: {{.*}}`__clang_trap_msg$Undefined Behavior Sanitizer$signed integer addition overflow in '2147483647 + 1'{{.*}} # CHECK: frame #1: {{.*}}`main at ubsan_add_overflow.c frame info >From 4605ee718f7b5beac312104962c813142b700f08 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Thu, 21 Aug 2025 15:49:24 -0700 Subject: [PATCH 02/11] Add `[[nodiscard]]` to `RuntimeTrapDiagnosticBuilder` --- clang/include/clang/Basic/Diagnostic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 92e8e9101e9db..a8bfe0a7f0527 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -1575,7 +1575,7 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) { /// \endcode /// /// -class RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { +class [[nodiscard]] RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { public: RuntimeTrapDiagnosticBuilder(DiagnosticsEngine *DiagObj, unsigned DiagID); ~RuntimeTrapDiagnosticBuilder(); >From 2622dd8269accb900e5b4799375f0ccfd33ecc61 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Thu, 21 Aug 2025 16:07:02 -0700 Subject: [PATCH 03/11] Remove unnecessary incldue --- clang/lib/CodeGen/CGExpr.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 1e8c321c88d2d..3b1d0a669247d 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -33,7 +33,6 @@ #include "clang/AST/StmtVisitor.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/CodeGenOptions.h" -#include "clang/Basic/DiagnosticParse.h" #include "clang/Basic/Module.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/STLExtras.h" >From d141bb662d0e0194c530aacb7f13164a89b10318 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Thu, 21 Aug 2025 16:11:03 -0700 Subject: [PATCH 04/11] Use modern header block for `DiagnosticTrap.h` and add a little bit of documentation. --- clang/include/clang/Basic/DiagnosticTrap.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticTrap.h b/clang/include/clang/Basic/DiagnosticTrap.h index 7c6a5dc5d40d7..716be525e0527 100644 --- a/clang/include/clang/Basic/DiagnosticTrap.h +++ b/clang/include/clang/Basic/DiagnosticTrap.h @@ -1,10 +1,16 @@ -//===--- DiagnosticTrap.h - Diagnostics for trap instructions ---*- 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 // //===----------------------------------------------------------------------===// +/// +/// \file +/// This file contains the necessary includes for using +/// `RuntimeTrapDiagnosticBuilder`. +/// +//===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_BASIC_TRAP_H #define LLVM_CLANG_BASIC_TRAP_H >From 03782c5f52e610ba0220add28d98c05c618736a3 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Thu, 21 Aug 2025 16:16:35 -0700 Subject: [PATCH 05/11] Fix some formatting --- clang/include/clang/Basic/DiagnosticIDs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h index a8c00fff0d9c5..06446cf580389 100644 --- a/clang/include/clang/Basic/DiagnosticIDs.h +++ b/clang/include/clang/Basic/DiagnosticIDs.h @@ -47,7 +47,7 @@ enum { DIAG_SIZE_ANALYSIS = 100, DIAG_SIZE_REFACTORING = 1000, DIAG_SIZE_INSTALLAPI = 100, - DIAG_SIZE_TRAP = 100, + DIAG_SIZE_TRAP = 100, }; // Start position for diagnostics. // clang-format off >From 3a215724a014ea6c306eb3d3a120ec00e49cd60b Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Thu, 21 Aug 2025 16:36:02 -0700 Subject: [PATCH 06/11] Use %enum_select --- clang/include/clang/Basic/DiagnosticTrapKinds.td | 6 +++++- clang/lib/CodeGen/CGExprScalar.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticTrapKinds.td b/clang/include/clang/Basic/DiagnosticTrapKinds.td index ce5b16747071e..edf44e19465e7 100644 --- a/clang/include/clang/Basic/DiagnosticTrapKinds.td +++ b/clang/include/clang/Basic/DiagnosticTrapKinds.td @@ -20,7 +20,11 @@ let CategoryName = "Undefined Behavior Sanitizer" in { def trap_ubsan_arith_overflow : Trap< "%select{unsigned|signed}0 integer " - "%select{addition|subtraction|multiplication}1 overflow in %2">; + "%enum_select<UBSanArithKind>{" + "%Add{addition}|" + "%Sub{subtraction}|" + "%Mul{multiplication}" + "}1 overflow in %2">; } } diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 0264c850534fc..0397fb2bbb6b7 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1840,21 +1840,21 @@ void ScalarExprEmitter::EmitBinOpCheck( StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty)); } else { // Arithmetic overflow (+, -, *). - unsigned ArithOverflowKind = 0; + int ArithOverflowKind = 0; switch (Opcode) { case BO_Add: { - ArithOverflowKind = 0; + ArithOverflowKind = diag::UBSanArithKind::Add; Check = SanitizerHandler::AddOverflow; break; } case BO_Sub: { Check = SanitizerHandler::SubOverflow; - ArithOverflowKind = 1; + ArithOverflowKind = diag::UBSanArithKind::Sub; break; } case BO_Mul: { Check = SanitizerHandler::MulOverflow; - ArithOverflowKind = 2; + ArithOverflowKind = diag::UBSanArithKind::Mul; break; } default: >From 9291aac673c524484d802461ea6f102a635cf39a Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Thu, 21 Aug 2025 16:54:02 -0700 Subject: [PATCH 07/11] Use llvm::SmallString instead of llvm::SmallVector --- clang/include/clang/Basic/Diagnostic.h | 3 ++- clang/lib/Basic/Diagnostic.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index a8bfe0a7f0527..aaa7bb99492e2 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -24,6 +24,7 @@ #include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Compiler.h" #include <cassert> @@ -1598,7 +1599,7 @@ class [[nodiscard]] RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { StringRef getCategory(); private: - llvm::SmallVector<char, 64> MessageStorage; + llvm::SmallString<64> MessageStorage; }; //===----------------------------------------------------------------------===// diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index 75729d5c8fd9e..d8a410303d94a 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -813,7 +813,7 @@ StringRef RuntimeTrapDiagnosticBuilder::getMessage() { Diagnostic Info(DiagObj, *this); Info.FormatDiagnostic(MessageStorage); } - return StringRef(MessageStorage.data(), MessageStorage.size()); + return MessageStorage; } StringRef RuntimeTrapDiagnosticBuilder::getCategory() { >From f37cc7adb8a2e2c12362bef98f1f98109de33e48 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Fri, 22 Aug 2025 14:20:50 -0700 Subject: [PATCH 08/11] Move `RuntimeTrapDiagnosticBuilder` to a safer API where the storage and transport of the "Trap Reason" is moved out of `RuntimeTrapDiagnosticBuilder` into a new helper class called `TrapReason`. --- clang/include/clang/Basic/Diagnostic.h | 81 ++++++++++++++++++-------- clang/lib/Basic/Diagnostic.cpp | 19 +++--- clang/lib/CodeGen/CGExpr.cpp | 14 ++--- clang/lib/CodeGen/CGExprScalar.cpp | 9 +-- clang/lib/CodeGen/CodeGenFunction.h | 5 +- clang/lib/CodeGen/CodeGenModule.h | 5 +- 6 files changed, 80 insertions(+), 53 deletions(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index aaa7bb99492e2..3cb6b01ee8226 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -23,8 +23,8 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" -#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Compiler.h" #include <cassert> @@ -1537,26 +1537,58 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) { } //===----------------------------------------------------------------------===// -// RuntimeTrapDiagnosticBuilder +// RuntimeTrapDiagnosticBuilder and helper classes //===----------------------------------------------------------------------===// -/// Class to make it convenient to construct "trap reasons" to attach to trap -/// instructions. + +/// Helper class for \class RuntimeTrapDiagnosticBuilder. This class stores the +/// "trap reason" built by \class RuntimeTrapDiagnosticBuilder. This consists of +/// a trap message and trap category. /// -/// Although this class inherits from `DiagnosticBuilder` it has very different -/// semantics. +/// It is intended that this object be allocated on the stack. +class TrapReason { +public: + TrapReason() {} + /// \return The trap message. Note the lifetime of the underlying storage for + /// the returned StringRef lives in this class which means the returned + /// StringRef should not be used after this class is destroyed. + StringRef getMessage() const { return Message; } + + /// \return the trap category (e.g. "Undefined Behavior Sanitizer") + StringRef getCategory() const { return Category; } + + bool isEmpty() const { return Message.size() == 0 && Category.size() == 0; } + + /// \return a pointer to this object if it contains a non-empty trap reason, + /// otherwise return `nullptr`. This is a convenient helper for passing + /// TrapReason objects to function calls that have a `TrapReason*` parameter. + TrapReason *getPtr() { + if (isEmpty()) + return nullptr; + return this; + } + +private: + llvm::SmallString<64> Message; + // The Category doesn't need its own storage because the StringRef points + // to a global constant string. + StringRef Category; + + // Only this class can set the private fields. + friend class RuntimeTrapDiagnosticBuilder; +}; + +/// Class to make it convenient to initialize TrapReason objects which can be +/// used to attach the "trap reason" to trap instructions. +/// +/// Although this class inherits from \class DiagnosticBuilder it has slightly +/// different semantics. /// /// * This class should only be used with trap diagnostics (declared in /// `DiagnosticTrapKinds.td`). /// * The `RuntimeTrapDiagnosticBuilder` does not emit diagnostics to the normal /// diagnostics consumers on destruction like normal Diagnostic builders. -/// Instead it does nothing on destruction. -/// * Users of this class that want to retrieve the "trap reason" should call -/// call the `getMessage()` and `getCategory()` and use those results before -/// the builder is destroyed. -/// * Unlike the `DiagnosticBuilder` the `RuntimeDiagnosticBuilder` should never -/// be created as a temporary (i.e. rvalue) and instead should be stored. This -/// is because the class is only useful if `getMessage()` and `getCategory()` -/// can be called. +/// Instead on destruction it assigns to the TrapReason object passed in to +/// the constructor. /// /// Given that this class inherits from `DiagnosticBuilder` it inherits all of /// its abilities to format diagnostic messages and consume various types in @@ -1569,16 +1601,17 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) { /// /// \code /// { -/// auto* RTDB = CGM.RuntimeDiag(diag::trap_diagnostic); -/// *RTDB << 0 << SomeExpr << SomeType; -/// consume(RTDB->getCategory(), RTDB->getMessage()); +/// TrapReason TR; +/// CGM.RuntimeDiag(diag::trap_diagnostic, TR) << 0 << SomeExpr << SomeType; +/// consume(TR.getPtr()); /// } /// \endcode /// /// -class [[nodiscard]] RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { +class RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { public: - RuntimeTrapDiagnosticBuilder(DiagnosticsEngine *DiagObj, unsigned DiagID); + RuntimeTrapDiagnosticBuilder(DiagnosticsEngine *DiagObj, unsigned DiagID, + TrapReason &TR); ~RuntimeTrapDiagnosticBuilder(); // Prevent accidentally copying or assigning @@ -1589,17 +1622,15 @@ class [[nodiscard]] RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder { RuntimeTrapDiagnosticBuilder(const RuntimeTrapDiagnosticBuilder &) = delete; RuntimeTrapDiagnosticBuilder(const RuntimeTrapDiagnosticBuilder &&) = delete; - /// \return Format the trap message and return it. Note the lifetime of - /// the underlying storage pointed to by the returned StringRef is the same - /// as the lifetime of this class. This means it is likely unsafe to store - /// the returned StringRef. - StringRef getMessage(); + /// \return Format the trap message into `Storage`. + void getMessage(SmallVectorImpl<char> &Storage); + /// \return Return the trap category. These are the `CategoryName` property /// of `trap` diagnostics declared in `DiagnosticTrapKinds.td`. StringRef getCategory(); private: - llvm::SmallString<64> MessageStorage; + TrapReason &TR; }; //===----------------------------------------------------------------------===// diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index d8a410303d94a..d1249ac75b1ae 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -796,24 +796,25 @@ DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D) } RuntimeTrapDiagnosticBuilder::RuntimeTrapDiagnosticBuilder( - DiagnosticsEngine *DiagObj, unsigned DiagID) - : DiagnosticBuilder(DiagObj, SourceLocation(), DiagID) { + DiagnosticsEngine *DiagObj, unsigned DiagID, TrapReason &TR) + : DiagnosticBuilder(DiagObj, SourceLocation(), DiagID), TR(TR) { assert(DiagObj->getDiagnosticIDs()->isTrapDiag(DiagID)); } RuntimeTrapDiagnosticBuilder::~RuntimeTrapDiagnosticBuilder() { + // Store the trap message and category into the TrapReason object. + getMessage(TR.Message); + TR.Category = getCategory(); + // Make sure that when `DiagnosticBuilder::~DiagnosticBuilder()` // calls `Emit()` that it does nothing. Clear(); } -StringRef RuntimeTrapDiagnosticBuilder::getMessage() { - if (MessageStorage.size() == 0) { - // Render the Diagnostic - Diagnostic Info(DiagObj, *this); - Info.FormatDiagnostic(MessageStorage); - } - return MessageStorage; +void RuntimeTrapDiagnosticBuilder::getMessage(SmallVectorImpl<char> &Storage) { + // Render the Diagnostic + Diagnostic Info(DiagObj, *this); + Info.FormatDiagnostic(Storage); } StringRef RuntimeTrapDiagnosticBuilder::getCategory() { diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 3b1d0a669247d..08ee48c3eb44e 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -3729,8 +3729,7 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF, void CodeGenFunction::EmitCheck( ArrayRef<std::pair<llvm::Value *, SanitizerKind::SanitizerOrdinal>> Checked, SanitizerHandler CheckHandler, ArrayRef<llvm::Constant *> StaticArgs, - ArrayRef<llvm::Value *> DynamicArgs, - std::unique_ptr<RuntimeTrapDiagnosticBuilder> RTDB) { + ArrayRef<llvm::Value *> DynamicArgs, const TrapReason *TR) { assert(IsSanitizerScope); assert(Checked.size() > 0); assert(CheckHandler >= 0 && @@ -3769,9 +3768,7 @@ void CodeGenFunction::EmitCheck( } if (TrapCond) - EmitTrapCheck(TrapCond, CheckHandler, NoMerge, - RTDB ? RTDB->getMessage() : "", - RTDB ? RTDB->getCategory() : ""); + EmitTrapCheck(TrapCond, CheckHandler, NoMerge, TR); if (!FatalCond && !RecoverableCond) return; @@ -4083,8 +4080,7 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) { void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID, - bool NoMerge, StringRef Message, - StringRef Category) { + bool NoMerge, const TrapReason *TR) { llvm::BasicBlock *Cont = createBasicBlock("cont"); // If we're optimizing, collapse all calls to trap down to just one per @@ -4096,9 +4092,9 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, llvm::DILocation *TrapLocation = Builder.getCurrentDebugLocation(); llvm::StringRef TrapMessage = - Message.size() > 0 ? Message : GetUBSanTrapForHandler(CheckHandlerID); + TR ? TR->getMessage() : GetUBSanTrapForHandler(CheckHandlerID); llvm::StringRef TrapCategory = - Category.size() > 0 ? Category : "Undefined Behavior Sanitizer"; + TR ? TR->getCategory() : "Undefined Behavior Sanitizer"; if (getDebugInfo() && !TrapMessage.empty() && CGM.getCodeGenOpts().SanitizeDebugTrapReasons && TrapLocation) { diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 0397fb2bbb6b7..729b2e8b4f7bf 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -1814,6 +1814,7 @@ void ScalarExprEmitter::EmitBinOpCheck( SmallVector<llvm::Constant *, 4> StaticData; SmallVector<llvm::Value *, 2> DynamicData; std::unique_ptr<RuntimeTrapDiagnosticBuilder> RTDB = nullptr; + TrapReason TR; BinaryOperatorKind Opcode = Info.Opcode; if (BinaryOperator::isCompoundAssignmentOp(Opcode)) @@ -1867,16 +1868,16 @@ void ScalarExprEmitter::EmitBinOpCheck( SanitizerKind::SignedIntegerOverflow)) { // Only pay the cost for constructing the trap diagnostic if they are // going to be used. - RTDB = CGF.CGM.RuntimeDiag(diag::trap_ubsan_arith_overflow); - *RTDB << Info.Ty->isSignedIntegerOrEnumerationType() - << ArithOverflowKind << Info.E; + CGF.CGM.RuntimeDiag(diag::trap_ubsan_arith_overflow, TR) + << Info.Ty->isSignedIntegerOrEnumerationType() << ArithOverflowKind + << Info.E; } } DynamicData.push_back(Info.LHS); DynamicData.push_back(Info.RHS); } - CGF.EmitCheck(Checks, Check, StaticData, DynamicData, std::move(RTDB)); + CGF.EmitCheck(Checks, Check, StaticData, DynamicData, TR.getPtr()); } //===----------------------------------------------------------------------===// diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 20b84c272ce31..07a5ed939a241 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -5281,7 +5281,7 @@ class CodeGenFunction : public CodeGenTypeCache { Checked, SanitizerHandler Check, ArrayRef<llvm::Constant *> StaticArgs, ArrayRef<llvm::Value *> DynamicArgs, - std::unique_ptr<RuntimeTrapDiagnosticBuilder> RTDB = nullptr); + const TrapReason *TR = nullptr); /// Emit a slow path cross-DSO CFI check which calls __cfi_slowpath /// if Cond if false. @@ -5297,8 +5297,7 @@ class CodeGenFunction : public CodeGenTypeCache { /// Create a basic block that will call the trap intrinsic, and emit a /// conditional branch to it, for the -ftrapv checks. void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID, - bool NoMerge = false, StringRef Message = "", - StringRef Category = ""); + bool NoMerge = false, const TrapReason *TR = nullptr); /// Emit a call to trap or debugtrap and attach function attribute /// "trap-func-name" if specified. diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 535dc968f648f..a183c35fa0396 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1826,9 +1826,8 @@ class CodeGenModule : public CodeGenTypeCache { } /// Helper function to construct a RuntimeTrapDiagnosticBuilder - [[nodiscard]] std::unique_ptr<RuntimeTrapDiagnosticBuilder> - RuntimeDiag(unsigned DiagID) { - return std::make_unique<RuntimeTrapDiagnosticBuilder>(&getDiags(), DiagID); + RuntimeTrapDiagnosticBuilder RuntimeDiag(unsigned DiagID, TrapReason &TR) { + return RuntimeTrapDiagnosticBuilder(&getDiags(), DiagID, TR); } private: >From fc8ab6928cc894a230e9b531d8c25fd35de18850 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Fri, 22 Aug 2025 14:38:10 -0700 Subject: [PATCH 09/11] Fix `tools/clang/test/Misc/warning-flags.c` by teaching `diagtool` to not treat trap diagnostics as a warning diagnostic. --- clang/lib/Basic/DiagnosticIDs.cpp | 2 ++ clang/tools/diagtool/ListWarnings.cpp | 3 +++ 2 files changed, 5 insertions(+) diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index a1d9d0f34d20d..9ae0c2c5a4b19 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -395,6 +395,8 @@ unsigned DiagnosticIDs::getCustomDiagID(CustomDiagDesc Diag) { } bool DiagnosticIDs::isWarningOrExtension(unsigned DiagID) const { + // FIXME: Returning `true` for notes, remarks, traps, and + // CLASS_INVALID classes looks wrong. return DiagID < diag::DIAG_UPPER_LIMIT ? getDiagClass(DiagID) != CLASS_ERROR : CustomDiagInfo->getDescription(DiagID).GetClass() != CLASS_ERROR; diff --git a/clang/tools/diagtool/ListWarnings.cpp b/clang/tools/diagtool/ListWarnings.cpp index 9f9647126dd8a..ce24f11bd1411 100644 --- a/clang/tools/diagtool/ListWarnings.cpp +++ b/clang/tools/diagtool/ListWarnings.cpp @@ -56,6 +56,9 @@ int ListWarnings::run(unsigned int argc, char **argv, llvm::raw_ostream &out) { if (DiagnosticIDs{}.isNote(diagID)) continue; + if (DiagnosticIDs{}.isTrapDiag(diagID)) + continue; + if (!DiagnosticIDs{}.isWarningOrExtension(diagID)) continue; >From 31be8abb00178f51b96e12884c1c25fc7704ac95 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Mon, 25 Aug 2025 12:32:43 -0700 Subject: [PATCH 10/11] Update clang/include/clang/Basic/Diagnostic.td Co-authored-by: Sirraide <aeternalm...@gmail.com> --- clang/include/clang/Basic/Diagnostic.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/Diagnostic.td b/clang/include/clang/Basic/Diagnostic.td index d190c70a401e3..b1d57d2b5ecc1 100644 --- a/clang/include/clang/Basic/Diagnostic.td +++ b/clang/include/clang/Basic/Diagnostic.td @@ -145,7 +145,7 @@ class Extension<string str> : Diagnostic<str, CLASS_EXTENSION, SEV_Ignored>; class ExtWarn<string str> : Diagnostic<str, CLASS_EXTENSION, SEV_Warning>; // Notes can provide supplementary information on errors, warnings, and remarks. class Note<string str> : Diagnostic<str, CLASS_NOTE, SEV_Fatal/*ignored*/>; -// Traps messages attached to traps in debug info +// Traps messages attached to traps in debug info. class Trap<string str> : Diagnostic<str, CLASS_TRAP, SEV_Fatal/*ignored*/>; class DefaultIgnore { Severity DefaultSeverity = SEV_Ignored; } >From 7aceb843c14727bbc4c9c5aadca8e10d3579f7c5 Mon Sep 17 00:00:00 2001 From: Dan Liew <d...@su-root.co.uk> Date: Mon, 25 Aug 2025 12:35:41 -0700 Subject: [PATCH 11/11] Update clang/include/clang/Basic/DiagnosticTrap.h Co-authored-by: Sirraide <aeternalm...@gmail.com> --- clang/include/clang/Basic/DiagnosticTrap.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticTrap.h b/clang/include/clang/Basic/DiagnosticTrap.h index 716be525e0527..b93f9ebc6f34c 100644 --- a/clang/include/clang/Basic/DiagnosticTrap.h +++ b/clang/include/clang/Basic/DiagnosticTrap.h @@ -12,8 +12,8 @@ /// //===----------------------------------------------------------------------===// -#ifndef LLVM_CLANG_BASIC_TRAP_H -#define LLVM_CLANG_BASIC_TRAP_H +#ifndef LLVM_CLANG_BASIC_DIAGNOSTICTRAP_H +#define LLVM_CLANG_BASIC_DIAGNOSTICTRAP_H #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticTrapInterface.inc" _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits