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

[Kevin Buettner <kevinb@redhat.com>]

On Tue, 22 Sep 2026 14:12:02 -0600
Tom Tromey <tromey@adacore.com> 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.

My AI-enabled patch review workflow confirmed Gwen's finding.
It said:

    Finding 1 -- the dominant uninterruptible loop is in char-print.c,
    not the array code.

    I profiled the exact repro from the thread (perf record on a
    patched build, 8 MiB static char p[], "print p"). 99% of samples
    are:

      print_command_1 -> print_value -> value_print -> c_value_print
	-> c_value_print_array -> printstr -> generic_printstr
	-> count_next_character -> iterate -> gdb_iconv -> iconv
	(98.98%)

    value_print_array_elements never appears. The reason is
    gdb/c-valprint.c:259: a char array that passes
    c_textual_element_type is printed through
    current_language->printstr -- the string path -- which entirely
    bypasses the code this patch modifies.

    Two measurements confirm this on the patched build (maint time
    1, print elements 10, max-value-size unlimited). A char array of
    8 MiB, printed through generic_printstr and iconv, takes 43.3 s.
    An int array of the same 8 MiB, which goes through
    value_print_array_elements, takes 0.29 s.

    So Guinevere's "you may have missed a spot" is correct, and the
    missed spot is char-print.c:279. The outer loop there, at
    char-print.c:525, already has a QUIT at line 529, but it only
    runs once per distinct character -- for a uniform array the
    inner while (1) consumes all 8M characters inside a single outer
    iteration, which is why the existing QUIT never fires.

    The fix is small: event-top.h is already included at
    char-print.c:21, so adding a QUIT as the first statement of the
    while (1) body at line 279 would do it, either in this patch or
    in an immediate follow-up. The commit message might also be
    softened: "This patch adds QUITs to the relevant spots" reads as
    a complete fix for the PR 17613 symptom, which it is not.

AI model used was GLM 5.3 Flash.  It had 3 other findings too, but
they were minor.

Kevin