* Re: [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter
2023-12-19 16:38 ` [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter Hannes Domani
@ 2024-01-03 10:39 ` Hannes Domani
2024-01-17 16:03 ` [PING 2] " Hannes Domani
2024-01-05 16:14 ` Guinevere Larsen
2024-01-29 13:19 ` Andrew Burgess
2 siblings, 1 reply; 7+ messages in thread
From: Hannes Domani @ 2024-01-03 10:39 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PING 2] [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter
2024-01-03 10:39 ` Hannes Domani
@ 2024-01-17 16:03 ` Hannes Domani
2024-01-26 17:04 ` Hannes Domani
0 siblings, 1 reply; 7+ messages in thread
From: Hannes Domani @ 2024-01-17 16:03 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PING 2] [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter
2024-01-17 16:03 ` [PING 2] " Hannes Domani
@ 2024-01-26 17:04 ` Hannes Domani
0 siblings, 0 replies; 7+ messages in thread
From: Hannes Domani @ 2024-01-26 17:04 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter
2023-12-19 16:38 ` [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter Hannes Domani
2024-01-03 10:39 ` Hannes Domani
@ 2024-01-05 16:14 ` Guinevere Larsen
2024-01-29 13:19 ` Andrew Burgess
2 siblings, 0 replies; 7+ messages in thread
From: Guinevere Larsen @ 2024-01-05 16:14 UTC (permalink / raw)
To: Hannes Domani, gdb-patches
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>
--
Cheers,
Guinevere Larsen
She/Her/Hers
> 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 ""
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter
2023-12-19 16:38 ` [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter Hannes Domani
2024-01-03 10:39 ` Hannes Domani
2024-01-05 16:14 ` Guinevere Larsen
@ 2024-01-29 13:19 ` Andrew Burgess
2024-01-29 16:04 ` Hannes Domani
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2024-01-29 13:19 UTC (permalink / raw)
To: Hannes Domani, gdb-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] Raise exception if ambiguous name is used in gdb.parameter
2024-01-29 13:19 ` Andrew Burgess
@ 2024-01-29 16:04 ` Hannes Domani
0 siblings, 0 replies; 7+ messages in thread
From: Hannes Domani @ 2024-01-29 16:04 UTC (permalink / raw)
To: gdb-patches, Andrew Burgess
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
^ permalink raw reply [flat|nested] 7+ messages in thread