Page MenuHomePhabricator

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

Authored by petpav01 on Sep 10 2018, 5:32 AM.

Details

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().

Diff Detail

Repository
rL LLVM

Event Timeline

petpav01 created this revision.Sep 10 2018, 5:32 AM

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 updated this revision to Diff 166073.Sep 19 2018, 12:39 AM

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

rsmith accepted this revision.Oct 3 2018, 6:08 PM

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

This revision is now accepted and ready to land.Oct 3 2018, 6:08 PM

No worries, thanks.

This revision was automatically updated to reflect the committed changes.