Skip to content

Commit 5044fa0

Browse files
committed
[UBSan][BoundsSafety] Implement support for more expressive "trap reasons"
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 `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
1 parent 17cc1dc commit 5044fa0

19 files changed

+259
-27
lines changed

clang/include/clang/Basic/AllDiagnosticKinds.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,5 @@
3030
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
3131
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
3232
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
33+
#include "clang/Basic/DiagnosticTrapKinds.inc"
3334
// clang-format on

clang/include/clang/Basic/AllDiagnostics.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "clang/Basic/DiagnosticRefactoring.h"
2727
#include "clang/Basic/DiagnosticSema.h"
2828
#include "clang/Basic/DiagnosticSerialization.h"
29+
#include "clang/Basic/DiagnosticTrap.h"
2930

3031
namespace clang {
3132
template <size_t SizeOfStr, typename FieldType> class StringSizerHelper {

clang/include/clang/Basic/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ clang_diag_gen(Parse)
3333
clang_diag_gen(Refactoring)
3434
clang_diag_gen(Sema)
3535
clang_diag_gen(Serialization)
36+
clang_diag_gen(Trap)
3637
clang_tablegen(DiagnosticGroups.inc -gen-clang-diag-groups
3738
SOURCE Diagnostic.td
3839
TARGET ClangDiagnosticGroups)

clang/include/clang/Basic/Diagnostic.h

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,6 +1233,7 @@ class DiagnosticBuilder : public StreamingDiagnostic {
12331233
friend class DiagnosticsEngine;
12341234
friend class PartialDiagnostic;
12351235
friend class Diagnostic;
1236+
friend class RuntimeTrapDiagnosticBuilder;
12361237

12371238
mutable DiagnosticsEngine *DiagObj = nullptr;
12381239

@@ -1534,6 +1535,72 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
15341535
return Report(SourceLocation(), DiagID);
15351536
}
15361537

1538+
//===----------------------------------------------------------------------===//
1539+
// RuntimeTrapDiagnosticBuilder
1540+
//===----------------------------------------------------------------------===//
1541+
/// Class to make it convenient to construct "trap reasons" to attach to trap
1542+
/// instructions.
1543+
///
1544+
/// Although this class inherits from `DiagnosticBuilder` it has very different
1545+
/// semantics.
1546+
///
1547+
/// * This class should only be used with trap diagnostics (declared in
1548+
/// `DiagnosticTrapKinds.td`).
1549+
/// * The `RuntimeTrapDiagnosticBuilder` does not emit diagnostics to the normal
1550+
/// diagnostics consumers on destruction like normal Diagnostic builders.
1551+
/// Instead it does nothing on destruction.
1552+
/// * Users of this class that want to retrieve the "trap reason" should call
1553+
/// call the `getMessage()` and `getCategory()` and use those results before
1554+
/// the builder is destroyed.
1555+
/// * Unlike the `DiagnosticBuilder` the `RuntimeDiagnosticBuilder` should never
1556+
/// be created as a temporary (i.e. rvalue) and instead should be stored. This
1557+
/// is because the class is only useful if `getMessage()` and `getCategory()`
1558+
/// can be called.
1559+
///
1560+
/// Given that this class inherits from `DiagnosticBuilder` it inherits all of
1561+
/// its abilities to format diagnostic messages and consume various types in
1562+
/// class (e.g. Type, Exprs, etc.). This makes it particularly suited to
1563+
/// printing types and expressions from the AST while codegen-ing runtime
1564+
/// checks.
1565+
///
1566+
///
1567+
/// Example use via the `CodeGenModule::RuntimeDiag` helper.
1568+
///
1569+
/// \code
1570+
/// {
1571+
/// auto* RTDB = CGM.RuntimeDiag(diag::trap_diagnostic);
1572+
/// *RTDB << 0 << SomeExpr << SomeType;
1573+
/// consume(RTDB->getCategory(), RTDB->getMessage());
1574+
/// }
1575+
/// \endcode
1576+
///
1577+
///
1578+
class RuntimeTrapDiagnosticBuilder : public DiagnosticBuilder {
1579+
public:
1580+
RuntimeTrapDiagnosticBuilder(DiagnosticsEngine *DiagObj, unsigned DiagID);
1581+
~RuntimeTrapDiagnosticBuilder();
1582+
1583+
// Prevent accidentally copying or assigning
1584+
RuntimeTrapDiagnosticBuilder &
1585+
operator=(const RuntimeTrapDiagnosticBuilder &) = delete;
1586+
RuntimeTrapDiagnosticBuilder &
1587+
operator=(const RuntimeTrapDiagnosticBuilder &&) = delete;
1588+
RuntimeTrapDiagnosticBuilder(const RuntimeTrapDiagnosticBuilder &) = delete;
1589+
RuntimeTrapDiagnosticBuilder(const RuntimeTrapDiagnosticBuilder &&) = delete;
1590+
1591+
/// \return Format the trap message and return it. Note the lifetime of
1592+
/// the underlying storage pointed to by the returned StringRef is the same
1593+
/// as the lifetime of this class. This means it is likely unsafe to store
1594+
/// the returned StringRef.
1595+
StringRef getMessage();
1596+
/// \return Return the trap category. These are the `CategoryName` property
1597+
/// of `trap` diagnostics declared in `DiagnosticTrapKinds.td`.
1598+
StringRef getCategory();
1599+
1600+
private:
1601+
llvm::SmallVector<char, 64> MessageStorage;
1602+
};
1603+
15371604
//===----------------------------------------------------------------------===//
15381605
// Diagnostic
15391606
//===----------------------------------------------------------------------===//

clang/include/clang/Basic/Diagnostic.td

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ def CLASS_REMARK : DiagClass;
3030
def CLASS_WARNING : DiagClass;
3131
def CLASS_EXTENSION : DiagClass;
3232
def CLASS_ERROR : DiagClass;
33+
def CLASS_TRAP : DiagClass;
3334

3435
// Responses to a diagnostic in a SFINAE context.
3536
class SFINAEResponse;
@@ -144,7 +145,8 @@ class Extension<string str> : Diagnostic<str, CLASS_EXTENSION, SEV_Ignored>;
144145
class ExtWarn<string str> : Diagnostic<str, CLASS_EXTENSION, SEV_Warning>;
145146
// Notes can provide supplementary information on errors, warnings, and remarks.
146147
class Note<string str> : Diagnostic<str, CLASS_NOTE, SEV_Fatal/*ignored*/>;
147-
148+
// Traps messages attached to traps in debug info
149+
class Trap<string str> : Diagnostic<str, CLASS_TRAP, SEV_Fatal/*ignored*/>;
148150

149151
class DefaultIgnore { Severity DefaultSeverity = SEV_Ignored; }
150152
class DefaultWarn { Severity DefaultSeverity = SEV_Warning; }
@@ -235,3 +237,4 @@ include "DiagnosticParseKinds.td"
235237
include "DiagnosticRefactoringKinds.td"
236238
include "DiagnosticSemaKinds.td"
237239
include "DiagnosticSerializationKinds.td"
240+
include "DiagnosticTrapKinds.td"

clang/include/clang/Basic/DiagnosticIDs.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ enum {
4747
DIAG_SIZE_ANALYSIS = 100,
4848
DIAG_SIZE_REFACTORING = 1000,
4949
DIAG_SIZE_INSTALLAPI = 100,
50+
DIAG_SIZE_TRAP = 100,
5051
};
5152
// Start position for diagnostics.
5253
// clang-format off
@@ -64,7 +65,8 @@ enum {
6465
DIAG_START_ANALYSIS = DIAG_START_SEMA + static_cast<int>(DIAG_SIZE_SEMA),
6566
DIAG_START_REFACTORING = DIAG_START_ANALYSIS + static_cast<int>(DIAG_SIZE_ANALYSIS),
6667
DIAG_START_INSTALLAPI = DIAG_START_REFACTORING + static_cast<int>(DIAG_SIZE_REFACTORING),
67-
DIAG_UPPER_LIMIT = DIAG_START_INSTALLAPI + static_cast<int>(DIAG_SIZE_INSTALLAPI)
68+
DIAG_START_TRAP = DIAG_START_INSTALLAPI + static_cast<int>(DIAG_SIZE_INSTALLAPI),
69+
DIAG_UPPER_LIMIT = DIAG_START_TRAP + static_cast<int>(DIAG_SIZE_TRAP)
6870
};
6971
// clang-format on
7072

@@ -189,7 +191,8 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
189191
CLASS_REMARK = 0x02,
190192
CLASS_WARNING = 0x03,
191193
CLASS_EXTENSION = 0x04,
192-
CLASS_ERROR = 0x05
194+
CLASS_ERROR = 0x05,
195+
CLASS_TRAP = 0x06
193196
};
194197

195198
static bool IsCustomDiag(diag::kind Diag) {
@@ -363,6 +366,10 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
363366
///
364367
bool isExtensionDiag(unsigned DiagID, bool &EnabledByDefault) const;
365368

369+
bool isTrapDiag(unsigned DiagID) const {
370+
return getDiagClass(DiagID) == CLASS_TRAP;
371+
}
372+
366373
/// Given a group ID, returns the flag that toggles the group.
367374
/// For example, for Group::DeprecatedDeclarations, returns
368375
/// "deprecated-declarations".
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//===--- DiagnosticTrap.h - Diagnostics for trap instructions ---*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_BASIC_TRAP_H
10+
#define LLVM_CLANG_BASIC_TRAP_H
11+
12+
#include "clang/Basic/Diagnostic.h"
13+
#include "clang/Basic/DiagnosticTrapInterface.inc"
14+
15+
#endif
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//==--- DiagnosticTrapKinds.td - CodeGen Diagnostics -------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
//===----------------------------------------------------------------------===//
10+
// Trap Diagnostics
11+
//
12+
// These are diagnostics that are emitted into Debug Info, rather than to the
13+
// traditional consumers like the terminal. Their primary purpose is to make
14+
// debugging traps (e.g. `-fsanitize-trap=undefined`) easier by attaching
15+
// a trap category and reason to the trap instruction that tools like a debugger
16+
// can show.
17+
//===----------------------------------------------------------------------===//
18+
let Component = "Trap" in {
19+
let CategoryName = "Undefined Behavior Sanitizer" in {
20+
21+
def trap_ubsan_arith_overflow : Trap<
22+
"%select{unsigned|signed}0 integer "
23+
"%select{addition|subtraction|multiplication}1 overflow in %2">;
24+
25+
}
26+
}

clang/lib/Basic/Diagnostic.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,8 @@ void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) {
664664

665665
void DiagnosticsEngine::Report(Level DiagLevel, const Diagnostic &Info) {
666666
assert(DiagLevel != Ignored && "Cannot emit ignored diagnostics!");
667+
assert(!getDiagnosticIDs()->isTrapDiag(Info.getID()) &&
668+
"Trap diagnostics should not be consumed by the DiagnosticsEngine");
667669
Client->HandleDiagnostic(DiagLevel, Info);
668670
if (Client->IncludeInDiagnosticCounts()) {
669671
if (DiagLevel == Warning)
@@ -793,6 +795,35 @@ DiagnosticBuilder::DiagnosticBuilder(const DiagnosticBuilder &D)
793795
D.Clear();
794796
}
795797

798+
RuntimeTrapDiagnosticBuilder::RuntimeTrapDiagnosticBuilder(
799+
DiagnosticsEngine *DiagObj, unsigned DiagID)
800+
: DiagnosticBuilder(DiagObj, SourceLocation(), DiagID) {
801+
assert(DiagObj->getDiagnosticIDs()->isTrapDiag(DiagID));
802+
}
803+
804+
RuntimeTrapDiagnosticBuilder::~RuntimeTrapDiagnosticBuilder() {
805+
// Make sure that when `DiagnosticBuilder::~DiagnosticBuilder()`
806+
// calls `Emit()` that it does nothing.
807+
Clear();
808+
}
809+
810+
StringRef RuntimeTrapDiagnosticBuilder::getMessage() {
811+
if (MessageStorage.size() == 0) {
812+
// Render the Diagnostic
813+
Diagnostic Info(DiagObj, *this);
814+
Info.FormatDiagnostic(MessageStorage);
815+
}
816+
return StringRef(MessageStorage.data(), MessageStorage.size());
817+
}
818+
819+
StringRef RuntimeTrapDiagnosticBuilder::getCategory() {
820+
auto CategoryID =
821+
DiagObj->getDiagnosticIDs()->getCategoryNumberForDiag(DiagID);
822+
if (CategoryID == 0)
823+
return "";
824+
return DiagObj->getDiagnosticIDs()->getCategoryNameFromID(CategoryID);
825+
}
826+
796827
Diagnostic::Diagnostic(const DiagnosticsEngine *DO,
797828
const DiagnosticBuilder &DiagBuilder)
798829
: DiagObj(DO), DiagLoc(DiagBuilder.DiagLoc), DiagID(DiagBuilder.DiagID),

clang/lib/Basic/DiagnosticIDs.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ enum DiagnosticClass {
6969
CLASS_WARNING = DiagnosticIDs::CLASS_WARNING,
7070
CLASS_EXTENSION = DiagnosticIDs::CLASS_EXTENSION,
7171
CLASS_ERROR = DiagnosticIDs::CLASS_ERROR,
72+
CLASS_TRAP = DiagnosticIDs::CLASS_TRAP,
7273
};
7374

7475
struct StaticDiagInfoRec {
@@ -139,6 +140,7 @@ VALIDATE_DIAG_SIZE(SEMA)
139140
VALIDATE_DIAG_SIZE(ANALYSIS)
140141
VALIDATE_DIAG_SIZE(REFACTORING)
141142
VALIDATE_DIAG_SIZE(INSTALLAPI)
143+
VALIDATE_DIAG_SIZE(TRAP)
142144
#undef VALIDATE_DIAG_SIZE
143145
#undef STRINGIFY_NAME
144146

@@ -171,6 +173,7 @@ const StaticDiagInfoRec StaticDiagInfo[] = {
171173
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
172174
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
173175
#include "clang/Basic/DiagnosticInstallAPIKinds.inc"
176+
#include "clang/Basic/DiagnosticTrapKinds.inc"
174177
// clang-format on
175178
#undef DIAG
176179
};
@@ -214,6 +217,7 @@ CATEGORY(SEMA, CROSSTU)
214217
CATEGORY(ANALYSIS, SEMA)
215218
CATEGORY(REFACTORING, ANALYSIS)
216219
CATEGORY(INSTALLAPI, REFACTORING)
220+
CATEGORY(TRAP, INSTALLAPI)
217221
#undef CATEGORY
218222

219223
// Avoid out of bounds reads.

0 commit comments

Comments
 (0)