[clang] 283e0a8 - [clang] Correct sanitizer behavior in union FAMs
Bill Wendling via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 20 16:08:25 PDT 2022
Author: Bill Wendling
Date: 2022-10-20T16:08:11-07:00
New Revision: 283e0a81ef35deec46aa231cb8b9d826060f532a
URL: https://github.com/llvm/llvm-project/commit/283e0a81ef35deec46aa231cb8b9d826060f532a
DIFF: https://github.com/llvm/llvm-project/commit/283e0a81ef35deec46aa231cb8b9d826060f532a.diff
LOG: [clang] Correct sanitizer behavior in union FAMs
Clang doesn't have the same behavior as GCC does with union flexible
array members. (Technically, union FAMs are probably not acceptable in
C99 and are an extension of GCC and Clang.)
Both Clang and GCC treat *all* arrays at the end of a structure as FAMs.
GCC does the same with unions. Clang does it for some arrays in unions
(incomplete, '0', and '1'), but not for all. Instead of having this
half-supported feature, sync Clang's behavior with GCC's.
Reviewed By: kees
Differential Revision: https://reviews.llvm.org/D135727
Added:
Modified:
clang/lib/AST/Expr.cpp
clang/test/CodeGen/bounds-checking-fam.c
clang/test/CodeGen/bounds-checking.c
clang/test/Sema/array-bounds-ptr-arith.c
Removed:
################################################################################
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 3fb780adc8bc7..739851c843996 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -210,20 +210,20 @@ bool Expr::isFlexibleArrayMemberLike(
// For compatibility with existing code, we treat arrays of length 0 or
// 1 as flexible array members.
- if (const auto *CAT = Context.getAsConstantArrayType(getType())) {
+ const auto *CAT = Context.getAsConstantArrayType(getType());
+ if (CAT) {
llvm::APInt Size = CAT->getSize();
// GCC extension, only allowed to represent a FAM.
if (Size == 0)
return true;
- // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
- // arrays to be treated as flexible-array-members, we still emit diagnostics
- // as if they are not. Pending further discussion...
using FAMKind = LangOptions::StrictFlexArraysLevelKind;
- if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete || Size.uge(2))
+ if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
return false;
+ if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
+ return false;
} else if (!Context.getAsIncompleteArrayType(getType()))
return false;
@@ -244,8 +244,13 @@ bool Expr::isFlexibleArrayMemberLike(
// 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>(ND)) {
- if (FD->getParent()->isUnion())
- return true;
+ // GCC treats an array memeber of a union as an FAM if the size is one or
+ // zero.
+ if (CAT) {
+ llvm::APInt Size = CAT->getSize();
+ if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
+ return true;
+ }
// Don't consider sizes resulting from macro expansions or template argument
// substitution to form C89 tail-padded arrays.
diff --git a/clang/test/CodeGen/bounds-checking-fam.c b/clang/test/CodeGen/bounds-checking-fam.c
index 1b4f183bee12b..2442e727e2ee4 100644
--- a/clang/test/CodeGen/bounds-checking-fam.c
+++ b/clang/test/CodeGen/bounds-checking-fam.c
@@ -9,16 +9,44 @@
// one-element array as the last member of a structure as an alternative.
// E.g. https://github.com/python/cpython/issues/84301
// Suppress such errors with -fstrict-flex-arrays=0.
+
+struct Incomplete {
+ int ignored;
+ int a[];
+};
+struct Zero {
+ int ignored;
+ int a[0];
+};
struct One {
+ int ignored;
int a[1];
};
struct Two {
+ int ignored;
int a[2];
};
struct Three {
+ int ignored;
int a[3];
};
+// CHECK-LABEL: define {{.*}} @{{.*}}test_incomplete{{.*}}(
+int test_incomplete(struct Incomplete *p, int i) {
+ // CHECK-STRICT-0-NOT: @__ubsan
+ // CHECK-STRICT-1-NOT: @__ubsan
+ // CHECK-STRICT-2-NOT: @__ubsan
+ return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_zero{{.*}}(
+int test_zero(struct Zero *p, int i) {
+ // CHECK-STRICT-0-NOT: @__ubsan
+ // CHECK-STRICT-1-NOT: @__ubsan
+ // CHECK-STRICT-2-NOT: @__ubsan
+ return p->a[i] + (p->a)[i];
+}
+
// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
int test_one(struct One *p, int i) {
// CHECK-STRICT-0-NOT: @__ubsan
@@ -29,7 +57,15 @@ int test_one(struct One *p, int i) {
// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
int test_two(struct Two *p, int i) {
- // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
+ // CHECK-STRICT-0-NOT: @__ubsan
+ // CHECK-STRICT-1: call void @__ubsan_handle_out_of_bounds_abort(
+ // CHECK-STRICT-2: call void @__ubsan_handle_out_of_bounds_abort(
+ return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
+int test_three(struct Three *p, int i) {
+ // CHECK-STRICT-0-NOT: @__ubsan
// CHECK-STRICT-1: call void @__ubsan_handle_out_of_bounds_abort(
// CHECK-STRICT-2: call void @__ubsan_handle_out_of_bounds_abort(
return p->a[i] + (p->a)[i];
@@ -60,29 +96,21 @@ int test_uzero(union uZero *p, int i) {
int test_uone(union uOne *p, int i) {
// CHECK-STRICT-0-NOT: @__ubsan
// CHECK-STRICT-1-NOT: @__ubsan
- // CHECK-STRICT-2: @__ubsan
+ // CHECK-STRICT-2: call void @__ubsan_handle_out_of_bounds_abort(
return p->a[i] + (p->a)[i];
}
// CHECK-LABEL: define {{.*}} @{{.*}}test_utwo{{.*}}(
int test_utwo(union uTwo *p, int i) {
- // CHECK-STRICT-0: @__ubsan
- // CHECK-STRICT-1: @__ubsan
- // CHECK-STRICT-2: @__ubsan
+ // CHECK-STRICT-0-NOT: @__ubsan
+ // CHECK-STRICT-1: call void @__ubsan_handle_out_of_bounds_abort(
+ // CHECK-STRICT-2: call void @__ubsan_handle_out_of_bounds_abort(
return p->a[i] + (p->a)[i];
}
// CHECK-LABEL: define {{.*}} @{{.*}}test_uthree{{.*}}(
int test_uthree(union uThree *p, int i) {
- // CHECK-STRICT-0: @__ubsan
- // CHECK-STRICT-1: @__ubsan
- // CHECK-STRICT-2: @__ubsan
- return p->a[i] + (p->a)[i];
-}
-
-// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
-int test_three(struct Three *p, int i) {
- // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
+ // CHECK-STRICT-0-NOT: @__ubsan
// CHECK-STRICT-1: call void @__ubsan_handle_out_of_bounds_abort(
// CHECK-STRICT-2: call void @__ubsan_handle_out_of_bounds_abort(
return p->a[i] + (p->a)[i];
@@ -96,6 +124,8 @@ struct Macro {
// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
int test_macro(struct Macro *p, int i) {
// CHECK-STRICT-0-NOT: @__ubsan
+ // CHECK-STRICT-1-NOT: @__ubsan
+ // CHECK-STRICT-2: call void @__ubsan_handle_out_of_bounds_abort(
return p->a[i] + (p->a)[i];
}
diff --git a/clang/test/CodeGen/bounds-checking.c b/clang/test/CodeGen/bounds-checking.c
index ca44adf5a1f27..1b9e28915e5d9 100644
--- a/clang/test/CodeGen/bounds-checking.c
+++ b/clang/test/CodeGen/bounds-checking.c
@@ -1,10 +1,10 @@
// RUN: %clang_cc1 -fsanitize=local-bounds -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 --check-prefixes=CHECK,NONLOCAL
+// RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s
//
// REQUIRES: x86-registered-target
-// CHECK-LABEL: @f
-double f(int b, int i) {
+// CHECK-LABEL: @f1
+double f1(int b, int i) {
double a[b];
// CHECK: call {{.*}} @llvm.{{(ubsan)?trap}}
return a[i];
@@ -31,29 +31,38 @@ void f3(void) {
a[2] = 1;
}
+// CHECK-LABEL: @f4
+__attribute__((no_sanitize("bounds")))
+int f4(int i) {
+ int b[64];
+ // CHECK-NOT: call void @llvm.trap()
+ // CHECK-NOT: trap:
+ // CHECK-NOT: cont:
+ return b[i];
+}
+
+// Union flexible-array memebers are a C99 extension. All array members with a
+// constant size should be considered FAMs.
+
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-LABEL: @f5
+int f5(union U *u, int i) {
+ // a is treated as a flexible array member.
// CHECK-NOT: @llvm.ubsantrap
- return u->a[i] + u->b[i];
- // CHECK: }
+ return u->a[i];
}
-// CHECK-LABEL: define {{.*}} @f5
-int f5(union U *u, int i) {
- // c is not a flexible array member.
- // NONLOCAL: call {{.*}} @llvm.ubsantrap
- return u->c[i];
- // CHECK: }
+// CHECK-LABEL: @f6
+int f6(union U *u, int i) {
+ // b is treated as a flexible array member.
+ // CHECK-NOT: call {{.*}} @llvm.{{(ubsan)?trap}}
+ return u->b[i];
}
-__attribute__((no_sanitize("bounds")))
-int f6(int i) {
- int b[64];
- // CHECK-NOT: call void @llvm.trap()
- // CHECK-NOT: trap:
- // CHECK-NOT: cont:
- return b[i];
+// CHECK-LABEL: @f7
+int f7(union U *u, int i) {
+ // c is treated as a flexible array member.
+ // CHECK-NOT: @llvm.ubsantrap
+ return u->c[i];
}
diff --git a/clang/test/Sema/array-bounds-ptr-arith.c b/clang/test/Sema/array-bounds-ptr-arith.c
index a76a8884120bc..974e420aa9e42 100644
--- a/clang/test/Sema/array-bounds-ptr-arith.c
+++ b/clang/test/Sema/array-bounds-ptr-arith.c
@@ -1,10 +1,12 @@
-// RUN: %clang_cc1 -verify=expected -Warray-bounds-pointer-arithmetic %s
-// RUN: %clang_cc1 -verify=expected -Warray-bounds-pointer-arithmetic %s -fstrict-flex-arrays=0
+// RUN: %clang_cc1 -verify=expected -Warray-bounds-pointer-arithmetic %s
+// RUN: %clang_cc1 -verify=expected -Warray-bounds-pointer-arithmetic %s -fstrict-flex-arrays=0
// RUN: %clang_cc1 -verify=expected,strict -Warray-bounds-pointer-arithmetic %s -fstrict-flex-arrays=2
// Test case from PR10615
struct ext2_super_block{
unsigned char s_uuid[8]; // expected-note {{declared here}}
+ int ignored; // Prevents "s_uuid" from being treated as a flexible array
+ // member.
};
void* ext2_statfs (struct ext2_super_block *es,int a) {
More information about the cfe-commits
mailing list