Skip to content

Commit

Permalink
[clang][CodeGen] Implicit Conversion Sanitizer: discover the world of…
Browse files Browse the repository at this point in the history
… CompoundAssign operators

Summary:
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)
```

However, the pre/post increment/decrement is still not handled.

Reviewers: rsmith, regehr, vsk, rjmccall, #sanitizers

Reviewed By: rjmccall

Subscribers: mclow.lists, cfe-commits, regehr

Tags: #clang, #sanitizers

Differential Revision: https://reviews.llvm.org/D53949

llvm-svn: 347258
  • Loading branch information
LebedevRI committed Nov 19, 2018
1 parent 238533e commit d677c3f
Show file tree
Hide file tree
Showing 5 changed files with 7,875 additions and 10 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ Undefined Behavior Sanitizer (UBSan)
is enabled by ``-fsanitize=integer``.
(as is ``-fsanitize=implicit-integer-truncation`` check)

* The Implicit Conversion Sanitizer (``-fsanitize=implicit-conversion``) has
learned to sanitize compound assignment operators.

Core Analysis Improvements
==========================

Expand Down
23 changes: 13 additions & 10 deletions clang/lib/CodeGen/CGExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,13 @@ class ScalarExprEmitter
: 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,
Expand Down Expand Up @@ -2191,13 +2198,8 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
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);
Expand Down Expand Up @@ -2866,9 +2868,10 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
// 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();
Expand Down
Loading

0 comments on commit d677c3f

Please sign in to comment.