Bug 234631
| Summary: | devel/valgrind: Fixes for FreeBSD 12.x support | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Julien Nadeau <vedge> | ||||||||
| Component: | Individual Port(s) | Assignee: | Niclas Zeising <zeising> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Only Me | CC: | doctorwhoguy, egil.hasting, janm, ml, natalino.picone, nhoyle, paulf, rozhuk.im, uqs, w.schwarzenfeld, zeising | ||||||||
| Priority: | --- | Keywords: | needs-patch, needs-qa | ||||||||
| Version: | Latest | Flags: | rozhuk.im:
maintainer-feedback-
koobs: merge-quarterly? |
||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
-PORTEPOCH= 1 +PORTEPOCH= 2 Should be a PORTREVISION bump Assign to new maintainer. 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. Created attachment 202919 [details]
pipe() -> pipe2(,0)
Also on FreeBSD 12 rquired: CFLAGS+= -mstack-alignment=8 looks like defaults now 16 and this brokes valgrind. Can you also integrate my patch: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234045 ? 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
(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! 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? Any update on this ? (In reply to Natalino Picone from comment #10) I've integrated these diffs into my github repo. Needs more testing. https://github.com/paulfloyd/freebsd_valgrind (In reply to rozhuk.im from comment #5) Do you have any further information on this? I haven't noticed it making any difference. (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. IMHO valgring should be in base, there is to many system and OS version specific things. (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. (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: valgrind11 valgrind12 valgrind-current or integrate it in FreeBSD source tree. (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. (In reply to Paul Floyd from comment #17) Use: __FreeBSD_version from sys/param.h no need to detect in build scripts. (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. This should now be fixed in devel/valgrind-devel and this item can be closed. Merged as of r543273, thanks for the fixes! |
Created attachment 200787 [details] Patch Add 12.x kevent, minherit, getrandom syscalls. Add 12.x struct stat. Suggest consolidating EXTRA_PATCHES so that 'update-patches' can work normally.