[PATCH] [gdb/python] Fix gdb.python/py-disasm.exp on arm-linux

[PATCH] [gdb/python] Fix gdb.python/py-disasm.exp on arm-linux

[Tom de Vries]

After fixing test-case gdb.python/py-disasm.exp to recognize the arm nop:
...
	nop	{0}
...
we run into:
...
disassemble test^M
Dump of assembler code for function test:^M
   0x004004d8 <+0>:     push    {r11}           @ (str r11, [sp, #-4]!)^M
   0x004004dc <+4>:     add     r11, sp, #0^M
   0x004004e0 <+8>:     nop     {0}^M
=> 0x004004e4 <+12>:    Python Exception <class 'ValueError'>: Buffer \
  returned from read_memory is sized 0 instead of the expected 4^M
^M
unknown disassembler error (error = -1)^M
(gdb) FAIL: $exp: global_disassembler=ShowInfoRepr: disassemble test
...

This is caused by this code in gdbpy_disassembler::read_memory_func:
...
  gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
                                              "read_memory",
                                              "KL", len, offset));
...
where len has type "unsigned int", and offset has type "LONGEST".

Indeed L means "long long", but K means "unsigned long long".

Fix this by using I instead, meaning "unsigned int".

Tested on arm-linux.

PR python/31845
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31845
---
 gdb/python/py-disasm.c                 | 2 +-
 gdb/testsuite/gdb.python/py-disasm.exp | 2 +-
 gdb/testsuite/gdb.python/py-disasm.py  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 2d8ce44a065..a545ec8d3cb 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -855,7 +855,7 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr, gdb_byte *buff,
      overridden by the user.  */
   gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
 					       "read_memory",
-					       "KL", len, offset));
+					       "IL", len, offset));
 
   /* Handle any exceptions.  */
   if (result_obj == nullptr)
diff --git a/gdb/testsuite/gdb.python/py-disasm.exp b/gdb/testsuite/gdb.python/py-disasm.exp
index 7c6f972a7e7..5d7d9221169 100644
--- a/gdb/testsuite/gdb.python/py-disasm.exp
+++ b/gdb/testsuite/gdb.python/py-disasm.exp
@@ -65,7 +65,7 @@ proc py_remove_all_disassemblers {} {
 #
 # Each different disassembler tests some different feature of the
 # Python disassembler API.
-set nop "(nop|nop\t0)"
+set nop "(nop|nop\t0|[string_to_regexp nop\t{0}])"
 set unknown_error_pattern "unknown disassembler error \\(error = -1\\)"
 set addr_pattern "\r\n=> ${curr_pc_pattern} <\[^>\]+>:\\s+"
 set base_pattern "${addr_pattern}${nop}"
diff --git a/gdb/testsuite/gdb.python/py-disasm.py b/gdb/testsuite/gdb.python/py-disasm.py
index c09eae811ee..2741fdb6c19 100644
--- a/gdb/testsuite/gdb.python/py-disasm.py
+++ b/gdb/testsuite/gdb.python/py-disasm.py
@@ -46,7 +46,7 @@ def check_building_disassemble_result():
 
 
 def is_nop(s):
-    return s == "nop" or s == "nop\t0"
+    return s == "nop" or s == "nop\t0" or s == "nop\t{0}"
 
 
 # Remove all currently registered disassemblers.

base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d
-- 
2.35.3

Re: [PATCH] [gdb/python] Fix gdb.python/py-disasm.exp on arm-linux

[Alexandra Petlanova Hajkova]

[-- Attachment #1: Type: text/plain, Size: 3502 bytes --]

On Fri, Jun 7, 2024 at 8:29 AM Tom de Vries <tdevries@suse.de> wrote:

> After fixing test-case gdb.python/py-disasm.exp to recognize the arm nop:
> ...
>         nop     {0}
> ...
> we run into:
> ...
> disassemble test^M
> Dump of assembler code for function test:^M
>    0x004004d8 <+0>:     push    {r11}           @ (str r11, [sp, #-4]!)^M
>    0x004004dc <+4>:     add     r11, sp, #0^M
>    0x004004e0 <+8>:     nop     {0}^M
> => 0x004004e4 <+12>:    Python Exception <class 'ValueError'>: Buffer \
>   returned from read_memory is sized 0 instead of the expected 4^M
> ^M
> unknown disassembler error (error = -1)^M
> (gdb) FAIL: $exp: global_disassembler=ShowInfoRepr: disassemble test
> ...
>
> This is caused by this code in gdbpy_disassembler::read_memory_func:
> ...
>   gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
>                                               "read_memory",
>                                               "KL", len, offset));
> ...
> where len has type "unsigned int", and offset has type "LONGEST".
>
> Indeed L means "long long", but K means "unsigned long long".
>
> Fix this by using I instead, meaning "unsigned int".
>
> Tested on arm-linux.
>
> PR python/31845
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31845
> ---
>  gdb/python/py-disasm.c                 | 2 +-
>  gdb/testsuite/gdb.python/py-disasm.exp | 2 +-
>  gdb/testsuite/gdb.python/py-disasm.py  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
> index 2d8ce44a065..a545ec8d3cb 100644
> --- a/gdb/python/py-disasm.c
> +++ b/gdb/python/py-disasm.c
> @@ -855,7 +855,7 @@ gdbpy_disassembler::read_memory_func (bfd_vma memaddr,
> gdb_byte *buff,
>       overridden by the user.  */
>    gdbpy_ref<> result_obj (PyObject_CallMethod ((PyObject *) obj,
>                                                "read_memory",
> -                                              "KL", len, offset));
> +                                              "IL", len, offset));
>
>    /* Handle any exceptions.  */
>    if (result_obj == nullptr)
> diff --git a/gdb/testsuite/gdb.python/py-disasm.exp
> b/gdb/testsuite/gdb.python/py-disasm.exp
> index 7c6f972a7e7..5d7d9221169 100644
> --- a/gdb/testsuite/gdb.python/py-disasm.exp
> +++ b/gdb/testsuite/gdb.python/py-disasm.exp
> @@ -65,7 +65,7 @@ proc py_remove_all_disassemblers {} {
>  #
>  # Each different disassembler tests some different feature of the
>  # Python disassembler API.
> -set nop "(nop|nop\t0)"
> +set nop "(nop|nop\t0|[string_to_regexp nop\t{0}])"
>  set unknown_error_pattern "unknown disassembler error \\(error = -1\\)"
>  set addr_pattern "\r\n=> ${curr_pc_pattern} <\[^>\]+>:\\s+"
>  set base_pattern "${addr_pattern}${nop}"
> diff --git a/gdb/testsuite/gdb.python/py-disasm.py
> b/gdb/testsuite/gdb.python/py-disasm.py
> index c09eae811ee..2741fdb6c19 100644
> --- a/gdb/testsuite/gdb.python/py-disasm.py
> +++ b/gdb/testsuite/gdb.python/py-disasm.py
> @@ -46,7 +46,7 @@ def check_building_disassemble_result():
>
>
>  def is_nop(s):
> -    return s == "nop" or s == "nop\t0"
> +    return s == "nop" or s == "nop\t0" or s == "nop\t{0}"
>
>
>  # Remove all currently registered disassemblers.
>
> base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d
> --
> 2.35.3
>
> Looks reasonable.
 Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>

Re: [PATCH] [gdb/python] Fix gdb.python/py-disasm.exp on arm-linux

[Tom de Vries]

On 6/7/24 18:10, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Indeed L means "long long", but K means "unsigned long long".
> 
> Tom> -					       "KL", len, offset));
> Tom> +					       "IL", len, offset));
>   
> This patch is ok but, I think, incomplete.
> 
> offset has type LONGEST, which is int64_t -- not necessarily "long long".

Thanks for the review.

I've fixed this by using gdb_py_longest, and submitted a v2 here ( 
https://sourceware.org/pipermail/gdb-patches/2024-June/209808.html ).

The v2 is now a series, which also contains a fix for the 
!HAVE_LONG_LONG scenario.

Thanks,
- Tom