I'd like to use valgrind to check the rr tool[1], and rr uses kcmp. Valgrind doesn't seem to support kcmp yet (at least for x86 processes). kcmp doesn't have memory side effects, so I suspect it would be easy to support and I may write the patch myself. (Hi Julian/Nick, if you watch this.) [1] http://rr-project.org/
Created attachment 88158 [details] Add kcmp support. Based on r14237. This seems to work for me. Not sure what tests are required with a somewhat trivial patch like this.
Chris, hi. Thanks for the patch. Committed as r14451.
Great, thanks!
hmm. Seems that idx1 and idx2 are only evaluated for type=KCMP_FILE. So valgrind might complain about an ininitialized system call parameter e.g. for type= KCMP_VM if the parameter register happens to contain an initialized value
(In reply to Christian Borntraeger from comment #4) > hmm. Seems that idx1 and idx2 are only evaluated for type=KCMP_FILE [..] Christian, I assume that you determined this by reading the kernel sources? If yes, can you tell me where?
Its even documented: http://man7.org/linux/man-pages/man2/kcmp.2.html But in the kernel code its also obvious: file kernel/kcmp.c look for SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, unsigned long, idx1, unsigned long, idx2) idx1 and idx2 are only used for KCMP_FILE
Thanks for the catch. Is it convention to back out or fix up the bug in place? (I won't have time to return to this for at least a few days, so please feel free to brutally revert.)
I think its better than before. Lets just fixup the pre handler in the next days...cant be that hard (hopefully)
something like Index: coregrind/m_syswrap/syswrap-linux.c =================================================================== --- coregrind/m_syswrap/syswrap-linux.c (revision 14461) +++ coregrind/m_syswrap/syswrap-linux.c (working copy) @@ -10103,10 +10103,16 @@ PRE(sys_kcmp) { - PRINT("kcmp ( %ld, %ld, %ld, %lu, %lu )", ARG1, ARG1, ARG3, ARG4, ARG5); - PRE_REG_READ5(long, "kcmp", - vki_pid_t, pid1, vki_pid_t, pid2, int, type, - unsigned long, idx1, unsigned long, idx2); + if (ARG3 == VKI_KCMP_FILE) { + PRINT("kcmp ( %ld, %ld, %ld, %lu, %lu )", ARG1, ARG1, ARG3, ARG4, ARG5); + PRE_REG_READ5(long, "kcmp", + vki_pid_t, pid1, vki_pid_t, pid2, int, type, + unsigned long, idx1, unsigned long, idx2) + } else { + PRINT("kcmp ( %ld, %ld, %ld )", ARG1, ARG1, ARG3); + PRE_REG_READ3(long, "kcmp", + vki_pid_t, pid1, vki_pid_t, pid2, int, type); + } } #undef PRE Index: include/vki/vki-linux.h =================================================================== --- include/vki/vki-linux.h (revision 14461) +++ include/vki/vki-linux.h (working copy) @@ -4502,6 +4502,18 @@ #define VKI_MEDIA_IOC_ENUM_LINKS _VKI_IOWR('|', 0x02, struct vki_media_links_enum) #define VKI_MEDIA_IOC_SETUP_LINK _VKI_IOWR('|', 0x03, struct vki_media_link_desc) +/* Comparison type */ +enum vki_kcmp_type { + VKI_KCMP_FILE, + VKI_KCMP_VM, + VKI_KCMP_FILES, + VKI_KCMP_FS, + VKI_KCMP_SIGHAND, + VKI_KCMP_IO, + VKI_KCMP_SYSVSEM, + + VKI_KCMP_TYPES, +}; #endif // __VKI_LINUX_H /*--------------------------------------------------------------------*/ should do the trick (untested) Julian, let me know if I should apply this.
Christian, thank you for the investigation and patch. Committed w/ a bit of change as r14473.
Thanks Christian and Julian!