Subject: Re: bug#76538: 31.0.50; 31.0.50; 31.0.50; feature/igc: using magit-section-cycle-global (S-TAB) and magit-section-toggle (TAB) in some random ways blocks GNU Emacs.

From: Pip Cet <[email-masked]>
Date: Mon, 03 Mar 2025 14:46:55 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Mon, 03 Mar 2025 10:42:25 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: Helmut Eller <[email-masked]>, Stefan Monnier
>> <[email-masked]>, Ihor Radchenko <[email-masked]>,
>> Eli Zaretskii <[email-masked]>, [email-masked],
>> [email-masked]
>>
>> I have no idea why I hit the assert which required the second patch.
>
> Which assertion was that?  Is there any way of triggering that on the
> master branch, or on the igc branch without first applying the patch?

Not on the master branch itself, I have to change the "5000" to "50000"
in the marker code, which I don't think should cause this assertion
violation.  The reproducer script is the magit.bash script Helmut
provided, this patch, and hitting C-p C-p C-n C-n when the magit window
opens.

diff --git a/src/marker.c b/src/marker.c
index 4ab68ec7bbe..61f7af3f904 100644
--- a/src/marker.c
+++ b/src/marker.c
@@ -97,6 +97,7 @@ #define CONSIDER(CHARPOS, BYTEPOS)					\
 {									\
   ptrdiff_t this_charpos = (CHARPOS);					\
   bool changed = false;							\
+  considered++;								\
 									\
   if (this_charpos == charpos)						\
     {									\
@@ -194,6 +195,7 @@ buf_charpos_to_bytepos (struct buffer *b, ptrdiff_t charpos)
      two best approximations is all single-byte,
      we interpolate the result immediately.  */
 
+  ptrdiff_t considered = 0;
   CONSIDER (BUF_PT (b), BUF_PT_BYTE (b));
   CONSIDER (BUF_GPT (b), BUF_GPT_BYTE (b));
   CONSIDER (BUF_BEGV (b), BUF_BEGV_BYTE (b));
@@ -219,7 +221,10 @@ buf_charpos_to_bytepos (struct buffer *b, ptrdiff_t charpos)
   eassert (best_below <= charpos && charpos <= best_above);
   if (charpos - best_below < best_above - charpos)
     {
-      bool record = charpos - best_below > 5000;
+      bool record = charpos - best_below > 50000;
+
+      if (record)
+	fprintf (stderr, "recording (1) starting at %ld %ld %ld\n", best_below, charpos, considered);
 
       while (best_below < charpos)
 	{
@@ -244,7 +249,10 @@ buf_charpos_to_bytepos (struct buffer *b, ptrdiff_t charpos)
     }
   else
     {
-      bool record = best_above - charpos > 5000;
+      bool record = best_above - charpos > 50000;
+
+      if (record)
+	fprintf (stderr, "recording (2) starting at %ld %ld %ld\n", charpos, best_above, considered);
 
       while (best_above > charpos)
 	{
@@ -278,6 +286,7 @@ #define CONSIDER(BYTEPOS, CHARPOS)					\
 {									\
   ptrdiff_t this_bytepos = (BYTEPOS);					\
   int changed = false;							\
+  considered++;								\
 									\
   if (this_bytepos == bytepos)						\
     {									\
@@ -342,6 +351,7 @@ buf_bytepos_to_charpos (struct buffer *b, ptrdiff_t bytepos)
   best_below = BEG;
   best_below_byte = BEG_BYTE;
 
+  ptrdiff_t considered = 0;
   CONSIDER (BUF_PT_BYTE (b), BUF_PT (b));
   CONSIDER (BUF_GPT_BYTE (b), BUF_GPT (b));
   CONSIDER (BUF_BEGV_BYTE (b), BUF_BEGV (b));
@@ -366,7 +376,10 @@ buf_bytepos_to_charpos (struct buffer *b, ptrdiff_t bytepos)
 
   if (bytepos - best_below_byte < best_above_byte - bytepos)
     {
-      bool record = bytepos - best_below_byte > 5000;
+      bool record = bytepos - best_below_byte > 50000;
+
+      if (record)
+	fprintf (stderr, "recording (3) starting at %ld %ld %ld\n", bytepos, best_below_byte, considered);
 
       while (best_below_byte < bytepos)
 	{
@@ -393,8 +406,10 @@ buf_bytepos_to_charpos (struct buffer *b, ptrdiff_t bytepos)
     }
   else
     {
-      bool record = best_above_byte - bytepos > 5000;
+      bool record = best_above_byte - bytepos > 50000;
 
+      if (record)
+	fprintf (stderr, "recording (4) starting at %ld %ld %ld\n", bytepos, best_above_byte, considered);
       while (best_above_byte > bytepos)
 	{
 	  best_above--;

it looks like this:

        it = {window = 0x555557caa01d, w = 0x555557caa018, f = 0x555555e74658, method = GET_FROM_BUFFER, stop_charpos = 27353858, prev_stop = 347705, base_level_stop = 0, end_charpos = 27353858, medium_narrowing_begv = 27244404, medium_narrowing_zv = 27353858, large_narrowing_begv = 27103730, large_narrowing_zv = 27353858, s = 0x0, string_nchars = 0, multibyte_p = true, tab_line_p = false, header_line_p = false, string_from_display_prop_p = false, string_from_prefix_prop_p = false, from_disp_prop_p = false, ellipsis_p = false, avoid_cursor_p = false, dp = 0x0, dpvec = 0x0, dpend = 0x0, dpvec_char_len = 0, dpvec_face_id = 0, saved_face_id = 0, ctl_chars = {0x0 <repeats 16 times>}, start = {pos = {charpos = 347705, bytepos = 348476}, overlay_string_index = -1, string_pos = {charpos = -1, bytepos = -1}, dpvec_index = -1}, current = {pos = {charpos = 27353857, bytepos = 27355172}, overlay_string_index = -1, string_pos = {charpos = -1, bytepos = -1}, dpvec_index = -1}, n_overlay_strings = 0, overlay_strings_charpos = 27353857, overlay_strings = {0x0 <repeats 16 times>}, string_overlays = {0x0 <repeats 16 times>}, string = 0x0, from_overlay = 0x0, stack = {{string = 0x0, string_nchars = 0, end_charpos = 0, stop_charpos = 0, prev_stop = 0, base_level_stop = 0, cmp_it = {stop_pos = 0, id = 0, ch = 0, rule_idx = 0, lookback = 0, nglyphs = 0, reversed_p = false, parent_it = 0x0, charpos = 0, nchars = 0, nbytes = 0, from = 0, to = 0, width = 0}, face_id = 0, u = {image = {object = 0x0, slice = {x = 0x0, y = 0x0, width = 0x0, height = 0x0}, image_id = 0}, stretch = {object = 0x0}, xwidget = {object = 0x0}}, position = {charpos = 0, bytepos = 0}, current = {pos = {charpos = 0, bytepos = 0}, overlay_string_index = 0, string_pos = {charpos = 0, bytepos = 0}, dpvec_index = 0}, from_overlay = 0x0, area = LEFT_MARGIN_AREA, method = GET_FROM_BUFFER, paragraph_embedding = NEUTRAL_DIR, multibyte_p = false, string_from_display_prop_p = false, string_from_prefix_prop_p = false, display_ellipsis_p = false, avoid_cursor_p = false, bidi_p = false, from_disp_prop_p = false, line_wrap = TRUNCATE, voffset = 0, space_width = 0x0, font_height = 0x0}, {string = 0x0, string_nchars = 0, end_charpos = 0, stop_charpos = 0, prev_stop = 0, base_level_stop = 0, cmp_it = {stop_pos = 0, id = 0, ch = 0, rule_idx = 0, lookback = 0, nglyphs = 0, reversed_p = false, parent_it = 0x0, charpos = 0, nchars = 0, nbytes = 0, from = 0, to = 0, width = 0}, face_id = 0, u = {image = {object = 0x0, slice = {x = 0x0, y = 0x0, width = 0x0, height = 0x0}, image_id = 0}, stretch = {object = 0x0}, xwidget = {object = 0x0}}, position = {charpos = 0, bytepos = 0}, current = {pos = {charpos = 0, bytepos = 0}, overlay_string_index = 0, string_pos = {charpos = 0, bytepos = 0}, dpvec_index = 0}, from_overlay = 0x0, area = LEFT_MARGIN_AREA, method = GET_FROM_BUFFER, paragraph_embedding = NEUTRAL_DIR, multibyte_p = false, string_from_display_prop_p = false, string_from_prefix_prop_p = false, display_ellipsis_p = false, avoid_cursor_p = false, bidi_p = false, from_disp_prop_p = false, line_wrap = TRUNCATE, voffset = 0, space_width = 0x0, font_height = 0x0}, {string = 0x0, string_nchars = 0, end_charpos = 0, stop_charpos = 0, prev_stop = 0, base_level_stop = 0, cmp_it = {stop_pos = 0, id = 0, ch = 0, rule_idx = 0, lookback = 0, nglyphs = 0, reversed_p = false, parent_it = 0x0, charpos = 0, nchars = 0, nbytes = 0, from = 0, to = 0, width = 0}, face_id = 0, u = {image = {object = 0x0, slice = {x = 0x0, y = 0x0, width = 0x0, height = 0x0}, image_id = 0}, stretch = {object = 0x0}, xwidget = {object = 0x0}}, position = {charpos = 0, bytepos = 0}, current = {pos = {charpos = 0, bytepos = 0}, overlay_string_index = 0, string_pos = {charpos = 0, bytepos = 0}, dpvec_index = 0}, from_overlay = 0x0, area = LEFT_MARGIN_AREA, method = GET_FROM_BUFFER, paragraph_embedding = NEUTRAL_DIR, multibyte_p = false, string_from_display_prop_p = false, string_from_prefix_prop_p = false, display_ellipsis_p = false, avoid_cursor_p = false, bidi_p = false, from_disp_prop_p = false, line_wrap = TRUNCATE, voffset = 0, space_width = 0x0, font_height = 0x0}, {string = 0x0, string_nchars = 0, end_charpos = 0, stop_charpos = 0, prev_stop = 0, base_level_stop = 0, cmp_it = {stop_pos = 0, id = 0, ch = 0, rule_idx = 0, lookback = 0, nglyphs = 0, reversed_p = false, parent_it = 0x0, charpos = 0, nchars = 0, nbytes = 0, from = 0, to = 0, width = 0}, face_id = 0, u = {image = {object = 0x0, slice = {x = 0x0, y = 0x0, width = 0x0, height = 0x0}, image_id = 0}, stretch = {object = 0x0}, xwidget = {object = 0x0}}, position = {charpos = 0, bytepos = 0}, current = {pos = {charpos = 0, bytepos = 0}, overlay_string_index = 0, string_pos = {charpos = 0, bytepos = 0}, dpvec_index = 0}, from_overlay = 0x0, area = LEFT_MARGIN_AREA, method = GET_FROM_BUFFER, paragraph_embedding = NEUTRAL_DIR, multibyte_p = false, string_from_display_prop_p = false, string_from_prefix_prop_p = false, display_ellipsis_p = false, avoid_cursor_p = false, bidi_p = false, from_disp_prop_p = false, line_wrap = TRUNCATE, voffset = 0, space_width = 0x0, font_height = 0x0}, {string = 0x0, string_nchars = 0, end_charpos = 0, stop_charpos = 0, prev_stop = 0, base_level_stop = 0, cmp_it = {stop_pos = 0, id = 0, ch = 0, rule_idx = 0, lookback = 0, nglyphs = 0, reversed_p = false, parent_it = 0x0, charpos = 0, nchars = 0, nbytes = 0, from = 0, to = 0, width = 0}, face_id = 0, u = {image = {object = 0x0, slice = {x = 0x0, y = 0x0, width = 0x0, height = 0x0}, image_id = 0}, stretch = {object = 0x0}, xwidget = {object = 0x0}}, position = {charpos = 0, bytepos = 0}, current = {pos = {charpos = 0, bytepos = 0}, overlay_string_index = 0, string_pos = {charpos = 0, bytepos = 0}, dpvec_index = 0}, from_overlay = 0x0, area = LEFT_MARGIN_AREA, method = GET_FROM_BUFFER, paragraph_embedding = NEUTRAL_DIR, multibyte_p = false, string_from_display_prop_p = false, string_from_prefix_prop_p = false, display_ellipsis_p = false, avoid_cursor_p = false, bidi_p = false, from_disp_prop_p = false, line_wrap = TRUNCATE, voffset = 0, space_width = 0x0, font_height = 0x0}}, sp = 0, selective = 0, what = IT_CHARACTER, face_id = 0, selective_display_ellipsis_p = true, ctl_arrow_p = true, face_box_p = false, start_of_box_run_p = false, end_of_box_run_p = false, overlay_strings_at_end_processed_p = false, ignore_overlay_strings_at_pos_p = false, glyph_not_available_p = false, starts_in_middle_of_char_p = false, face_before_selective_p = false, constrain_row_ascent_descent_p = false, line_number_produced_p = false, align_visually_p = false, line_wrap = TRUNCATE, base_face_id = 0, c = 10, len = 0, cmp_it = {stop_pos = 27353857, id = -1, ch = -2, rule_idx = 0, lookback = 0, nglyphs = 0, reversed_p = false, parent_it = 0x7fffffffb8f0, charpos = 0, nchars = 0, nbytes = 0, from = 0, to = 0, width = 0}, char_to_display = 0, glyphless_method = GLYPHLESS_DISPLAY_THIN_SPACE, image_id = 0, xwidget = 0x0, slice = {x = 0x0, y = 0x0, width = 0x0, height = 0x0}, space_width = 0x0, voffset = 0, tab_width = 8, font_height = 0x0, object = 0x555557cdf2bd, position = {charpos = 27353857, bytepos = 27355172}, truncation_pixel_width = 9, continuation_pixel_width = 0, first_visible_x = 0, last_visible_x = 1881, last_visible_y = 1836, extra_line_spacing = 0, max_extra_line_spacing = 0, override_ascent = -1, override_descent = 0, override_boff = 0, glyph_row = 0x0, area = TEXT_AREA, nglyphs = 1, pixel_width = 0, ascent = 0, descent = 0, max_ascent = 0, max_descent = 0, phys_ascent = 0, phys_descent = 0, max_phys_ascent = 0, max_phys_descent = 0, current_x = 0, wrap_prefix_width = 0, continuation_lines_width = 0, eol_pos = {charpos = 0, bytepos = 0}, current_y = 396, first_vpos = 0, vpos = 22, hpos = 0, lnum = 0, lnum_bytepos = 0, lnum_width = 0, lnum_pixel_width = 0, pt_lnum = 0, stretch_adjust = 0, left_user_fringe_bitmap = 0, right_user_fringe_bitmap = 0, left_user_fringe_face_id = 0, right_user_fringe_face_id = 0, bidi_p = true, bidi_it = {bytepos = 27355172, charpos = 27353857, ch = 10, nchars = 1, ch_len = 1, type = NEUTRAL_B, type_after_wn = NEUTRAL_B, orig_type = NEUTRAL_B, resolved_level = 0 '\000', isolate_level = 0 '\000', invalid_levels = 0, invalid_isolates = 0, prev = {charpos = 0, type = UNKNOWN_BT, orig_type = UNKNOWN_BT}, last_strong = {charpos = 0, type = UNKNOWN_BT, orig_type = UNKNOWN_BT}, next_for_neutral = {charpos = -1, type = UNKNOWN_BT, orig_type = UNKNOWN_BT}, prev_for_neutral = {charpos = -1, type = UNKNOWN_BT, orig_type = UNKNOWN_BT}, next_for_ws = {charpos = 0, type = UNKNOWN_BT, orig_type = UNKNOWN_BT}, bracket_pairing_pos = -1, bracket_enclosed_type = UNKNOWN_BT, next_en_pos = 0, next_en_type = UNKNOWN_BT, sos = L2R, scan_dir = 0, disp_pos = -1, disp_prop = 0, stack_idx = 0, level_stack = {{next_for_neutral_pos = 0, next_for_neutral_type = 0, last_strong_type = 0, prev_for_neutral_type = 0, level = 0 '\000', flags = 0 '\000'} <repeats 128 times>}, string = {lstring = 0x0, s = 0x0, schars = 0, bufpos = 0, from_disp_str = false, unibyte = false}, w = 0x555557caa018, paragraph_dir = NEUTRAL_DIR, separator_limit = -1, first_elt = false, new_paragraph = true, frame_window_p = true}, paragraph_embedding = NEUTRAL_DIR, min_width_property = 0x0, min_width_start = 0}

Should this combination ever be valid?

it->what = IT_CHARACTER
it->c = 10
it->len = 0

>> It seems to me like a bug in the xdisp.c code, but it's xdisp.c, so
>> who knows?
>
> I suspect it->len is not used when the character is composed, because

The character isn't composed, is it? It's a plain newline.

>> -	it->c = STRING_CHAR (s);
>> +	{
>> +	  it->c = STRING_CHAR (s);
>> +	  it->len = CHAR_BYTES (it->c);
>> +	}
>
> This is sub-optimal; please use string_char_and_length instead.

Will do, if we need to fix this at all.

>> @@ -6724,6 +6730,7 @@ handle_composition_prop (struct it *it)
>>        pos_byte = IT_BYTEPOS (*it);
>>        string = Qnil;
>>        it->c = FETCH_CHAR (pos_byte);
>> +      it->len = it->multibyte_p ? CHAR_BYTES (it->c) : 1;
>
> Likewise here.

I was looking for fetch_char_and_length, but I guess
string_char_and_length (BYTE_POS_ADDR (pos_byte), &it->len) works, too?

I'll try reducing the patch to just the 5000 -> 50000 thing next.

Pip