https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/188340
>From f33e24dfe26a15ba6a58c658a182211ad6cbe329 Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Tue, 24 Mar 2026 13:06:48 -0700 Subject: [PATCH] [Clang] Add missing __ob_trap check for sign change Add a missing OBTrapInvolved check before EmitIntegerSignChangeCheck(). This is considered "missing" as a previous attempt [1] to properly add an __ob_trap backdoor missed this particular instance. This backdoor is needed because __ob_trap types are very picky about implicit conversions (including implicit sign change): ```c unsigned int __ob_trap big = 4294967295; (signed int)big; // should trap! ``` [1]: https://github.com/llvm/llvm-project/pull/185772 Signed-off-by: Justin Stitt <[email protected]> --- clang/lib/CodeGen/CGExprScalar.cpp | 53 +++++++++-------- .../CodeGen/overflow-behavior-types-scl.c | 17 ++++++ clang/test/CodeGen/overflow-behavior-types.c | 59 +++++++++++++++++-- 3 files changed, 100 insertions(+), 29 deletions(-) diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index d208b760396b6..a8dcf22992983 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -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, diff --git a/clang/test/CodeGen/overflow-behavior-types-scl.c b/clang/test/CodeGen/overflow-behavior-types-scl.c index 19022f083f7eb..76b7aaf5e3264 100644 --- a/clang/test/CodeGen/overflow-behavior-types-scl.c +++ b/clang/test/CodeGen/overflow-behavior-types-scl.c @@ -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; +} diff --git a/clang/test/CodeGen/overflow-behavior-types.c b/clang/test/CodeGen/overflow-behavior-types.c index 5c844d321e8ab..109888f707506 100644 --- a/clang/test/CodeGen/overflow-behavior-types.c +++ b/clang/test/CodeGen/overflow-behavior-types.c @@ -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); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
