Created attachment 119842 [details] patch valgrind currently does not support copy_file_range syscall. My patch adds the support for the various architectures.
Very nice. This looks good. (disclaimer, Sasha and I already did discuss this patch on irc). I did two tweaks. Removed the args from vgtest file (the testcase doesn't need arguments anymore). And added a NEWS entry/this bug number (which is indeed kinda circular, you only know the bug number after submitting the bug). commit 5f00db054a6f59502e9deeeb59ace2261207ee31 Author: Alexandra Hajkova <ahajkova@redhat.com> Date: Thu May 2 08:24:02 2019 -0400 Add support for the copy_file_range syscall Support amd64, x86, arm64, ppc64, ppc32 and s390x architectures. Also add sys-copy_file_range test case. I'll test on arm32. I believe the code should work as is there too. Petar, could you check whether mip32/mips64 can also be hooked up?
Tested on Fedora f29 armv7. Works as expected (modulo a strange backtrace issue, but that has nothing to do with this code or test). Pushed: commit bd27ad3ff31555484b7fdb310c4b033620882e44 Author: Mark Wielaard <mark@klomp.org> Date: Sun May 5 16:01:41 2019 +0200 Hook linux copy_file_range syscall on arm. diff --git a/coregrind/m_syswrap/syswrap-arm-linux.c b/coregrind/m_syswrap/syswrap-arm-linux.c index 9f1bdab..9ba0665 100644 --- a/coregrind/m_syswrap/syswrap-arm-linux.c +++ b/coregrind/m_syswrap/syswrap-arm-linux.c @@ -1016,6 +1016,8 @@ static SyscallTableEntry syscall_main_table[] = { LINXY(__NR_getrandom, sys_getrandom), // 384 LINXY(__NR_memfd_create, sys_memfd_create), // 385 + LINX_(__NR_copy_file_range, sys_copy_file_range), // 391 + LINXY(__NR_statx, sys_statx), // 397 };
(In reply to Mark Wielaard from comment #1) > Very nice. This looks good. (disclaimer, Sasha and I already did discuss > this patch on irc). I did two tweaks. Removed the args from vgtest file (the > testcase doesn't need arguments anymore). And added a NEWS entry/this bug > number (which is indeed kinda circular, you only know the bug number after > submitting the bug). > > commit 5f00db054a6f59502e9deeeb59ace2261207ee31 > Author: Alexandra Hajkova <ahajkova@redhat.com> > Date: Thu May 2 08:24:02 2019 -0400 > > Add support for the copy_file_range syscall > > Support amd64, x86, arm64, ppc64, ppc32 and s390x architectures. > Also add sys-copy_file_range test case. > > I'll test on arm32. I believe the code should work as is there too. > > Petar, could you check whether mip32/mips64 can also be hooked up? Mark, the sys-copy_file_range.stderr.exp does not cover all the cases. Here is a exp diff on mips32: 5a6,10 > Syscall param copy_file_range(off_in) points to unaddressable byte(s) > ... > by 0x........: main (sys-copy_file_range.c:57) > Address 0x........ is not stack'd, malloc'd or (recently) free'd > 21c26 < ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) --- > ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0) In short, on mips32 one more issue is reported. How do you want to handle this? New exp file or something else?
(In reply to Petar Jovanovic from comment #3) > Mark, the sys-copy_file_range.stderr.exp does not cover all the cases. > > Here is a exp diff on mips32: > > 5a6,10 > > Syscall param copy_file_range(off_in) points to unaddressable byte(s) > > ... > > by 0x........: main (sys-copy_file_range.c:57) > > Address 0x........ is not stack'd, malloc'd or (recently) free'd > > > 21c26 > < ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) > --- > > ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0) > > In short, on mips32 one more issue is reported. How do you want to handle > this? New exp file or something else? O, lovely. So in that case the undefined memory points to an invalid address... So we want the pointer to be defined to something valid and undefined. Does the following work on mips32? (patch explicitly doesn't change the line numbers of the test function calls to keep the exp file the same): diff --git a/memcheck/tests/linux/sys-copy_file_range.c b/memcheck/tests/linux/sys-copy_file_range.c index 83981c6..589399c 100644 --- a/memcheck/tests/linux/sys-copy_file_range.c +++ b/memcheck/tests/linux/sys-copy_file_range.c @@ -3,8 +3,8 @@ #include <stdio.h> #include <stdlib.h> #include <sys/stat.h> -#include <sys/syscall.h> #include <unistd.h> +#include "../../memcheck.h" int main(int argc, char **argv) { @@ -51,7 +51,7 @@ int main(int argc, char **argv) /* Check valgrind will produce expected warnings for the various wrong arguments. */ do { - void *t; + void *t = 0; VALGRIND_MAKE_MEM_UNDEFINED (&t, sizeof (void *)); void *z = (void *) -1; ret = copy_file_range(fd_in, t, fd_out, NULL, len, 0);
(In reply to Mark Wielaard from comment #4) > > Does the following work on mips32? Yes, it does. Can you commit the change? Thanks.
(In reply to Petar Jovanovic from comment #5) > (In reply to Mark Wielaard from comment #4) > > Does the following work on mips32? > > Yes, it does. Can you commit the change? commit c212b72a63e43be323a4e028bbdbe8b023c22be8 Author: Mark Wielaard <mark@klomp.org> Date: Wed May 15 21:30:00 2019 +0200 Explicitly make testcase variable for sys-copy_file_range undefined. On some systems an extra warning could occur when a variable in the memcheck/tests/linux/sys-copy_file_range testcase was undefined, but (accidentially) pointed to known bad memory. Fix by defining the variable as 0, but then marking it explicitly undefined using memcheck VALGRIND_MAKE_MEM_UNDEFINED. Followup for https://bugs.kde.org/show_bug.cgi?id=407218
commit 033d013bebeb3471c0da47060deb9a5771e6c913 Author: Mark Wielaard <mark@klomp.org> Date: Fri May 24 21:51:31 2019 +0200 Fix memcheck/tests/linux/sys-copy_file_range open call (mode). sys-copy_file_range.c calls open with O_CREAT flag and so must provide a mode argument. valgrind memcheck actually caught this ommission on some arches (fedora rawhide i686 specifically). This is a small additional fixup for https://bugs.kde.org/show_bug.cgi?id=407218 diff --git a/memcheck/tests/linux/sys-copy_file_range.c b/memcheck/tests/linux/s index 589399c..3022fa1 100644 --- a/memcheck/tests/linux/sys-copy_file_range.c +++ b/memcheck/tests/linux/sys-copy_file_range.c @@ -12,7 +12,7 @@ int main(int argc, char **argv) struct stat stat; loff_t len, ret; - fd_in = open("copy_file_range_source", O_CREAT | O_RDWR); + fd_in = open("copy_file_range_source", O_CREAT | O_RDWR, 0644); if (fd_in == -1) { perror("open copy_file_range_source"); exit(EXIT_FAILURE);
*** Bug 443833 has been marked as a duplicate of this bug. ***