From 95584d3b3309ff4da4cc458254df383f5cff044b Mon Sep 17 00:00:00 2001 From: Liubov Dmitrieva Date: Sun, 23 Oct 2011 13:34:15 -0400 Subject: [PATCH] Fix signedness in wcscmp comparison --- ChangeLog | 11 +++ string/test-strcmp.c | 89 ++++++++++++++--- sysdeps/i386/i686/multiarch/wcscmp-sse2.S | 158 ++++++++++++++++-------------- sysdeps/x86_64/wcscmp.S | 109 ++++++++++++--------- wcsmbs/wcscmp.c | 4 +- 5 files changed, 239 insertions(+), 132 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5c234d5..c8f504c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2011-09-22 Liubov Dmitrieva + + * sysdeps/x86_64/wcscmp.S: Update. + Fix wrong comparison semantics. + wcscmp shall use signed comparison not unsigned. + Don't use substraction to avoid overflow bug. + * sysdeps/i386/i686/multiarch/wcscmp-sse2.S: Likewise. + * wcsmbc/wcscmp.c: Likewise. + * string/test-strcmp.c: Likewise. + Add new tests to check cases with negative values. + 2011-10-23 Ulrich Drepper * sysdeps/ieee754/dbl-64/dla.h: Move DLA_FMA definition to... diff --git a/string/test-strcmp.c b/string/test-strcmp.c index b8d85dc..0e43f85 100644 --- a/string/test-strcmp.c +++ b/string/test-strcmp.c @@ -1,4 +1,4 @@ -/* Test and measure STRCMP functions. +/* Test and measure strcmp and wcscmp functions. Copyright (C) 1999, 2002, 2003, 2005, 2011 Free Software Foundation, Inc. This file is part of the GNU C Library. Written by Jakub Jelinek , 1999. @@ -23,7 +23,6 @@ #include "test-string.h" #ifdef WIDE -# include # include # define L(str) L##str @@ -34,12 +33,53 @@ # define SIMPLE_STRCMP simple_wcscmp # define STUPID_STRCMP stupid_wcscmp # define CHAR wchar_t -# define UCHAR uint32_t +# define UCHAR wchar_t # define CHARBYTES 4 # define CHARBYTESLOG 2 # define CHARALIGN __alignof__ (CHAR) # define MIDCHAR 0x7fffffff # define LARGECHAR 0xfffffffe +# define CHAR__MAX WCHAR_MAX +# define CHAR__MIN WCHAR_MIN + +/* Wcscmp uses signed semantics for comparison, not unsigned */ +/* Avoid using substraction since possible overflow */ + +int +simple_wcscmp (const wchar_t *s1, const wchar_t *s2) +{ + wchar_t c1, c2; + do + { + c1 = *s1++; + c2 = *s2++; + if (c2 == L'\0') + return c1 - c2; + } + while (c1 == c2); + + return c1 < c2 ? -1 : 1; +} + +int +stupid_wcscmp (const wchar_t *s1, const wchar_t *s2) +{ + size_t ns1 = wcslen (s1) + 1; + size_t ns2 = wcslen (s2) + 1; + size_t n = ns1 < ns2 ? ns1 : ns2; + int ret = 0; + + wchar_t c1, c2; + + while (n--) { + c1 = *s1++; + c2 = *s2++; + if ((ret = c1 < c2 ? -1 : c1 == c2 ? 0 : 1) != 0) + break; + } + return ret; +} + #else # define L(str) str # define STRCMP strcmp @@ -55,31 +95,35 @@ # define CHARALIGN 1 # define MIDCHAR 0x7f # define LARGECHAR 0xfe -#endif -typedef int (*proto_t) (const CHAR *, const CHAR *); +# define CHAR__MAX CHAR_MAX +# define CHAR__MIN CHAR_MIN +/* Strcmp uses unsigned semantics for comparison. */ int -SIMPLE_STRCMP (const CHAR *s1, const CHAR *s2) +simple_strcmp (const char *s1, const char *s2) { int ret; - while ((ret = *(UCHAR *) s1 - *(UCHAR *) s2++) == 0 && *s1++); + while ((ret = *(unsigned char *) s1 - *(unsigned char*) s2++) == 0 && *s1++); return ret; } int -STUPID_STRCMP (const CHAR *s1, const CHAR *s2) +stupid_strcmp (const char *s1, const char *s2) { - size_t ns1 = STRLEN (s1) + 1; - size_t ns2 = STRLEN (s2) + 1; + size_t ns1 = strlen (s1) + 1; + size_t ns2 = strlen (s2) + 1; size_t n = ns1 < ns2 ? ns1 : ns2; int ret = 0; while (n--) - if ((ret = *(UCHAR *) s1++ - *(UCHAR *) s2++) != 0) + if ((ret = *(unsigned char *) s1++ - *(unsigned char *) s2++) != 0) break; return ret; } +#endif + +typedef int (*proto_t) (const CHAR *, const CHAR *); IMPL (STUPID_STRCMP, 1) IMPL (SIMPLE_STRCMP, 1) @@ -276,14 +320,31 @@ check (void) STRCPY(s1, L("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrs")); STRCPY(s2, L("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkLMNOPQRSTUV")); + /* Check correct working for negatives values */ + + s1[0] = 1; + s2[0] = 1; + s1[1] = 1; + s2[1] = 1; + s1[2] = -1; + s2[2] = 3; + s1[3] = 0; + s2[3] = -1; + + /* Check possible overflow bug, actual more for wcscmp */ + + s1[7] = CHAR__MIN; + s2[7] = CHAR__MAX; + size_t l1 = STRLEN (s1); size_t l2 = STRLEN (s2); + for (size_t i1 = 0; i1 < l1; i1++) for (size_t i2 = 0; i2 < l2; i2++) { - int exp_result = SIMPLE_STRCMP (s1 + i1, s2 + i2); - FOR_EACH_IMPL (impl, 0) - check_result (impl, s1 + i1, s2 + i2, exp_result); + int exp_result = SIMPLE_STRCMP (s1 + i1, s2 + i2); + FOR_EACH_IMPL (impl, 0) + check_result (impl, s1 + i1, s2 + i2, exp_result); } } diff --git a/sysdeps/i386/i686/multiarch/wcscmp-sse2.S b/sysdeps/i386/i686/multiarch/wcscmp-sse2.S index 404a9a4..cca0d83 100644 --- a/sysdeps/i386/i686/multiarch/wcscmp-sse2.S +++ b/sysdeps/i386/i686/multiarch/wcscmp-sse2.S @@ -21,7 +21,6 @@ #ifndef NOT_IN_libc # include -# include "asm-syntax.h" # define CFI_PUSH(REG) \ cfi_adjust_cfa_offset (4); \ @@ -34,18 +33,16 @@ # define PUSH(REG) pushl REG; CFI_PUSH (REG) # define POP(REG) popl REG; CFI_POP (REG) -# ifndef STRCMP -# define STRCMP __wcscmp_sse2 -# endif - # define ENTRANCE PUSH(%esi); PUSH(%edi) # define RETURN POP(%edi); POP(%esi); ret; CFI_PUSH(%esi); CFI_PUSH(%edi); # define PARMS 4 # define STR1 PARMS # define STR2 STR1+4 +/* Note: wcscmp uses signed comparison, not unsugned as in strcmp function. */ + .text -ENTRY (STRCMP) +ENTRY (__wcscmp_sse2) /* * This implementation uses SSE to compare up to 16 bytes at a time. */ @@ -131,7 +128,7 @@ L(continue_48_48): jne L(nequal) test %ecx, %ecx jz L(equal) - + movdqu 16(%edi), %xmm1 movdqu 16(%esi), %xmm2 pcmpeqd %xmm1, %xmm0 /* Any null double_word? */ @@ -264,21 +261,21 @@ L(continue_00_48): test %ecx, %ecx jnz L(less4_double_words1) - sub (%esi), %eax - jnz L(return) - + cmp (%esi), %eax + jne L(nequal) + mov 4(%edi), %eax - sub 4(%esi), %eax - jnz L(return) + cmp 4(%esi), %eax + jne L(nequal) mov 8(%edi), %eax - sub 8(%esi), %eax - jnz L(return) + cmp 8(%esi), %eax + jne L(nequal) mov 12(%edi), %eax - sub 12(%esi), %eax - jnz L(return) - + cmp 12(%esi), %eax + jne L(nequal) + movdqu 16(%esi), %xmm2 pcmpeqd %xmm2, %xmm0 /* Any null double_word? */ pcmpeqd 16(%edi), %xmm2 /* compare first 4 double_words for equality */ @@ -381,7 +378,7 @@ L(continue_32_48): movdqu 48(%esi), %xmm2 pcmpeqd %xmm1, %xmm0 /* Any null double_word? */ pcmpeqd %xmm2, %xmm1 /* compare first 4 double_words for equality */ - psubb %xmm0, %xmm1 /* packed sub of comparison results*/ + psubb %xmm0, %xmm1 /* packed sub of comparison results */ pmovmskb %xmm1, %edx sub $0xffff, %edx /* if first 4 double_words are same, edx == 0xffff */ jnz L(less4_double_words_48) @@ -585,21 +582,21 @@ L(continue_48_00): test %ecx, %ecx jnz L(less4_double_words1) - sub (%esi), %eax - jnz L(return) - + cmp (%esi), %eax + jne L(nequal) + mov 4(%edi), %eax - sub 4(%esi), %eax - jnz L(return) + cmp 4(%esi), %eax + jne L(nequal) mov 8(%edi), %eax - sub 8(%esi), %eax - jnz L(return) + cmp 8(%esi), %eax + jne L(nequal) mov 12(%edi), %eax - sub 12(%esi), %eax - jnz L(return) - + cmp 12(%esi), %eax + jne L(nequal) + movdqu 16(%edi), %xmm1 pcmpeqd %xmm1, %xmm0 /* Any null double_word? */ pcmpeqd 16(%esi), %xmm1 /* compare first 4 double_words for equality */ @@ -839,142 +836,161 @@ L(less4_double_words1): test %ecx, %ecx jz L(equal) - mov 12(%esi), %edx - mov 12(%edi), %eax - sub %edx, %eax + mov 12(%esi), %ecx + cmp %ecx, 12(%edi) + jne L(nequal) + xor %eax, %eax RETURN .p2align 4 L(less4_double_words): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words) and $15, %dl jz L(second_double_word) - mov (%edi), %eax - sub (%esi), %eax + mov (%esi), %ecx + cmp %ecx, (%edi) + jne L(nequal) RETURN .p2align 4 L(second_double_word): - mov 4(%edi), %eax - sub 4(%esi), %eax + mov 4(%esi), %ecx + cmp %ecx, 4(%edi) + jne L(nequal) RETURN .p2align 4 L(next_two_double_words): and $15, %dh jz L(fourth_double_word) - mov 8(%edi), %eax - sub 8(%esi), %eax + mov 8(%esi), %ecx + cmp %ecx, 8(%edi) + jne L(nequal) RETURN .p2align 4 L(fourth_double_word): - mov 12(%edi), %eax - sub 12(%esi), %eax + mov 12(%esi), %ecx + cmp %ecx, 12(%edi) + jne L(nequal) RETURN .p2align 4 L(less4_double_words_16): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words_16) and $15, %dl jz L(second_double_word_16) - mov 16(%edi), %eax - sub 16(%esi), %eax + mov 16(%esi), %ecx + cmp %ecx, 16(%edi) + jne L(nequal) RETURN .p2align 4 L(second_double_word_16): - mov 20(%edi), %eax - sub 20(%esi), %eax + mov 20(%esi), %ecx + cmp %ecx, 20(%edi) + jne L(nequal) RETURN .p2align 4 L(next_two_double_words_16): and $15, %dh jz L(fourth_double_word_16) - mov 24(%edi), %eax - sub 24(%esi), %eax + mov 24(%esi), %ecx + cmp %ecx, 24(%edi) + jne L(nequal) RETURN .p2align 4 L(fourth_double_word_16): - mov 28(%edi), %eax - sub 28(%esi), %eax + mov 28(%esi), %ecx + cmp %ecx, 28(%edi) + jne L(nequal) RETURN .p2align 4 L(less4_double_words_32): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words_32) and $15, %dl jz L(second_double_word_32) - mov 32(%edi), %eax - sub 32(%esi), %eax + mov 32(%esi), %ecx + cmp %ecx, 32(%edi) + jne L(nequal) RETURN .p2align 4 L(second_double_word_32): - mov 36(%edi), %eax - sub 36(%esi), %eax + mov 36(%esi), %ecx + cmp %ecx, 36(%edi) + jne L(nequal) RETURN .p2align 4 L(next_two_double_words_32): and $15, %dh jz L(fourth_double_word_32) - mov 40(%edi), %eax - sub 40(%esi), %eax + mov 40(%esi), %ecx + cmp %ecx, 40(%edi) + jne L(nequal) RETURN .p2align 4 L(fourth_double_word_32): - mov 44(%edi), %eax - sub 44(%esi), %eax + mov 44(%esi), %ecx + cmp %ecx, 44(%edi) + jne L(nequal) RETURN .p2align 4 L(less4_double_words_48): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words_48) and $15, %dl jz L(second_double_word_48) - mov 48(%edi), %eax - sub 48(%esi), %eax + mov 48(%esi), %ecx + cmp %ecx, 48(%edi) + jne L(nequal) RETURN .p2align 4 L(second_double_word_48): - mov 52(%edi), %eax - sub 52(%esi), %eax + mov 52(%esi), %ecx + cmp %ecx, 52(%edi) + jne L(nequal) RETURN .p2align 4 L(next_two_double_words_48): and $15, %dh jz L(fourth_double_word_48) - mov 56(%edi), %eax - sub 56(%esi), %eax + mov 56(%esi), %ecx + cmp %ecx, 56(%edi) + jne L(nequal) RETURN .p2align 4 L(fourth_double_word_48): - mov 60(%edi), %eax - sub 60(%esi), %eax - RETURN - - .p2align 4 -L(return): + mov 60(%esi), %ecx + cmp %ecx, 60(%edi) + jne L(nequal) RETURN .p2align 4 L(nequal): mov $1, %eax - ja L(nequal_bigger) + jg L(return) neg %eax + RETURN -L(nequal_bigger): + .p2align 4 +L(return): RETURN .p2align 4 @@ -988,7 +1004,7 @@ L(equal): .p2align 4 L(neq): mov $1, %eax - ja L(neq_bigger) + jg L(neq_bigger) neg %eax L(neq_bigger): @@ -999,5 +1015,5 @@ L(eq): xorl %eax, %eax ret -END (STRCMP) +END (__wcscmp_sse2) #endif diff --git a/sysdeps/x86_64/wcscmp.S b/sysdeps/x86_64/wcscmp.S index 991ecb2..12bfdaf 100644 --- a/sysdeps/x86_64/wcscmp.S +++ b/sysdeps/x86_64/wcscmp.S @@ -20,6 +20,8 @@ #include +/* Note: wcscmp uses signed comparison, not unsighed as in strcmp function. */ + .text ENTRY (wcscmp) /* @@ -76,7 +78,7 @@ L(continue_48_48): jne L(nequal) test %ecx, %ecx jz L(equal) - + movdqu 16(%rdi), %xmm1 movdqu 16(%rsi), %xmm2 pcmpeqd %xmm1, %xmm0 /* Any null double_word? */ @@ -209,21 +211,21 @@ L(continue_00_48): test %ecx, %ecx jnz L(less4_double_words1) - sub (%rsi), %eax - jnz L(return) - + cmp (%rsi), %eax + jne L(nequal) + mov 4(%rdi), %eax - sub 4(%rsi), %eax - jnz L(return) + cmp 4(%rsi), %eax + jne L(nequal) mov 8(%rdi), %eax - sub 8(%rsi), %eax - jnz L(return) + cmp 8(%rsi), %eax + jne L(nequal) mov 12(%rdi), %eax - sub 12(%rsi), %eax - jnz L(return) - + cmp 12(%rsi), %eax + jne L(nequal) + movdqu 16(%rsi), %xmm2 pcmpeqd %xmm2, %xmm0 /* Any null double_word? */ pcmpeqd 16(%rdi), %xmm2 /* compare first 4 double_words for equality */ @@ -530,21 +532,21 @@ L(continue_48_00): test %ecx, %ecx jnz L(less4_double_words1) - sub (%rsi), %eax - jnz L(return) - + cmp (%rsi), %eax + jne L(nequal) + mov 4(%rdi), %eax - sub 4(%rsi), %eax - jnz L(return) + cmp 4(%rsi), %eax + jne L(nequal) mov 8(%rdi), %eax - sub 8(%rsi), %eax - jnz L(return) + cmp 8(%rsi), %eax + jne L(nequal) mov 12(%rdi), %eax - sub 12(%rsi), %eax - jnz L(return) - + cmp 12(%rsi), %eax + jne L(nequal) + movdqu 16(%rdi), %xmm1 pcmpeqd %xmm1, %xmm0 /* Any null double_word? */ pcmpeqd 16(%rsi), %xmm1 /* compare first 4 double_words for equality */ @@ -784,25 +786,29 @@ L(less4_double_words1): test %ecx, %ecx jz L(equal) - mov 12(%rsi), %edx - mov 12(%rdi), %eax - sub %edx, %eax + mov 12(%rsi), %ecx + cmp %ecx, 12(%rdi) + jne L(nequal) + xor %eax, %eax ret .p2align 4 L(less4_double_words): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words) and $15, %dl jz L(second_double_word) mov (%rdi), %eax - sub (%rsi), %eax + cmp (%rsi), %eax + jne L(nequal) ret .p2align 4 L(second_double_word): mov 4(%rdi), %eax - sub 4(%rsi), %eax + cmp 4(%rsi), %eax + jne L(nequal) ret .p2align 4 @@ -810,29 +816,34 @@ L(next_two_double_words): and $15, %dh jz L(fourth_double_word) mov 8(%rdi), %eax - sub 8(%rsi), %eax + cmp 8(%rsi), %eax + jne L(nequal) ret .p2align 4 L(fourth_double_word): mov 12(%rdi), %eax - sub 12(%rsi), %eax + cmp 12(%rsi), %eax + jne L(nequal) ret .p2align 4 L(less4_double_words_16): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words_16) and $15, %dl jz L(second_double_word_16) mov 16(%rdi), %eax - sub 16(%rsi), %eax + cmp 16(%rsi), %eax + jne L(nequal) ret .p2align 4 L(second_double_word_16): mov 20(%rdi), %eax - sub 20(%rsi), %eax + cmp 20(%rsi), %eax + jne L(nequal) ret .p2align 4 @@ -840,29 +851,34 @@ L(next_two_double_words_16): and $15, %dh jz L(fourth_double_word_16) mov 24(%rdi), %eax - sub 24(%rsi), %eax + cmp 24(%rsi), %eax + jne L(nequal) ret .p2align 4 L(fourth_double_word_16): mov 28(%rdi), %eax - sub 28(%rsi), %eax + cmp 28(%rsi), %eax + jne L(nequal) ret .p2align 4 L(less4_double_words_32): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words_32) and $15, %dl jz L(second_double_word_32) mov 32(%rdi), %eax - sub 32(%rsi), %eax + cmp 32(%rsi), %eax + jne L(nequal) ret .p2align 4 L(second_double_word_32): mov 36(%rdi), %eax - sub 36(%rsi), %eax + cmp 36(%rsi), %eax + jne L(nequal) ret .p2align 4 @@ -870,29 +886,34 @@ L(next_two_double_words_32): and $15, %dh jz L(fourth_double_word_32) mov 40(%rdi), %eax - sub 40(%rsi), %eax + cmp 40(%rsi), %eax + jne L(nequal) ret .p2align 4 L(fourth_double_word_32): mov 44(%rdi), %eax - sub 44(%rsi), %eax + cmp 44(%rsi), %eax + jne L(nequal) ret .p2align 4 L(less4_double_words_48): + xor %eax, %eax test %dl, %dl jz L(next_two_double_words_48) and $15, %dl jz L(second_double_word_48) mov 48(%rdi), %eax - sub 48(%rsi), %eax + cmp 48(%rsi), %eax + jne L(nequal) ret .p2align 4 L(second_double_word_48): mov 52(%rdi), %eax - sub 52(%rsi), %eax + cmp 52(%rsi), %eax + jne L(nequal) ret .p2align 4 @@ -900,23 +921,21 @@ L(next_two_double_words_48): and $15, %dh jz L(fourth_double_word_48) mov 56(%rdi), %eax - sub 56(%rsi), %eax + cmp 56(%rsi), %eax + jne L(nequal) ret .p2align 4 L(fourth_double_word_48): mov 60(%rdi), %eax - sub 60(%rsi), %eax - ret - - .p2align 4 -L(return): + cmp 60(%rsi), %eax + jne L(nequal) ret .p2align 4 L(nequal): mov $1, %eax - ja L(nequal_bigger) + jg L(nequal_bigger) neg %eax L(nequal_bigger): diff --git a/wcsmbs/wcscmp.c b/wcsmbs/wcscmp.c index 6c93f70..ddbd4aa 100644 --- a/wcsmbs/wcscmp.c +++ b/wcsmbs/wcscmp.c @@ -37,11 +37,11 @@ WCSCMP (s1, s2) { c1 = (wint_t) *s1++; c2 = (wint_t) *s2++; - if (c1 == L'\0') + if (c2 == L'\0') return c1 - c2; } while (c1 == c2); - return c1 - c2; + return c1 < c2 ? -1 : 1; } libc_hidden_def (WCSCMP) -- 2.7.4