From d677c3fc6199aa375808a6250517b118ba66b40a Mon Sep 17 00:00:00 2001
From: Roman Lebedev <lebedev.ri@gmail.com>
Date: Mon, 19 Nov 2018 19:56:43 +0000
Subject: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of
 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
---
 clang/lib/CodeGen/CGExprScalar.cpp | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

(limited to 'clang/lib/CodeGen/CGExprScalar.cpp')

diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 801f4a9fc1a..e7c63ac11e9 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -332,6 +332,13 @@ public:
         : 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,
@@ -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);
@@ -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();
-- 
cgit v1.2.3