Re: [RFC] Target Layer Python Interface

[Kieran Bingham <kieran dot bingham at linaro dot org>]

On 05/02/16 16:47, Kieran Bingham wrote:
> 
> 
> On 05/02/16 16:38, Jeff Mahoney wrote:
>> On 2/5/16 11:36 AM, Kieran Bingham wrote:
>>> On 04/02/16 22:16, Ales Novak wrote:
>>>> Hello,
>>>>
>>>> On 2016-2-1 19:19, Kieran Bingham wrote:
>>>>> [...] The API, I would expect to match that of the Target API
>>>>> operations. I would expect a one-to-one mapping of (required)
>>>>> operation names to perform functions.
>>>>>
>>>>> For instance, to support thread listings, I would expect
>>>>> something along the lines of:
>>>>>
>>>>> .to_update_thread_list .to_pid_to_str .to_extra_thread_info 
>>>>> .to_thread_alive .to_fetch_registers ...
>>>>
>>>> FTR I've slightly tweaked your gdb.Target to process
>>>> "to_xfer_partial", the respective commit is:
>>>>
>>>> https://github.com/alesax/gdb-kdump/commit/efba160691273ef3c154711255
>> 4584088b5dba75
>>>>
>>>>
>>>>
>>>>
>> (and the respective branch is "gdb-target")
>>>>
>>>> Then the target code which is accessing virtual (!) memory of the
>>>> kernel dump on the disk (using libkdumpfile library) is as small
>>>> as:
>>>>
>>>> === from gdb import Target from _kdumpfile import kdumpfile
>>>>
>>>> class MyTarget(Target): def __init__(self, fil): self.kdump =
>>>> kdumpfile(fil) self.kdump.symbol_func = \ lambda nam:
>>>> long(gdb.lookup_minimal_symbol(nam).value()) 
>>>> self.kdump.vtop_init() super(MyTarget, self).__init__() def
>>>> to_xfer_partial(self, obj, annex, readbuf, writebuf, offset,
>>>> ln): if obj == self.TARGET_OBJECT_MEMORY: r = self.kdump.read
>>>> (self.kdump.KDUMP_KVADDR, offset, ln) readbuf[:] = r return ln
>>>>
>>>> MyTarget(file("/tmp/vmcore")) ===
>>>>
>>>> which is really nice, I'd say. Now it would be interesting
>>
>>> Were you going to say something else here? ... looks like it got
>>> chopped!
>>
>>
>>> Pulling the const's through is pretty neat, and that does make an 
>>> effective way to implement the read overrides to a file!
>>
>>
>>> By the way, I'd started to add thread support - but I'm on holiday
>>> now.
>>
>>> Adding an add_thread(pid,lwp,tid) method to the inferior allows
>>
>> Ok, I have code doing that too.  There's a big messy commit at the top
>> of my repo last night that I'm going to refactor but I got most of
>> this working.
> 
> Fantastic, you've probably spent more time on it than me so I'll bet
> your version works before I'm back from holiday!
> 
> I'll go have a quick look now. :D
> 
> I felt a bit like we need a bit better encapsulation on the ptid_t
> object. Might need a small object to wrap it up for passing around
> instead of just throwing Tuples everywhere.



+    if (!result)
+      error("Callback failed.\n");
+
+    ret = python_string_to_host_string (result);
     do_cleanups(cleanup);
+    return ret;
 }

Watch out for the memory leak here ...
It's one of the annoyances of this layer. No way to free the pointers
when we pass them back up to the caller:

In my implementation of pid_to_str, I have a scratch buffer to copy the
string into and then free the object.

It's probably worth putting one scratch buffer in the layer to share
across all functions that need it. We should be can_async_p = 0 anyway I
think.

https://git.linaro.org/people/kieran.bingham/binutils-gdb.git/commitdiff/ecd824755584f5c3f2083f7d9ee4b5e25eb357f3

Ha - I say that - and then I see I didn't free the char array from
python_string_to_host_string (result); in my version! That'll have to be
a fixup later :)

--
Kieran

> 
> --
> Kieran
> 
>>
>> -Jeff
>>
>>> def to_update_thread_list(self): 
>>> gdb.write("LX.to_update_thread_list\n") inferior =
>>> gdb.selected_inferior() threads = inferior.threads() for task in
>>> tasks.task_lists(): # Build ptid_t ... class object better here
>>> still ptid = (inferior.pid, 0, task['pid'])  # (pid, lwp, tid) if
>>> ptid not in threads: gdb.write("- New Task [{} {}]\n" 
>>> .format(task['pid'], task['comm'].string())) 
>>> inferior.add_thread(ptid)
>>
>>
>>> The 'if ptid not in tasks' is not working yet. That was going to be
>>> next on my list.
>>
>>> I think the comparison function is in the wrong place, it should
>>> be implementing __contains__ instead of compare I think.
>>
>>> Then it's just a matter of wiring up Jeff's Regcache ...
>>
>>> If you're interested: My latest patches are at:
>>
>>> http://git.linaro.org/people/kieran.bingham/binutils-gdb.git
>>> lkd-python
>>
>>> And the Kernel Awareness object is at 
>>> http://git.linaro.org/people/kieran.bingham/linux.git lkd-python
>>
>>> Feel free to have a go at wiring up while I'm away if it's useful
>>> to you.
>>
>>
>>>>
>>>>
>>>>> And having seen Jeff's work today, we could utilise Jeff's
>>>>> py-regcache object quite effectively
>>>>>
>>>>> [...]
>>>>>
>>>>> Yes, I suspect some of the functionality to implement will be
>>>>> very repeatable throughout each of the operation call
>>>>> implementations.
>>>>>
>>>>>
>>>>> However, the more I look into it - the more I see each function
>>>>> is likely to need very specific bindings, as it is not simple
>>>>> passing from c function to c function.
>>>>
>>>> Yes, the mentioned to_xfer_partial being a good example (of not
>>>> simple passing).
>>
>>> Indeed - but probably not too many hooks to implement to get
>>> thread integration through python.
>>
>>>>
>>>>> Perhaps we can factor out commonality as we go - and try to
>>>>> keep as DRY as possible, but I suspect it will be an iterative
>>>>> implementation process.
>>>>>
>>>>>
>>>>>> I can't comment without more details though. My initial
>>>>>> reaction though is yeah, this sounds useful and exciting.
>>>>>
>>>>> Perfect :)
>>>>
>>>> Yes, this definitely is worth pursuing.
>>>>
>>
>>> I'm glad you like the concept. I think it can work well with the
>>> recent code Jeff has written.
>>
>>> Although I may be slightly diverted for a bit when I get back from 
>>> holiday - so if it can go somewhere for you guys ... do have a go
>>> until I return. (And let me know how it goes!)
>>
>>> Regards
>>
>>> Kieran
>>
>>
>>
>>