topic Re: memcmp() in CW is broken in CodeWarrior for MCU

HC12: memcmp() in CW is broken

pittbull — Tue, 24 Oct 2006 21:48:03 GMT

Hello,
I'm using CW for S12X (IDE version 5.7.0.1714).
The memcmp() function does not work correctly.

Trying the below program on a Windows PC I get:
PASS!
PASS!
Code:

int my_memcmp_impl (void *s1, void *s2, unsigned int n){unsigned char *cs1 = s1;unsigned char *cs2 = s2;for (;n-- > 0; cs1++, cs2++)if (*cs1 != *cs2)return *cs1 - *cs2;return 0;}void test_memcmp (void){static unsigned char snonce[] = {0x0b, 0x97, 0x39, 0x0f, 0x37, 0x51, 0x78, 0x09, 0x81, 0x1e, 0xfd, 0x9c, 0x6e, 0x65, 0x94, 0x2b, 0x63, 0x2c, 0xe9, 0x53, 0x89, 0x38, 0x08, 0xba, 0x36, 0x0b, 0x03, 0x7c, 0xd1, 0x85, 0xe4, 0x14, };static unsigned char anonce[] = {0xd8, 0xa4, 0x80, 0x43, 0xb8, 0x37, 0x74, 0x02, 0xb9, 0x6a, 0xff, 0x49, 0xa5, 0x08, 0x9e, 0x93, 0x29, 0x37, 0x03, 0x47, 0x0e, 0xee, 0xa3, 0x17, 0xb4, 0x74, 0x30, 0xc6, 0xa6, 0x8d, 0x75, 0x2e, };debug_printf ("Testing memcmp (CW):\n");if (memcmp(snonce, anonce, sizeof(snonce)) > 0)debug_printf ("!!!! FAIL !!!!\n");elsedebug_printf ("PASS!\n");debug_printf ("Testing memcmp (my implementation):\n");if (my_memcmp_impl(snonce, anonce, sizeof(snonce)) > 0)debug_printf ("!!!! FAIL !!!!\n");elsedebug_printf ("PASS!\n");debug_printf ("\n");}

the same program running on an S12X target gives me
!!!! FAIL !!!!
PASS!

Message Edited by Alban on 2006-10-26 11:37 AM

Message Edited by CrasyCat on 2007-04-13 01:29 PM

Re: memcmp() in CW is broken

CrasyCat — Wed, 25 Oct 2006 16:23:01 GMT

Hello

You have defined cs1 and cs2 as pointer to unsigned char.

When you are doing the operation *cs1 - *cs2, the operation is performed on unsigned value (no sign extension done prior to performing the operation).

If you define cs1 and cs2 as pointer to signed char, you get the expected FAIL message.

I hope this helps.

CrasyCat

Re: memcmp() in CW is broken

Lundin — Thu, 26 Oct 2006 13:55:58 GMT

Or simply write

return (int)*cs1 - (int)*cs2;

Re: memcmp() in CW is broken

pittbull — Thu, 26 Oct 2006 18:18:48 GMT

Hello,

From the C spec:
-----------------
7.21.4 Comparison functions
1 The sign of a nonzero value returned by the comparison functions memcmp, strcmp,
and strncmp is determined by the sign of the difference between the values of the first
pair of characters (both interpreted as unsigned char) that differ in the objects being
compared.

As you can see, my function does the right thing but the CW library uses 'signed' comparison (which is wrong according to the ISO/IEC document

Re: memcmp() in CW is broken

Sten — Thu, 26 Oct 2006 19:31:54 GMT

I agree with pittbull, it seems that CW does the memcmp() wrong.

I did a test on CW S12X v. 4.5 and when comparing two strings beginning with 0x0b and 0xd8, memcmp returns 0x33 which is a positive integer suggesting that 0x0b > 0xd8.

The right return value would have been 0xff35.

Message Edited by Sten on 2006-10-2603:33 PM

Re: memcmp() in CW is broken

CrasyCat — Thu, 26 Oct 2006 20:34:17 GMT

Hello

OK in that case submit a service request though our on line support web site.

This way the defect will be processed appropriately by our support team.

CrasyCat

Re: memcmp() in CW is broken

pittbull — Fri, 27 Oct 2006 02:08:34 GMT

Hello CrasyCat,

CrasyCat wrote:
OK in that case submit a service request though our on line support web site....

Too late for me

This bad 'memcmp' caused a bug in a cryptographic function that was very hard to track down, but I already fixed it.

Nevertheless, I hope that one of the Freescale compiler folks find this thread and correct the library.

Cheers,

-> pittbull

Re: memcmp() in CW is broken

Lundin — Fri, 27 Oct 2006 14:58:23 GMT

I assume that you have this enabled:

Edit->Project settings->Type sizes-> char = signed

In ANSI C, the char type or may not be signed by default. On the other hand, one would have to assume that Metrowerks have adapted their function to suit the compiler's default char type.

Re: memcmp() in CW is broken

pittbull — Sat, 28 Oct 2006 01:19:10 GMT

Lundin wrote:
In ANSI C, the char type or may not be signed by default.

Yes, it's implementation-defined but 'memcmp' must treat the input as unsigned...