Re: [PATCH] Add QUIT to inner loop in value_print_array_elements

[Guinevere Larsen <guinevere@redhat.com>]

On 9/22/26 5:12 PM, Tom Tromey wrote:
> When printing an array, some work is done to detect repeated elements.
> This work is not interruptible, and it is somewhat slow (see
> PR exp/32681).
>
> This patch adds QUITs to the relevant spots.
>
> I am not sure there is a way to write a non-racy test for this.

Hi Tom!

The code change seems pretty obvious. I tried to give a test some 
thought, and based on how long it takes to print a 1M sized array, we 
might be able to add a wait for a second or two and attempt to send a 
Ctrl+C signal, and check for a timeout, I think. Without real testing 
(and using the code in the bugzilla example), I wonder if this is workable:

gdb_test_no_output "set max-value-size unlimited"
send_gdb "p *p\n"
# make sure that we're in the code looking for repeats
sleep 1
gdb_test_multiple "\003" "cancel printing" {
   -ex -wrap "" {
     pass $gdb_test_name
   }
}

I think this may work. The main racy issue I can see would be false 
positive, where we send QUIT before reaching the problem code, which 
isn't as much of an issue as the false negatives I don't think.

That said, I just tried applying this and running that test manually and 
it didn't quit, there's still a large uninterruptible pause going on, so 
I think you may have missed a spot?

-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)

>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=17613
> ---
>   gdb/ada-valprint.c | 2 ++
>   gdb/f-valprint.c   | 5 +++++
>   gdb/valprint.c     | 2 ++
>   3 files changed, 9 insertions(+)
>
> diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
> index d66f9f84d52..020478bdf40 100644
> --- a/gdb/ada-valprint.c
> +++ b/gdb/ada-valprint.c
> @@ -183,6 +183,8 @@ val_print_packed_array_elements (struct type *type, const gdb_byte *valaddr,
>   					   bitsize, elttype);
>         while (1)
>   	{
> +	  QUIT;
> +
>   	  /* Make sure to free any values in the inner loop.  */
>   	  scoped_value_mark free_values;
>   
> diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
> index d0b2030554f..2b54c8bf1c0 100644
> --- a/gdb/f-valprint.c
> +++ b/gdb/f-valprint.c
> @@ -36,6 +36,7 @@
>   #include "cli/cli-style.h"
>   #include "gdbarch.h"
>   #include "f-array-walker.h"
> +#include "event-top.h"
>   
>   static void f77_get_dynamic_length_of_aggregate (struct type *);
>   
> @@ -188,6 +189,8 @@ class fortran_array_printer_impl : public fortran_array_walker_base_impl
>   			  struct type *elt_type, LONGEST elt_off,
>   			  LONGEST index, bool last_p)
>     {
> +    QUIT;
> +
>       size_t dim_indx = m_dimension - 1;
>       struct type *elt_type_prev = m_elt_type_prev;
>       LONGEST elt_off_prev = m_elt_off_prev;
> @@ -262,6 +265,8 @@ class fortran_array_printer_impl : public fortran_array_walker_base_impl
>       LONGEST elt_off_prev = m_elt_off_prev;
>       bool repeated = false;
>   
> +    QUIT;
> +
>       if (m_options.repeat_count_threshold < UINT_MAX
>   	&& elt_type_prev != nullptr)
>         {
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index 83a442dcef5..7e41943f66f 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -2038,6 +2038,8 @@ value_print_array_elements (struct value *val, struct ui_file *stream,
>   
>   	  while (rep1 < len)
>   	    {
> +	      QUIT;
> +
>   	      /* When printing large arrays this spot is called frequently, so
>   		 clean up temporary values asap to prevent allocating a large
>   		 amount of them.  */
>
> base-commit: 123e4a5eab2db300b44288dcc1a29c903f9622f9