lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, regehr, vsk, rjmccall, Sanitizers.
lebedev.ri added projects: clang, Sanitizers.
Not yet for an //actual// review, needs tests.
As reported by @regehr (thanks!) on twitter
(https://twitter.com/johnregehr/status/1057681496255815686),
we (me) has completely forgot about the binary assignment operator.
In AST, it isn't represented as separate `ImplicitCastExpr`'s,
but as a single `CompoundAssignOperator`, that does all the casts internally.
Which means, out of these two, only the first one is diagnosed:
auto foo() {
unsigned char c = 255;
c = c + 1;
return c;
}
auto bar() {
unsigned char c = 255;
c += 1;
return c;
}
https://godbolt.org/z/JNyVc4
This patch does handle the `CompoundAssignOperator`:
int main() {
unsigned char c = 255;
c += 1;
return c;
}
$ ./bin/clang -g -fsanitize=integer /tmp/test.c && ./a.out
/tmp/test.c:3:5: runtime error: implicit conversion from type 'int' of value
256 (32-bit, signed) to type 'unsigned char' changed the value to 0 (8-bit,
unsigned)
#0 0x2392b8 in main /tmp/test.c:3:5
#1 0x7fec4a612b16 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x22b16)
#2 0x214029 in _start (/build/llvm-build-GCC-release/a.out+0x214029)
Repository:
rC Clang
https://reviews.llvm.org/D53949
Files:
lib/CodeGen/CGExprScalar.cpp
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -332,6 +332,13 @@
: TreatBooleanAsSigned(false),
EmitImplicitIntegerTruncationChecks(false),
EmitImplicitIntegerSignChangeChecks(false) {}
+
+ ScalarConversionOpts(clang::SanitizerSet SanOpts)
+ : TreatBooleanAsSigned(false),
+ EmitImplicitIntegerTruncationChecks(
+ SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation)),
+ EmitImplicitIntegerSignChangeChecks(
+ SanOpts.has(SanitizerKind::ImplicitIntegerSignChange)) {}
};
Value *
EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
@@ -2198,13 +2205,8 @@
case CK_IntegralCast: {
ScalarConversionOpts Opts;
if (auto *ICE = dyn_cast<ImplicitCastExpr>(CE)) {
- if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitConversion) &&
- !ICE->isPartOfExplicitCast()) {
- Opts.EmitImplicitIntegerTruncationChecks =
- CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation);
- Opts.EmitImplicitIntegerSignChangeChecks =
- CGF.SanOpts.has(SanitizerKind::ImplicitIntegerSignChange);
- }
+ if (!ICE->isPartOfExplicitCast())
+ Opts = ScalarConversionOpts(CGF.SanOpts);
}
return EmitScalarConversion(Visit(E), E->getType(), DestTy,
CE->getExprLoc(), Opts);
@@ -2872,9 +2874,10 @@
// Expand the binary operator.
Result = (this->*Func)(OpInfo);
- // Convert the result back to the LHS type.
- Result =
- EmitScalarConversion(Result, E->getComputationResultType(), LHSTy, Loc);
+ // Convert the result back to the LHS type,
+ // potentially with Implicit Conversion sanitizer check.
+ Result = EmitScalarConversion(Result, E->getComputationResultType(), LHSTy,
+ Loc, ScalarConversionOpts(CGF.SanOpts));
if (atomicPHI) {
llvm::BasicBlock *opBB = Builder.GetInsertBlock();
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -332,6 +332,13 @@
: TreatBooleanAsSigned(false),
EmitImplicitIntegerTruncationChecks(false),
EmitImplicitIntegerSignChangeChecks(false) {}
+
+ ScalarConversionOpts(clang::SanitizerSet SanOpts)
+ : TreatBooleanAsSigned(false),
+ EmitImplicitIntegerTruncationChecks(
+ SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation)),
+ EmitImplicitIntegerSignChangeChecks(
+ SanOpts.has(SanitizerKind::ImplicitIntegerSignChange)) {}
};
Value *
EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
@@ -2198,13 +2205,8 @@
case CK_IntegralCast: {
ScalarConversionOpts Opts;
if (auto *ICE = dyn_cast<ImplicitCastExpr>(CE)) {
- if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitConversion) &&
- !ICE->isPartOfExplicitCast()) {
- Opts.EmitImplicitIntegerTruncationChecks =
- CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitIntegerTruncation);
- Opts.EmitImplicitIntegerSignChangeChecks =
- CGF.SanOpts.has(SanitizerKind::ImplicitIntegerSignChange);
- }
+ if (!ICE->isPartOfExplicitCast())
+ Opts = ScalarConversionOpts(CGF.SanOpts);
}
return EmitScalarConversion(Visit(E), E->getType(), DestTy,
CE->getExprLoc(), Opts);
@@ -2872,9 +2874,10 @@
// Expand the binary operator.
Result = (this->*Func)(OpInfo);
- // Convert the result back to the LHS type.
- Result =
- EmitScalarConversion(Result, E->getComputationResultType(), LHSTy, Loc);
+ // Convert the result back to the LHS type,
+ // potentially with Implicit Conversion sanitizer check.
+ Result = EmitScalarConversion(Result, E->getComputationResultType(), LHSTy,
+ Loc, ScalarConversionOpts(CGF.SanOpts));
if (atomicPHI) {
llvm::BasicBlock *opBB = Builder.GetInsertBlock();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
