NetBSD Problem Report #57141

From www@netbsd.org  Wed Dec 28 13:27:07 2022
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 2C9471A9239
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 28 Dec 2022 13:27:07 +0000 (UTC)
Message-Id: <20221228132705.BAD231A923A@mollari.NetBSD.org>
Date: Wed, 28 Dec 2022 13:27:05 +0000 (UTC)
From: bugs@compiler.ai
Reply-To: bugs@compiler.ai
To: gnats-bugs@NetBSD.org
Subject: Bug in swab()
X-Send-Pr-Version: www-1.0

>Number:         57141
>Category:       lib
>Synopsis:       Bug in swab()
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Dec 28 13:30:00 +0000 2022
>Closed-Date:    Tue Jul 25 10:42:07 +0000 2023
>Last-Modified:  Tue Jul 25 10:42:07 +0000 2023
>Originator:     Jai Arora, Abhishek Rose, Shubhani Gupta, Sorav Bansal
>Release:        `trunk` branch of the src repository
>Organization:
CompilerAI Research Group, IIT Delhi
>Environment:
5.4.0-135-generic #152-Ubuntu SMP Wed Nov 23 20:19:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
>Description:
The bug is in the C implementation of swab as located in lib/libc/string/ directory of the NetBSD repository. The source code was downloaded from the official git repository(https://github.com/NetBSD/src, top level commit on the `trunk` branch).

Linux[0] manpage for swab() specifies that the function copies n bytes from the array pointed to by from to the array pointed to by to, exchanging adjacent even and odd bytes. NetBSD's implementation incorrectly rounds n to a multiple of 8, leading one less memory swap than expected.

0: https://man7.org/linux/man-pages/man3/swab.3.html
>How-To-Repeat:
An example input is:
  const char src[] = {90, 91, 1, 2};
  char dst[4] = {'A', 'B', 'C', 'D'};
  swab(src, dst, 4);
  if (dst[0] != 91 || dst[1] != 90 || dst[2] != 2 || dst[3] != 1) {
    printf("BUG!\n");
  }

The file that demonstrates the bug can be found here: https://github.com/compilerai/bug-reports/blob/master/bug_files/netbsd_swab_bug.c
>Fix:
A patch that applies the necessary fix is available here: https://github.com/compilerai/bug-reports/blob/master/patch/netbsd_swab.patch

>Release-Note:

>Audit-Trail:
From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57141 CVS commit: src/lib/libc/string
Date: Wed, 28 Dec 2022 14:32:05 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Wed Dec 28 14:32:04 UTC 2022

 Modified Files:
 	src/lib/libc/string: swab.c

 Log Message:
 PR lib/57141 - never decrement len without actually performing a STEP.


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.19 src/lib/libc/string/swab.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: kre@NetBSD.org
State-Changed-When: Wed, 28 Dec 2022 14:46:03 +0000
State-Changed-Why:
This should be fixed in HEAD now (though I am not sure how long
it will take to propogate to the git repository).

I will request pullups to NetBSD 8, 9, and 10_BETA soon.

However note that:

   NetBSD's implementation incorrectly rounds n to a multiple of 8,

is not really a correct chacterisation of the problem, n is the
input arg (number of bytes), the value being rounded was n/2,
and is just an implementation technique to make less loop iterations
in cases where n is large.

It was indeed being done incorrectly however.   Thanks for the report.

Note also that most of the common NetBSD supported architectures don't
use that version of swab() - they instead use an archtecture specific
version written in assembler.

Please let us know if the updated version (once it becomes available)
seems OK.


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57141 CVS commit: src
Date: Wed, 28 Dec 2022 15:34:19 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Dec 28 15:34:19 UTC 2022

 Modified Files:
 	src/lib/libc/string: swab.c
 	src/tests/lib/libc/string: t_swab.c

 Log Message:
 swab(3): Rewrite this to be understandable.

 And make the tests work, and exercise all lengths up to 100.

 Evidently the previous definition, presumably tightly optimized for
 1980s-era compilers and CPUs, was too hard to understand, because it
 was incorrectly tested for two decades and broken for years.

 PR lib/57141

 XXX pullup-8
 XXX pullup-9
 XXX pullup-10


 To generate a diff of this commit:
 cvs rdiff -u -r1.19 -r1.20 src/lib/libc/string/swab.c
 cvs rdiff -u -r1.2 -r1.3 src/tests/lib/libc/string/t_swab.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57141 CVS commit: [netbsd-10] src
Date: Wed, 22 Feb 2023 18:47:00 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Feb 22 18:47:00 UTC 2023

 Modified Files:
 	src/lib/libc/string [netbsd-10]: swab.c
 	src/tests/lib/libc/string [netbsd-10]: t_swab.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #98):

 	tests/lib/libc/string/t_swab.c: revision 1.3
 	lib/libc/string/swab.c: revision 1.20

 swab(3): Rewrite this to be understandable.

 And make the tests work, and exercise all lengths up to 100.
 Evidently the previous definition, presumably tightly optimized for
 1980s-era compilers and CPUs, was too hard to understand, because it
 was incorrectly tested for two decades and broken for years.

 PR lib/57141


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.18.56.1 src/lib/libc/string/swab.c
 cvs rdiff -u -r1.2 -r1.2.52.1 src/tests/lib/libc/string/t_swab.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57141 CVS commit: [netbsd-9] src
Date: Wed, 22 Feb 2023 18:49:17 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Feb 22 18:49:17 UTC 2023

 Modified Files:
 	src/lib/libc/string [netbsd-9]: swab.c
 	src/tests/lib/libc/string [netbsd-9]: t_swab.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #1601):

 	tests/lib/libc/string/t_swab.c: revision 1.3
 	lib/libc/string/swab.c: revision 1.20

 swab(3): Rewrite this to be understandable.

 And make the tests work, and exercise all lengths up to 100.
 Evidently the previous definition, presumably tightly optimized for
 1980s-era compilers and CPUs, was too hard to understand, because it
 was incorrectly tested for two decades and broken for years.

 PR lib/57141


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.18.48.1 src/lib/libc/string/swab.c
 cvs rdiff -u -r1.2 -r1.2.44.1 src/tests/lib/libc/string/t_swab.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57141 CVS commit: [netbsd-8] src
Date: Wed, 22 Feb 2023 18:50:42 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Wed Feb 22 18:50:42 UTC 2023

 Modified Files:
 	src/lib/libc/string [netbsd-8]: swab.c
 	src/tests/lib/libc/string [netbsd-8]: t_swab.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #1798):

 	tests/lib/libc/string/t_swab.c: revision 1.3
 	lib/libc/string/swab.c: revision 1.20

 swab(3): Rewrite this to be understandable.

 And make the tests work, and exercise all lengths up to 100.
 Evidently the previous definition, presumably tightly optimized for
 1980s-era compilers and CPUs, was too hard to understand, because it
 was incorrectly tested for two decades and broken for years.

 PR lib/57141


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.18.38.1 src/lib/libc/string/swab.c
 cvs rdiff -u -r1.2 -r1.2.34.1 src/tests/lib/libc/string/t_swab.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Tue, 25 Jul 2023 10:42:07 +0000
State-Changed-Why:
feedback timeout, problem fixed on all live branches


>Unformatted:

Home	PR Database Search