[v2] dump-guest-memory.py: fix python 2 support

Python GDB support may use Python 2 or 3.

Inferior.read_memory() may return a 'buffer' with Python 2 or a
'memoryview' with Python 3 (see also
https://sourceware.org/gdb/onlinedocs/gdb/Inferiors-In-Python.html)

The elf.add_vmcoreinfo_note() method expects a "bytes" object. Wrap
the returned memory with bytes(), which works with both 'memoryview'
and 'buffer'.

Fixes a regression introduced with commit
d23bfa91b7789534d16ede6cb7d925bfac3f3c4c ("add vmcoreinfo").

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/dump-guest-memory.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 01/19/18 17:25, Marc-André Lureau wrote:
> Python GDB support may use Python 2 or 3.
> 
> Inferior.read_memory() may return a 'buffer' with Python 2 or a
> 'memoryview' with Python 3 (see also
> https://sourceware.org/gdb/onlinedocs/gdb/Inferiors-In-Python.html)
> 
> The elf.add_vmcoreinfo_note() method expects a "bytes" object. Wrap
> the returned memory with bytes(), which works with both 'memoryview'
> and 'buffer'.
> 
> Fixes a regression introduced with commit
> d23bfa91b7789534d16ede6cb7d925bfac3f3c4c ("add vmcoreinfo").
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/dump-guest-memory.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 09bec92b50..03fbf69f8a 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -564,7 +564,7 @@ shape and this command should mostly work."""
>  
>          vmcoreinfo = self.phys_memory_read(addr, size)
>          if vmcoreinfo:
> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
> +            self.elf.add_vmcoreinfo_note(bytes(vmcoreinfo))
>  
>      def invoke(self, args, from_tty):
>          """Handles command invocation from gdb."""
> 

Ugh, can you please point me to the v1->v2 difference?

Thanks
Laszlo

On 01/19/2018 01:18 PM, Laszlo Ersek wrote:
> On 01/19/18 17:25, Marc-André Lureau wrote:
>> Python GDB support may use Python 2 or 3.
>>
>> Inferior.read_memory() may return a 'buffer' with Python 2 or a
>> 'memoryview' with Python 3 (see also
>> https://sourceware.org/gdb/onlinedocs/gdb/Inferiors-In-Python.html)
>>
>> The elf.add_vmcoreinfo_note() method expects a "bytes" object. Wrap
>> the returned memory with bytes(), which works with both 'memoryview'
>> and 'buffer'.
>>

If I understand it, the issue stems from the fact that vmcoreinfo is a
bytes buffer under python2, but a memoryview under python3.  Let's
compare the patches:

The V1 patch:
https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg04010.html

>
>          if vmcoreinfo:
> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
> +            # Python 2.7 returns a buffer
> +            vmciview = memoryview(vmcoreinfo)
> +            self.elf.add_vmcoreinfo_note(vmciview.tobytes())

did an unconditional memoryview(vmcoreinfo) followed by a .tobytes()
call. Under python 2.7, vmcoreinfo is converted from a buffer to a
memoryview, then back to bytes; under python 3, memoryview(vmcoreinfo)
is a no-op (since it is already a memory view), then that is converted
to bytes; the double conversion was necessary because bytes.tobytes()
doesn't exist.  But python 2.6 doesn't have the memoryview conversion,
so we've excluded older python users.

Now looking at the V2 patch:


>> @@ -564,7 +564,7 @@ shape and this command should mostly work."""
>>  
>>          vmcoreinfo = self.phys_memory_read(addr, size)
>>          if vmcoreinfo:
>> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
>> +            self.elf.add_vmcoreinfo_note(bytes(vmcoreinfo))

Under python2, and bytes(buffer) is still bytes; under python3,
vmcoreinfo is a memoryview but bytes(buffer) is equivalent to
buffer.to_bytes().  We've avoided the intermediate conversion through
memoryview, and thus work regardless of python version.

> Ugh, can you please point me to the v1->v2 difference?

Hope that helped (and hope I got it right, I'm learning as well from
this thread).

On 01/19/18 20:48, Eric Blake wrote:
> On 01/19/2018 01:18 PM, Laszlo Ersek wrote:
>> On 01/19/18 17:25, Marc-André Lureau wrote:
>>> Python GDB support may use Python 2 or 3.
>>>
>>> Inferior.read_memory() may return a 'buffer' with Python 2 or a
>>> 'memoryview' with Python 3 (see also
>>> https://sourceware.org/gdb/onlinedocs/gdb/Inferiors-In-Python.html)
>>>
>>> The elf.add_vmcoreinfo_note() method expects a "bytes" object. Wrap
>>> the returned memory with bytes(), which works with both 'memoryview'
>>> and 'buffer'.
>>>
> 
> If I understand it, the issue stems from the fact that vmcoreinfo is a
> bytes buffer under python2, but a memoryview under python3.  Let's
> compare the patches:
> 
> The V1 patch:
> https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg04010.html
> 
>>
>>          if vmcoreinfo:
>> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
>> +            # Python 2.7 returns a buffer
>> +            vmciview = memoryview(vmcoreinfo)
>> +            self.elf.add_vmcoreinfo_note(vmciview.tobytes())
> 
> did an unconditional memoryview(vmcoreinfo) followed by a .tobytes()
> call. Under python 2.7, vmcoreinfo is converted from a buffer to a
> memoryview, then back to bytes; under python 3, memoryview(vmcoreinfo)
> is a no-op (since it is already a memory view), then that is converted
> to bytes; the double conversion was necessary because bytes.tobytes()
> doesn't exist.  But python 2.6 doesn't have the memoryview conversion,
> so we've excluded older python users.
> 
> Now looking at the V2 patch:
> 
> 
>>> @@ -564,7 +564,7 @@ shape and this command should mostly work."""
>>>  
>>>          vmcoreinfo = self.phys_memory_read(addr, size)
>>>          if vmcoreinfo:
>>> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
>>> +            self.elf.add_vmcoreinfo_note(bytes(vmcoreinfo))
> 
> Under python2, and bytes(buffer) is still bytes; under python3,
> vmcoreinfo is a memoryview but bytes(buffer) is equivalent to
> buffer.to_bytes().  We've avoided the intermediate conversion through
> memoryview, and thus work regardless of python version.

Heh, "semantics" :)

> 
>> Ugh, can you please point me to the v1->v2 difference?
> 
> Hope that helped (and hope I got it right, I'm learning as well from
> this thread).
> 

It sure helped a lot, thank you very much! In fact, you've done all the
review work, so technically, "R-b: Laszlo" would be misappropriation.
Can you give your R-b?

I'll manage an "I agree" statement:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

On 01/19/2018 10:25 AM, Marc-André Lureau wrote:
> Python GDB support may use Python 2 or 3.
> 
> Inferior.read_memory() may return a 'buffer' with Python 2 or a
> 'memoryview' with Python 3 (see also
> https://sourceware.org/gdb/onlinedocs/gdb/Inferiors-In-Python.html)
> 
> The elf.add_vmcoreinfo_note() method expects a "bytes" object. Wrap
> the returned memory with bytes(), which works with both 'memoryview'
> and 'buffer'.
> 
> Fixes a regression introduced with commit
> d23bfa91b7789534d16ede6cb7d925bfac3f3c4c ("add vmcoreinfo").
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/dump-guest-memory.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>