[Intel-gfx] [PATCH igt 02/10] igt/gem_mmap_gtt: Simulate gdb inspecting a GTT mmap using ptrace()

[Tvrtko Ursulin]

On 31/07/2017 11:17, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-31 10:41:06)
>>
>> On 28/07/2017 13:08, Chris Wilson wrote:
>>> gdb uses ptrace() to peek and poke bytes of the target's address space.
>>> The kernel must implement an vm_ops->access() handler or else gdb will
>>> be unable to inspect the pointer and report it as out-of-bounds. Worse
>>> than useless as it causes immediate suspicion of the valid GTT pointer.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> ---
>>>    tests/gem_mmap_gtt.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 79 insertions(+)
>>>
>>> diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
>>> index 4ff5e7f1..61c08406 100644
>>> --- a/tests/gem_mmap_gtt.c
>>> +++ b/tests/gem_mmap_gtt.c
>>> @@ -36,6 +36,8 @@
>>>    #include <errno.h>
>>>    #include <sys/stat.h>
>>>    #include <sys/ioctl.h>
>>> +#include <sys/ptrace.h>
>>> +#include <sys/wait.h>
>>>    #include "drm.h"
>>>    
>>>    #include "igt.h"
>>> @@ -310,6 +312,81 @@ test_write_gtt(int fd)
>>>        munmap(src, OBJECT_SIZE);
>>>    }
>>>    
>>> +static void *memchr_inv(const void *s, int c, size_t n)
>>> +{
>>> +     const uint8_t *us = s;
>>> +     const uint8_t uc = c;
>>> +
>>> +#pragma GCC diagnostic push
>>> +#pragma GCC diagnostic ignored "-Wcast-qual"
>>> +     while (n--) {
>>> +             if (*us != uc)
>>> +                     return (void *) us;
>>> +             us++;
>>> +     }
>>> +#pragma GCC diagnostic pop
>>
>> Stripping away constness wouldn't work with a simpler:
> 
> Or just saying const void *memchr_inv().
> 
> In this case we don't need memchr_inv, as just a bool will do,
> so memchr_eq().
> 
>>
>> uint8_t *us = (uint8_t *)s?
>>
>>> +
>>> +     return NULL;
>>> +}
>>> +
>>> +static void
>>> +test_ptrace(int fd)
>>> +{
>>> +     long AA, CC;
>>> +     long *gtt, *copy;
>>> +     uint32_t bo;
>>> +     pid_t pid;
>>> +
>>> +     memset(&AA, 0xaa, sizeof(AA));
>>> +     memset(&CC, 0x55, sizeof(CC));
>>> +
>>> +     copy = malloc(OBJECT_SIZE);
>>> +     memset(copy, AA, OBJECT_SIZE);
>>> +
>>> +     bo = gem_create(fd, OBJECT_SIZE);
>>> +     gtt = mmap_bo(fd, bo);
>>> +     memset(gtt, CC, OBJECT_SIZE);
>>> +     gem_close(fd, bo);
>>> +
>>> +     igt_assert(!memchr_inv(gtt, CC, OBJECT_SIZE));
>>> +     igt_assert(!memchr_inv(copy, AA, OBJECT_SIZE));
>>> +
>>> +     switch ((pid = fork())) {
>>> +             case -1:
>>> +                     igt_assert(pid != -1);
>>> +                     break;
>>> +
>>> +             case 0:
>>> +                     ptrace(PTRACE_TRACEME, 0, NULL, NULL);
>>> +                     raise(SIGSTOP);
>>> +                     raise(SIGKILL);
>>> +                     exit(0);
>>> +                     break;
>>> +
>>> +             default:
>>> +                     /* Wait for the child to ready themselves */
>>> +                     wait(NULL);
>>> +
>>> +                     ptrace(PTRACE_ATTACH, pid, NULL, NULL);
>>> +                     for (int i = 0; i < OBJECT_SIZE/sizeof(long); i++) {
>>> +                             copy[i] = ptrace(PTRACE_PEEKDATA, pid, gtt+i, NULL);
>>> +                             ptrace(PTRACE_POKEDATA, pid, gtt + i, AA);
>>
>> Inconsistent whitespace in the above three lines. First and second need
>> a tidy.
> 
> ? The whitespace carries meaning for me.

"gtt+i" / "gtt + i" ? And "OBJECT_SIZE/sizeof". I know it's only IGT but 
lets be consistent and follow our guidelines.

Regards,

Tvrtko