| Summary: | Perf Events ioctls not implemented | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Keno Fischer <keno> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | wishlist | CC: | mark |
| Priority: | NOR | ||
| Version: | 3.12 SVN | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Attachments: |
Patch to add valgrind support for perf events ioctls.
Updated patch. |
||
|
Description
Keno Fischer
2016-09-07 19:41:02 UTC
Created attachment 100976 [details]
Patch to add valgrind support for perf events ioctls.
Here's a patched I whipped up that seems to work fine. This is my first patch to valgrind, please let me know if I should do anything else.
Keno, thank you for the patch. It looks OK, apart from this fragment
+ case VKI_PERF_EVENT_IOC_SET_FILTER: {
+ char *filter = (char *)ARG3;
+ PRE_MEM_READ("ioctl(VKI_PERF_EVENT_IOC_SET_FILTER)",
+ (Addr)ARG3, VG_(strnlen)(filter, VKI_PAGE_SIZE)+1);
+ break;
+ }
If I interpret this correctly, ARG3 is a C style zero-terminated string that the syscall
reads. Is that correct?
If so, there's already a macro for that: PRE_MEM_RASCIIZ. I think you want
something like PRE_MEM_RASCIIZ(ioctl(VKI_PERF_EVENT_IOC_SET_FILTER).filter", ARG3).
Can you redo the patch and re-test it? Thanks.
> If I interpret this correctly, ARG3 is a C style zero-terminated string that the syscall
> reads. Is that correct?
Essentially yes, though with the caveat that if there's no NUL after PAGE_SIZE-1 bytes, it'll still accept that as far as I know (as a PAGE_SIZE-1 sized string), since it uses strndup_user(str, PAGE_SIZE) on the kernel side of things.
(In reply to Keno Fischer from comment #3) > > If I interpret this correctly, ARG3 is a C style zero-terminated string that the syscall > > reads. Is that correct? > > Essentially yes, though with the caveat that if there's no NUL after > PAGE_SIZE-1 bytes, it'll still accept that as far as I know (as a > PAGE_SIZE-1 sized string), since it uses strndup_user(str, PAGE_SIZE) on the > kernel side of things. That seems a funny corner case that I cannot imagine anybody relies on (the caller would have to explicitly not pass a zero terminated string larger than PAGE_SIZE, which is different on different arches). If someone does I think a warning from valgrind seems somewhat justified. So just using PRE_MEM_RASCIIZ here should be fine. Created attachment 101305 [details]
Updated patch.
Committed, r16077. Thanks for the patch. |