[PATCH] D83502: Change behavior with zero-sized static array extents
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 9 13:00:18 PDT 2020
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, echristo, dblaikie, rjmccall.
Currently, Clang diagnoses this code by default: `void f(int a[static 0]);` saying that "static has no effect on zero-length arrays" and this diagnostic is accurate for our implementation.
However, static array extents require that the caller of the function pass a nonnull pointer to an array of *at least* that number of elements, but it can pass more (see C17 6.7.6.3p6). Given that we allow zero-sized arrays as a GNU extension and that it's valid to pass more elements than specified by the static array extent, I think we should support zero-sized static array extents with the usual semantics because it can be useful, as pointed out to me on Twitter (https://twitter.com/rep_stosq_void/status/1280892279998291973):
void my_bzero(char p[static 0], int n);
my_bzero(&c+1, 0); //ok
my_bzero(t+k,n-k); //ok, pattern from actual code
https://reviews.llvm.org/D83502
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CodeGen/vla.c
clang/test/Sema/static-array.c
Index: clang/test/Sema/static-array.c
===================================================================
--- clang/test/Sema/static-array.c
+++ clang/test/Sema/static-array.c
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only -fblocks -pedantic -verify %s
-void cat0(int a[static 0]) {} // expected-warning {{'static' has no effect on zero-length arrays}}
+void cat0(int a[static 0]) {} // expected-warning {{zero size arrays are an extension}} \
+ // expected-note {{callee declares array parameter as static here}}
void cat(int a[static 3]) {} // expected-note 4 {{callee declares array parameter as static here}} expected-note 2 {{passing argument to parameter 'a' here}}
@@ -9,7 +10,7 @@
void f(int *p) {
int a[2], b[3], c[4];
- cat0(0);
+ cat0(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
cat(0); // expected-warning {{null passed to a callee that requires a non-null argument}}
cat(a); // expected-warning {{array argument is too small; contains 2 elements, callee requires at least 3}}
Index: clang/test/CodeGen/vla.c
===================================================================
--- clang/test/CodeGen/vla.c
+++ clang/test/CodeGen/vla.c
@@ -206,3 +206,6 @@
// NULL-INVALID: define void @test9(i32 %n, i32* nonnull %a)
// NULL-VALID: define void @test9(i32 %n, i32* %a)
+// Make sure a zero-sized static array extent is still required to be nonnull.
+void test10(int a[static 0]) {}
+// CHECK: define void @test10(i32* nonnull %a)
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2393,13 +2393,6 @@
? diag::err_typecheck_zero_array_size
: diag::ext_typecheck_zero_array_size)
<< ArraySize->getSourceRange();
-
- if (ASM == ArrayType::Static) {
- Diag(ArraySize->getBeginLoc(),
- diag::warn_typecheck_zero_static_array_size)
- << ArraySize->getSourceRange();
- ASM = ArrayType::Normal;
- }
} else if (!T->isDependentType() && !T->isVariablyModifiedType() &&
!T->isIncompleteType() && !T->isUndeducedType()) {
// Is the array too large?
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2500,12 +2500,20 @@
if (ArrTy->getSizeModifier() == ArrayType::Static) {
QualType ETy = ArrTy->getElementType();
uint64_t ArrSize = ArrTy->getSize().getZExtValue();
- if (!ETy->isIncompleteType() && ETy->isConstantSizeType() &&
- ArrSize) {
- llvm::AttrBuilder Attrs;
- Attrs.addDereferenceableAttr(
- getContext().getTypeSizeInChars(ETy).getQuantity()*ArrSize);
- AI->addAttrs(Attrs);
+ if (!ETy->isIncompleteType() && ETy->isConstantSizeType()) {
+ // If the type is complete and we know the array size is non-
+ // zero, then we know the dereferencable range of memory.
+ // Otherwise, if the array size is zero, we only know that the
+ // memory must be nonnull.
+ if (ArrSize) {
+ llvm::AttrBuilder Attrs;
+ Attrs.addDereferenceableAttr(
+ getContext().getTypeSizeInChars(ETy).getQuantity() *
+ ArrSize);
+ AI->addAttrs(Attrs);
+ } else {
+ AI->addAttr(llvm::Attribute::NonNull);
+ }
} else if (getContext().getTargetAddressSpace(ETy) == 0 &&
!CGM.getCodeGenOpts().NullPointerIsValid) {
AI->addAttr(llvm::Attribute::NonNull);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5395,9 +5395,6 @@
"zero size arrays are an extension">, InGroup<ZeroLengthArray>;
def err_typecheck_zero_array_size : Error<
"zero-length arrays are not permitted in C++">;
-def warn_typecheck_zero_static_array_size : Warning<
- "'static' has no effect on zero-length arrays">,
- InGroup<ArrayBounds>;
def err_array_size_non_int : Error<"size of array has non-integer type %0">;
def err_init_element_not_constant : Error<
"initializer element is not a compile-time constant">;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83502.276809.patch
Type: text/x-patch
Size: 4812 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200709/6915a586/attachment-0001.bin>
More information about the cfe-commits
mailing list