| Summary: | FreeBSD support, part 1 | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Paul Floyd <pjfloyd> |
| Component: | vex | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | emaste, gharris, mark, tom |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | FreeBSD | ||
| See Also: | https://bugs.kde.org/show_bug.cgi?id=208531 | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
diff in vex directory
diff in vex directory diff in vex directory |
||
Forgot to add, there are also a few minor nits that clang was moaning about. (In reply to Paul Floyd from comment #0) > Created attachment 136050 [details] > diff in vex directory > > Primarily these changes concern the nature of alignment generated signals. > On Linux, these produce SIGSEGV and n FreeBSD they produce SIGBUS. On most if not all *BSDs they produce SIGBUS. That *might* be the case on Solaris as well. (I remember it being the case on SunOS on Sun-2, where 16-bit and wider quantities had to be aligned on 16-bit boundaries, and on SPARC; that probably carried over to SunOS 5.) Personally, I think that's how it *should* be, but.... I'm fairly sure that this behaviour is not in SunOS 5 amd64. Work was done on a SPARC port but it never made it to the official repo (this was around the time that Orcale dropped the ball). Valgrind does run on Solaris 11.3, 11.4 and Illumos amd64. I'll double check next time I boot my trusty old Solaris PC. If you think it would be better to rename the function and have a single ifdef there (and I definitely agree that it would be) then why not post that patch rather than one that you think is suboptimal? Changing the function name would reduce code duplication (virtually all of the SEGV vs BUS functions are the same) and also reduce the amount of ifdefery dramatically. In general I've tried to tread very carefully and make as few changes as possible that would affect Linux. This probably took that too far. I'll update the code and the patch. Created attachment 136983 [details]
diff in vex directory
Change to using generic SIGNAL functions for all platforms, and put the #ifdefed code inside gen_SIGNAL_if_not_XX_align
Created attachment 141832 [details]
diff in vex directory
I cannot comment on the freebsd specific changes, I assume they are correct. The changes in VEX/priv/guest_amd64_toIR.c seem fine. As do the changes in VEX/priv/host_amd64_defs.c and VEX/priv/host_amd64_isel.c. MIPS and PPC already use synth_sigbus, so this should work. The change to the initialization of vex_control is correct. The change in VEX/pub/libvex.h looks OK too to help clang. The patch compiles fine on Fedora x86_64, please apply. These changes pushed with commit 33012dd82ba5f3502d3c6c5d95cf22b7af48f6b3 |
Created attachment 136050 [details] diff in vex directory Primarily these changes concern the nature of alignment generated signals. On Linux, these produce SIGSEGV and n FreeBSD they produce SIGBUS. It would be cleaner in my opinion to rename the functions to be gen_SIGNAL_if_not_XX_aligned gen_SIGNAL_if_not_16_aligned etc. and just have the different code as follows static void gen_SIGNAL_if_not_XX_aligned ( IRTemp effective_addr, ULong mask ) { stmt( IRStmt_Exit( binop(Iop_CmpNE64, binop(Iop_And64,mkexpr(effective_addr),mkU64(mask)), mkU64(0)), #if !defined(VGO_FREEBSD) Ijk_SigSEGV, #else Ijk_SigBUS, #endif IRConst_U64(guest_RIP_curr_instr), OFFB_RIP ) ); }