[v2] Raise exception if ambiguous name is used in gdb.parameter

  Currently gdb.parameter doesn't raise an exception if an
ambiguous name is used, it instead returns the value of the
last partly matching parameter:
```
(gdb) show print sym
Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
(gdb) show print symbol-loading
Printing of symbol loading messages is "full".
(gdb) py print(gdb.parameter("print sym"))
full
```

It's because lookup_cmd_composition_1 tries to detect
ambigous names by checking the return value of find_cmd
for CMD_LIST_AMBIGUOUS, which never happens, since only
lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
Instead the nfound argument contains the number of found
matches.

By using it instead, and by setting *CMD to the special value
CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
the appropriate error message:
```
(gdb) py print(gdb.parameter("print sym"))
Traceback (most recent call last):
  File "<string>", line 1, in <module>
RuntimeError: Parameter `print sym' is ambiguous.
Error while executing Python code.
(gdb) py print(gdb.parameter("print symbol"))
True
(gdb) py print(gdb.parameter("print symbol-"))
Traceback (most recent call last):
  File "<string>", line 1, in <module>
RuntimeError: Parameter `print symbol-' is ambiguous.
Error while executing Python code.
(gdb) py print(gdb.parameter("print symbol-load"))
full
```

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
---
v2:
- Small changes in commit message.
- Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
  ambiguous commands, so gdbpy_parameter can show a better
  error message.
---
 gdb/cli/cli-decode.c                      | 13 ++++++---
 gdb/python/python.c                       |  5 +++-
 gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)
  

Ping.


Am Dienstag, 19. Dezember 2023, 17:39:36 MEZ hat Hannes Domani <ssbssa@yahoo.de> Folgendes geschrieben:

> Currently gdb.parameter doesn't raise an exception if an
> ambiguous name is used, it instead returns the value of the
> last partly matching parameter:
> ```
> (gdb) show print sym
> Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
> (gdb) show print symbol-loading
> Printing of symbol loading messages is "full".
> (gdb) py print(gdb.parameter("print sym"))
> full
> ```
>
> It's because lookup_cmd_composition_1 tries to detect
> ambigous names by checking the return value of find_cmd
> for CMD_LIST_AMBIGUOUS, which never happens, since only
> lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
> Instead the nfound argument contains the number of found
> matches.
>
> By using it instead, and by setting *CMD to the special value
> CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
> the appropriate error message:
> ```
> (gdb) py print(gdb.parameter("print sym"))
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> RuntimeError: Parameter `print sym' is ambiguous.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol"))
> True
> (gdb) py print(gdb.parameter("print symbol-"))
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> RuntimeError: Parameter `print symbol-' is ambiguous.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol-load"))
> full
> ```
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
> ---
> v2:
> - Small changes in commit message.
> - Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
>   ambiguous commands, so gdbpy_parameter can show a better
>   error message.
> ---
> gdb/cli/cli-decode.c                      | 13 ++++++---
> gdb/python/python.c                      |  5 +++-
> gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
> 3 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 940cd6a2c8e..db57764578d 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -2549,7 +2549,8 @@ deprecated_cmd_warning (const char *text, struct cmd_list_element *list)
>     If TEXT is a subcommand (i.e. one that is preceded by a prefix
>     command) set *PREFIX_CMD.
>
> -  Set *CMD to point to the command TEXT indicates.
> +  Set *CMD to point to the command TEXT indicates, or to
> +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
>
>     If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
>     exist, they are NULL when we return.
> @@ -2589,7 +2590,12 @@ lookup_cmd_composition_1 (const char *text,
>       *cmd = find_cmd (command.c_str (), len, cur_list, 1, &nfound);
>
>       /* We only handle the case where a single command was found.  */
> -      if (*cmd == CMD_LIST_AMBIGUOUS || *cmd == nullptr)
> +      if (nfound > 1)
> +    {
> +      *cmd = CMD_LIST_AMBIGUOUS;
> +      return 0;
> +    }
> +      else if (*cmd == nullptr)
>     return 0;
>       else
>     {
> @@ -2623,7 +2629,8 @@ lookup_cmd_composition_1 (const char *text,
>     If TEXT is a subcommand (i.e. one that is preceded by a prefix
>     command) set *PREFIX_CMD.
>
> -  Set *CMD to point to the command TEXT indicates.
> +  Set *CMD to point to the command TEXT indicates, or to
> +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
>
>     If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
>     exist, they are NULL when we return.
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 951de82ef06..fa0b1ca6604 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -559,7 +559,10 @@ gdbpy_parameter (PyObject *self, PyObject *args)
>       GDB_PY_HANDLE_EXCEPTION (ex);
>     }
>
> -  if (!found)
> +  if (cmd == CMD_LIST_AMBIGUOUS)
> +    return PyErr_Format (PyExc_RuntimeError,
> +            _("Parameter `%s' is ambiguous."), arg);
> +  else if (!found)
>     return PyErr_Format (PyExc_RuntimeError,
>             _("Could not find parameter `%s'."), arg);
>
> diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
> index c1f8a80c6b7..828fe84694d 100644
> --- a/gdb/testsuite/gdb.python/py-parameter.exp
> +++ b/gdb/testsuite/gdb.python/py-parameter.exp
> @@ -588,6 +588,37 @@ proc_with_prefix test_language {} {
>     gdb_test_no_output "set lang auto"
> }
>
> +proc_with_prefix test_ambiguous_parameter {} {
> +    gdb_test_multiline "create parameter" \
> +    "python" "" \
> +    "class TestAmbiguousParam (gdb.Parameter):" "" \
> +    "  def __init__ (self, name, value):" "" \
> +    "      super (TestAmbiguousParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_INTEGER)" "" \
> +    "      self.value = value" "" \
> +    "end"
> +
> +    # Create parameters.
> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-1', 1)" ""
> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-2-extra', 2)" ""
> +    gdb_test "python TestAmbiguousParam('test-ambiguous', 3)" ""
> +
> +    # Test unambiguous matches.
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1'))" "1"
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2-extra'))" "2"
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2'))" "2"
> +    gdb_test "python print(gdb.parameter('test-ambiguous'))" "3"
> +
> +    # Test ambiguous names.
> +    gdb_test "python print(gdb.parameter('test-ambiguou'))" \
> +    "Parameter .* is ambiguous.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-'))" \
> +    "Parameter .* is ambiguous.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-v'))" \
> +    "Parameter .* is ambiguous.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1a'))" \
> +    "Could not find parameter.*Error while executing Python code."
> +}
> +
> test_directories
> test_data_directory
> test_boolean_parameter
> @@ -600,5 +631,6 @@ test_gdb_parameter
> test_integer_parameter
> test_throwing_parameter
> test_language
> +test_ambiguous_parameter
>
> rename py_param_test_maybe_no_output ""
> --
> 2.35.1
  

On 19/12/2023 17:38, Hannes Domani wrote:
> Currently gdb.parameter doesn't raise an exception if an
> ambiguous name is used, it instead returns the value of the
> last partly matching parameter:
> ```
> (gdb) show print sym
> Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
> (gdb) show print symbol-loading
> Printing of symbol loading messages is "full".
> (gdb) py print(gdb.parameter("print sym"))
> full
> ```
>
> It's because lookup_cmd_composition_1 tries to detect
> ambigous names by checking the return value of find_cmd
> for CMD_LIST_AMBIGUOUS, which never happens, since only
> lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
> Instead the nfound argument contains the number of found
> matches.
>
> By using it instead, and by setting *CMD to the special value
> CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
> the appropriate error message:
> ```
> (gdb) py print(gdb.parameter("print sym"))
> Traceback (most recent call last):
>    File "<string>", line 1, in <module>
> RuntimeError: Parameter `print sym' is ambiguous.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol"))
> True
> (gdb) py print(gdb.parameter("print symbol-"))
> Traceback (most recent call last):
>    File "<string>", line 1, in <module>
> RuntimeError: Parameter `print symbol-' is ambiguous.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol-load"))
> full
> ```
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
> ---
> v2:
> - Small changes in commit message.
> - Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
>    ambiguous commands, so gdbpy_parameter can show a better
>    error message.
> ---

I've taken a look at this, and it looks good. I think your solution of 
returning CMD_LIST_AMBIGUOUS is better than the original suggestion of 
returning nullptr, and this test doesn't introduce any regressions.

Reviewed-By: Guinevere Larsen <blarsen@redhat.com>
  

Ping.


On 03.01.2024 11:39, Hannes Domani wrote:
>   Ping.
> 
> 
> Am Dienstag, 19. Dezember 2023, 17:39:36 MEZ hat Hannes Domani <ssbssa@yahoo.de> Folgendes geschrieben:
> 
>> Currently gdb.parameter doesn't raise an exception if an
>> ambiguous name is used, it instead returns the value of the
>> last partly matching parameter:
>> ```
>> (gdb) show print sym
>> Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
>> (gdb) show print symbol-loading
>> Printing of symbol loading messages is "full".
>> (gdb) py print(gdb.parameter("print sym"))
>> full
>> ```
>>
>> It's because lookup_cmd_composition_1 tries to detect
>> ambigous names by checking the return value of find_cmd
>> for CMD_LIST_AMBIGUOUS, which never happens, since only
>> lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
>> Instead the nfound argument contains the number of found
>> matches.
>>
>> By using it instead, and by setting *CMD to the special value
>> CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
>> the appropriate error message:
>> ```
>> (gdb) py print(gdb.parameter("print sym"))
>> Traceback (most recent call last):
>>     File "<string>", line 1, in <module>
>> RuntimeError: Parameter `print sym' is ambiguous.
>> Error while executing Python code.
>> (gdb) py print(gdb.parameter("print symbol"))
>> True
>> (gdb) py print(gdb.parameter("print symbol-"))
>> Traceback (most recent call last):
>>     File "<string>", line 1, in <module>
>> RuntimeError: Parameter `print symbol-' is ambiguous.
>> Error while executing Python code.
>> (gdb) py print(gdb.parameter("print symbol-load"))
>> full
>> ```
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
>> ---
>> v2:
>> - Small changes in commit message.
>> - Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
>>     ambiguous commands, so gdbpy_parameter can show a better
>>     error message.
>> ---
>> gdb/cli/cli-decode.c                      | 13 ++++++---
>> gdb/python/python.c                      |  5 +++-
>> gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
>> 3 files changed, 46 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
>> index 940cd6a2c8e..db57764578d 100644
>> --- a/gdb/cli/cli-decode.c
>> +++ b/gdb/cli/cli-decode.c
>> @@ -2549,7 +2549,8 @@ deprecated_cmd_warning (const char *text, struct cmd_list_element *list)
>>       If TEXT is a subcommand (i.e. one that is preceded by a prefix
>>       command) set *PREFIX_CMD.
>>
>> -  Set *CMD to point to the command TEXT indicates.
>> +  Set *CMD to point to the command TEXT indicates, or to
>> +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
>>
>>       If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
>>       exist, they are NULL when we return.
>> @@ -2589,7 +2590,12 @@ lookup_cmd_composition_1 (const char *text,
>>         *cmd = find_cmd (command.c_str (), len, cur_list, 1, &nfound);
>>
>>         /* We only handle the case where a single command was found.  */
>> -      if (*cmd == CMD_LIST_AMBIGUOUS || *cmd == nullptr)
>> +      if (nfound > 1)
>> +    {
>> +      *cmd = CMD_LIST_AMBIGUOUS;
>> +      return 0;
>> +    }
>> +      else if (*cmd == nullptr)
>>       return 0;
>>         else
>>       {
>> @@ -2623,7 +2629,8 @@ lookup_cmd_composition_1 (const char *text,
>>       If TEXT is a subcommand (i.e. one that is preceded by a prefix
>>       command) set *PREFIX_CMD.
>>
>> -  Set *CMD to point to the command TEXT indicates.
>> +  Set *CMD to point to the command TEXT indicates, or to
>> +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
>>
>>       If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
>>       exist, they are NULL when we return.
>> diff --git a/gdb/python/python.c b/gdb/python/python.c
>> index 951de82ef06..fa0b1ca6604 100644
>> --- a/gdb/python/python.c
>> +++ b/gdb/python/python.c
>> @@ -559,7 +559,10 @@ gdbpy_parameter (PyObject *self, PyObject *args)
>>         GDB_PY_HANDLE_EXCEPTION (ex);
>>       }
>>
>> -  if (!found)
>> +  if (cmd == CMD_LIST_AMBIGUOUS)
>> +    return PyErr_Format (PyExc_RuntimeError,
>> +            _("Parameter `%s' is ambiguous."), arg);
>> +  else if (!found)
>>       return PyErr_Format (PyExc_RuntimeError,
>>               _("Could not find parameter `%s'."), arg);
>>
>> diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
>> index c1f8a80c6b7..828fe84694d 100644
>> --- a/gdb/testsuite/gdb.python/py-parameter.exp
>> +++ b/gdb/testsuite/gdb.python/py-parameter.exp
>> @@ -588,6 +588,37 @@ proc_with_prefix test_language {} {
>>       gdb_test_no_output "set lang auto"
>> }
>>
>> +proc_with_prefix test_ambiguous_parameter {} {
>> +    gdb_test_multiline "create parameter" \
>> +    "python" "" \
>> +    "class TestAmbiguousParam (gdb.Parameter):" "" \
>> +    "  def __init__ (self, name, value):" "" \
>> +    "      super (TestAmbiguousParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_INTEGER)" "" \
>> +    "      self.value = value" "" \
>> +    "end"
>> +
>> +    # Create parameters.
>> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-1', 1)" ""
>> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-2-extra', 2)" ""
>> +    gdb_test "python TestAmbiguousParam('test-ambiguous', 3)" ""
>> +
>> +    # Test unambiguous matches.
>> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1'))" "1"
>> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2-extra'))" "2"
>> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2'))" "2"
>> +    gdb_test "python print(gdb.parameter('test-ambiguous'))" "3"
>> +
>> +    # Test ambiguous names.
>> +    gdb_test "python print(gdb.parameter('test-ambiguou'))" \
>> +    "Parameter .* is ambiguous.*Error while executing Python code."
>> +    gdb_test "python print(gdb.parameter('test-ambiguous-'))" \
>> +    "Parameter .* is ambiguous.*Error while executing Python code."
>> +    gdb_test "python print(gdb.parameter('test-ambiguous-v'))" \
>> +    "Parameter .* is ambiguous.*Error while executing Python code."
>> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1a'))" \
>> +    "Could not find parameter.*Error while executing Python code."
>> +}
>> +
>> test_directories
>> test_data_directory
>> test_boolean_parameter
>> @@ -600,5 +631,6 @@ test_gdb_parameter
>> test_integer_parameter
>> test_throwing_parameter
>> test_language
>> +test_ambiguous_parameter
>>
>> rename py_param_test_maybe_no_output ""
>> --
>> 2.35.1
  

Ping.


Am Mittwoch, 17. Januar 2024, 17:03:43 MEZ hat Hannes Domani <ssbssa@yahoo.de> Folgendes geschrieben:

> Ping.
>
>
> On 03.01.2024 11:39, Hannes Domani wrote:
> >  Ping.
> >
> >
> > Am Dienstag, 19. Dezember 2023, 17:39:36 MEZ hat Hannes Domani <ssbssa@yahoo.de> Folgendes geschrieben:
> >
> >> Currently gdb.parameter doesn't raise an exception if an
> >> ambiguous name is used, it instead returns the value of the
> >> last partly matching parameter:
> >> ```
> >> (gdb) show print sym
> >> Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
> >> (gdb) show print symbol-loading
> >> Printing of symbol loading messages is "full".
> >> (gdb) py print(gdb.parameter("print sym"))
> >> full
> >> ```
> >>
> >> It's because lookup_cmd_composition_1 tries to detect
> >> ambigous names by checking the return value of find_cmd
> >> for CMD_LIST_AMBIGUOUS, which never happens, since only
> >> lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
> >> Instead the nfound argument contains the number of found
> >> matches.
> >>
> >> By using it instead, and by setting *CMD to the special value
> >> CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
> >> the appropriate error message:
> >> ```
> >> (gdb) py print(gdb.parameter("print sym"))
> >> Traceback (most recent call last):
> >>     File "<string>", line 1, in <module>
> >> RuntimeError: Parameter `print sym' is ambiguous.
> >> Error while executing Python code.
> >> (gdb) py print(gdb.parameter("print symbol"))
> >> True
> >> (gdb) py print(gdb.parameter("print symbol-"))
> >> Traceback (most recent call last):
> >>     File "<string>", line 1, in <module>
> >> RuntimeError: Parameter `print symbol-' is ambiguous.
> >> Error while executing Python code.
> >> (gdb) py print(gdb.parameter("print symbol-load"))
> >> full
> >> ```
> >>
> >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
> >> ---
> >> v2:
> >> - Small changes in commit message.
> >> - Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
> >>     ambiguous commands, so gdbpy_parameter can show a better
> >>     error message.
> >> ---
> >> gdb/cli/cli-decode.c                      | 13 ++++++---
> >> gdb/python/python.c                      |  5 +++-
> >> gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
> >> 3 files changed, 46 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> >> index 940cd6a2c8e..db57764578d 100644
> >> --- a/gdb/cli/cli-decode.c
> >> +++ b/gdb/cli/cli-decode.c
> >> @@ -2549,7 +2549,8 @@ deprecated_cmd_warning (const char *text, struct cmd_list_element *list)
> >>       If TEXT is a subcommand (i.e. one that is preceded by a prefix
> >>       command) set *PREFIX_CMD.
> >>
> >> -  Set *CMD to point to the command TEXT indicates.
> >> +  Set *CMD to point to the command TEXT indicates, or to
> >> +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
> >>
> >>       If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
> >>       exist, they are NULL when we return.
> >> @@ -2589,7 +2590,12 @@ lookup_cmd_composition_1 (const char *text,
> >>         *cmd = find_cmd (command.c_str (), len, cur_list, 1, &nfound);
> >>
> >>         /* We only handle the case where a single command was found.  */
> >> -      if (*cmd == CMD_LIST_AMBIGUOUS || *cmd == nullptr)
> >> +      if (nfound > 1)
> >> +    {
> >> +      *cmd = CMD_LIST_AMBIGUOUS;
> >> +      return 0;
> >> +    }
> >> +      else if (*cmd == nullptr)
> >>       return 0;
> >>         else
> >>       {
> >> @@ -2623,7 +2629,8 @@ lookup_cmd_composition_1 (const char *text,
> >>       If TEXT is a subcommand (i.e. one that is preceded by a prefix
> >>       command) set *PREFIX_CMD.
> >>
> >> -  Set *CMD to point to the command TEXT indicates.
> >> +  Set *CMD to point to the command TEXT indicates, or to
> >> +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
> >>
> >>       If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
> >>       exist, they are NULL when we return.
> >> diff --git a/gdb/python/python.c b/gdb/python/python.c
> >> index 951de82ef06..fa0b1ca6604 100644
> >> --- a/gdb/python/python.c
> >> +++ b/gdb/python/python.c
> >> @@ -559,7 +559,10 @@ gdbpy_parameter (PyObject *self, PyObject *args)
> >>         GDB_PY_HANDLE_EXCEPTION (ex);
> >>       }
> >>
> >> -  if (!found)
> >> +  if (cmd == CMD_LIST_AMBIGUOUS)
> >> +    return PyErr_Format (PyExc_RuntimeError,
> >> +            _("Parameter `%s' is ambiguous."), arg);
> >> +  else if (!found)
> >>       return PyErr_Format (PyExc_RuntimeError,
> >>               _("Could not find parameter `%s'."), arg);
> >>
> >> diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
> >> index c1f8a80c6b7..828fe84694d 100644
> >> --- a/gdb/testsuite/gdb.python/py-parameter.exp
> >> +++ b/gdb/testsuite/gdb.python/py-parameter.exp
> >> @@ -588,6 +588,37 @@ proc_with_prefix test_language {} {
> >>       gdb_test_no_output "set lang auto"
> >> }
> >>
> >> +proc_with_prefix test_ambiguous_parameter {} {
> >> +    gdb_test_multiline "create parameter" \
> >> +    "python" "" \
> >> +    "class TestAmbiguousParam (gdb.Parameter):" "" \
> >> +    "  def __init__ (self, name, value):" "" \
> >> +    "      super (TestAmbiguousParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_INTEGER)" "" \
> >> +    "      self.value = value" "" \
> >> +    "end"
> >> +
> >> +    # Create parameters.
> >> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-1', 1)" ""
> >> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-2-extra', 2)" ""
> >> +    gdb_test "python TestAmbiguousParam('test-ambiguous', 3)" ""
> >> +
> >> +    # Test unambiguous matches.
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1'))" "1"
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2-extra'))" "2"
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2'))" "2"
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous'))" "3"
> >> +
> >> +    # Test ambiguous names.
> >> +    gdb_test "python print(gdb.parameter('test-ambiguou'))" \
> >> +    "Parameter .* is ambiguous.*Error while executing Python code."
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous-'))" \
> >> +    "Parameter .* is ambiguous.*Error while executing Python code."
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous-v'))" \
> >> +    "Parameter .* is ambiguous.*Error while executing Python code."
> >> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1a'))" \
> >> +    "Could not find parameter.*Error while executing Python code."
> >> +}
> >> +
> >> test_directories
> >> test_data_directory
> >> test_boolean_parameter
> >> @@ -600,5 +631,6 @@ test_gdb_parameter
> >> test_integer_parameter
> >> test_throwing_parameter
> >> test_language
> >> +test_ambiguous_parameter
> >>
> >> rename py_param_test_maybe_no_output ""
> >> --
> >> 2.35.1
  

Hannes Domani <ssbssa@yahoo.de> writes:

> Currently gdb.parameter doesn't raise an exception if an
> ambiguous name is used, it instead returns the value of the
> last partly matching parameter:
> ```
> (gdb) show print sym
> Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
> (gdb) show print symbol-loading
> Printing of symbol loading messages is "full".
> (gdb) py print(gdb.parameter("print sym"))
> full
> ```
>
> It's because lookup_cmd_composition_1 tries to detect
> ambigous names by checking the return value of find_cmd
> for CMD_LIST_AMBIGUOUS, which never happens, since only
> lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
> Instead the nfound argument contains the number of found
> matches.
>
> By using it instead, and by setting *CMD to the special value
> CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
> the appropriate error message:
> ```
> (gdb) py print(gdb.parameter("print sym"))
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> RuntimeError: Parameter `print sym' is ambiguous.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol"))
> True
> (gdb) py print(gdb.parameter("print symbol-"))
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> RuntimeError: Parameter `print symbol-' is ambiguous.
> Error while executing Python code.
> (gdb) py print(gdb.parameter("print symbol-load"))
> full
> ```
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
> ---
> v2:
> - Small changes in commit message.
> - Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
>   ambiguous commands, so gdbpy_parameter can show a better
>   error message.
> ---
>  gdb/cli/cli-decode.c                      | 13 ++++++---
>  gdb/python/python.c                       |  5 +++-
>  gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
>  3 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 940cd6a2c8e..db57764578d 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -2549,7 +2549,8 @@ deprecated_cmd_warning (const char *text, struct cmd_list_element *list)
>     If TEXT is a subcommand (i.e. one that is preceded by a prefix
>     command) set *PREFIX_CMD.
>  
> -   Set *CMD to point to the command TEXT indicates.
> +   Set *CMD to point to the command TEXT indicates, or to
> +   CMD_LIST_AMBIGUOUS if there are multiple possible matches.
>  
>     If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
>     exist, they are NULL when we return.
> @@ -2589,7 +2590,12 @@ lookup_cmd_composition_1 (const char *text,
>        *cmd = find_cmd (command.c_str (), len, cur_list, 1, &nfound);
>  
>        /* We only handle the case where a single command was found.  */
> -      if (*cmd == CMD_LIST_AMBIGUOUS || *cmd == nullptr)
> +      if (nfound > 1)
> +	{
> +	  *cmd = CMD_LIST_AMBIGUOUS;
> +	  return 0;

This changes the API of lookup_cmd_composition.  You need to audit all
the calls to this function to check that non are depending on CMD being
set to nullptr.  I only looked at one in cli/cli-script.c in the
function do_document_command that is a problem, try this:

  (gdb) define foo_1
  Type commands for definition of "foo_1".
  End with a line saying just "end".
  >echo aa\n
  >end
  (gdb) define foo_2
  Type commands for definition of "foo_2".
  End with a line saying just "end".
  >echo bb\n
  >end
  (gdb) document foo
  
  
  Fatal signal: Segmentation fault
  ... etc ...

Not every use of lookup_cmd_composition is a problem, e.g. in
alias_command in cli/cli-cmds.c it's not an issue, so you'll need to
check 'em all.

If you do find cases that would have triggered bugs with this change, it
would be awesome if you could add tests to cover these cases, though I
guess that's not a hard requirement.

Thanks,
Andrew


> +	}
> +      else if (*cmd == nullptr)
>  	return 0;
>        else
>  	{
> @@ -2623,7 +2629,8 @@ lookup_cmd_composition_1 (const char *text,
>     If TEXT is a subcommand (i.e. one that is preceded by a prefix
>     command) set *PREFIX_CMD.
>  
> -   Set *CMD to point to the command TEXT indicates.
> +   Set *CMD to point to the command TEXT indicates, or to
> +   CMD_LIST_AMBIGUOUS if there are multiple possible matches.
>  
>     If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
>     exist, they are NULL when we return.
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 951de82ef06..fa0b1ca6604 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -559,7 +559,10 @@ gdbpy_parameter (PyObject *self, PyObject *args)
>        GDB_PY_HANDLE_EXCEPTION (ex);
>      }
>  
> -  if (!found)
> +  if (cmd == CMD_LIST_AMBIGUOUS)
> +    return PyErr_Format (PyExc_RuntimeError,
> +			 _("Parameter `%s' is ambiguous."), arg);
> +  else if (!found)
>      return PyErr_Format (PyExc_RuntimeError,
>  			 _("Could not find parameter `%s'."), arg);
>  
> diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp
> index c1f8a80c6b7..828fe84694d 100644
> --- a/gdb/testsuite/gdb.python/py-parameter.exp
> +++ b/gdb/testsuite/gdb.python/py-parameter.exp
> @@ -588,6 +588,37 @@ proc_with_prefix test_language {} {
>      gdb_test_no_output "set lang auto"
>  }
>  
> +proc_with_prefix test_ambiguous_parameter {} {
> +    gdb_test_multiline "create parameter" \
> +	"python" "" \
> +	"class TestAmbiguousParam (gdb.Parameter):" "" \
> +	"   def __init__ (self, name, value):" "" \
> +	"      super (TestAmbiguousParam, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_INTEGER)" "" \
> +	"      self.value = value" "" \
> +	"end"
> +
> +    # Create parameters.
> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-1', 1)" ""
> +    gdb_test "python TestAmbiguousParam('test-ambiguous-value-2-extra', 2)" ""
> +    gdb_test "python TestAmbiguousParam('test-ambiguous', 3)" ""
> +
> +    # Test unambiguous matches.
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1'))" "1"
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2-extra'))" "2"
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-2'))" "2"
> +    gdb_test "python print(gdb.parameter('test-ambiguous'))" "3"
> +
> +    # Test ambiguous names.
> +    gdb_test "python print(gdb.parameter('test-ambiguou'))" \
> +	"Parameter .* is ambiguous.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-'))" \
> +	"Parameter .* is ambiguous.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-v'))" \
> +	"Parameter .* is ambiguous.*Error while executing Python code."
> +    gdb_test "python print(gdb.parameter('test-ambiguous-value-1a'))" \
> +	"Could not find parameter.*Error while executing Python code."
> +}
> +
>  test_directories
>  test_data_directory
>  test_boolean_parameter
> @@ -600,5 +631,6 @@ test_gdb_parameter
>  test_integer_parameter
>  test_throwing_parameter
>  test_language
> +test_ambiguous_parameter
>  
>  rename py_param_test_maybe_no_output ""
> -- 
> 2.35.1
  

Am Montag, 29. Januar 2024 um 14:19:23 MEZ hat Andrew Burgess <aburgess@redhat.com> Folgendes geschrieben:

> Hannes Domani <ssbssa@yahoo.de> writes:
>
> > Currently gdb.parameter doesn't raise an exception if an
> > ambiguous name is used, it instead returns the value of the
> > last partly matching parameter:
> > ```
> > (gdb) show print sym
> > Ambiguous show print command "sym": symbol, symbol-filename, symbol-loading.
> > (gdb) show print symbol-loading
> > Printing of symbol loading messages is "full".
> > (gdb) py print(gdb.parameter("print sym"))
> > full
> > ```
> >
> > It's because lookup_cmd_composition_1 tries to detect
> > ambigous names by checking the return value of find_cmd
> > for CMD_LIST_AMBIGUOUS, which never happens, since only
> > lookup_cmd_1 returns CMD_LIST_AMBIGUOUS.
> > Instead the nfound argument contains the number of found
> > matches.
> >
> > By using it instead, and by setting *CMD to the special value
> > CMD_LIST_AMBIGUOUS in this case, gdbpy_parameter can now show
> > the appropriate error message:
> > ```
> > (gdb) py print(gdb.parameter("print sym"))
> > Traceback (most recent call last):
> >  File "<string>", line 1, in <module>
> > RuntimeError: Parameter `print sym' is ambiguous.
> > Error while executing Python code.
> > (gdb) py print(gdb.parameter("print symbol"))
> > True
> > (gdb) py print(gdb.parameter("print symbol-"))
> > Traceback (most recent call last):
> >  File "<string>", line 1, in <module>
> > RuntimeError: Parameter `print symbol-' is ambiguous.
> > Error while executing Python code.
> > (gdb) py print(gdb.parameter("print symbol-load"))
> > full
> > ```
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14639
> > ---
> > v2:
> > - Small changes in commit message.
> > - Use CMD_LIST_AMBIGUOUS in lookup_cmd_composition_1 to mark
> >  ambiguous commands, so gdbpy_parameter can show a better
> >  error message.
> > ---
> >  gdb/cli/cli-decode.c                      | 13 ++++++---
> >  gdb/python/python.c                      |  5 +++-
> >  gdb/testsuite/gdb.python/py-parameter.exp | 32 +++++++++++++++++++++++
> >  3 files changed, 46 insertions(+), 4 deletions(-)
> >
> > diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> > index 940cd6a2c8e..db57764578d 100644
> > --- a/gdb/cli/cli-decode.c
> > +++ b/gdb/cli/cli-decode.c
> > @@ -2549,7 +2549,8 @@ deprecated_cmd_warning (const char *text, struct cmd_list_element *list)
> >    If TEXT is a subcommand (i.e. one that is preceded by a prefix
> >    command) set *PREFIX_CMD.
> >
> > -  Set *CMD to point to the command TEXT indicates.
> > +  Set *CMD to point to the command TEXT indicates, or to
> > +  CMD_LIST_AMBIGUOUS if there are multiple possible matches.
> >
> >    If any of *ALIAS, *PREFIX_CMD, or *CMD cannot be determined or do not
> >    exist, they are NULL when we return.
> > @@ -2589,7 +2590,12 @@ lookup_cmd_composition_1 (const char *text,
> >        *cmd = find_cmd (command.c_str (), len, cur_list, 1, &nfound);
> >
> >        /* We only handle the case where a single command was found.  */
> > -      if (*cmd == CMD_LIST_AMBIGUOUS || *cmd == nullptr)
> > +      if (nfound > 1)
> > +    {
> > +      *cmd = CMD_LIST_AMBIGUOUS;
> > +      return 0;
>
> This changes the API of lookup_cmd_composition.  You need to audit all
> the calls to this function to check that non are depending on CMD being
> set to nullptr.  I only looked at one in cli/cli-script.c in the
> function do_document_command that is a problem, try this:
>
>   (gdb) define foo_1
>   Type commands for definition of "foo_1".
>   End with a line saying just "end".
>   >echo aa\n
>   >end
>   (gdb) define foo_2
>   Type commands for definition of "foo_2".
>   End with a line saying just "end".
>   >echo bb\n
>   >end
>   (gdb) document foo
>  
>  
>   Fatal signal: Segmentation fault
>   ... etc ...
>
> Not every use of lookup_cmd_composition is a problem, e.g. in
> alias_command in cli/cli-cmds.c it's not an issue, so you'll need to
> check 'em all.
>
> If you do find cases that would have triggered bugs with this change, it
> would be awesome if you could add tests to cover these cases, though I
> guess that's not a hard requirement.

Thanks for catching this.
I've now checked all uses of lookup_cmd_composition, and could not find
another problem (either it bailed because of lookup_cmd_composition return
value, or the returned *cmd was not used at all afterwards).

So I've fixed do_document_command, and added a test for this, in the v3:
https://sourceware.org/pipermail/gdb-patches/2024-January/206257.html


Hannes