Re: [PATCH v6 5/8] GDB/Python: Use None for `var_zuinteger_unlimited' value set to `unlimited'

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

Hi,

I'm seeing a number of new failures on armhf Ubuntu 20.04 for both gdb.python/py-format-address.exp and gdb.python/py-parameter.exp.

Since this series touched those, I'm wondering if those are known.

FAIL: gdb.python/py-format-address.exp: symbol_filename=off: gdb.format_address, result should have an offset
FAIL: gdb.python/py-format-address.exp: symbol_filename=on: gdb.format_address, result should have an offset
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test default value
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test default value via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test set to 0
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test set to 1
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test set to 5
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of -1
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of -1 via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of -5 via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of 0 via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of 1
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of 1 via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of 5 via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of None
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: test value of None via gdb.parameter
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: {test set to -1}
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: {test set to -5}
FAIL: gdb.python/py-parameter.exp: test_integer_parameter: kind=PARAM_UINTEGER: {test set to None}

On 8/17/22 23:04, Maciej W. Rozycki wrote:
> Consistently with parameters of the PARAM_UINTEGER and PARAM_INTEGER
> types return the special value of `None' for a PARAM_ZUINTEGER_UNLIMITED
> parameter set to `unlimited', fixing an inconsistency introduced with
> commit 0489430a0e1a ("Handle var_zuinteger and var_zuinteger_unlimited
> from Python"); cf. PR python/20084.  Adjust the testsuite accordingly.
> 
> This makes all the three parameter types consistent with each other as
> far as the use of `None' is concerned, and similar to the Guile/Scheme
> interface where the `#:unlimited' keyword is likewise used.  We have a
> precedent already documented for a similar API correction:
> 
>   -- Function: gdb.breakpoints ()
>       Return a sequence holding all of GDB's breakpoints.  *Note
>       Breakpoints In Python::, for more information.  In GDB version 7.11
>       and earlier, this function returned 'None' if there were no
>       breakpoints.  This peculiarity was subsequently fixed, and now
>       'gdb.breakpoints' returns an empty sequence in this case.
> 
> made in the past.
> 
> And then we have documentation not matching the interface as far as the
> value of `None' already returned for parameters of the PARAM_UINTEGER
> and PARAM_INTEGER types is concerned, and the case of an incorrect
> assertion for PARAM_UINTEGER and PARAM_ZUINTEGER_UNLIMITED parameters in
> the sibling Guile/Scheme module making such parameters unusable that has
> never been reported, both indicating that these features may indeed not
> be heavily used, and therefore that the risk from such an API change is
> likely lower than the long-term burden of the inconsistency.
> 
> And where the value read from a parameter is only used for presentation
> purposes, then such a change is expected to be transparent.
> ---
> Changes from v5:
> 
> - Refer to Python parameter types in the change description rather than
>    underlying GDB variable types in preparation for breaking the tight
>    coupling between the two later in this series.
> 
> - Document existing and updated semantics in the GDB manual.
> 
> - Update the testsuite adjustment to fit in the now expanded test case.
> 
> - Add a NEWS entry.
> 
> No change from v4.
> 
> New change in v4.
> ---
>   gdb/NEWS                                  |    7 +++++++
>   gdb/doc/python.texi                       |   30 +++++++++++++++++++-----------
>   gdb/python/python.c                       |   10 +++++++++-
>   gdb/testsuite/gdb.python/py-parameter.exp |   16 +++++-----------
>   4 files changed, 40 insertions(+), 23 deletions(-)
> 
> Index: src/gdb/NEWS
> ===================================================================
> --- src.orig/gdb/NEWS
> +++ src/gdb/NEWS
> @@ -175,6 +175,13 @@ GNU/Linux/LoongArch (gdbserver)	loongarc
>        gdb.BreakpointLocation objects specifying the locations where the
>        breakpoint is inserted into the debuggee.
>   
> +  ** Parameters of gdb.PARAM_ZUINTEGER_UNLIMITED type now return the
> +     value of None for the 'unlimited' setting, consistently with
> +     parameters of gdb.PARAM_UINTEGER and gdb.PARAM_INTEGER types.
> +     Parameters of all the three types now accept the value of None
> +     to mean 'unlimited'.  The use of internal integer representation
> +     for the 'unlimited' setting is now deprecated.
> +
>   * New features in the GDB remote stub, GDBserver
>   
>     ** GDBserver is now supported on LoongArch GNU/Linux.
> Index: src/gdb/doc/python.texi
> ===================================================================
> --- src.orig/gdb/doc/python.texi
> +++ src/gdb/doc/python.texi
> @@ -4598,14 +4598,16 @@ Python, true and false are represented u
>   @findex PARAM_UINTEGER
>   @findex gdb.PARAM_UINTEGER
>   @item gdb.PARAM_UINTEGER
> -The value is an unsigned integer.  The value of 0 should be
> -interpreted to mean ``unlimited''.
> +The value is an unsigned integer.  The value of @code{None} should be
> +interpreted to mean ``unlimited'', and the value of 0 is reserved and
> +should not be used.
>   
>   @findex PARAM_INTEGER
>   @findex gdb.PARAM_INTEGER
>   @item gdb.PARAM_INTEGER
> -The value is a signed integer.  The value of 0 should be interpreted
> -to mean ``unlimited''.
> +The value is a signed integer.  The value of @code{None} should be
> +interpreted to mean ``unlimited'', and the value of 0 is reserved and
> +should not be used.
>   
>   @findex PARAM_STRING
>   @findex gdb.PARAM_STRING
> @@ -4635,21 +4637,27 @@ The value is a filename.  This is just l
>   @findex PARAM_ZINTEGER
>   @findex gdb.PARAM_ZINTEGER
>   @item gdb.PARAM_ZINTEGER
> -The value is an integer.  This is like @code{PARAM_INTEGER}, except 0
> -is interpreted as itself.
> +The value is a signed integer.  This is like @code{PARAM_INTEGER},
> +except that 0 is allowed and the value of @code{None} is not supported.
>   
>   @findex PARAM_ZUINTEGER
>   @findex gdb.PARAM_ZUINTEGER
>   @item gdb.PARAM_ZUINTEGER
> -The value is an unsigned integer.  This is like @code{PARAM_INTEGER},
> -except 0 is interpreted as itself, and the value cannot be negative.
> +The value is an unsigned integer.  This is like @code{PARAM_UINTEGER},
> +except that 0 is allowed and the value of @code{None} is not supported.
>   
>   @findex PARAM_ZUINTEGER_UNLIMITED
>   @findex gdb.PARAM_ZUINTEGER_UNLIMITED
>   @item gdb.PARAM_ZUINTEGER_UNLIMITED
> -The value is a signed integer.  This is like @code{PARAM_ZUINTEGER},
> -except the special value -1 should be interpreted to mean
> -``unlimited''.  Other negative values are not allowed.
> +The value is a signed integer.  This is like @code{PARAM_INTEGER}
> +including that the value of @code{None} should be interpreted to mean
> +``unlimited'', except that 0 is allowed, and the value cannot be negative.
> +
> +In GDB version 12 and earlier, a parameter of this type when read would
> +return -1 rather than @code{None} for the setting of ``unlimited''.
> +This peculiarity was subsequently fixed, for consistency with parameters
> +of @code{PARAM_UINTEGER} and @code{PARAM_INTEGER} types, so that all the
> +three types return the value of @code{None} for ``unlimited''.
>   
>   @findex PARAM_ENUM
>   @findex gdb.PARAM_ENUM
> Index: src/gdb/python/python.c
> ===================================================================
> --- src.orig/gdb/python/python.c
> +++ src/gdb/python/python.c
> @@ -509,9 +509,17 @@ gdbpy_parameter_value (const setting &va
>   	Py_RETURN_NONE;
>         /* Fall through.  */
>       case var_zinteger:
> -    case var_zuinteger_unlimited:
>         return gdb_py_object_from_longest (var.get<int> ()).release ();
>   
> +    case var_zuinteger_unlimited:
> +      {
> +	int val = var.get<int> ();
> +
> +	if (val == -1)
> +	  Py_RETURN_NONE;
> +	return gdb_py_object_from_longest (val).release ();
> +      }
> +
>       case var_uinteger:
>         {
>   	unsigned int val = var.get<unsigned int> ();
> Index: src/gdb/testsuite/gdb.python/py-parameter.exp
> ===================================================================
> --- src.orig/gdb/testsuite/gdb.python/py-parameter.exp
> +++ src/gdb/testsuite/gdb.python/py-parameter.exp
> @@ -346,22 +346,16 @@ proc_with_prefix test_gdb_parameter { }
>   	    "listsize" {
>   		set param_get_zero None
>   		set param_get_minus_one -1
> -		set param_get_none None
> -		set param_get_unlimited None
>   		set param_set_minus_one ""
>   	    }
>   	    "print elements" {
>   		set param_get_zero None
>   		set param_get_minus_one None
> -		set param_get_none None
> -		set param_get_unlimited None
>   		set param_set_minus_one $param_range_error
>   	    }
>   	    "max-completions" {
>   		set param_get_zero 0
> -		set param_get_minus_one -1
> -		set param_get_none -1
> -		set param_get_unlimited -1
> +		set param_get_minus_one None
>   		set param_set_minus_one ""
>   	    }
>   	    default {
> @@ -392,13 +386,13 @@ proc_with_prefix test_gdb_parameter { }
>   	    "test set to None"
>   
>   	gdb_test "python print(gdb.parameter('$param'))" \
> -	    $param_get_none "test value of None"
> +	    None "test value of None"
>   
>   	gdb_test_no_output "python gdb.set_parameter('$param', 'unlimited')" \
>   	    "test set to 'unlimited'"
>   
>   	gdb_test "python print(gdb.parameter('$param'))" \
> -	    $param_get_unlimited "test value of 'unlimited'"
> +	    None "test value of 'unlimited'"
>       }
>   
>       clean_restart
> @@ -468,9 +462,9 @@ proc_with_prefix test_integer_parameter
>   	    }
>   	    PARAM_ZUINTEGER_UNLIMITED {
>   		set param_get_zero 0
> -		set param_get_minus_one -1
> +		set param_get_minus_one None
>   		set param_get_minus_five 1
> -		set param_get_none -1
> +		set param_get_none None
>   		set param_set_minus_one ""
>   		set param_set_minus_five $param_range_error
>   		set param_set_none ""