From nobody Fri Aug 25 21:53:04 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RXYbl6cxJz4qqs8; Fri, 25 Aug 2023 21:53:07 +0000 (UTC) (envelope-from fuz@fuz.su) Received: from fuz.su (fuz.su [IPv6:2001:41d0:8:e508::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "fuz.su", Issuer "fuz.su" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RXYbl0JNFz4BNh; Fri, 25 Aug 2023 21:53:06 +0000 (UTC) (envelope-from fuz@fuz.su) Authentication-Results: mx1.freebsd.org; none Received: from fuz.su (localhost [127.0.0.1]) by fuz.su (8.16.1/8.16.1) with ESMTPS id 37PLr4aV037443 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Fri, 25 Aug 2023 23:53:04 +0200 (CEST) (envelope-from fuz@fuz.su) Received: (from fuz@localhost) by fuz.su (8.16.1/8.16.1/Submit) id 37PLr4pt037442; Fri, 25 Aug 2023 23:53:04 +0200 (CEST) (envelope-from fuz) Date: Fri, 25 Aug 2023 23:53:04 +0200 From: Robert Clausecker To: Jessica Clarke Cc: Robert Clausecker , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: 3d8ef251aa9d - main - lib/libc/amd64/string/strchrnul.S: fix edge case in scalar code Message-ID: References: <202308251923.37PJN6EM082298@gitrepo.freebsd.org> <457A2B01-A4B0-4373-BD06-6BEBED922980@freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="hHmT2Y9f8phfyPZ/" Content-Disposition: inline In-Reply-To: <457A2B01-A4B0-4373-BD06-6BEBED922980@freebsd.org> X-Rspamd-Queue-Id: 4RXYbl0JNFz4BNh X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:16276, ipnet:2001:41d0::/32, country:FR] --hHmT2Y9f8phfyPZ/ Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Jessica, > That such bugs are being found in even the *scalar* code is concerning. > How confident are we that all the various implementations have in fact > been adequately tested if this slipped through? We have the 14 release > coming up, and this makes me nervous about whether all this code is > really ready for prime time yet. This seems like the kind of thing that > needs a long time to bake in -CURRENT so that it can be used in anger > and, hopefully, have all the edge cases hit. What have they been tested > against, anyway? Can we borrow test suites from other projects? Please understand that "scalar" means "no SIMD." It doesn't mean "na=EFve implementation" (it was requested that it be possible to use libc string functions without them touching the SSE state, so I preserved existing scalar implementations and added them where only generic code existed). In fact, scalar implementations are usually more complicated than the SIMD variants as x86 does not have SIMD-like operations for general purpose registers, so they have to be emulated with bit manipulation tricks. Specifically, I used the hasless(x,n) approach from Anderson's Bit Twiddling Hacks [1]. This approach is also used by the generic strchrnul() function, although withough the part (processing the head of the string until alignment is reached) that caused the bug. > That such bugs are being found in even the *scalar* code is concerning. > How confident are we that all the various implementations have in fact > been adequately tested if this slipped through? We have the 14 release > coming up, and this makes me nervous about whether all this code is > really ready for prime time yet. This seems like the kind of thing that > needs a long time to bake in -CURRENT so that it can be used in anger > and, hopefully, have all the edge cases hit. What have they been tested > against, anyway? Can we borrow test suites from other projects? Rest assured that I try my best to thoroughly test the code. We have comprehensive tests for libc string functions (many from NetBSD) and where they are missing or lacking, I have tried to extend the test suite. The ultimate test is building world with the changes applied where many different programs thoroughly exercise all sorts of string functions. This is incidentally how I caught this error: while implementing memchr() using the same approach as strchrnul(), I found that clang would crash when using this function. Debugging turned up this weird corner case, reflecting my own misunderstanding of the properties of the hasless(x,n) function (it tells you where the first match is; subsequent matches are not guaranteed to be accurate). I will check with re@ how to proceed with respect to these changes. If desired, they can be postponed to 14.1, though I think it would be nice to get them in now rather than in a year. [1]: https://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord PS: if you like, I can add you as a subscriber to future submissions of this project so you can assist in code review and early testing. Currently, D41520, D41528, D41557, and D41567 are pending review. Yours, Robert Clausecker --=20 () ascii ribbon campaign - for an 8-bit clean world=20 /\ - against html email - against proprietary attachments --hHmT2Y9f8phfyPZ/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKTBAABCgB9FiEExWcBrcoFY7LMaPxvWXxDScqS3gUFAmTpIrtfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEM1 NjcwMUFEQ0EwNTYzQjJDQzY4RkM2RjU5N0M0MzQ5Q0E5MkRFMDUACgkQWXxDScqS 3gXWHg/+LgDIG1bteXOeWoGY6Y1wN44MU/EgCKpZ1Eq+CKADqM35+pxd2oYAXwWl YHytN8JNZWOGKgW30xcVWiwzAk7sfa8iX+0TcFM2ZNTpkS1zlltA3THTzggytIuf DCu20KNmx/h41yEhBeO1z6rKtEFzFzW6qoSL4E8lYdJfbehaKiH/UoHTKD9FVQaD XXzvu7FfrnPfetME+wrzSUsNyhEh6XNiI3rLXhid0SSskQTW/putVn+BUrKsDCro 9duqB+U08mboONXrJLZawP8+yGjtZOl6uP7MFlF6Sl6f6QzoNtIAo4gHqdnkFzLU GrrFOETb5JZ2ybDrrEvUAPWdSj0Op/16kyuDLgOK/vqkWcCSBhJnvI9fzBGD24ym KUIWejwUq5ewr18FGnZfARhMT1KAxRhgshLfKVKIKGrfPbyD8uK6I7nVuLqxiier S+iFQa4WbiVNoelJe+tnjWhSdNtMdIIY0Lp93bdLKTCHNC7qpZjYQucP+sqgoo0x 9cOPsxLva4zDSVt8dZgYePHzEIfw4VS6p/fVXf0Dsaeur+Ace87KkIEFtGxhswdP kq2eT/DuD7G4M8R7TDsKKyql3A2DRGvbsnEMMSsaGclvcnUnRm6wugajcUpVUxkZ F46deBkNbLsGWnPcpYa8s/OBy2FaiEte2PVHd3ciPiwJXHVwTPY= =fKWa -----END PGP SIGNATURE----- --hHmT2Y9f8phfyPZ/--