[PATCH] Fix the argument index error of __builtin___memccpy_chk

[Steven Wu]

Add the testcase. The assertion failure only happens when size_t is not the same as int, so I add the test to builtin-object-size.c.
I can also add it to builtins.c as an alternative, it won’t assertion failed but it will give the wrong diagnostics. Let me know if the other way is better.

New patch:
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -365,7 +365,6 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   // check secure string manipulation functions where overflows
   // are detectable at compile time
   case Builtin::BI__builtin___memcpy_chk:
-  case Builtin::BI__builtin___memccpy_chk:
   case Builtin::BI__builtin___memmove_chk:
   case Builtin::BI__builtin___memset_chk:
   case Builtin::BI__builtin___strlcat_chk:
@@ -375,6 +374,9 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   case Builtin::BI__builtin___stpncpy_chk:
     SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3);
     break;
+  case Builtin::BI__builtin___memccpy_chk:
+    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4);
+    break;
   case Builtin::BI__builtin___snprintf_chk:
   case Builtin::BI__builtin___vsnprintf_chk:
     SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3);
diff --git a/test/Sema/builtin-object-size.c b/test/Sema/builtin-object-size.c
index e4b6560..b1bda06 100644
--- a/test/Sema/builtin-object-size.c
+++ b/test/Sema/builtin-object-size.c
@@ -43,3 +43,12 @@ void f5(void)
   memcpy((char *)NULL + 0x10000, buf, 0x10);
   memcpy1((char *)NULL + 0x10000, buf, 0x10); // expected-error {{argument should be a value from 0 to 3}}
 }
+
+// rdar://18431336
+void f6(void)
+{
+  char b[5];
+  char buf[10];
+  __builtin___memccpy_chk (buf, b, '\0', sizeof(b), __builtin_object_size (buf, 0));
+  __builtin___memccpy_chk (b, buf, '\0', sizeof(buf), __builtin_object_size (b, 0));  // expected-warning {{'__builtin___memccpy_chk' will always overflow destination buffer}}
+}

> On Sep 23, 2014, at 4:12 PM, jahanian <fjahanian at apple.com> wrote:
> 
> Ok. But please provide a test case. See builtin-object-size.c for some samples.
> 
> - Fariborz
> 
> 
> On Sep 23, 2014, at 4:03 PM, Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>> wrote:
> 
>> Fix an assertion failure in CheckBuiltinFunctionCall.
>> memccpy_chk should have source and dest size at arg 3 and 4.
>> 
>> ---
>> lib/Sema/SemaChecking.cpp | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
>> index 7462869..5ea862c 100644
>> --- a/lib/Sema/SemaChecking.cpp
>> +++ b/lib/Sema/SemaChecking.cpp
>> @@ -365,7 +365,6 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
>>   // check secure string manipulation functions where overflows
>>   // are detectable at compile time
>>   case Builtin::BI__builtin___memcpy_chk:
>> -  case Builtin::BI__builtin___memccpy_chk:
>>   case Builtin::BI__builtin___memmove_chk:
>>   case Builtin::BI__builtin___memset_chk:
>>   case Builtin::BI__builtin___strlcat_chk:
>> @@ -375,6 +374,9 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
>>   case Builtin::BI__builtin___stpncpy_chk:
>>     SemaBuiltinMemChkCall(*this, FDecl, TheCall, 2, 3);
>>     break;
>> +  case Builtin::BI__builtin___memccpy_chk:
>> +    SemaBuiltinMemChkCall(*this, FDecl, TheCall, 3, 4);
>> +    break;
>>   case Builtin::BI__builtin___snprintf_chk:
>>   case Builtin::BI__builtin___vsnprintf_chk:
>>     SemaBuiltinMemChkCall(*this, FDecl, TheCall, 1, 3);
>> 
>> <memccpy.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140923/99ada780/attachment.html>