[clang] f18233d - Fix -fsanitize=array-bound to treat T[0] union members as flexible array

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-03-18T15:47:24-07:00
New Revision: f18233dad4669eaf59ca996dca79beae8d364247

URL: https://github.com/llvm/llvm-project/commit/f18233dad4669eaf59ca996dca79beae8d364247
DIFF: https://github.com/llvm/llvm-project/commit/f18233dad4669eaf59ca996dca79beae8d364247.diff

LOG: Fix -fsanitize=array-bound to treat T[0] union members as flexible array
members regardless of whether they're the last member of the union.

Added: 
    

Modified: 
    clang/lib/CodeGen/CGExpr.cpp
    clang/test/CodeGen/bounds-checking.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 6457852f1b6d..671ada019cec 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -866,8 +866,12 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
 static bool isFlexibleArrayMemberExpr(const Expr *E) {
   // For compatibility with existing code, we treat arrays of length 0 or
   // 1 as flexible array members.
+  // FIXME: This is inconsistent with the warning code in SemaChecking. Unify
+  // the two mechanisms.
   const ArrayType *AT = E->getType()->castAsArrayTypeUnsafe();
   if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
+    // FIXME: Sema doesn't treat [1] as a flexible array member if the bound
+    // was produced by macro expansion.
     if (CAT->getSize().ugt(1))
       return false;
   } else if (!isa<IncompleteArrayType>(AT))
@@ -880,6 +884,10 @@ static bool isFlexibleArrayMemberExpr(const Expr *E) {
     // FIXME: If the base type of the member expr is not FD->getParent(),
     // this should not be treated as a flexible array member access.
     if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
+      // FIXME: Sema doesn't treat a T[1] union member as a flexible array
+      // member, only a T[0] or T[] member gets that treatment.
+      if (FD->getParent()->isUnion())
+        return true;
       RecordDecl::field_iterator FI(
           DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
       return ++FI == FD->getParent()->field_end();

diff  --git a/clang/test/CodeGen/bounds-checking.c b/clang/test/CodeGen/bounds-checking.c
index 2e6a08650dd9..15cef8c007a5 100644
--- a/clang/test/CodeGen/bounds-checking.c
+++ b/clang/test/CodeGen/bounds-checking.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -fsanitize=local-bounds -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fsanitize=local-bounds -fexperimental-new-pass-manager -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s
-// RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -fexperimental-new-pass-manager -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s --check-prefixes=CHECK,NONLOCAL
+// RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -fexperimental-new-pass-manager -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s --check-prefixes=CHECK,NONLOCAL
 //
 // REQUIRES: x86-registered-target
 
@@ -31,3 +31,21 @@ void f3() {
   // CHECK: call {{.*}} @llvm.trap
   a[2] = 1;
 }
+
+union U { int a[0]; int b[1]; int c[2]; };
+
+// CHECK-LABEL: define {{.*}} @f4
+int f4(union U *u, int i) {
+  // a and b are treated as flexible array members.
+  // CHECK-NOT: @llvm.trap
+  return u->a[i] + u->b[i];
+  // CHECK: }
+}
+
+// CHECK-LABEL: define {{.*}} @f5
+int f5(union U *u, int i) {
+  // c is not a flexible array member.
+  // NONLOCAL: call {{.*}} @llvm.trap
+  return u->c[i];
+  // CHECK: }
+}