[UBSan][BoundsSafety] Implement support for more expressive "trap reasons" #154618
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
delcypher
requested review from
AaronBallman,
Michael137,
PiJoules,
Sirraide,
fmayer,
hnrklssn,
ilovepi,
patrykstefanski,
petrhosek,
rapidsna,
shafik,
thurstond,
usama54321 and
vitalybuka
delcypher
added
the
clang:bounds-safety
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-lldb Author: Dan Liew (delcypher) ChangesIn 29992cf (#145967) support was added for "trap reasons" on traps emitted in UBSan in trapping mode (e.g. A limitation of that patch is that the trap reason string is hard-coded for each 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:
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
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 2. Apply the new infrastructure to UBSan checks for arithmetic overflow To demonstrate using Previously for code like The trap reason string looked like now the trap message looks like: This string is much more specific because
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 Patch is 27.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/154618.diff 19 Files Affected:
diff --git a/clang/include/clang/Basic/AllDiagnosticKinds.inc b/clang/include/clang/Basic/AllDiagnosticKinds.inc
index a946b4a640ac6..0ecc55b07efa9 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/DiagnosticCodeGenKinds.inc"
// clang-format on
diff --git a/clang/include/clang/Basic/AllDiagnostics.h b/clang/include/clang/Basic/AllDiagnostics.h
index e64634cc138f7..f9446dccfa7cb 100644
--- a/clang/include/clang/Basic/AllDiagnostics.h
+++ b/clang/include/clang/Basic/AllDiagnostics.h
@@ -26,6 +26,7 @@
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/DiagnosticSerialization.h"
#include "clang/Basic/DiagnosticRefactoring.h"
+#include "clang/Basic/DiagnosticCodeGen.h"
namespace clang {
template <size_t SizeOfStr, typename FieldType>
diff --git a/clang/include/clang/Basic/CMakeLists.txt b/clang/include/clang/Basic/CMakeLists.txt
index 0cf661a57dfa8..dc91c782c8fc2 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(CodeGen)
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..e65df3e43b9a7 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
+/// `DiagnosticCodeGenKinds.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 `DiagnosticCodeGenKinds.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..9cedb3a200ff0 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 "DiagnosticCodeGenKinds.td"
diff --git a/clang/include/clang/Basic/DiagnosticCodeGen.h b/clang/include/clang/Basic/DiagnosticCodeGen.h
new file mode 100644
index 0000000000000..06ed516c65c3d
--- /dev/null
+++ b/clang/include/clang/Basic/DiagnosticCodeGen.h
@@ -0,0 +1,15 @@
+//===--- DiagnosticCodeGen.h - Diagnostics for libcodegen -------*- 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_DIAGNOSTICCODEGEN_H
+#define LLVM_CLANG_BASIC_DIAGNOSTICCODEGEN_H
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticCodeGenInterface.inc"
+
+#endif
diff --git a/clang/include/clang/Basic/DiagnosticCodeGenKinds.td b/clang/include/clang/Basic/DiagnosticCodeGenKinds.td
new file mode 100644
index 0000000000000..6e9deae0c7f37
--- /dev/null
+++ b/clang/include/clang/Basic/DiagnosticCodeGenKinds.td
@@ -0,0 +1,26 @@
+//==--- DiagnosticCodeGenKinds.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 = "CodeGen" 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/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h
index b21a3b6232fc0..b9435043ae89b 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -47,6 +47,7 @@ namespace clang {
DIAG_SIZE_ANALYSIS = 100,
DIAG_SIZE_REFACTORING = 1000,
DIAG_SIZE_INSTALLAPI = 100,
+ DIAG_SIZE_CODEGEN = 10,
};
// Start position for diagnostics.
enum {
@@ -63,7 +64,8 @@ namespace clang {
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_CODEGEN = DIAG_START_INSTALLAPI + static_cast<int>(DIAG_SIZE_INSTALLAPI),
+ DIAG_UPPER_LIMIT = DIAG_START_CODEGEN + static_cast<int>(DIAG_SIZE_CODEGEN)
};
class CustomDiagInfo;
@@ -186,7 +188,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) {
@@ -360,6 +363,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/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index e33e843db6a44..2159ecf8294e3 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -793,6 +793,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..34fc7ffc18ba1 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(CODEGEN)
#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/DiagnosticCodeGenKinds.inc"
// clang-format on
#undef DIAG
};
@@ -214,6 +217,7 @@ CATEGORY(SEMA, CROSSTU)
CATEGORY(ANALYSIS, SEMA)
CATEGORY(REFACTORING, ANALYSIS)
CATEGORY(INSTALLAPI, REFACTORING)
+CATEGORY(CODEGEN, INSTALLAPI)
#undef CATEGORY
// Avoid out of bounds reads.
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d229d81d6b934..8f24a605c5bb2 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"
@@ -3721,7 +3722,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 &&
@@ -3760,7 +3762,9 @@ void CodeGenFunction::EmitCheck(
}
if (TrapCond)
- EmitTrapCheck(TrapCond, CheckHandler, NoMerge);
+ EmitTrapCheck(TrapCond, CheckHandler, NoMerge,
+ RTDB ? RTDB->getMessage() : "",
+ RTDB ? RTDB->getCategory() : "");
if (!FatalCond && !RecoverableCond)
return;
@@ -4072,7 +4076,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
@@ -4083,12 +4088,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 155b80df36715..5dafb2da89f2c 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 ad318f289ee83..4cb0a46d7ae8e 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -5273,7 +5273,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.
@@ -5289,7 +5290,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..786dc6d266ab1 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/DiagnosticCodeGen.h" // For runtime trap diagnostics
#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
+...
[truncated]
|
delcypher
force-pushed
the
dliew/sketch_runtime_diag_v2
branch
from
24a3e9e to
b0ec5d8
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
Author
|
Looks like I accidentally broke |
Contributor
Author
|
The formatting failure is due to some code that |
not treat trap diagnostics as a warning diagnostic.
Co-authored-by: Sirraide <[email protected]>
Co-authored-by: Sirraide <[email protected]>
Co-authored-by: Sirraide <[email protected]>
Co-authored-by: Sirraide <[email protected]>
Co-authored-by: Sirraide <[email protected]>
Co-authored-by: Sirraide <[email protected]>
either `none`, `basic`, or `detailed`. The old boolean flags are included for compatibility and are aliases of the new flag. The `basic` mode is the behavior that existed before this patch, and `detailed` is the newer more expressive trap reasons.
unoptimized-vs-optimized builds
delcypher
force-pushed
the
dliew/sketch_runtime_diag_v2
branch
from
b73434a to
cd62acd
Compare
Contributor
Author
|
@thurstond Thanks for the feedback. I've tried to address your comments and I also added a brief discussion of optimized-vs-unoptimized builds. |
thurstond
approved these changes
Aug 27, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/14447 Here is the relevant piece of the build log for the reference |
jyknight
added a commit
that referenced
this pull request
Aug 28, 2025
delcypher
added a commit
to delcypher/apple-llvm-project
that referenced
this pull request
Sep 9, 2025
…sons" (llvm#154618) In 29992cf (llvm#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 `Trap` 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 `DiagnosticTrapKinds.td` to describe the different trap reasons. * Add the `TrapReasonBuilder` and `TrapReason` classes 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::BuildTrapReason()` as a convenient constructor for the `TrapReasonBuilder`. 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 `TrapReasonBuilder` 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 `TrapReasonBuilder` 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 `TrapReasonBuilder` this patch applies it to UBSan traps for arithmetic overflow. The intention is that we would iteratively switch to using the `TrapReasonBuilder` 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 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. To allow users to avoid this a new driver/cc1 flag `-fsanitize-debug-trap-reasons=` has been added which can either be `none` (disable trap reasons entirely), `basic` (use the per `SanitizerKind` hard coded strings), and `detailed` (use the new expressive trap reasons implemented in this patch). The default is `detailed` to give the best out-of-the-box debugging experience. The existing `-fsanitize-debug-trap-reasons` and `-fno-sanitize-debug-trap-reasons` have been kept for compatibility and are aliases of the new flag with `detailed` and `none` arguments passed respectively. rdar://158612755 Conflicts: clang/include/clang/Basic/AllDiagnosticKinds.inc clang/include/clang/Basic/DiagnosticIDs.h clang/lib/Basic/DiagnosticIDs.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.h clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticIDs.h rdar://159302620 (cherry picked from commit 6d385c3) (cherry picked from commit f1ee047)
delcypher
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Sep 9, 2025
…lvm#154628) When trying to add a new diagnostic category (e.g. llvm#154618) I discovered `clang-format` really wanted to reformat these files. My initial attempt was just to suppress the reformatting with `// clang-format (on|off)` directives but reviewers preferred just reformatting the files so these two files have been completely reformatted. `clang-format` has been disabled for the enum that declares the `DIAG_START_*` constants because its much less readable after formatting. (cherry picked from commit 0961200) Conflicts: clang/include/clang/Basic/DiagnosticIDs.h
delcypher
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Sep 9, 2025
…sons" (llvm#154618) In 29992cf (llvm#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 `Trap` 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 `DiagnosticTrapKinds.td` to describe the different trap reasons. * Add the `TrapReasonBuilder` and `TrapReason` classes 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::BuildTrapReason()` as a convenient constructor for the `TrapReasonBuilder`. 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 `TrapReasonBuilder` 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 `TrapReasonBuilder` 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 `TrapReasonBuilder` this patch applies it to UBSan traps for arithmetic overflow. The intention is that we would iteratively switch to using the `TrapReasonBuilder` 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 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. To allow users to avoid this a new driver/cc1 flag `-fsanitize-debug-trap-reasons=` has been added which can either be `none` (disable trap reasons entirely), `basic` (use the per `SanitizerKind` hard coded strings), and `detailed` (use the new expressive trap reasons implemented in this patch). The default is `detailed` to give the best out-of-the-box debugging experience. The existing `-fsanitize-debug-trap-reasons` and `-fno-sanitize-debug-trap-reasons` have been kept for compatibility and are aliases of the new flag with `detailed` and `none` arguments passed respectively. rdar://158612755 Conflicts: clang/include/clang/Basic/AllDiagnosticKinds.inc clang/include/clang/Basic/DiagnosticIDs.h clang/lib/Basic/DiagnosticIDs.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CodeGenFunction.h clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticIDs.h rdar://159302620 (cherry picked from commit 6d385c3) (cherry picked from commit f1ee047)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 29992cf (#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
SanitizerKindeven 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:
Trapdiagnostic kind to represent trap diagnostics in TableGen files.Trapdiagnostic component. While this part probably isn't technically necessary it seemed like I should follow the existing convention used by the diagnostic system.DiagnosticTrapKinds.tdto describe the different trap reasons.TrapReasonBuilderandTrapReasonclasses to provide an interface for constructing trap reason strings and the trap category. Note this API while similar toDiagnosticBuilderhas different semantics which are described in the code comments. In particular the behavior when the destructor is called is very different.CodeGenModule::BuildTrapReason()as a convenient constructor for theTrapReasonBuilder.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
TrapReasonBuilderis 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:%select) can be used.Type,Expr, etc.) just work out-of-the-box.TrapReasonBuilderAPI is very similar toDiagnosticBuilderwhich 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-safetyimplementation (currently exists downstream).2. Apply the new infrastructure to UBSan checks for arithmetic overflow
To demonstrate using
TrapReasonBuilderthis patch applies it to UBSan traps for arithmetic overflow. The intention is that we would iteratively switch to using theTrapReasonBuilderfor all UBSan traps where it makes sense in future patches.Previously for code like
The trap reason string looked like
now the trap message looks like:
This string is much more specific because
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. To allow users to avoid this a new driver/cc1 flag
-fsanitize-debug-trap-reasons=has been added which can either benone(disable trap reasons entirely),basic(use the perSanitizerKindhard coded strings), anddetailed(use the new expressive trap reasons implemented in this patch). The default isdetailedto give the best out-of-the-box debugging experience. The existing-fsanitize-debug-trap-reasonsand-fno-sanitize-debug-trap-reasonshave been kept for compatibility and are aliases of the new flag withdetailedandnonearguments passed respectively.rdar://158612755