Assertion failure in sixel code #494

Closed
opened 6 months ago by r_c_f · 4 comments
r_c_f commented 6 months ago

During the PGO stage of the build in the foot-git PKGBUILD, my rather-pathetic laptop display combined with a tiling WM leads to sporadic assertion failures when trying to process the output of

../scripts/generate-alt-random-writes.py --scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel

once I've got enough other terminals open to give it only a quarter of the screen. Trying it outside the makepkg environment, I was able to log the output (foot.txt) and get a backtrace (gdb.txt), which I've attached along with the borkage2 file from the offending test.

During the PGO stage of the build in the `foot-git` PKGBUILD, my rather-pathetic laptop display combined with a tiling WM leads to sporadic assertion failures when trying to process the output of ``` ../scripts/generate-alt-random-writes.py --scroll --scroll-region --colors-regular --colors-bright --colors-256 --colors-rgb --attr-bold --attr-italic --attr-underline --sixel ``` once I've got enough other terminals open to give it only a quarter of the screen. Trying it outside the makepkg environment, I was able to log the output (foot.txt) and get a backtrace (gdb.txt), which I've attached along with the `borkage2` file from the offending test.
23 MiB
1.8 KiB
1.7 KiB
Owner

Ouch, thanks for reporting!

Ouch, thanks for reporting!
dnkl added the
bug
label 6 months ago
Owner

I was able to reproduce with a similar setup:

./foot --font FantasqueSansMono:pixelsize=13 --window-size-chars 84x24

I had to cat the file twice, but doing so gives me:

 err: sixel.c:802: BUG in sixel_unhook(): assertion failed: 'image.rows < term->grid->num_rows'

Stack trace:
    #0 0x7fe745630279 in __sanitizer_print_stack_trace /build/gcc/src/gcc/libsanitizer/asan/asan_stack.cpp:86
    #1 0x55ef8fc5b01f in print_stack_trace ../../debug.c:20
    #2 0x55ef8fc5b2ab in bug ../../debug.c:44
    #3 0x55ef8fc1b4ff in sixel_unhook ../../sixel.c:802
    #4 0x55ef8fc4c81a in dcs_unhook ../../dcs.c:110
    #5 0x55ef8fc28a8c in action_unhook ../../vt.c:534
    #6 0x55ef8fc2c9dc in state_dcs_passthrough_switch ../../vt.c:1183
    #7 0x55ef8fc2d504 in vt_from_slave ../../vt.c:1309
    #8 0x55ef8fbd0511 in fdm_ptmx ../../terminal.c:255
    #9 0x55ef8fad3335 in fdm_poll ../../fdm.c:459
    #10 0x55ef8fafec4a in main ../../main.c:535
    #11 0x7fe7445f1b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #12 0x55ef8fa846fd in _start (/home/daniel/src/foot/bld/debug/foot+0x3186fd)
I was able to reproduce with a similar setup: ``` ./foot --font FantasqueSansMono:pixelsize=13 --window-size-chars 84x24 ``` I had to `cat` the file twice, but doing so gives me: ``` err: sixel.c:802: BUG in sixel_unhook(): assertion failed: 'image.rows < term->grid->num_rows' Stack trace: #0 0x7fe745630279 in __sanitizer_print_stack_trace /build/gcc/src/gcc/libsanitizer/asan/asan_stack.cpp:86 #1 0x55ef8fc5b01f in print_stack_trace ../../debug.c:20 #2 0x55ef8fc5b2ab in bug ../../debug.c:44 #3 0x55ef8fc1b4ff in sixel_unhook ../../sixel.c:802 #4 0x55ef8fc4c81a in dcs_unhook ../../dcs.c:110 #5 0x55ef8fc28a8c in action_unhook ../../vt.c:534 #6 0x55ef8fc2c9dc in state_dcs_passthrough_switch ../../vt.c:1183 #7 0x55ef8fc2d504 in vt_from_slave ../../vt.c:1309 #8 0x55ef8fbd0511 in fdm_ptmx ../../terminal.c:255 #9 0x55ef8fad3335 in fdm_poll ../../fdm.c:459 #10 0x55ef8fafec4a in main ../../main.c:535 #11 0x7fe7445f1b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) #12 0x55ef8fa846fd in _start (/home/daniel/src/foot/bld/debug/foot+0x3186fd) ```
Owner

Oh come on... looks like it's just the assert that's wrong - it's off by one:

assert(image.rows < term->grid->num_rows);

We're comparing two row counters; one being the number of rows used/required by the current sixel chunk, and the other the total number of scrollback rows. The sixel chunk may, of course, use all available rows...

However, applying

diff --git a/sixel.c b/sixel.c
index 0e8c58d..910e4a5 100644
--- a/sixel.c
+++ b/sixel.c
@@ -799,7 +799,7 @@ sixel_unhook(struct terminal *term)
             .opaque = !term->sixel.transparent_bg,
         };
 
-        xassert(image.rows < term->grid->num_rows);
+        xassert(image.rows <= term->grid->num_rows);
         xassert(image.pos.row + image.rows - 1 < term->grid->num_rows);
 
         LOG_DBG("generating %dx%d pixman image at %d-%d",

now gives us this instead:

 err: sixel.c:227: BUG in verify_scrollback_consistency(): assertion failed: 'last_row < row_no'

Stack trace:
    #0 0x7ffff768d279 in __sanitizer_print_stack_trace /build/gcc/src/gcc/libsanitizer/asan/asan_stack.cpp:86
    #1 0x555555a4301f in print_stack_trace ../../debug.c:20
    #2 0x555555a432ab in bug ../../debug.c:44
    #3 0x5555559f533b in verify_scrollback_consistency ../../sixel.c:227
    #4 0x5555559f61af in verify_sixels ../../sixel.c:279
    #5 0x5555559f7d92 in sixel_insert ../../sixel.c:305
    #6 0x555555a03eef in sixel_unhook ../../sixel.c:855
    #7 0x555555a3481a in dcs_unhook ../../dcs.c:110
    #8 0x555555a10a8c in action_unhook ../../vt.c:534
    #9 0x555555a149dc in state_dcs_passthrough_switch ../../vt.c:1183
    #10 0x555555a15504 in vt_from_slave ../../vt.c:1309
    #11 0x5555559b8511 in fdm_ptmx ../../terminal.c:255
    #12 0x5555558bb335 in fdm_poll ../../fdm.c:459
    #13 0x5555558e6c4a in main ../../main.c:535
    #14 0x7ffff664eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #15 0x55555586c6fd in _start (/home/daniel/src/foot/bld/debug/foot+0x3186fd)

The hunt continues...

Oh come on... looks like it's just the assert that's wrong - it's off by one: ```c assert(image.rows < term->grid->num_rows); ``` We're comparing two row counters; one being the number of rows used/required by the current sixel chunk, and the other the total number of scrollback rows. The sixel chunk may, of course, use all available rows... However, applying ```diff diff --git a/sixel.c b/sixel.c index 0e8c58d..910e4a5 100644 --- a/sixel.c +++ b/sixel.c @@ -799,7 +799,7 @@ sixel_unhook(struct terminal *term) .opaque = !term->sixel.transparent_bg, }; - xassert(image.rows < term->grid->num_rows); + xassert(image.rows <= term->grid->num_rows); xassert(image.pos.row + image.rows - 1 < term->grid->num_rows); LOG_DBG("generating %dx%d pixman image at %d-%d", ``` now gives us this instead: ``` err: sixel.c:227: BUG in verify_scrollback_consistency(): assertion failed: 'last_row < row_no' Stack trace: #0 0x7ffff768d279 in __sanitizer_print_stack_trace /build/gcc/src/gcc/libsanitizer/asan/asan_stack.cpp:86 #1 0x555555a4301f in print_stack_trace ../../debug.c:20 #2 0x555555a432ab in bug ../../debug.c:44 #3 0x5555559f533b in verify_scrollback_consistency ../../sixel.c:227 #4 0x5555559f61af in verify_sixels ../../sixel.c:279 #5 0x5555559f7d92 in sixel_insert ../../sixel.c:305 #6 0x555555a03eef in sixel_unhook ../../sixel.c:855 #7 0x555555a3481a in dcs_unhook ../../dcs.c:110 #8 0x555555a10a8c in action_unhook ../../vt.c:534 #9 0x555555a149dc in state_dcs_passthrough_switch ../../vt.c:1183 #10 0x555555a15504 in vt_from_slave ../../vt.c:1309 #11 0x5555559b8511 in fdm_ptmx ../../terminal.c:255 #12 0x5555558bb335 in fdm_poll ../../fdm.c:459 #13 0x5555558e6c4a in main ../../main.c:535 #14 0x7ffff664eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) #15 0x55555586c6fd in _start (/home/daniel/src/foot/bld/debug/foot+0x3186fd) ``` The hunt continues...
Owner

Note to self, since I'll probably have to put this on paus soon:

The problem is that we're trying to emit a sixel with exactly as many rows as the scrollback and a newline. Normally, the newline would trigger logic that removes the scrolled out image. But in this case, the image hasn't been emitted yet (we're "pre-allocating" space for it by emitting newlines).

Thank god for the verify_*() functions...

Easiest option is to skip "too large" sixels. Again, normal behavior is to remove the whole sixel as soon as some part of it is scrolled out from the scrollback.

Note to self, since I'll probably have to put this on paus soon: The problem is that we're trying to emit a sixel with exactly as many rows as the scrollback **and a newline**. Normally, the newline would trigger logic that removes the scrolled out image. But in this case, the image hasn't been emitted yet (we're "pre-allocating" space for it by emitting newlines). Thank god for the `verify_*()` functions... Easiest option is to skip "too large" sixels. Again, normal behavior is to remove the whole sixel as soon as some part of it is scrolled out from the scrollback.
dnkl referenced this issue from a commit 6 months ago
dnkl closed this issue 6 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.