strverscmp is buggy in Cygwin 3.4.6

strverscmp is buggy in Cygwin 3.4.6

[Bruno Haible]

Hi,

Here's a test case of strverscmp, from Dmitry Bogatov [1]

#include <string.h>
int main ()
{
  return strverscmp ("UNKNOWN", "2.2.0") <= 0;
}

It succeeds on glibc and musl libc 1.2.4, but fails on musl libc 1.2.3
and Cygwin 2.9.0 and 3.4.6.

The cause is apparently that Cygwin's strverscmp implementation was
borrowed from musl libc (Cygwin commit 59e09b6419cdf400be3c73b61ac9c22560dc397e)
at a time when musl libc's implementation was buggy. In musl libc, it is
meanwhile fixed, through
https://git.musl-libc.org/cgit/musl/commit/src/string/strverscmp.c?id=b50eb8c36c20f967bd0ed70c0b0db38a450886ba

Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2024-01/msg00002.html

Re: strverscmp is buggy in newlib 4.4.0 (was: Cygwin 3.4.6)

[Brian Inglis]

On 2024-01-02 03:23, Bruno Haible via Cygwin wrote:
> Here's a test case of strverscmp, from Dmitry Bogatov [1]
> #include <string.h>
> int main ()
> {
>    return strverscmp ("UNKNOWN", "2.2.0") <= 0;
> }
> It succeeds on glibc and musl libc 1.2.4, but fails on musl libc 1.2.3
> and Cygwin 2.9.0 and 3.4.6.
> The cause is apparently that Cygwin's strverscmp implementation was
> borrowed from musl libc (Cygwin commit 59e09b6419cdf400be3c73b61ac9c22560dc397e)
> at a time when musl libc's implementation was buggy. In musl libc, it is
> meanwhile fixed, through
> https://git.musl-libc.org/cgit/musl/commit/src/string/strverscmp.c?id=b50eb8c36c20f967bd0ed70c0b0db38a450886ba
> [1] https://lists.gnu.org/archive/html/bug-gnulib/2024-01/msg00002.html

Issue is in newlib (x-post and followups set):
https://sourceware.org/cgit/newlib-cygwin/tree/newlib/libc/string/strverscmp.c

patch with git log msg:
https://git.musl-libc.org/cgit/musl/patch/src/string/strverscmp.c?id=b50eb8c36c20f967bd0ed70c0b0db38a450886ba

Gnulib discussion:
https://lists.gnu.org/archive/html/bug-gnulib/2024-01/msg00002.html

Musl thread starts:
https://www.openwall.com/lists/musl/2022/11/06/1

bare patch without git log msg attached to:
https://www.openwall.com/lists/musl/2022/11/08/1

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

Re: strverscmp is buggy in Cygwin 3.4.6

[matthew patton]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]

> The cause is apparently that Cygwin's strverscmp implementation wasborrowed from musl libc 
would it make sense to use git submodules when "borrowing" code so the upstream reference is not lost, and keeping it abreast is relatively trivial exercise?

Re: strverscmp is buggy in newlib 4.4.0 (was: Cygwin 3.4.6)

[Brian Inglis]

On 2024-01-02 13:29, matthew patton via Cygwin wrote:
>> The cause is apparently that Cygwin's strverscmp implementation wasborrowed from musl libc
> would it make sense to use git submodules when "borrowing" code so the
> upstream reference is not lost, and keeping it abreast is relatively trivial
> exercise?
Issue is in newlib (x-post and followups set):
https://sourceware.org/cgit/newlib-cygwin/tree/newlib/libc/string/strverscmp.c

Docs for man pages are embedded in newlib (but not Cygwin unfortunately) 
sources, but it would be nice if folks added a subsection or reference with 
{Free,Open,Net}BSD/musl/etc. links, so we could see where and when the code was 
copied, although some sources include BSD revision headers and copyrights, other 
sources have no identifying header.

-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

Re: strverscmp is buggy in newlib 4.4.0 (was Cygwin 3.4.6)

[Brian Inglis]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

On 2024-01-02 10:36, Brian Inglis via Cygwin wrote:
> On 2024-01-02 03:23, Bruno Haible via Cygwin wrote:
>> Here's a test case of strverscmp, from Dmitry Bogatov [1]
>> #include <string.h>
>> int main ()
>> {
>>    return strverscmp ("UNKNOWN", "2.2.0") <= 0;
>> }
>> It succeeds on glibc and musl libc 1.2.4, but fails on musl libc 1.2.3
>> and Cygwin 2.9.0 and 3.4.6.
>> The cause is apparently that Cygwin's strverscmp implementation was
>> borrowed from musl libc (Cygwin commit 59e09b6419cdf400be3c73b61ac9c22560dc397e)
>> at a time when musl libc's implementation was buggy. In musl libc, it is
>> meanwhile fixed, through
>> https://git.musl-libc.org/cgit/musl/commit/src/string/strverscmp.c?id=b50eb8c36c20f967bd0ed70c0b0db38a450886ba
>> [1] https://lists.gnu.org/archive/html/bug-gnulib/2024-01/msg00002.html
> 
> Issue is in newlib (x-post and followups set):
> https://sourceware.org/cgit/newlib-cygwin/tree/newlib/libc/string/strverscmp.c
> 
> patch with git log msg:
> https://git.musl-libc.org/cgit/musl/patch/src/string/strverscmp.c?id=b50eb8c36c20f967bd0ed70c0b0db38a450886ba

This musl patch applies cleanly to newlib (patch offsets) and is attached.

> Gnulib discussion:
> https://lists.gnu.org/archive/html/bug-gnulib/2024-01/msg00002.html
> 
> Musl thread starts:
> https://www.openwall.com/lists/musl/2022/11/06/1
> 
> bare patch without git log msg attached to:
> https://www.openwall.com/lists/musl/2022/11/08/1
-- 
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                 -- Antoine de Saint-Exupéry

[-- Attachment #2: musl_string_strverscmp.patch --]
[-- Type: text/plain, Size: 1493 bytes --]

From b50eb8c36c20f967bd0ed70c0b0db38a450886ba Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Mon, 7 Nov 2022 22:17:55 -0500
Subject: fix strverscmp comparison of digit sequence with non-digits

the rule that longest digit sequence not beginning with a zero is
greater only applies when both sequences being compared are
non-degenerate. this is spelled out explicitly in the man page, which
may be deemed authoritative for this nonstandard function: "If one or
both of these is empty, then return what strcmp(3) would have
returned..."

we were wrongly treating any sequence of digits not beginning with a
zero as greater than a non-digit in the other string.
---
 src/string/strverscmp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'src/string/strverscmp.c')

diff --git a/src/string/strverscmp.c b/src/string/strverscmp.c
index 4daf276d..16c1da22 100644
--- a/src/string/strverscmp.c
+++ b/src/string/strverscmp.c
@@ -18,9 +18,9 @@ int strverscmp(const char *l0, const char *r0)
 		else if (c!='0') z=0;
 	}
 
-	if (l[dp]!='0' && r[dp]!='0') {
-		/* If we're not looking at a digit sequence that began
-		 * with a zero, longest digit string is greater. */
+	if (l[dp]-'1'<9U && r[dp]-'1'<9U) {
+		/* If we're looking at non-degenerate digit sequences starting
+		 * with nonzero digits, longest digit string is greater. */
 		for (j=i; isdigit(l[j]); j++)
 			if (!isdigit(r[j])) return 1;
 		if (isdigit(r[j])) return -1;
-- 
cgit v1.2.1