Bug 234631 - devel/valgrind: Fixes for FreeBSD 12.x support
Summary: devel/valgrind: Fixes for FreeBSD 12.x support
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Niclas Zeising
Keywords: needs-patch, needs-qa
Depends on:
Reported: 2019-01-05 08:16 UTC by Julien Nadeau
Modified: 2021-03-23 17:00 UTC (History)
11 users (show)

See Also:
rozhuk.im: maintainer-feedback-
koobs: merge-quarterly?

Patch (37.77 KB, patch)
2019-01-05 08:16 UTC, Julien Nadeau
no flags Details | Diff
pipe() -> pipe2(,0) (603 bytes, patch)
2019-03-16 18:30 UTC, rozhuk.im
no flags Details | Diff
update patch (43.36 KB, patch)
2019-03-17 22:02 UTC, rozhuk.im
rozhuk.im: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Nadeau 2019-01-05 08:16:39 UTC
Created attachment 200787 [details]

Add 12.x kevent, minherit, getrandom syscalls.
Add 12.x struct stat.

Suggest consolidating EXTRA_PATCHES so that 'update-patches' can work normally.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2019-01-05 10:19:59 UTC

Should be a PORTREVISION bump
Comment 2 Walter Schwarzenfeld freebsd_triage 2019-03-10 11:48:04 UTC
Assign to new maintainer.
Comment 3 rozhuk.im 2019-03-16 18:04:14 UTC
There is one thing that not done: this will not work if kernel build without COMPAT_FREEBSD10 - wich remove pipe().
For all other soft this handled by libc (IMHO, not sure), it convert it to pipe2(,0), but valgring try to call directly via syscall and fail.
Almost same thing with COMPAT_FREEBSD11.
I try to make patch to fix it and add it here.
Comment 4 rozhuk.im 2019-03-16 18:30:22 UTC
Created attachment 202919 [details]
pipe() -> pipe2(,0)
Comment 5 rozhuk.im 2019-03-16 19:21:52 UTC
Also on FreeBSD 12 rquired:
CFLAGS+=	-mstack-alignment=8

looks like defaults now 16 and this brokes valgrind.
Comment 6 rozhuk.im 2019-03-16 19:32:54 UTC
Can you also integrate my patch: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234045 ?
Comment 7 rozhuk.im 2019-03-17 22:02:50 UTC
Created attachment 202943 [details]
update patch

- pipe() -> pipe2(,0) for 11.0
- add getfsstat() for 12.0
- add statfs() for 12.0
- add sigwait()
- use -mstack-alignment=8 to avoid crash
Comment 8 Egil Hasting 2019-07-02 15:46:06 UTC
(In reply to rozhuk.im from comment #7)
Tested the patch on: FreeBSD 12.0-RELEASE-p3 FreeBSD 12.0-RELEASE-p3 GENERIC  amd64

Works as expected, thanks!
Comment 9 nhoyle 2019-12-05 16:35:25 UTC
This very much doesn't affect only you. Been waiting for the fix to be merged for months now, since 12 came out, and now we're on 12.1. Is there any movement on getting this merged?
Comment 10 Natalino Picone 2020-01-31 11:15:04 UTC
Any update on this ?
Comment 11 Paul Floyd 2020-03-27 19:35:01 UTC
(In reply to Natalino Picone from comment #10)

I've integrated these diffs into my github repo. Needs more testing.

Comment 12 Paul Floyd 2020-03-27 19:36:13 UTC
(In reply to rozhuk.im from comment #5)

Do you have any further information on this? I haven't noticed it making any difference.
Comment 13 rozhuk.im 2020-04-01 12:38:29 UTC
(In reply to Paul Floyd from comment #12)

I do not remember all details of this.
It just fix crashes that happens after some update, probably is was clang8 issue, I did not dig deep, just found this simple fix/workaround.
Comment 14 rozhuk.im 2020-04-16 08:30:03 UTC
IMHO valgring should be in base, there is to many system and OS version specific things.
Comment 15 Paul Floyd 2020-04-16 08:56:10 UTC
(In reply to rozhuk.im from comment #14)

One of my goals is to have correct support for several versions of FreeBSD. For the moment I've only tried 12.1 and 11.3, but I am planning on going back at least as far as 10.
Comment 16 rozhuk.im 2020-04-16 10:03:47 UTC
(In reply to Paul Floyd from comment #15)

10x is outdated.
Some syscals renamed to ****_compat10 and new with same name added.
It is not possible (very difficult with to many #if ver > ***) to have valgrind version that works on all versions of FreeBSD.

Better to have:
or integrate it in FreeBSD source tree.
Comment 17 Paul Floyd 2020-04-16 10:25:17 UTC
(In reply to rozhuk.im from comment #16)

I don't want to go down the route of having multiple branches. This is not the way that upstream Valgrind works. There is the occasional feature development branch, but otherwise everything goes into HEAD.

For the moment I'm doing detection of the FreeBSD version in the configure script. This puts the version in config.h. Generally it is not a problem if newer data structures and syscalls get compiled into a Valgrind for an older FreeBSD. For instance FreeBSD 11 will never use a 551 fstat syscall. The only thing is that it would be nice to use the right names for any error messages, which is why for the moment I'm using #ifs where I can. I might reconsider that if the code starts to become unmaintainable spaghetti.
Comment 18 rozhuk.im 2020-04-16 10:37:36 UTC
(In reply to Paul Floyd from comment #17)

Use: __FreeBSD_version
from sys/param.h
no need to detect in build scripts.
Comment 19 Paul Floyd 2020-04-16 11:18:38 UTC
(In reply to rozhuk.im from comment #18)

I might do that on a case by case basis. configure/config.h has the advantage of being OS independent, which will be more acceptable upstream.
Comment 20 Paul Floyd 2020-07-30 11:38:59 UTC
This should now be fixed in devel/valgrind-devel and this item can be closed.
Comment 21 Ulrich Spörlein freebsd_committer 2021-03-23 17:00:08 UTC
Merged as of r543273, thanks for the fixes!