[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [cfe-commits] [Review] [ubsan] Fix type reported in compound assignment operations
From: Will Dietz <wdietz2 () uiuc ! edu>
Date: 2012-12-30 22:13:25
Message-ID: CAKGWAO8YGGMOaX0C93-yi65=fLqqfyVyMMJd+oePC8j++nCfiw () mail ! gmail ! com
[Download RAW message or body]
Thanks for the feedback, updated patches attached. Moved regression
test to clang, and also fix similar issue with "/=" using the wrong
type.
On Sun, Dec 30, 2012 at 2:48 AM, Richard Smith <richard@metafoo.co.uk> wrote:
> - StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.E->getType()));
> + QualType ResTy = Info.E->getType();
> + if (const CompoundAssignOperator *C =
> + dyn_cast<CompoundAssignOperator>(Info.E))
> + ResTy = C->getComputationResultType();
> + StaticData.push_back(CGF.EmitCheckTypeDescriptor(ResTy));
>
> Could you just use Info.Ty here?
Good call, much better.
>
> Please add a test for Clang's test suite too. The tests in compiler-rt
> are intended to test compiler-rt itself (the formatting of the
> diagnostics, etc), not for testing that Clang produces correct data
> blocks (in particular, we should have sufficient coverage that we can
> refactor Clang's IRGen without running the compiler-rt tests).
>
Understood, thanks for the explanation. Makes good sense.
> On Sun, Dec 30, 2012 at 12:13 AM, Will Dietz <wdietz2@uiuc.edu> wrote:
>> See attached patches, thanks!
>>
>> Description:
>>
>> When checking "a += b" we were using the type of 'a' in the
>> diagnostic, instead of the type of the overflowing expression "a+b".
>> This was particularly problematic when 'a' was signed and 'b' was
>> unsigned.
>>
>> Okay to commit?
>>
>> ~Will
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits@cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
["0001-ubsan-Use-correct-type-for-compound-assignment-ops.patch" (application/octet-stream)]
From f00cc17f92d255f1a53f3fa74bc680a8e0a0ad04 Mon Sep 17 00:00:00 2001
From: Will Dietz <w@wdtz.org>
Date: Sun, 30 Dec 2012 00:39:15 -0600
Subject: [PATCH] [ubsan] Use correct type for compound assignment ops.
---
lib/CodeGen/CGExprScalar.cpp | 4 +-
test/CodeGen/compound-assign-overflow.c | 36 +++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 2 deletions(-)
create mode 100644 test/CodeGen/compound-assign-overflow.c
diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index 61126e1..0bf8547 100644
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -833,7 +833,7 @@ void ScalarExprEmitter::EmitBinOpCheck(Value *Check, const \
BinOpInfo &Info) { } else if (Opcode == BO_Div || Opcode == BO_Rem) {
// Divide or modulo by zero, or signed overflow (eg INT_MAX / -1).
CheckName = "divrem_overflow";
- StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.E->getType()));
+ StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty));
} else {
// Signed arithmetic overflow (+, -, *).
switch (Opcode) {
@@ -842,7 +842,7 @@ void ScalarExprEmitter::EmitBinOpCheck(Value *Check, const \
BinOpInfo &Info) { case BO_Mul: CheckName = "mul_overflow"; break;
default: llvm_unreachable("unexpected opcode for bin op check");
}
- StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.E->getType()));
+ StaticData.push_back(CGF.EmitCheckTypeDescriptor(Info.Ty));
}
DynamicData.push_back(Info.LHS);
DynamicData.push_back(Info.RHS);
diff --git a/test/CodeGen/compound-assign-overflow.c \
b/test/CodeGen/compound-assign-overflow.c new file mode 100644
index 0000000..a178e9b
--- /dev/null
+++ b/test/CodeGen/compound-assign-overflow.c
@@ -0,0 +1,36 @@
+// Verify proper type emitted for compound assignments
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s \
-fsanitize=signed-integer-overflow,unsigned-integer-overflow | FileCheck %s +
+#include <stdint.h>
+
+// CHECK: @[[INT:.*]] = private unnamed_addr constant { i16, i16, [6 x i8] } { i16 \
0, i16 11, [6 x i8] c"'int'\00" } +// CHECK: @[[LINE_100:.*]] = private unnamed_addr \
constant {{.*}}, i32 100, i32 5 {{.*}} @[[INT]] +// CHECK: @[[UINT:.*]] = private \
unnamed_addr constant { i16, i16, [15 x i8] } { i16 0, i16 10, [15 x i8] c"'unsigned \
int'\00" } +// CHECK: @[[LINE_200:.*]] = private unnamed_addr constant {{.*}}, i32 \
200, i32 5 {{.*}} @[[UINT]] +// CHECK: @[[DIVINT:.*]] = private unnamed_addr constant \
{ i16, i16, [6 x i8] } { i16 0, i16 11, [6 x i8] c"'int'\00" } +// CHECK: \
@[[LINE_300:.*]] = private unnamed_addr constant {{.*}}, i32 300, i32 5 {{.*}} \
@[[DIVINT]] +
+int32_t x;
+
+// CHECK: @compaddsigned
+void compaddsigned() {
+#line 100
+ x += ((int32_t)1);
+ // CHECK: @__ubsan_handle_add_overflow(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), \
{{.*}}) +}
+
+// CHECK: @compaddunsigned
+void compaddunsigned() {
+#line 200
+ x += ((uint32_t)1U);
+ // CHECK: @__ubsan_handle_add_overflow(i8* bitcast ({{.*}} @[[LINE_200]] to i8*), \
{{.*}}) +}
+
+int8_t a, b;
+
+// CHECK: @compdiv
+void compdiv() {
+#line 300
+ a /= b;
+ // CHECK: @__ubsan_handle_divrem_overflow(i8* bitcast ({{.*}} @[[LINE_300]] to \
i8*), {{.*}}) +}
--
1.7.1
["0001-ubsan-Check-for-appropriate-types-on-compound-assign.patch" (application/octet-stream)]
From c4d01c1af748a0425e07e16c78ab5bc5b9af175d Mon Sep 17 00:00:00 2001
From: Will Dietz <w@wdtz.org>
Date: Sun, 30 Dec 2012 01:43:13 -0600
Subject: [PATCH] [ubsan] Check for appropriate types on compound assignment overflow \
diagnostics.
---
lib/ubsan/lit_tests/Integer/add-overflow.cpp | 2 +-
lib/ubsan/lit_tests/Integer/no-recover.cpp | 4 ++--
lib/ubsan/lit_tests/Integer/uadd-overflow.cpp | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/ubsan/lit_tests/Integer/add-overflow.cpp \
b/lib/ubsan/lit_tests/Integer/add-overflow.cpp index 4477638..8054352 100644
--- a/lib/ubsan/lit_tests/Integer/add-overflow.cpp
+++ b/lib/ubsan/lit_tests/Integer/add-overflow.cpp
@@ -13,7 +13,7 @@ int main() {
#ifdef ADD_I32
int32_t k = 0x12345678;
k += 0x789abcde;
- // CHECK-ADD_I32: add-overflow.cpp:[[@LINE-1]]:5: runtime error: signed integer \
overflow: 305419896 + 2023406814 cannot be represented in type 'int32_t' (aka 'int') \
+ // CHECK-ADD_I32: add-overflow.cpp:[[@LINE-1]]:5: runtime error: signed integer \
overflow: 305419896 + 2023406814 cannot be represented in type 'int' #endif
#ifdef ADD_I64
diff --git a/lib/ubsan/lit_tests/Integer/no-recover.cpp \
b/lib/ubsan/lit_tests/Integer/no-recover.cpp index b25d440..e200fea 100644
--- a/lib/ubsan/lit_tests/Integer/no-recover.cpp
+++ b/lib/ubsan/lit_tests/Integer/no-recover.cpp
@@ -13,8 +13,8 @@ int main() {
uint32_t k = 0x87654321;
k += 0xedcba987;
- // RECOVER: no-recover.cpp:[[@LINE-1]]:5: runtime error: unsigned integer \
overflow: 2271560481 + 3989547399 cannot be represented in type 'uint32_t' (aka \
'unsigned int')
- // ABORT: no-recover.cpp:[[@LINE-2]]:5: runtime error: unsigned integer overflow: \
2271560481 + 3989547399 cannot be represented in type 'uint32_t' (aka 'unsigned int') \
+ // RECOVER: no-recover.cpp:[[@LINE-1]]:5: runtime error: unsigned integer \
overflow: 2271560481 + 3989547399 cannot be represented in type 'unsigned int' + // \
ABORT: no-recover.cpp:[[@LINE-2]]:5: runtime error: unsigned integer overflow: \
2271560481 + 3989547399 cannot be represented in type 'unsigned int'
(void)(uint64_t(10000000000000000000ull) + uint64_t(9000000000000000000ull));
// RECOVER: 10000000000000000000 + 9000000000000000000 cannot be represented in \
type 'unsigned long'
diff --git a/lib/ubsan/lit_tests/Integer/uadd-overflow.cpp \
b/lib/ubsan/lit_tests/Integer/uadd-overflow.cpp index d7b43d0..0edb100 100644
--- a/lib/ubsan/lit_tests/Integer/uadd-overflow.cpp
+++ b/lib/ubsan/lit_tests/Integer/uadd-overflow.cpp
@@ -13,7 +13,7 @@ int main() {
#ifdef ADD_I32
uint32_t k = 0x87654321;
k += 0xedcba987;
- // CHECK-ADD_I32: uadd-overflow.cpp:[[@LINE-1]]:5: runtime error: unsigned integer \
overflow: 2271560481 + 3989547399 cannot be represented in type 'uint32_t' (aka \
'unsigned int') + // CHECK-ADD_I32: uadd-overflow.cpp:[[@LINE-1]]:5: runtime error: \
unsigned integer overflow: 2271560481 + 3989547399 cannot be represented in type \
'unsigned int' #endif
#ifdef ADD_I64
--
1.7.1
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic