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

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

[Paul Pluzhnikov]

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
+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>.
     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

Re: [PATCH] Fix bad interaction between element limit and repeated values (BZ#24331).

[Keith Seitz]

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

Re: [PATCH] Fix bad interaction between element limit and repeated values (BZ#24331).

[Andrew Burgess]

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

[PATCH] Update comments for the gdb/24331 fix.

[Paul Pluzhnikov]

On Mon, May 15, 2023 at 2:43 AM Andrew Burgess <aburgess@redhat.com> wrote:
>
> It's fine to reference the bug, but could you expand this comment to say
> what's actually being tested, e.g.

Sorry, I've already committed previous patch.
Ok to commit this one?

Thanks,

---
 gdb/testsuite/gdb.base/printcmds.exp          | 5 ++++-
 gdb/testsuite/gdb.python/py-format-string.exp | 2 --
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index f2be549f2a5..2506f745f20 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -1077,7 +1077,10 @@ proc test_repeat_bytes {} {
     }
 }
 
-# Test for BZ#24331
+# Test that 'set elements' correctly limits the number of characters
+# printed from a string when a string ends with repeated characters.
+# This is gdb/24331
+
 proc test_repeat_bytes_limit {} {
    gdb_test "print -elem 3 -- \"AAAAA\"" "= \"AAA\"..."
    gdb_test "print -char 3 -elem 10 -- \"AAAAA\"" "= \"AAA\"..."
diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
index 2a011ebd45a..55a37766bff 100644
--- a/gdb/testsuite/gdb.python/py-format-string.exp
+++ b/gdb/testsuite/gdb.python/py-format-string.exp
@@ -745,8 +745,6 @@ proc test_max_string_one { setting unlimited } {
       "${default_pointer_regexp} \"hel\"..."
     check_format_string "a_binary_string" $opts \
       "${default_pointer_regexp} \"hel\"..."
-    # This will print four characters instead of three, see
-    # <https://sourceware.org/bugzilla/show_bug.cgi?id=24331>.
     check_format_string "a_binary_string_array" $opts \
       "\"hel\"..."
     check_format_string "a_big_string" $opts \
-- 
2.40.1.606.ga4b1b128d6-goog

Re: [PATCH] Update comments for the gdb/24331 fix.

[Andrew Burgess]

Paul Pluzhnikov <ppluzhnikov@google.com> writes:

> On Mon, May 15, 2023 at 2:43 AM Andrew Burgess <aburgess@redhat.com> wrote:
>>
>> It's fine to reference the bug, but could you expand this comment to say
>> what's actually being tested, e.g.
>
> Sorry, I've already committed previous patch.
> Ok to commit this one?

LGTM.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>
> Thanks,
>
> ---
>  gdb/testsuite/gdb.base/printcmds.exp          | 5 ++++-
>  gdb/testsuite/gdb.python/py-format-string.exp | 2 --
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index f2be549f2a5..2506f745f20 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -1077,7 +1077,10 @@ proc test_repeat_bytes {} {
>      }
>  }
>  
> -# Test for BZ#24331
> +# Test that 'set elements' correctly limits the number of characters
> +# printed from a string when a string ends with repeated characters.
> +# This is gdb/24331
> +
>  proc test_repeat_bytes_limit {} {
>     gdb_test "print -elem 3 -- \"AAAAA\"" "= \"AAA\"..."
>     gdb_test "print -char 3 -elem 10 -- \"AAAAA\"" "= \"AAA\"..."
> diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
> index 2a011ebd45a..55a37766bff 100644
> --- a/gdb/testsuite/gdb.python/py-format-string.exp
> +++ b/gdb/testsuite/gdb.python/py-format-string.exp
> @@ -745,8 +745,6 @@ proc test_max_string_one { setting unlimited } {
>        "${default_pointer_regexp} \"hel\"..."
>      check_format_string "a_binary_string" $opts \
>        "${default_pointer_regexp} \"hel\"..."
> -    # This will print four characters instead of three, see
> -    # <https://sourceware.org/bugzilla/show_bug.cgi?id=24331>.
>      check_format_string "a_binary_string_array" $opts \
>        "\"hel\"..."
>      check_format_string "a_big_string" $opts \
> -- 
> 2.40.1.606.ga4b1b128d6-goog