Fix bad interaction between element limit and repeated values (BZ#24331).

  Currently

  print -elements=3 -- "AAAAAA"

prints complete string, which is not what the user asked for.

Fix two buggy tests exposed by the fix, and add a new test.
---
 gdb/testsuite/gdb.base/printcmds.exp          | 15 +++++++++++++
 gdb/testsuite/gdb.python/py-format-string.exp |  2 +-
 gdb/valprint.c                                | 22 ++++++++++++++-----
 3 files changed, 33 insertions(+), 6 deletions(-)
  

Hi,

On 5/7/23 18:22, Paul Pluzhnikov via Gdb-patches wrote:
> Currently
> 
>    print -elements=3 -- "AAAAAA"
> 
> prints complete string, which is not what the user asked for.
> 
> Fix two buggy tests exposed by the fix, and add a new test.

Thank you for the patch. I just have one small ask.

> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index b05d90dd26b..b770c93d805 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -2363,16 +2363,19 @@ print_converted_chars_to_obstack (struct obstack *obstack,
>   				  const std::vector<converted_character> &chars,
>   				  int quote_char, int width,
>   				  enum bfd_endian byte_order,
> -				  const struct value_print_options *options)
> +				  const struct value_print_options *options,
> +				  int *finished)

The comment preceding this function explains the meaning of all of its
arguments. Please add `finished', too.

>   {
> -  unsigned int idx;
> +  unsigned int idx, num_elements;
>     const converted_character *elem;
>     enum {START, SINGLE, REPEAT, INCOMPLETE, FINISH} state, last;
>     gdb_wchar_t wide_quote_char = gdb_btowc (quote_char);
>     bool need_escape = false;
> +  const int print_max = options->print_max_chars > 0
> +      ? options->print_max_chars : options->print_max;
>   
>     /* Set the start state.  */
> -  idx = 0;
> +  idx = num_elements = 0;
>     last = state = START;
>     elem = NULL;
>   

Otherwise LGTM.

Reviewed-by: Keith Seitz <keiths@redhat.com>

Keith
  

Paul Pluzhnikov via Gdb-patches <gdb-patches@sourceware.org> writes:

> Currently
>
>   print -elements=3 -- "AAAAAA"
>
> prints complete string, which is not what the user asked for.
>
> Fix two buggy tests exposed by the fix, and add a new test.
> ---
>  gdb/testsuite/gdb.base/printcmds.exp          | 15 +++++++++++++
>  gdb/testsuite/gdb.python/py-format-string.exp |  2 +-
>  gdb/valprint.c                                | 22 ++++++++++++++-----
>  3 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index db57769c303..f2be549f2a5 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -1027,6 +1027,9 @@ proc test_repeat_bytes {} {
>      set fmt(SR) "\"%s\", %s"
>      set fmt(SS) "\"%s%s\""
>  
> +    # We have up to 60 elements to print.
> +    gdb_test_no_output "set print elements 100"
> +
>      # Set the target-charset to ASCII, because the output varies from
>      # different charset.
>      with_target_charset "ASCII" {
> @@ -1074,6 +1077,17 @@ proc test_repeat_bytes {} {
>      }
>  }
>  
> +# Test for BZ#24331

It's fine to reference the bug, but could you expand this comment to say
what's actually being tested, e.g.

  # Test that 'set elements' correctly limits the number of characters
  # printed from a string.  This is PR gdb/24331.

> +proc test_repeat_bytes_limit {} {
> +   gdb_test "print -elem 3 -- \"AAAAA\"" "= \"AAA\"..."
> +   gdb_test "print -char 3 -elem 10 -- \"AAAAA\"" "= \"AAA\"..."
> +   gdb_test "print -elem 3 -- \"ABBBB\"" "= \"ABB\"..."
> +   gdb_test "print -char 3 -elem 10 -- \"ABBBB\"" "= \"ABB\"..."
> +
> +   gdb_test "print -elem 3 -repeats 2 -- \"AAABBB\"" "= 'A' <repeats 3 times>..."
> +   gdb_test "print -elem 4 -repeats 2 -- \"AAABBB\"" "= 'A' <repeats 3 times>, 'B' <repeats 3 times>"
> +}
> +
>  # Test printf of convenience variables.
>  # These tests can be done with or without a running inferior.
>  # PREFIX ensures uniqueness of test names.
> @@ -1199,3 +1213,4 @@ test_printf_with_strings
>  test_print_symbol
>  test_repeat_bytes
>  test_radices
> +test_repeat_bytes_limit
> diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
> index 1d50f59dee3..2a011ebd45a 100644
> --- a/gdb/testsuite/gdb.python/py-format-string.exp
> +++ b/gdb/testsuite/gdb.python/py-format-string.exp
> @@ -748,7 +748,7 @@ proc test_max_string_one { setting unlimited } {
>      # This will print four characters instead of three, see
>      # <https://sourceware.org/bugzilla/show_bug.cgi?id=24331>.

This comment seems to be out of date now and likely needs updating.

Thanks,
Andrew

>      check_format_string "a_binary_string_array" $opts \
> -      "\"hell\"..."
> +      "\"hel\"..."
>      check_format_string "a_big_string" $opts \
>        [get_cut_big_string 3]
>      if { $setting == "elements"} {
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index b05d90dd26b..b770c93d805 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -2363,16 +2363,19 @@ print_converted_chars_to_obstack (struct obstack *obstack,
>  				  const std::vector<converted_character> &chars,
>  				  int quote_char, int width,
>  				  enum bfd_endian byte_order,
> -				  const struct value_print_options *options)
> +				  const struct value_print_options *options,
> +				  int *finished)
>  {
> -  unsigned int idx;
> +  unsigned int idx, num_elements;
>    const converted_character *elem;
>    enum {START, SINGLE, REPEAT, INCOMPLETE, FINISH} state, last;
>    gdb_wchar_t wide_quote_char = gdb_btowc (quote_char);
>    bool need_escape = false;
> +  const int print_max = options->print_max_chars > 0
> +      ? options->print_max_chars : options->print_max;
>  
>    /* Set the start state.  */
> -  idx = 0;
> +  idx = num_elements = 0;
>    last = state = START;
>    elem = NULL;
>  
> @@ -2400,7 +2403,13 @@ print_converted_chars_to_obstack (struct obstack *obstack,
>  		obstack_grow (obstack, &wide_quote_char, sizeof (gdb_wchar_t));
>  	      }
>  	    /* Output the character.  */
> -	    for (j = 0; j < elem->repeat_count; ++j)
> +	    int repeat_count = elem->repeat_count;
> +	    if (print_max < repeat_count + num_elements)
> +	      {
> +		repeat_count = print_max - num_elements;
> +		*finished = 0;
> +	      }
> +	    for (j = 0; j < repeat_count; ++j)
>  	      {
>  		if (elem->result == wchar_iterate_ok)
>  		  print_wchar (elem->chars[0], elem->buf, elem->buflen, width,
> @@ -2408,6 +2417,7 @@ print_converted_chars_to_obstack (struct obstack *obstack,
>  		else
>  		  print_wchar (gdb_WEOF, elem->buf, elem->buflen, width,
>  			       byte_order, obstack, quote_char, &need_escape);
> +		num_elements += 1;
>  	      }
>  	  }
>  	  break;
> @@ -2439,6 +2449,7 @@ print_converted_chars_to_obstack (struct obstack *obstack,
>  	    obstack_grow_wstr (obstack, LCST ("'"));
>  	    std::string s = string_printf (_(" <repeats %u times>"),
>  					   elem->repeat_count);
> +	    num_elements += elem->repeat_count;
>  	    for (j = 0; s[j]; ++j)
>  	      {
>  		gdb_wchar_t w = gdb_btowc (s[j]);
> @@ -2463,6 +2474,7 @@ print_converted_chars_to_obstack (struct obstack *obstack,
>  	  print_wchar (gdb_WEOF, elem->buf, elem->buflen, width, byte_order,
>  		       obstack, 0, &need_escape);
>  	  obstack_grow_wstr (obstack, LCST (">"));
> +	  num_elements += 1;
>  
>  	  /* We do not attempt to output anything after this.  */
>  	  state = FINISH;
> @@ -2597,7 +2609,7 @@ generic_printstr (struct ui_file *stream, struct type *type,
>  
>    /* Print the output string to the obstack.  */
>    print_converted_chars_to_obstack (&wchar_buf, converted_chars, quote_char,
> -				    width, byte_order, options);
> +				    width, byte_order, options, &finished);
>  
>    if (force_ellipses || !finished)
>      obstack_grow_wstr (&wchar_buf, LCST ("..."));
> -- 
> 2.40.1.521.gf1e218fcd8-goog