Skip to content

Commit c1ba3a6

Browse files
committed
[NFC][BoundsSafety] Adopt TrapReason in CodeGenFunction::EmitBoundsSafetyTrapCheck
This is the first step in adopting the new trap reasons infrastructure. This patch introduces the `trap_bs_fallback` diagnostic and this just uses the existing infrastructure for computing trap reason reasons. Thus there is no user visible change in behavior with this patch. Future patches will start introducing new diagnostics so more specific trap reason messages can be created. rdar://158623471
1 parent 2fb7ea2 commit c1ba3a6

File tree

4 files changed

+51
-22
lines changed

4 files changed

+51
-22
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//==--- DiagnosticBoundsSafetyTrapKinds.td --------------------------------===//
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+
// These are the trap diagnostics used by -fbounds-safety
9+
//===----------------------------------------------------------------------===//
10+
11+
let Component = "Trap" in {
12+
let CategoryName = "Bounds check failed" in {
13+
14+
// Used for falling back to the legacy infrastructure for constructing
15+
// trap reason strings.
16+
def trap_bs_fallback : Trap<"%0">;
17+
18+
}
19+
}

clang/include/clang/Basic/DiagnosticTrapKinds.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,7 @@ def trap_ubsan_arith_overflow : Trap<
2828

2929
}
3030
}
31+
32+
// TO_UPSTREAM(BoundsSafety) ON
33+
include "DiagnosticBoundsSafetyTrapKinds.td"
34+
// TO_UPSTREAM(BoundsSafety) OFF

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "clang/AST/StmtVisitor.h"
3737
#include "clang/Basic/Builtins.h"
3838
#include "clang/Basic/CodeGenOptions.h"
39+
#include "clang/Basic/DiagnosticTrap.h" // TO_UPSTREAM(BoundsSafety)
3940
#include "clang/Basic/Module.h"
4041
#include "clang/Basic/SourceManager.h"
4142
#include "llvm/ADT/STLExtras.h"
@@ -4855,8 +4856,7 @@ void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
48554856
void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
48564857
SanitizerHandler CheckHandlerID,
48574858
bool NoMerge, const TrapReason *TR,
4858-
StringRef Annotation,
4859-
StringRef BoundsSafetyTrapMessage) {
4859+
StringRef Annotation) {
48604860
llvm::BasicBlock *Cont = createBasicBlock("cont");
48614861

48624862
// If we're optimizing, collapse all calls to trap down to just one per
@@ -4870,26 +4870,24 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
48704870
llvm::StringRef TrapMessage;
48714871
llvm::StringRef TrapCategory;
48724872
auto DebugTrapReasonKind = CGM.getCodeGenOpts().getSanitizeDebugTrapReasons();
4873+
4874+
/* TO_UPSTREAM(BoundsSafety) ON*/
48734875
if (TR && !TR->isEmpty() &&
4874-
DebugTrapReasonKind ==
4875-
CodeGenOptions::SanitizeDebugTrapReasonKind::Detailed) {
4876+
(DebugTrapReasonKind ==
4877+
CodeGenOptions::SanitizeDebugTrapReasonKind::Detailed ||
4878+
CheckHandlerID == SanitizerHandler::BoundsSafety)) {
48764879
TrapMessage = TR->getMessage();
48774880
TrapCategory = TR->getCategory();
48784881
} else {
4879-
/* TO_UPSTREAM(BoundsSafety) ON*/
4880-
// FIXME: Move to using `TrapReason` (rdar://158623471).
4881-
if (CheckHandlerID == SanitizerHandler::BoundsSafety) {
4882-
TrapMessage = BoundsSafetyTrapMessage;
4883-
TrapCategory = GetBoundsSafetyTrapMessagePrefix();
4884-
} else {
4885-
TrapMessage = GetUBSanTrapForHandler(CheckHandlerID);
4886-
TrapCategory = "Undefined Behavior Sanitizer";
4887-
}
4882+
assert(CheckHandlerID != SanitizerHandler::BoundsSafety);
4883+
TrapMessage = GetUBSanTrapForHandler(CheckHandlerID);
4884+
TrapCategory = "Undefined Behavior Sanitizer";
48884885
}
48894886

48904887
if (getDebugInfo() && !(TrapMessage.empty() && TrapCategory.empty()) &&
4891-
DebugTrapReasonKind !=
4892-
CodeGenOptions::SanitizeDebugTrapReasonKind::None &&
4888+
(DebugTrapReasonKind !=
4889+
CodeGenOptions::SanitizeDebugTrapReasonKind::None ||
4890+
CheckHandlerID == SanitizerHandler::BoundsSafety) &&
48934891
TrapLocation) {
48944892
TrapLocation = getDebugInfo()->CreateTrapFailureMessageFor(
48954893
TrapLocation, TrapCategory, TrapMessage);
@@ -4979,19 +4977,26 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
49794977
EmitBlock(Cont);
49804978
}
49814979

4982-
void CodeGenFunction::EmitBoundsSafetyTrapCheck(llvm::Value *Checked,
4983-
BoundsSafetyTrapKind kind,
4984-
BoundsSafetyTrapCtx::Kind TrapCtx) {
4980+
void CodeGenFunction::EmitBoundsSafetyTrapCheck(
4981+
llvm::Value *Checked, BoundsSafetyTrapKind kind,
4982+
BoundsSafetyTrapCtx::Kind TrapCtx, TrapReason *TR) {
49854983
auto OptRemark = GetBoundsSafetyOptRemarkForTrap(kind);
49864984
assert(BoundsSafetyOptRemarkScope::InScope(this, OptRemark));
49874985

4986+
// Fallback: If a TrapReason object isn't provided use the legacy approach
4987+
// for constructing
4988+
TrapReason TempTR;
4989+
if (!TR) {
4990+
CGM.BuildTrapReason(diag::trap_bs_fallback, TempTR)
4991+
<< GetBoundsSafetyTrapMessageSuffix(kind, TrapCtx);
4992+
}
4993+
49884994
// We still need to pass `OptRemark` because not all emitted instructions
49894995
// can be covered by BoundsSafetyOptRemarkScope. This is because EmitTrapCheck
49904996
// caches basic blocks that contain instructions that need annotating.
49914997
EmitTrapCheck(Checked, SanitizerHandler::BoundsSafety,
49924998
/*NoMerge=*/CGM.getCodeGenOpts().BoundsSafetyUniqueTraps,
4993-
/*TR=*/nullptr, GetBoundsSafetyOptRemarkString(OptRemark),
4994-
GetBoundsSafetyTrapMessageSuffix(kind, TrapCtx));
4999+
TR ? TR : &TempTR, GetBoundsSafetyOptRemarkString(OptRemark));
49955000
}
49965001

49975002
llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5425,7 +5425,7 @@ class CodeGenFunction : public CodeGenTypeCache {
54255425
/// conditional branch to it, for the -ftrapv checks.
54265426
void EmitTrapCheck(llvm::Value *Checked, SanitizerHandler CheckHandlerID,
54275427
bool NoMerge = false, const TrapReason *TR = nullptr,
5428-
StringRef Annotation = "", StringRef TrapMessage = "");
5428+
StringRef Annotation = "");
54295429

54305430
/* TO_UPSTREAM(BoundsSafety) ON*/
54315431
/// Create a basic block that will call the trap intrinsic for -fbounds-safety, and
@@ -5435,7 +5435,8 @@ class CodeGenFunction : public CodeGenTypeCache {
54355435
/// must be in scope when this method is called.
54365436
void EmitBoundsSafetyTrapCheck(
54375437
llvm::Value *Checked, BoundsSafetyTrapKind kind,
5438-
BoundsSafetyTrapCtx::Kind TrapCtx = BoundsSafetyTrapCtx::UNKNOWN);
5438+
BoundsSafetyTrapCtx::Kind TrapCtx = BoundsSafetyTrapCtx::UNKNOWN,
5439+
TrapReason *TR = nullptr);
54395440
/* TO_UPSTREAM(BoundsSafety) OFF*/
54405441

54415442
/// Emit a call to trap or debugtrap and attach function attribute

0 commit comments

Comments
 (0)