[clang] Fix crash during template sema checking

[gchatelet]

Summary

If the size parameter of __builtin_memcpy_inline comes from an un-instantiated template parameter current code would crash.

[gchatelet]

Buggy trace here

[courbet]

clang/lib/Sema/SemaChecking.cpp

1655

Are the first two necessary ? IIRC only EvaluateKnownConstInt() does not like template-dependent stuff.

[gchatelet]

Address comments

clang/lib/Sema/SemaChecking.cpp

1655

That's right I don't think it's realistic to pass the pointers as template parameters so whatever CheckNonNullArgument is doing it will probably never end up crashing in the same way.

I'll limit the fix to SizeOp then.

[efriedma]

Please post patches against master, not previous versions of the patch.

I think you want to check isValueDependent(); isInstantiationDependent() includes some other stuff that isn't relevant here.

[gchatelet]

Address comments

[efriedma]

LGTM

clang/lib/Sema/SemaChecking.cpp

1655

"<<"?

clang/lib/Sema/SemaChecking.cpp

Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   case Builtin::BI__sync_synchronize:
     Diag(TheCall->getBeginLoc(), diag::warn_atomic_implicit_seq_cst)
         << TheCall->getCallee()->getSourceRange();
     break;
   case Builtin::BI__builtin_nontemporal_load:
   case Builtin::BI__builtin_nontemporal_store:
     return SemaBuiltinNontemporalOverloaded(TheCallResult);
   case Builtin::BI__builtin_memcpy_inline: {
-    // __builtin_memcpy_inline size argument is a constant by definition.
-    if (TheCall->getArg(2)->EvaluateKnownConstInt(Context).isNullValue())
+    clang::Expr *SizeOp = TheCall->getArg(2);
+    // We warn about copying to or from `nullptr` pointers when `size` is
+    // greater than 0. When `size` is value dependent we cannot evaluate its
+    // value so we bail out.
+    if (SizeOp->isValueDependent())
       break;
+    if (!SizeOp->EvaluateKnownConstInt(Context).isNullValue()) {
       CheckNonNullArgument(*this, TheCall->getArg(0), TheCall->getExprLoc());
       CheckNonNullArgument(*this, TheCall->getArg(1), TheCall->getExprLoc());
+    }
     break;
   }
 #define BUILTIN(ID, TYPE, ATTRS)
 #define ATOMIC_BUILTIN(ID, TYPE, ATTRS) \
   case Builtin::BI##ID: \
     return SemaAtomicOpsOverloaded(TheCallResult, AtomicExpr::AO##ID);
 #include "clang/Basic/Builtins.def"
   case Builtin::BI__annotation:

clang/test/Sema/builtins-memcpy-inline.cpp

void test_memcpy_inline_null_buffer_is_ok_if_size_is_zero(void *ptr) {
   __builtin_memcpy_inline(ptr, NULL, /*size */ 0);
   __builtin_memcpy_inline(NULL, ptr, /*size */ 0);
   __builtin_memcpy_inline(NULL, NULL, /*size */ 0);
 }
 
 void test_memcpy_inline_non_constant_size(void *dst, const void *src, unsigned size) {
   __builtin_memcpy_inline(dst, src, size); // expected-error {{argument to '__builtin_memcpy_inline' must be a constant integer}}
 }
+
+template <unsigned size>
+void test_memcpy_inline_template(void *dst, const void *src) {
+  // we do not try to evaluate size in non intantiated templates.
+  __builtin_memcpy_inline(dst, src, size);
+}