Hi there, I'm working on a patch for libc & libkern to change qsort_r(3)'s prototype to match that of glibc. By doing this, the POSIX working group can go ahead with standardising qsort_r(3). I would therefore like to request an exp-run with the following patch applied to the base system (patch only tested on HEAD): https://reviews.freebsd.org/D17083 https://reviews.freebsd.org/D17083?download=true <- raw diff Thanks in advance!
This was rejected before, why try to do it again?
Hi Antoine, d'oh! I forgot all about the previous exp-runs cem@ requested. I just commented on D17083 to refer to those. Back then the main issue was that we couldn't reliably detect bad calls to qsort_r(), but in the meantime Clang's implementation of _Generic() seems to have improved. For example, I am no longer able to reproduce the issue I demonstrated in https://reviews.freebsd.org/D9169#189476. Because of this, I think it's worth revisiting this once more. Thanks!
Can you provide a patch that can be applied?
Created attachment 197013 [details] Patch for fixing up qsort_r() Ah, that's annoying. The patch that I linked (from Phabricator) does not deal with renames/additions in such a way that it can be applied easily. :-/ I've just attached a patch that I've created with 'svn diff --patch-compatible'. I was able to apply it using 'svn patch'. Hope that helps!
(In reply to Ed Schouten from comment #2) At least 1 port builds successfully despite the incorrect call: devel/pecl-qb Also there is something strange with at least ruby: checking for qsort_r... yes checking whether qsort_r is GNU version... no checking whether qsort_r is BSD version... no Is this normal?
Hi Antoine, I just looked at those ports and observed the following: - pecl-qb: This package calls qsort_r(), explicitly casting all parameters to 'void *'. This makes it impossible to derive anything useful. Considering that this package doesn't use Autoconf for picking the right flavor, let's patch this to take __FreeBSD_version into account. - Ruby: I filed a pull request for that: https://github.com/ruby/ruby/pull/1954 Let's import that patch once upstream merges it. Be sure to let me know if there are any other ports that behave out of the ordinary. Thanks!
A commit references this bug: Author: ed Date: Thu Sep 13 19:26:18 UTC 2018 New revision: 479692 URL: https://svnweb.freebsd.org/changeset/ports/479692 Log: lang/ruby2[345]: Improve qsort_r() detection This change merges the following upstream pull request into the Ruby interpreter ports: https://github.com/ruby/ruby/pull/1954 Adding this patch to these ports will ensure that once we patch up qsort_r() to be compatible with glibc, Ruby will automatically pick them up. Ruby should also build fine without this patch, but this will cause it to use its own implementation, which blows up the binary size slightly. Poudriere runs seem to pass for 11.x amd64. Logs indicate that this doesn't negatively affect the existing qsort_r() detection: checking whether qsort_r is GNU version... no checking whether qsort_r is BSD version... yes PR: 231256 Approved by: sunpoet Differential Revision: https://reviews.freebsd.org/D17157 Changes: head/lang/ruby23/files/patch-configure.in head/lang/ruby24/files/patch-configure.in head/lang/ruby25/files/patch-configure.ac
Around 700 ports were skipped due to new failures. New failure logs on i386: http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/astrometry-0.65_3.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/drm-legacy-kmod-g20180826.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/hashcat-4.2.1,1.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/monodevelop-7.6.3.1.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/orange3-3.15.0.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/pointcloud-1.2.0.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/raptor2-2.0.15_9.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/singular-4.1.1.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/synthpod-lv2-g20170810_2.log http://package18.nyi.freebsd.org/data/headi386PR231256-default/2018-09-13_06h10m40s/logs/errors/tvheadend-4.2.6_4.log
Hi Antoine, Thanks for the list! The exp-run has finished, I presume? Assuming there are also one or two breakages in the remaining 700 ports, it means around a dozen ports are thus affected. None of them seem to be very high-profile. Not too bad. I checked the sources of the ports you listed below. Unlike Ruby, none of these seem to use some kind of auto-detection scheme for qsort_r(). They should all be patched up to use __FreeBSD_version after the fact. I'll make sure to send out code reviews/file bugs with patches after D17083 lands.
(In reply to Ed Schouten from comment #9) The reviews/file bugs with patches have to happen before, not after.
How may that be done if the value of __FreeBSD_version is not yet known?
(In reply to Ed Schouten from comment #9) textproc/raptor2 is kind of high profile, it causes 537 skipped.
Created attachment 197107 [details] Crude patch to keep raptor2 buildable? Hi Antoine, With that number of reverse dependencies it would probably make sense to at least provide some workaround for that specific port. Would it be acceptable to add a patch like this to raptor2 and clean it up after the src changes land?
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=cd28784c67bfe64bd95afc8136732dba6750bf27 commit cd28784c67bfe64bd95afc8136732dba6750bf27 Author: Ed Schouten <ed@FreeBSD.org> AuthorDate: 2022-09-30 16:59:40 +0000 Commit: Xin LI <delphij@FreeBSD.org> CommitDate: 2022-09-30 22:28:02 +0000 textproc/raptor2: Improve sort_r(3) API detection. In a future version of FreeBSD, we would adopt POSIX qsort_r(3) API which is different from traditional FreeBSD qsort_r(3). PR: ports/231256 Exp-run by: antoine Approved by: portmgr textproc/raptor2/Makefile | 2 +- textproc/raptor2/files/patch-src_sort__r.h (new) | 34 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)