[constexpr] Fix ICE when memcpy() is given a pointer to an incomplete array

[petpav01]

Summary

Trying to compile the following example results in a clang crash:

$ cat char.c
void *memcpy(void *, const void *, unsigned int);
extern char array2[];
extern char array[];
void test(void) { memcpy(&array, &array2, 9 * sizeof(char)); }

$ ./llvm/build/bin/clang -target armv8a-none-eabi -c char.c
clang-8: /work/llvm/lib/Support/APInt.cpp:1785: static void llvm::APInt::udivrem(const llvm::APInt&, uint64_t, llvm::APInt&, uint64_t&): Assertion `RHS != 0 && "Divide by zero?"' failed.
[...]

The problem occurs since rC338941. The change added support for constant evaluation of __builtin_memcpy/memmove() but it does not always cope well with incomplete types.

The AST for the memcpy() call looks as follows:

CallExpr 0x7416d0 'void *'
|-ImplicitCastExpr 0x7416b8 'void *(*)(void *, const void *, unsigned int)' <FunctionToPointerDecay>
| `-DeclRefExpr 0x741518 'void *(void *, const void *, unsigned int)' Function 0x741198 'memcpy' 'void *(void *, const void *, unsigned int)'
|-ImplicitCastExpr 0x741710 'void *' <BitCast>
| `-UnaryOperator 0x741598 'char (*)[]' prefix '&' cannot overflow
|   `-DeclRefExpr 0x741540 'char []' lvalue Var 0x741358 'array' 'char []'
|-ImplicitCastExpr 0x741728 'const void *' <BitCast>
| `-UnaryOperator 0x7415e0 'char (*)[]' prefix '&' cannot overflow
|   `-DeclRefExpr 0x7415b8 'char []' lvalue Var 0x741298 'array2' 'char []'
`-BinaryOperator 0x741668 'unsigned int' '*'
  |-ImplicitCastExpr 0x741650 'unsigned int' <IntegralCast>
  | `-IntegerLiteral 0x741600 'int' 9
  `-UnaryExprOrTypeTraitExpr 0x741630 'unsigned int' sizeof 'char'

The following happens in PointerExprEvaluator::VisitBuiltinCallExpr(), label Builtin::BI__builtin_memcpy:

• Types T and SrcT are determined as:

IncompleteArrayType 0x741250 'char []'
`-BuiltinType 0x6ff430 'char'

• Method ASTContext::getTypeSizeInChars() is called to obtain size of type T. It returns 0 because the type is incomplete. The result is stored in variable TSize.
• Following call to llvm::APInt::udivrem(OrigN, TSize, N, Remainder) fails because it attempts a divide by zero.

The proposed patch fixes the problem by adding a check that no incomplete type is getting copied prior to the call to ASTContext::getTypeSizeInChars().

[rsmith]

Thank you!

include/clang/Basic/DiagnosticASTKinds.td

172 ↗

(On Diff #164656)

Nit: add an underscore between incomplete and type.

lib/AST/ExprConstant.cpp

6225–6228 ↗

(On Diff #164656)

Please reorder this before the trivial-copyability check. We don't know whether a type is trivially-copyable if it's not complete, so it makes more sense to check these things in the opposite order. Testcase:

struct A; extern A x, y; constexpr void f() { __builtin_memcpy(&x, &y, 4); }

... currently produces a bogus diagnostic about A not being trivially-copyable but instead should produce your new diagnostic that A is incomplete.

[petpav01]

Thanks for having a look at this patch. Updated version addresses the review comments.

[rsmith]

Looks good. Thanks again, and sorry for the delay.

[petpav01]

No worries, thanks.

cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td

def note_constexpr_memcpy_null : Note<
   "'%select{%select{memcpy|wmemcpy}1|%select{memmove|wmemmove}1}0' "
   "is %3">;
 def note_constexpr_memcpy_type_pun : Note<
   "cannot constant evaluate '%select{memcpy|memmove}0' from object of "
   "type %1 to object of type %2">;
 def note_constexpr_memcpy_nontrivial : Note<
   "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
   "non-trivially-copyable type %1">;
+def note_constexpr_memcpy_incomplete_type : Note<
+  "cannot constant evaluate '%select{memcpy|memmove}0' between objects of "
+  "incomplete type %1">;
 def note_constexpr_memcpy_overlap : Note<
   "'%select{memcpy|wmemcpy}0' between overlapping memory regions">;
 def note_constexpr_memcpy_unsupported : Note<
   "'%select{%select{memcpy|wmemcpy}1|%select{memmove|wmemmove}1}0' "
   "not supported: %select{"
   "size to copy (%4) is not a multiple of size of element type %3 (%5)|"
   "source is not a contiguous array of at least %4 elements of type %3|"
   "destination is not a contiguous array of at least %4 elements of type %3}2">;

cfe/trunk/lib/AST/ExprConstant.cpp

case Builtin::BI__builtin_wmemmove: {
     // trivially-copyable type. (For the wide version, the designator will be
     // invalid if the designated object is not a wchar_t.)
     QualType T = Dest.Designator.getType(Info.Ctx);
     QualType SrcT = Src.Designator.getType(Info.Ctx);
     if (!Info.Ctx.hasSameUnqualifiedType(T, SrcT)) {
       Info.FFDiag(E, diag::note_constexpr_memcpy_type_pun) << Move << SrcT << T;
       return false;
     }
+    if (T->isIncompleteType()) {
+      Info.FFDiag(E, diag::note_constexpr_memcpy_incomplete_type) << Move << T;
+      return false;
+    }
     if (!T.isTriviallyCopyableType(Info.Ctx)) {
       Info.FFDiag(E, diag::note_constexpr_memcpy_nontrivial) << Move << T;
       return false;
     }
 
     // Figure out how many T's we're copying.
     uint64_t TSize = Info.Ctx.getTypeSizeInChars(T).getQuantity();
     if (!WChar) {

cfe/trunk/test/CodeGen/builtin-memfns.c

 void test11() {
   typedef struct { int a; } b;
   int d;
   b e;
   // CHECK: call void @llvm.memcpy{{.*}}(
   memcpy(&d, (char *)&e.a, sizeof(e));
 }
 
+// CHECK-LABEL: @test12
+extern char dest_array[];
+extern char src_array[];
+void test12() {
+  // CHECK: call void @llvm.memcpy{{.*}}(
+  memcpy(&dest_array, &dest_array, 2);
+}

cfe/trunk/test/SemaCXX/constexpr-string.cpp

constexpr int test_derived_to_base(int n) {
     return arr[0].a * 1000 + arr[0].b * 100 + arr[1].a * 10 + arr[1].b;
   }
   static_assert(test_derived_to_base(0) == 1234);
   static_assert(test_derived_to_base(1) == 3234);
   // FIXME: We could consider making this work by stripping elements off both
   // designators until we have a long enough matching size, if both designators
   // point to the start of their respective final elements.
   static_assert(test_derived_to_base(2) == 3434); // expected-error {{constant}} expected-note {{in call}}
+
+  // Check that when address-of an array is passed to a tested function the
+  // array can be fully copied.
+  constexpr int test_address_of_const_array_type() {
+    int arr[4] = {1, 2, 3, 4};
+    __builtin_memmove(&arr, &arr, sizeof(arr));
+    return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+  }
+  static_assert(test_address_of_const_array_type() == 1234);
+
+  // Check that an incomplete array is rejected.
+  constexpr int test_incomplete_array_type() { // expected-error {{never produces a constant}}
+    extern int arr[];
+    __builtin_memmove(arr, arr, 4 * sizeof(arr[0]));
+    // expected-note@-1 2{{'memmove' not supported: source is not a contiguous array of at least 4 elements of type 'int'}}
+    return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+  }
+  static_assert(test_incomplete_array_type() == 1234); // expected-error {{constant}} expected-note {{in call}}
+
+  // Check that a pointer to an incomplete array is rejected.
+  constexpr int test_address_of_incomplete_array_type() { // expected-error {{never produces a constant}}
+    extern int arr[];
+    __builtin_memmove(&arr, &arr, 4 * sizeof(arr[0]));
+    // expected-note@-1 2{{cannot constant evaluate 'memmove' between objects of incomplete type 'int []'}}
+    return arr[0] * 1000 + arr[1] * 100 + arr[2] * 10 + arr[3];
+  }
+  static_assert(test_address_of_incomplete_array_type() == 1234); // expected-error {{constant}} expected-note {{in call}}
+
+  // Check that a pointer to an incomplete struct is rejected.
+  constexpr bool test_address_of_incomplete_struct_type() { // expected-error {{never produces a constant}}
+    struct Incomplete;
+    extern Incomplete x, y;
+    __builtin_memcpy(&x, &x, 4);
+    // expected-note@-1 2{{cannot constant evaluate 'memcpy' between objects of incomplete type 'Incomplete'}}
+    return true;
+  }
+  static_assert(test_address_of_incomplete_struct_type()); // expected-error {{constant}} expected-note {{in call}}
 }