NetBSD Problem Report #54441

From www@netbsd.org  Tue Aug  6 03:24:15 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-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 9054A7A1C1
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  6 Aug 2019 03:24:15 +0000 (UTC)
Message-Id: <20190806032414.85E587A1E0@mollari.NetBSD.org>
Date: Tue,  6 Aug 2019 03:24:14 +0000 (UTC)
From: nis8192@gmail.com
Reply-To: nis8192@gmail.com
To: gnats-bugs@NetBSD.org
Subject: devel/liubusb undefined behaviour
X-Send-Pr-Version: www-1.0

>Number:         54441
>Category:       pkg
>Synopsis:       devel/liubusb undefined behaviour
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Aug 06 03:25:00 +0000 2019
>Closed-Date:    Mon Jul 19 04:28:52 +0000 2021
>Last-Modified:  Mon Jul 19 04:28:52 +0000 2021
>Originator:     Shingo Nishioka
>Release:        Tpkgsrc-2019Q2
>Organization:
>Environment:
8.1_STABLE
>Description:
As func. usb_parse_descriptor contains some undefined behavior code, which violates strict aliasing rules, when compiled with clang -O2, the library does not work properly.
>How-To-Repeat:
Add follwing lines to /etc/mk.conf and rebuild the library

MKGCC=no
MKLLVM=yes
HAVE_LLVM=yes
PKGSRC_COMPILER=clang
CLANGBASE=/usr

>Fix:
One of the followings will do:

1. apply following patch

--- libusb/descriptor.c.orig	2019-08-02 09:59:25.784968424 +0900
+++ libusb/descriptor.c	2019-08-02 09:59:44.009134412 +0900
@@ -54,7 +54,9 @@
 	for (cp = descriptor; *cp; cp++) {
 		switch (*cp) {
 			case 'b':	/* 8-bit byte */
-				*dp++ = *sp++;
+				memcpy(dp, sp, 1);
+				dp += 1;
+				sp += 1;
 				break;
 			case 'w':	/* 16-bit word, convert from little endian to CPU */
 				dp += ((uintptr_t)dp & 1);	/* Align to word boundary */
@@ -63,7 +65,7 @@
 					memcpy(dp, sp, 2);
 				} else {
 					w = (sp[1] << 8) | sp[0];
-					*((uint16_t *)dp) = w;
+					memcpy(dp, &w, 4);
 				}
 				sp += 2;
 				dp += 2;
@@ -76,7 +78,7 @@
 				} else {
 					d = (sp[3] << 24) | (sp[2] << 16) |
 						(sp[1] << 8) | sp[0];
-					*((uint32_t *)dp) = d;
+					memcpy(dp, &d, 4);
 				}
 				sp += 4;
 				dp += 4;



2. disable -O2 level optimization (-O 1 will do)

3. allow strict aliasing rules violation (-fno-strict-aliasing)

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: maya@NetBSD.org
State-Changed-When: Tue, 06 Aug 2019 08:54:35 +0000
State-Changed-Why:
Does it work with the change? (I see the testsuite is segfaulting...)
It would be good to report this upstream.


From: "Maya Rashish" <maya@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54441 CVS commit: pkgsrc/devel/libusb1
Date: Tue, 6 Aug 2019 08:50:28 +0000

 Module Name:	pkgsrc
 Committed By:	maya
 Date:		Tue Aug  6 08:50:28 UTC 2019

 Modified Files:
 	pkgsrc/devel/libusb1: Makefile distinfo
 Added Files:
 	pkgsrc/devel/libusb1/patches: patch-ub

 Log Message:
 libusb1: patch some undefined behaviour, disable strict aliasing, change
 -O2 to -O1 when building with clang.

 This isn't in a separate hacks.mk file because I think that hides the
 problem too much, it's an issue with the code in the package, not with
 the compiler's choices.

 Fixes functionality when built with clang.

 From Shingo Nishioka in PR pkg/54441.


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.19 pkgsrc/devel/libusb1/Makefile
 cvs rdiff -u -r1.10 -r1.11 pkgsrc/devel/libusb1/distinfo
 cvs rdiff -u -r0 -r1.1 pkgsrc/devel/libusb1/patches/patch-ub

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

From: "Maya Rashish" <maya@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54441 CVS commit: pkgsrc/devel/libusb
Date: Tue, 6 Aug 2019 09:12:10 +0000

 Module Name:	pkgsrc
 Committed By:	maya
 Date:		Tue Aug  6 09:12:10 UTC 2019

 Modified Files:
 	pkgsrc/devel/libusb: Makefile distinfo
 Added Files:
 	pkgsrc/devel/libusb/patches: patch-descriptors.c

 Log Message:
 libusb: avoid unaligned access. Improve code consistency.
 XXX does this package also need -O1 -fno-strict-aliasing on clang?

 Noted by Shingo Nishioka in PR pkg/54441


 To generate a diff of this commit:
 cvs rdiff -u -r1.43 -r1.44 pkgsrc/devel/libusb/Makefile
 cvs rdiff -u -r1.27 -r1.28 pkgsrc/devel/libusb/distinfo
 cvs rdiff -u -r0 -r1.1 pkgsrc/devel/libusb/patches/patch-descriptors.c

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

From: coypu@sdf.org
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: pkg/54441: devel/liubusb undefined behaviour
Date: Tue, 6 Aug 2019 09:13:59 +0000

 Additionally:
 libusb1 and libusb both seemed wrong, but I wasn't sure if both need -O1
 -fno-strict-aliasing.

 Which one were you referring to?

From: "Maya Rashish" <maya@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54441 CVS commit: pkgsrc/devel/libusb
Date: Tue, 6 Aug 2019 09:27:31 +0000

 Module Name:	pkgsrc
 Committed By:	maya
 Date:		Tue Aug  6 09:27:31 UTC 2019

 Modified Files:
 	pkgsrc/devel/libusb: Makefile distinfo
 Removed Files:
 	pkgsrc/devel/libusb/patches: patch-descriptors.c

 Log Message:
 libusb: revert previous patch. this code is not doing unaligned access.

 PR pkg/54441


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.45 pkgsrc/devel/libusb/Makefile
 cvs rdiff -u -r1.28 -r1.29 pkgsrc/devel/libusb/distinfo
 cvs rdiff -u -r1.1 -r0 pkgsrc/devel/libusb/patches/patch-descriptors.c

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

From: "Benny Siegert" <bsiegert@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54441 CVS commit: [pkgsrc-2019Q2] pkgsrc/devel/libusb1
Date: Sat, 10 Aug 2019 12:32:34 +0000

 Module Name:	pkgsrc
 Committed By:	bsiegert
 Date:		Sat Aug 10 12:32:34 UTC 2019

 Modified Files:
 	pkgsrc/devel/libusb1 [pkgsrc-2019Q2]: Makefile distinfo
 Added Files:
 	pkgsrc/devel/libusb1/patches [pkgsrc-2019Q2]: patch-ub

 Log Message:
 Pullup ticket #6027 - requested by maya
 devel/libusb1: clang build fix

 Revisions pulled up:
 - devel/libusb1/Makefile                                        1.19
 - devel/libusb1/distinfo                                        1.11
 - devel/libusb1/patches/patch-ub                                1.1

 ---
    Module Name:	pkgsrc
    Committed By:	maya
    Date:		Tue Aug  6 08:50:28 UTC 2019

    Modified Files:
    	pkgsrc/devel/libusb1: Makefile distinfo
    Added Files:
    	pkgsrc/devel/libusb1/patches: patch-ub

    Log Message:
    libusb1: patch some undefined behaviour, disable strict aliasing, change
    -O2 to -O1 when building with clang.

    This isn't in a separate hacks.mk file because I think that hides the
    problem too much, it's an issue with the code in the package, not with
    the compiler's choices.

    Fixes functionality when built with clang.

    >From Shingo Nishioka in PR pkg/54441.


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.18.2.1 pkgsrc/devel/libusb1/Makefile
 cvs rdiff -u -r1.10 -r1.10.2.1 pkgsrc/devel/libusb1/distinfo
 cvs rdiff -u -r0 -r1.1.2.2 pkgsrc/devel/libusb1/patches/patch-ub

 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: maya@NetBSD.org
State-Changed-When: Fri, 24 Apr 2020 14:00:19 +0000
State-Changed-Why:
nia@ ended up testing this package in practice (and finding an error in the patch, too). So I assume after she successfully used the package after her changes, we can consider it fixed, maybe.


State-Changed-From-To: closed->feedback
State-Changed-By: maya@NetBSD.org
State-Changed-When: Fri, 24 Apr 2020 14:05:09 +0000
State-Changed-Why:
Eh, on second thought, let's ask for feedback. The package changed more than I imagined.


From: maya@NetBSD.org
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: pkg/54441 (devel/liubusb undefined behaviour)
Date: Fri, 24 Apr 2020 14:02:21 +0000

 My main concern with the patch was that I broke the package from working
 by not being able to test it. Feel free to note if you are having issues
 with unaligned access, I believe nia tested it with GCC rather than
 clang.

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 19 Jul 2021 04:28:52 +0000
State-Changed-Why:
feedback timeout, assume it is in fact fixed


>Unformatted:

Home	PR Database Search