Re: [PATCHv3 2/2] gdb/python: extend the Python Disassembler API to allow for styling

[Tom de Vries <tdevries@suse.de>]

On 5/17/23 19:46, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>> On 5/12/23 19:50, Andrew Burgess via Gdb-patches wrote:
>>> This commit extends the Python Disassembler API to allow for styling
>>> of the instructions.
>>>
>>
>> I'm running into:
>> ...
>> FAIL: gdb.python/py-disasm.exp:
>> global_disassembler=ErrorCreatingTextPart_NoArgs: disassemble test
>> FAIL: gdb.python/py-disasm.exp:
>> global_disassembler=ErrorCreatingAddressPart_NoArgs: disassemble test
>> FAIL: gdb.python/py-disasm.exp:
>> global_disassembler=ErrorCreatingTextPart_NoString: disassemble test
>> FAIL: gdb.python/py-disasm.exp:
>> global_disassembler=ErrorCreatingTextPart_NoStyle: disassemble test
>> FAIL: gdb.python/py-disasm.exp: try to create a DisassemblerResult
>> without a length argument
>> ...
>>
>> First in more detail:
>> ...
>> (gdb) disassemble test^M
>> Dump of assembler code for function test:^M
>>      0x00000000004004a7 <+0>:     push   %rbp^M
>>      0x00000000004004a8 <+1>:     mov    %rsp,%rbp^M
>>      0x00000000004004ab <+4>:     nop^M
>> => 0x00000000004004ac <+5>:     Python Exception <class 'TypeError'>:
>> Required argument 'style' (pos 1) not found^M
>> ^M
>> unknown disassembler error (error = -1)^M
>> (gdb) FAIL: gdb.python/py-disasm.exp:
>> global_disassembler=ErrorCreatingTextPart_NoArgs: disassemble test
>> ...
>>
>> This seems to be due to the test-case expecting " function missing
>> required argument ..." but getting "Required argument ..."
> 
> Sorry for that.

Np of course :)

>  I guess you must be using an older (3.6.x or earlier)
> version of Python?
> 

/me checks:
...
$ python3 --version
Python 3.6.15
...
Yes.

> The patch below should resolve the issues, I'll push this tomorrow.
> 

I've checked, it does, thanks.

- Tom

> Thanks,
> Andrew
> 
> ---
> 
> commit 416267af2e8272f677461fe9674bb10fcbd19098
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Wed May 17 18:39:26 2023 +0100
> 
>      gdb/testsuite: handle older Python versions in gdb.python/py-disasm.exp
>      
>      It was pointed out on the mailing list that the new tests added in
>      this commit:
>      
>        commit 4de4e48514fc47aeb4ca95cd4091e2a333fbe9e1
>        Date:   Tue Jan 24 15:35:45 2023 +0000
>      
>            gdb/python: extend the Python Disassembler API to allow for styling
>      
>      will fail when GDB is built with Python 3.6 or earlier.  This is
>      because the error that is emitted when a function argument is missing
>      changed in Python 3.7, instead of an error like this:
>      
>        Python Exception <class 'TypeError'>: function missing required argument 'style' (pos 1)
>      
>      earlier versions of Python emit:
>      
>        Python Exception <class 'TypeError'>: Required argument 'style' (pos 1) not found
>      
>      and the new tests didn't allow for this.
>      
>      This commit fixes this by allowing either pattern.  I've tested this
>      building GDB against Python 3.7.9 and 3.6.15, with this commit all
>      tests in gdb.python/py-disasm.exp now pass.
> 
> diff --git a/gdb/testsuite/gdb.python/py-disasm.exp b/gdb/testsuite/gdb.python/py-disasm.exp
> index 304393f71ab..f2f9225168a 100644
> --- a/gdb/testsuite/gdb.python/py-disasm.exp
> +++ b/gdb/testsuite/gdb.python/py-disasm.exp
> @@ -75,6 +75,16 @@ proc make_exception_pattern { type msg } {
>       return "${::addr_pattern}Python Exception <class '$type'>: $msg\r\n\r\n${::unknown_error_pattern}"
>   }
>   
> +# Helper proc to build a pattern for the text Python emits when a
> +# function argument is missing.  This string changed in Python 3.7 and
> +# later.  NAME is the parameter name, and POS is its integer position
> +# in the argument list.
> +proc missing_arg_pattern { name pos } {
> +    set pattern_1 "function missing required argument '$name' \\(pos $pos\\)"
> +    set pattern_2 "Required argument '$name' \\(pos $pos\\) not found"
> +    return "(?:${pattern_1}|${pattern_2})"
> +}
> +
>   set test_plans \
>       [list \
>   	 [list "" "${base_pattern}\r\n.*"] \
> @@ -113,16 +123,16 @@ set test_plans \
>   		   "Result is not a DisassemblerResult."]] \
>   	 [list "ErrorCreatingTextPart_NoArgs" \
>   	      [make_exception_pattern "TypeError" \
> -		  "function missing required argument 'style' \\(pos 1\\)"]] \
> +		   [missing_arg_pattern "style" 1]]] \
>   	 [list "ErrorCreatingAddressPart_NoArgs" \
>   	      [make_exception_pattern "TypeError" \
> -		  "function missing required argument 'address' \\(pos 1\\)"]] \
> +		   [missing_arg_pattern "address" 1]]] \
>   	 [list "ErrorCreatingTextPart_NoString" \
>   	      [make_exception_pattern "TypeError" \
> -		  "function missing required argument 'string' \\(pos 2\\)"]] \
> +		   [missing_arg_pattern "string" 2]]] \
>   	 [list "ErrorCreatingTextPart_NoStyle" \
>   	      [make_exception_pattern "TypeError" \
> -		  "function missing required argument 'style' \\(pos 1\\)"]] \
> +		   [missing_arg_pattern "style" 1]]] \
>   	 [list "All_Text_Part_Styles" "${addr_pattern}p1p2p3p4p5p6p7p8p9p10\r\n.*"] \
>   	 [list "ErrorCreatingTextPart_StringAndParts" \
>   	      [make_exception_pattern "ValueError" \
> @@ -273,7 +283,7 @@ with_test_prefix "check inheritance" {
>   # Test some error conditions when creating a DisassemblerResult object.
>   gdb_test "python result = gdb.disassembler.DisassemblerResult()" \
>       [multi_line \
> -	 "TypeError: function missing required argument 'length' \\(pos 1\\)" \
> +	 "TypeError: [missing_arg_pattern length 1]" \
>   	 "Error while executing Python code\\."] \
>       "try to create a DisassemblerResult without a length argument"
>   
>