When using GDB built with python 2.7,
Inferior.read_memory (address, length)
returns a buffer object. When using GDB built with python 3.X,
it returns a memoryview object, which cannot be added to another
memoryview object.
Replace the addition (+) of 2 python memoryview objects
with the addition of 2 'bytes' objects.
Create a read_memoryview() function that always return a memoryview
object.
Change the read_u16 function so it doesn't need to use ord()
anymore.
Tested with Python 3.4 and gdb 7.7
Signed-off-by: Dom Cote <[email protected]>
---
scripts/gdb/linux/dmesg.py | 9 +++++----
scripts/gdb/linux/utils.py | 9 +++++++--
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
index 927d0d2a3145..96f4732157d8 100644
--- a/scripts/gdb/linux/dmesg.py
+++ b/scripts/gdb/linux/dmesg.py
@@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
if log_first_idx < log_next_idx:
log_buf_2nd_half = -1
length = log_next_idx - log_first_idx
- log_buf = inf.read_memory(start, length)
+ log_buf = utils.read_memoryview(inf, start, length).tobytes()
else:
log_buf_2nd_half = log_buf_len - log_first_idx
- log_buf = inf.read_memory(start, log_buf_2nd_half) + \
- inf.read_memory(log_buf_addr, log_next_idx)
+ a = utils.read_memoryview(inf, start, log_buf_2nd_half)
+ b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
+ log_buf = a.tobytes() + b.tobytes()
pos = 0
while pos < log_buf.__len__():
@@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
text = log_buf[pos + 16:pos + 16 + text_len]
time_stamp = utils.read_u64(log_buf[pos:pos + 8])
- for line in memoryview(text).tobytes().splitlines():
+ for line in text.splitlines():
gdb.write("[{time:12.6f}] {line}\n".format(
time=time_stamp / 1000000000.0,
line=line))
diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
index 0893b326a28b..c2b779e7bd26 100644
--- a/scripts/gdb/linux/utils.py
+++ b/scripts/gdb/linux/utils.py
@@ -87,11 +87,16 @@ def get_target_endianness():
return target_endianness
+# Compat between GDB built with python 2.7 vs 3.X
+def read_memoryview(inf, start, length):
+ return memoryview(inf.read_memory(start, length))
+
+
def read_u16(buffer):
if get_target_endianness() == LITTLE_ENDIAN:
- return ord(buffer[0]) + (ord(buffer[1]) << 8)
+ return buffer[0] + (buffer[1] << 8)
else:
- return ord(buffer[1]) + (ord(buffer[0]) << 8)
+ return buffer[1] + (buffer[0] << 8)
def read_u32(buffer):
--
1.9.1
Hi Dom,
I've just tested your patch quickly, and it generates an error on Python 2.7
On 05/04/16 19:38, Dom Cote wrote:
> When using GDB built with python 2.7,
>
> Inferior.read_memory (address, length)
>
> returns a buffer object. When using GDB built with python 3.X,
> it returns a memoryview object, which cannot be added to another
> memoryview object.
>
> Replace the addition (+) of 2 python memoryview objects
> with the addition of 2 'bytes' objects.
>
> Create a read_memoryview() function that always return a memoryview
> object.
The change to memoryview appears to work - but I don't think it needs to
be indirected by a function definition?
> Change the read_u16 function so it doesn't need to use ord()
> anymore.
This change is separate to the memoryview object, and should be in it's
own patch. One patch should fix one thing independently.
For example, the change to memoryview object appears to be functional,
and the read_u16 is not. If these changes are in separate patches, then
working changes can be accepted sooner, and tested easier.
> Tested with Python 3.4 and gdb 7.7
>
> Signed-off-by: Dom Cote <[email protected]>
> ---
> scripts/gdb/linux/dmesg.py | 9 +++++----
> scripts/gdb/linux/utils.py | 9 +++++++--
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
> index 927d0d2a3145..96f4732157d8 100644
> --- a/scripts/gdb/linux/dmesg.py
> +++ b/scripts/gdb/linux/dmesg.py
> @@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
> if log_first_idx < log_next_idx:
> log_buf_2nd_half = -1
> length = log_next_idx - log_first_idx
> - log_buf = inf.read_memory(start, length)
> + log_buf = utils.read_memoryview(inf, start, length).tobytes()
This looks like it could just call memoryview() directly ...
> else:
> log_buf_2nd_half = log_buf_len - log_first_idx
> - log_buf = inf.read_memory(start, log_buf_2nd_half) + \
> - inf.read_memory(log_buf_addr, log_next_idx)
> + a = utils.read_memoryview(inf, start, log_buf_2nd_half)
> + b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
Likewise here I presume ...
> + log_buf = a.tobytes() + b.tobytes()
>
> pos = 0
> while pos < log_buf.__len__():
> @@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
> text = log_buf[pos + 16:pos + 16 + text_len]
> time_stamp = utils.read_u64(log_buf[pos:pos + 8])
>
> - for line in memoryview(text).tobytes().splitlines():
> + for line in text.splitlines():
This looks like a separate change, not related to the bug fix?
If this is an improvement, rather than a fix - it should probably also
have it's own patch. (at first glance it looks like an improvement :D )
I just haven't seen yet if it depends on the other change.
> gdb.write("[{time:12.6f}] {line}\n".format(
> time=time_stamp / 1000000000.0,
> line=line))
> diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> index 0893b326a28b..c2b779e7bd26 100644
> --- a/scripts/gdb/linux/utils.py
> +++ b/scripts/gdb/linux/utils.py
> @@ -87,11 +87,16 @@ def get_target_endianness():
> return target_endianness
>
>
> +# Compat between GDB built with python 2.7 vs 3.X
> +def read_memoryview(inf, start, length):
> + return memoryview(inf.read_memory(start, length))
This simply always returns a memoryview object, so why not just change
the respective lines, which you have already had to modify to call/use
memoryview directly?
> +
> +
> def read_u16(buffer):
> if get_target_endianness() == LITTLE_ENDIAN:
> - return ord(buffer[0]) + (ord(buffer[1]) << 8)
> + return buffer[0] + (buffer[1] << 8)
> else:
> - return ord(buffer[1]) + (ord(buffer[0]) << 8)
> + return buffer[1] + (buffer[0] << 8)
This breaks for me, but returning these lines to use ord() shows that
the memoryview changes appear to work.
What was the need for changing these lines? Does it cause a break on 3.x?
This causes the following error on 2.7 for me:
(gdb) lx-dmesg
Traceback (most recent call last):
File
"/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/dmesg.py",
line 45, in invoke
length = utils.read_u16(log_buf[pos + 8:pos + 10])
File
"/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/utils.py",
line 97, in read_u16
return buffer[0] + (buffer[1] << 8)
TypeError: unsupported operand type(s) for <<: 'str' and 'int'
Error occurred in Python command: unsupported operand type(s) for <<:
'str' and 'int'
>
>
> def read_u32(buffer):
>
--
Regards
Kieran
Hi Dom,
On 08/04/16 04:33, Dom Cote wrote:
> Hi Kieran,
>
> Thanks for the feedback.
>
> I am going to need to build a version of gdb with python 2.7 to see what
> the differences are, and try to abstract them in the best possible way.
>
> Then I will send a new patch and incorporate what you just highlighted
> in it.
I was just wondering if you had chance to look at any of this ? (I was
looking at it again myself)
> Regards
>
> Dom
>
>
> On Thu, Apr 7, 2016 at 11:02 PM, Kieran Bingham
> <[email protected] <mailto:[email protected]>> wrote:
>
> Hi Dom,
>
> I've just tested your patch quickly, and it generates an error on
> Python 2.7
>
>
> On 05/04/16 19:38, Dom Cote wrote:
> > When using GDB built with python 2.7,
> >
> > Inferior.read_memory (address, length)
> >
> > returns a buffer object. When using GDB built with python 3.X,
> > it returns a memoryview object, which cannot be added to another
> > memoryview object.
> >
> > Replace the addition (+) of 2 python memoryview objects
> > with the addition of 2 'bytes' objects.
> >
> > Create a read_memoryview() function that always return a memoryview
> > object.
>
> The change to memoryview appears to work - but I don't think it needs to
> be indirected by a function definition?
>
>
> > Change the read_u16 function so it doesn't need to use ord()
> > anymore.
>
> This change is separate to the memoryview object, and should be in it's
> own patch. One patch should fix one thing independently.
>
> For example, the change to memoryview object appears to be functional,
> and the read_u16 is not. If these changes are in separate patches, then
> working changes can be accepted sooner, and tested easier.
>
>
> > Tested with Python 3.4 and gdb 7.7
> >
> > Signed-off-by: Dom Cote <[email protected] <mailto:[email protected]>>
> > ---
> > scripts/gdb/linux/dmesg.py | 9 +++++----
> > scripts/gdb/linux/utils.py | 9 +++++++--
> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
> > index 927d0d2a3145..96f4732157d8 100644
> > --- a/scripts/gdb/linux/dmesg.py
> > +++ b/scripts/gdb/linux/dmesg.py
> > @@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
> > if log_first_idx < log_next_idx:
> > log_buf_2nd_half = -1
> > length = log_next_idx - log_first_idx
> > - log_buf = inf.read_memory(start, length)
> > + log_buf = utils.read_memoryview(inf, start, length).tobytes()
>
> This looks like it could just call memoryview() directly ...
I just looked at the line sizes:
- log_buf = inf.read_memory(start, length)
+ log_buf = utils.read_memoryview(inf, start, length).tobytes()
+ log_buf = memoryview(inf.read_memory(start, length)).tobytes()
There's not a lot in it, and saving one char is probably not enough
reason to add a function call.
>
> > else:
> > log_buf_2nd_half = log_buf_len - log_first_idx
> > - log_buf = inf.read_memory(start, log_buf_2nd_half) + \
> > - inf.read_memory(log_buf_addr, log_next_idx)
> > + a = utils.read_memoryview(inf, start, log_buf_2nd_half)
> > + b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
>
> Likewise here I presume ...
>
> > + log_buf = a.tobytes() + b.tobytes()
> >
> > pos = 0
> > while pos < log_buf.__len__():
> > @@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
> > text = log_buf[pos + 16:pos + 16 + text_len]
> > time_stamp = utils.read_u64(log_buf[pos:pos + 8])
> >
> > - for line in memoryview(text).tobytes().splitlines():
> > + for line in text.splitlines():
>
> This looks like a separate change, not related to the bug fix?
> If this is an improvement, rather than a fix - it should probably also
> have it's own patch. (at first glance it looks like an improvement :D )
>
> I just haven't seen yet if it depends on the other change.
>
> > gdb.write("[{time:12.6f}] {line}\n".format(
> > time=time_stamp / 1000000000.0,
> > line=line))
> > diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> > index 0893b326a28b..c2b779e7bd26 100644
> > --- a/scripts/gdb/linux/utils.py
> > +++ b/scripts/gdb/linux/utils.py
> > @@ -87,11 +87,16 @@ def get_target_endianness():
> > return target_endianness
> >
> >
> > +# Compat between GDB built with python 2.7 vs 3.X
> > +def read_memoryview(inf, start, length):
> > + return memoryview(inf.read_memory(start, length))
>
> This simply always returns a memoryview object, so why not just change
> the respective lines, which you have already had to modify to call/use
> memoryview directly?
>
> > +
> > +
> > def read_u16(buffer):
> > if get_target_endianness() == LITTLE_ENDIAN:
> > - return ord(buffer[0]) + (ord(buffer[1]) << 8)
> > + return buffer[0] + (buffer[1] << 8)
> > else:
> > - return ord(buffer[1]) + (ord(buffer[0]) << 8)
> > + return buffer[1] + (buffer[0] << 8)
>
> This breaks for me, but returning these lines to use ord() shows that
> the memoryview changes appear to work.
>
> What was the need for changing these lines? Does it cause a break on
> 3.x?
>
> This causes the following error on 2.7 for me:
>
> (gdb) lx-dmesg
> Traceback (most recent call last):
> File
> "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/dmesg.py",
> line 45, in invoke
> length = utils.read_u16(log_buf[pos + 8:pos + 10])
> File
> "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/utils.py",
> line 97, in read_u16
> return buffer[0] + (buffer[1] << 8)
> TypeError: unsupported operand type(s) for <<: 'str' and 'int'
> Error occurred in Python command: unsupported operand type(s) for <<:
> 'str' and 'int'
I guess we're going to have to do something along the lines of
PY2 = (sys.version_info[0] == 2)
if PY2:
# Do Py2 compatible code
else:
# Do Py3+ stuff
to support both targets. (Ugh...) But this isn't the first time I've
been asked if we support both Python 2 and Python 3 so I guess it will
become an issue.
Anyway, let me know if there's anything I can do to help / test.
--
Regards
Kieran Bingham