This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: New 64 bit wcscmp implementation


Yes, we need to fix signed/unsigned wrong semantics in simple_wcscmp,
stupid_wcscmp as well.

Small additional change makes it:

The patch:
diff --git a/string/test-strcmp.c b/string/test-strcmp.c
index eac3b5b..0cc47e9 100644
--- a/string/test-strcmp.c
+++ b/string/test-strcmp.c
@@ -34,7 +34,7 @@
 # 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)


So at the results I fixed signed/unsigned wrong semantics:

in simple_wcscmp, stupid_wcscmp (from test)
in i386/i686/multiarch/wcscmp-sse2.S
in x86_64/wcscmp.S

Make check passes without errors now.


But, Yes, we still have overflow bug but not only in simple_wcscmp,
stupid_wcscmp. It seems we have it everywhere:

in simple_wcscmp, stupid_wcscmp (from test)
in i386/i686/multiarch/wcscmp-sse2.S
in x86_64/wcscmp.S
even in wcsmbs/wcscmp.c

I will fix it in an other patch.

BTW, there are no test cases for it in glibc now.
My changes in test cover only signed/unsigned wrong semantics bug.

--
Liubov Dmitrieva

2011/9/16 Joseph S. Myers <joseph@codesourcery.com>:
> On Fri, 16 Sep 2011, Dmitrieva Liubov wrote:
>
>> Hello,
>>
>> The example mentioned by Joseph doesn't show that for old wcscmp
>> version the comparisons are always done in an unsigned type even when
>> wchar_t is signed.
>
> Well - it compares in an unsigned type then subtracts in an unsigned type
> and returns the result in a signed type (the sign after conversion to
> signed possibly being wrong). ?The precise form of the bug isn't
> particularly significant.
>
> In properly fixed testcases, UCHAR (the type for the comparison) should be
> wchar_t not uint32_t in the wide-character case, and the return value in
> STUPID_STRCMP etc. can't be determined directly by a subtraction that
> might overflow, it needs to be of the form (a < b ? -1 : a == b ? 0 : 1)
> or similar.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]