[Clang] Add missing __ob_trap check for sign change (#188340)

Add a missing OBTrapInvolved check before EmitIntegerSignChangeCheck().

This is considered "missing" as a previous attempt (https://github.com/llvm/llvm-project/pull/185772) to properly add an `__ob_trap` backdoor missed this particular instance.

This backdoor is needed because we want `__ob_trap` types to be picky about implicit conversions (including implicit sign change):

```c
	unsigned int __ob_trap big = 4294967295;
	(signed int)big; // should trap!
```

Move the `OBTrapInvolved` setup to the top of the function so it can be used in all the places we need it.
This commit is contained in:
Justin Stitt 2026-04-02 10:51:46 -07:00 committed by GitHub
parent 8e61085291
commit 43233b8aae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 100 additions and 29 deletions

View File

@ -1355,14 +1355,18 @@ void ScalarExprEmitter::EmitIntegerSignChangeCheck(Value *Src, QualType SrcType,
return;
}
// Does an SSCL have an entry for the DstType under its respective sanitizer
// section?
if (DstSigned && CGF.getContext().isTypeIgnoredBySanitizer(
SanitizerKind::ImplicitSignedIntegerTruncation, DstType))
return;
if (!DstSigned &&
CGF.getContext().isTypeIgnoredBySanitizer(
SanitizerKind::ImplicitUnsignedIntegerTruncation, DstType))
return;
// section? Don't check this if an __ob_trap type is involved as it has
// priority to emit checks regardless of sanitizer case lists.
if (!OBTrapInvolved) {
if (DstSigned &&
CGF.getContext().isTypeIgnoredBySanitizer(
SanitizerKind::ImplicitSignedIntegerTruncation, DstType))
return;
if (!DstSigned &&
CGF.getContext().isTypeIgnoredBySanitizer(
SanitizerKind::ImplicitUnsignedIntegerTruncation, DstType))
return;
}
// That's it. We can't rule out any more cases with the data we have.
auto CheckHandler = SanitizerHandler::ImplicitConversion;
@ -1670,6 +1674,20 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
llvm::Type *DstTy = ConvertType(DstType);
// Determine whether an overflow behavior of 'trap' has been specified for
// either the destination or the source types. If so, we can elide sanitizer
// capability checks as this overflow behavior kind is also capable of
// emitting traps without runtime sanitizer support.
// Also skip instrumentation if either source or destination has 'wrap'
// behavior - the user has explicitly indicated they accept wrapping
// semantics. Use non-canonical types to preserve OBT annotations.
const auto *DstOBT = NoncanonicalDstType->getAs<OverflowBehaviorType>();
const auto *SrcOBT = NoncanonicalSrcType->getAs<OverflowBehaviorType>();
bool OBTrapInvolved =
(DstOBT && DstOBT->isTrapKind()) || (SrcOBT && SrcOBT->isTrapKind());
bool OBWrapInvolved =
(DstOBT && DstOBT->isWrapKind()) || (SrcOBT && SrcOBT->isWrapKind());
// Cast from half through float if half isn't a native type.
if (SrcType->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
// Cast to FP using the intrinsic if the half type itself isn't supported.
@ -1696,9 +1714,10 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
// Ignore conversions like int -> uint.
if (SrcTy == DstTy) {
if (Opts.EmitImplicitIntegerSignChangeChecks)
if (Opts.EmitImplicitIntegerSignChangeChecks ||
(OBTrapInvolved && !OBWrapInvolved))
EmitIntegerSignChangeCheck(Src, NoncanonicalSrcType, Src,
NoncanonicalDstType, Loc);
NoncanonicalDstType, Loc, OBTrapInvolved);
return Src;
}
@ -1829,20 +1848,6 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
}
}
// Determine whether an overflow behavior of 'trap' has been specified for
// either the destination or the source types. If so, we can elide sanitizer
// capability checks as this overflow behavior kind is also capable of
// emitting traps without runtime sanitizer support.
// Also skip instrumentation if either source or destination has 'wrap'
// behavior - the user has explicitly indicated they accept wrapping
// semantics. Use non-canonical types to preserve OBT annotations.
const auto *DstOBT = NoncanonicalDstType->getAs<OverflowBehaviorType>();
const auto *SrcOBT = NoncanonicalSrcType->getAs<OverflowBehaviorType>();
bool OBTrapInvolved =
(DstOBT && DstOBT->isTrapKind()) || (SrcOBT && SrcOBT->isTrapKind());
bool OBWrapInvolved =
(DstOBT && DstOBT->isWrapKind()) || (SrcOBT && SrcOBT->isWrapKind());
if ((Opts.EmitImplicitIntegerTruncationChecks || OBTrapInvolved) &&
!OBWrapInvolved && !Opts.PatternExcluded)
EmitIntegerTruncationCheck(Src, NoncanonicalSrcType, Res,

View File

@ -11,6 +11,10 @@
// RUN: -fexperimental-overflow-behavior-types -fsanitize=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation \
// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=TRUNC
// RUN: %clang_cc1 -triple x86_64-linux-gnu %t/test.c -fsanitize-ignorelist=%t/sign-change.scl \
// RUN: -fexperimental-overflow-behavior-types -fsanitize=implicit-integer-sign-change \
// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=SIGNCHG
//--- sio.scl
[signed-integer-overflow]
# ignore signed-integer-overflow instrumentation across all types
@ -25,6 +29,10 @@ type:*
[{implicit-unsigned-integer-truncation,implicit-signed-integer-truncation}]
type:*
//--- sign-change.scl
[implicit-integer-sign-change]
type:*
//--- test.c
#define __wrap __attribute__((overflow_behavior("wrap")))
#define __no_trap __attribute__((overflow_behavior("trap")))
@ -62,3 +70,12 @@ void bar(int value) {
// TRUNC-NEXT: br i1 %[[TCHECK]], {{.*}}%handler.implicit_conversion
unsigned char __ob_trap a = value;
}
// SIGNCHG-LABEL: define {{.*}} @obt_priority_over_scl_for_sign_change_sanitizer
void obt_priority_over_scl_for_sign_change_sanitizer(unsigned int __ob_trap a) {
// SIGNCHG: %[[T0:.*]] = load i32, ptr %a.addr
// SIGNCHG-NEXT: %[[NEG0:.*]] = icmp slt i32 %[[T0]], 0
// SIGNCHG-NEXT: %[[SIGN0:.*]] = icmp eq i1 false, %[[NEG0]]
// SIGNCHG-NEXT: br i1 %[[SIGN0]], {{.*}} %handler.implicit_conversion
(signed int)a;
}

View File

@ -175,9 +175,18 @@ int explicit_truncation_cast(__ob_trap unsigned long long result) {
return (int)result;
}
// EXCL-LABEL: define {{.*}} @pattern_exclusion_priority_over_obt
void pattern_exclusion_priority_over_obt(unsigned char __ob_trap count) {
// EXCL: while.cond
// EXCL: br i1 %tobool, label %while.body
// EXCL: br label %while.cond
while (count--) {}
}
// Make sure __ob_trap types warn on sign change with
// -fsanitize=implicit-integer-sign-change or trap otherwise.
// DEFAULT-LABEL: define {{.*}} @unsigned_to_signed_cast
// NOSAN-LABEL: define {{.*}} @unsigned_to_signed_cast
void unsigned_to_signed_cast(__ob_trap unsigned long long a) {
// DEFAULT: %[[T0:.*]] = load i64, ptr %a.addr
// DEFAULT-NEXT: %[[CONV0:.*]] = trunc i64 %[[T0]] to i8
@ -195,9 +204,49 @@ void unsigned_to_signed_cast(__ob_trap unsigned long long a) {
(signed char)(a);
}
// EXCL-LABEL: define {{.*}} @pattern_exclusion_priority_over_obt
void pattern_exclusion_priority_over_obt(unsigned char __ob_trap count) {
// EXCL: br label %while.cond
// EXCL-NOT: %truncheck
while (count--) {}
// DEFAULT-LABEL: define {{.*}} @signed_to_unsigned_cast
// NOSAN-LABEL: define {{.*}} @signed_to_unsigned_cast
void signed_to_unsigned_cast(__ob_trap signed long long a) {
// DEFAULT: %[[T0:.*]] = load i64, ptr %a.addr
// DEFAULT-NEXT: %[[CONV0:.*]] = trunc i64 %[[T0]] to i8
// DEFAULT: %[[TRUNC0:.*]] = icmp eq i64 {{.*}}, %[[T0]]
// DEFAULT-NEXT: br i1 %[[TRUNC0]], {{.*}} %handler.implicit_conversion
// NOSAN: %[[T0:.*]] = load i64, ptr %a.addr
// NOSAN-NEXT: %[[CONV0:.*]] = trunc i64 %[[T0]] to i8
// NOSAN: %[[TRUNC0:.*]] = icmp eq i64 {{.*}}, %[[T0]]
// NOSAN-NEXT: br i1 %[[TRUNC0]], {{.*}} %trap
(unsigned char)(a);
}
// DEFAULT-LABEL: define {{.*}} @unsigned_to_signed_cast_same_size
// NOSAN-LABEL: define {{.*}} @unsigned_to_signed_cast_same_size
void unsigned_to_signed_cast_same_size(__ob_trap unsigned int a) {
// DEFAULT: %[[T0:.*]] = load i32, ptr %a.addr
// DEFAULT-NEXT: %[[NEG:.*]] = icmp slt i32 %[[T0]], 0
// DEFAULT-NEXT: %[[SIGN0:.*]] = icmp eq i1 false, %[[NEG]]
// DEFUALT-NEXT: br i1 %[[SIGN0]], {{.*}}, label %handler.implicit_conversion
// NOSAN: %[[T0:.*]] = load i32, ptr %a.addr
// NOSAN-NEXT: %[[NEG:.*]] = icmp slt i32 %[[T0]], 0
// NOSAN-NEXT: %[[SIGN0:.*]] = icmp eq i1 false, %[[NEG]]
// NOSAN: %[[T1:.*]] = and i1 %[[SIGN0]]
// NOSAN-NEXT: br i1 %[[T1]], {{.*}}, label %trap
(signed int)(a);
}
// DEFAULT-LABEL: define {{.*}} @signed_to_unsigned_same_size
// NOSAN-LABEL: define {{.*}} @signed_to_unsigned_same_size
void signed_to_unsigned_same_size(__ob_trap signed int a) {
// DEFAULT: %[[T0:.*]] = load i32, ptr %a.addr
// DEFAULT-NEXT: %[[NEG:.*]] = icmp slt i32 %[[T0]], 0
// DEFAULT-NEXT: %[[SIGN0:.*]] = icmp eq i1 %[[NEG]], false
// DEFUALT-NEXT: br i1 %[[SIGN0]], {{.*}}, label %handler.implicit_conversion
// NOSAN: %[[T0:.*]] = load i32, ptr %a.addr
// NOSAN-NEXT: %[[NEG:.*]] = icmp slt i32 %[[T0]], 0
// NOSAN-NEXT: %[[SIGN0:.*]] = icmp eq i1 %[[NEG]], false
// NOSAN: %[[T1:.*]] = and i1 %[[SIGN0]]
// NOSAN-NEXT: br i1 %[[T1]], {{.*}}, label %trap
(unsigned int)(a);
}