[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

[Adam Magier via Phabricator via cfe-commits]

AdamMagierFOSS updated this revision to Diff 399057.
AdamMagierFOSS added a comment.

Updating based on feedback from rjmccall.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116048/new/

https://reviews.llvm.org/D116048

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/catch-undef-behavior.c


Index: clang/test/CodeGen/catch-undef-behavior.c
===================================================================
--- clang/test/CodeGen/catch-undef-behavior.c
+++ clang/test/CodeGen/catch-undef-behavior.c
@@ -17,6 +17,7 @@
 // CHECK-UBSAN: @[[LINE_700:.*]] = {{.*}}, i32 700, i32 14 {{.*}} @[[STRUCT_S]], i8 2, i8 3 }
 // CHECK-UBSAN: @[[LINE_800:.*]] = {{.*}}, i32 800, i32 12 {{.*}} @{{.*}} }
 // CHECK-UBSAN: @[[LINE_900:.*]] = {{.*}}, i32 900, i32 11 {{.*}} @{{.*}} }
+// CHECK-UBSAN: @[[LINE_1000:.*]] = {{.*}}, i32 1000, i32 11 {{.*}} @{{.*}} }
 // CHECK-UBSAN: @[[FP16:.*]] = private unnamed_addr constant { i16, i16, [9 x i8] } { i16 1, i16 16, [9 x i8] c"'__fp16'\00" }
 // CHECK-UBSAN: @[[LINE_1200:.*]] = {{.*}}, i32 1200, i32 10 {{.*}} @{{.*}} }
 // CHECK-UBSAN: @[[LINE_1300:.*]] = {{.*}}, i32 1300, i32 10 {{.*}} @{{.*}} }
@@ -186,6 +187,16 @@
   int arr[n * 3];
 }
 
+// CHECK-UBSAN-LABEL: @vla_bound_unsigned
+void vla_bound_unsigned(unsigned int n) {
+  // CHECK-UBSAN:      icmp ugt i32 %[[PARAM:.*]], 0
+  //
+  // CHECK-UBSAN:      %[[ARG:.*]] = zext i32 %[[PARAM]] to i64
+  // CHECK-UBSAN-NEXT: call void @__ubsan_handle_vla_bound_not_positive(i8* bitcast ({{.*}} @[[LINE_1000]] to i8*), i64 %[[ARG]])
+#line 1000
+  int arr[n * 3];
+}
+
 // CHECK-UBSAN-LABEL: @int_float_no_overflow
 float int_float_no_overflow(__int128 n) {
   // CHECK-UBSAN-NOT: call void @__ubsan_handle
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2247,32 +2247,35 @@
 
       // Unknown size indication requires no size computation.
       // Otherwise, evaluate and record it.
-      if (const Expr *size = vat->getSizeExpr()) {
+      if (const Expr *sizeExpr = vat->getSizeExpr()) {
         // It's possible that we might have emitted this already,
         // e.g. with a typedef and a pointer to it.
-        llvm::Value *&entry = VLASizeMap[size];
+        llvm::Value *&entry = VLASizeMap[sizeExpr];
         if (!entry) {
-          llvm::Value *Size = EmitScalarExpr(size);
+          llvm::Value *size = EmitScalarExpr(sizeExpr);
+          clang::QualType sizeExprType = sizeExpr->getType();
 
           // C11 6.7.6.2p5:
           //   If the size is an expression that is not an integer constant
           //   expression [...] each time it is evaluated it shall have a value
           //   greater than zero.
-          if (SanOpts.has(SanitizerKind::VLABound) &&
-              size->getType()->isSignedIntegerType()) {
+          if (SanOpts.has(SanitizerKind::VLABound)) {
             SanitizerScope SanScope(this);
-            llvm::Value *Zero = llvm::Constant::getNullValue(Size->getType());
+            llvm::Value *Zero = llvm::Constant::getNullValue(size->getType());
+            llvm::Value *CheckCondition =
+                sizeExprType->isSignedIntegerType()
+                    ? Builder.CreateICmpSGT(size, Zero)
+                    : Builder.CreateICmpUGT(size, Zero);
             llvm::Constant *StaticArgs[] = {
-                EmitCheckSourceLocation(size->getBeginLoc()),
-                EmitCheckTypeDescriptor(size->getType())};
-            EmitCheck(std::make_pair(Builder.CreateICmpSGT(Size, Zero),
-                                     SanitizerKind::VLABound),
-                      SanitizerHandler::VLABoundNotPositive, StaticArgs, Size);
+                EmitCheckSourceLocation(sizeExpr->getBeginLoc()),
+                EmitCheckTypeDescriptor(sizeExprType)};
+            EmitCheck(std::make_pair(CheckCondition, SanitizerKind::VLABound),
+                      SanitizerHandler::VLABoundNotPositive, StaticArgs, size);
           }
 
           // Always zexting here would be wrong if it weren't
           // undefined behavior to have a negative bound.
-          entry = Builder.CreateIntCast(Size, SizeTy, /*signed*/ false);
+          entry = Builder.CreateIntCast(size, SizeTy, /*signed*/ false);
         }
       }
       type = vat->getElementType();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116048.399057.patch
Type: text/x-patch
Size: 4080 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220111/33a75ea5/attachment.bin>