Skip Menu |

This queue is for tickets about the Tickit CPAN distribution.

Report information
The Basics
Id: 101716
Status: resolved
Priority: 0/
Queue: Tickit

People
Owner: Nobody in particular
Requestors: TEAM [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.49
Fixed in: 0.54



Subject: Segfault in Tickit::RenderBuffer->get_cell
Tests are passing, but a program that uses Tickit::Widget::Layout::Desktop is raising a segfault whenever it hits the ->get_cell call. perl-5.20.0, 32-bit. I'll try to narrow it down to a proper test case, here's some initial info from a gdb session (with -g flags hacked in): Program received signal SIGSEGV, Segmentation fault. 0xb72ae577 in XS_Tickit__RenderBuffer__xs_get_cell (cv=0x900d688) at lib/Tickit.xs:1062 1062 SvPOK_on(text); SvUTF8_on(text); SvCUR_set(text, len); (gdb) bt #0 0xb72ae577 in XS_Tickit__RenderBuffer__xs_get_cell (cv=0x900d688) at lib/Tickit.xs:1062 #1 0x080edfde in Perl_pp_entersub () #2 0x080e713b in Perl_runops_standard () #3 0x0807f07e in perl_run () #4 0x08060b8d in main () (gdb) l 1057 } 1058 1059 len = tickit_renderbuffer_get_cell_text(rb, line, col, NULL, 0); 1060 text = newSV(len + 1); 1061 tickit_renderbuffer_get_cell_text(rb, line, col, SvPVX(text), len + 1); 1062 SvPOK_on(text); SvUTF8_on(text); SvCUR_set(text, len); 1063 XPUSHs(sv_2mortal(text)); 1064 1065 mPUSHs(newSVpen(tickit_pen_clone(tickit_renderbuffer_get_cell_pen(rb, line, col)), NULL)); 1066 (gdb) p text $1 = (SV *) 0x9aca808 (gdb) p *text $2 = {sv_any = 0x0, sv_refcnt = 1, sv_flags = 536888320, sv_u = {svu_pv = 0x9af1120 "", svu_iv = 162468128, svu_uv = 162468128, svu_rv = 0x9af1120, svu_rx = 0x9af1120, svu_array = 0x9af1120, svu_hash = 0x9af1120, svu_gp = 0x9af1120, svu_fp = 0x9af1120}} (gdb) p len $3 = 4294967295 (gdb) p len + 1 $4 = 0 (gdb) p line $5 = 0 (gdb) p col $6 = 42 (gdb) p rb $7 = (TickitRenderBuffer *) 0x9ad4708 (gdb) p *rb $8 = {lines = 35, cols = 127, cells = 0x9ad67c8, vc_pos_set = 0, vc_line = 161989160, vc_col = 161777348, xlate_line = -6, xlate_col = 42, clip = {top = 0, left = 42, lines = 1, cols = 26}, pen = 0x9a7bdf0, depth = 4, stack = 0x9a77948, texts = 0x9a7aa18, n_texts = 0, size_texts = 4, tmp = 0x9af0d98 "", tmplen = 0, tmpsize = 256}
I'm guessing that it's due to line + xlate_line < 0, and that we're seeing a -1 return value that's not being handled here. If that's the case, not really sure what the best way of reporting it back to the user would be - just raise an exception maybe?
Confirmed that changing that section as follows: len = tickit_renderbuffer_get_cell_text(rb, line, col, NULL, 0); /* len is a size_t, so -1 => ~0 */ if(1 + len == 0) croak("$line or $col out of range"); text = newSV(len + 1); raises an exception rather than a segfault, so libtickit itself is handling this gracefully enough. Might want to check line+xlate_line / col+xlate_col rather than assuming this is the failure reason - seems that hitting a CONT cell could also trigger this. Test case should be along the lines of: $rb->translate(-1,0); $rb->get_cell(0,0); if I'm reading the gdb output correctly... anyway, marking this as "patched", since the workaround is enough to keep perl code running (and diagnose whatever's translating things out of the buffer). cheers, Tom
Original segfault test case from 98211 still raises a segfault so I'm attaching that to make it easier to find. cheers, Tom
Subject: get_cell_segfault.t
use strict; use warnings; use Test::More; use Test::Fatal; use Tickit::Window; use Tickit::Test; my ( $term, $win ) = mk_term_and_window; my $float = $win->make_float( 2, 2, 3, 3 ); $float->set_on_expose(with_rb => sub { my ($win, $rb, $rect) = @_; $rb->clear; my $ch = ord 'x'; $rb->char_at(0, 0, $ch); is(exception { $rb->get_cell(300, 300)->char }, undef, 'can request position outside renderbuffer'); }); $win->expose; flush_tickit; done_testing;
I think this fixes it nicely. A slight disagreement on the return value of _get_cell_active(), which returns true/false, but -1 on "error". Which of course appears true. -- Paul Evans
Subject: rt101716.diff
=== modified file 'lib/Tickit.xs' --- lib/Tickit.xs 2015-07-21 19:32:09 +0000 +++ lib/Tickit.xs 2015-08-08 21:13:39 +0000 @@ -1072,7 +1072,7 @@ TickitRenderBufferLineMask mask; PPCODE: rb = self; - if(!tickit_renderbuffer_get_cell_active(rb, line, col)) { + if(tickit_renderbuffer_get_cell_active(rb, line, col) != 1) { XPUSHs(&PL_sv_undef); XPUSHs(&PL_sv_undef); XSRETURN(2); === added file 't/90rt101716.t' --- t/90rt101716.t 1970-01-01 00:00:00 +0000 +++ t/90rt101716.t 2015-08-08 21:13:39 +0000 @@ -0,0 +1,27 @@ +use strict; +use warnings; + +use Test::More; +use Test::Fatal; +use Tickit::Window; +use Tickit::Test; + +my ( $term, $win ) = mk_term_and_window; +my $float = $win->make_float( 2, 2, 3, 3 ); + +$float->set_on_expose(with_rb => sub { + my ( $win, $rb, $rect ) = @_; + $rb->clear; + + my $ch = ord 'x'; + $rb->char_at(0, 0, $ch); + + is( exception { + $rb->get_cell(300, 300)->char + }, undef, 'can request position outside renderbuffer'); +}); + +$win->expose; +flush_tickit; + +done_testing;
Fix was released back in 0.54 -- Paul Evans