Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LinearLayout->BoxView->Panel->TextArea can panic #148

Closed
simmons opened this issue Aug 14, 2017 · 2 comments
Closed

LinearLayout->BoxView->Panel->TextArea can panic #148

simmons opened this issue Aug 14, 2017 · 2 comments

Comments

@simmons
Copy link

simmons commented Aug 14, 2017

The TextArea::compute_rows() function panics with an "attempt to subtract with overflow" error if required_size() is called with (0,0). This can occur in normal conditions when LinearLayout::required_size()'s budget layout algorithm is invoked on a horizontally oriented layout, which calls the child required_size() with (1,_), and Panel transforms this to (0,0) when trying to accommodate the border.

I've observed this on the latest master (134854e), and the problem appears to go back to 14cfe36. This is on Linux with the default ncurses backend.

Example code:

extern crate cursive;
use cursive::Cursive;
use cursive::view::*;
use cursive::views::*;
fn main() {
    let mut cursive = Cursive::new();
    cursive.add_layer(LinearLayout::horizontal()
        .child(BoxView::new(SizeConstraint::Full,
                            SizeConstraint::Full,
                            Panel::new(TextArea::new().content("one"))))
        .child(BoxView::new(SizeConstraint::Full,
                            SizeConstraint::Full,
                            Panel::new(TextArea::new().content("two")))));
    cursive.run();
}

Panic with backtrace:

thread 'main' panicked at 'attempt to subtract with overflow', /Users/simmons/work/ref/Cursive/src/views/text_area.rs:205
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::panicking::panic
   9: cursive::views::text_area::TextArea::compute_rows
  10: <cursive::views::text_area::TextArea as cursive::view::View>::required_size
  11: <cursive::views::panel::Panel<V> as cursive::view::view_wrapper::ViewWrapper>::wrap_required_size
  12: cursive::view::view_wrapper::<impl cursive::view::View for T>::required_size
  13: <cursive::views::box_view::BoxView<T> as cursive::view::view_wrapper::ViewWrapper>::wrap_required_size
  14: cursive::view::view_wrapper::<impl cursive::view::View for T>::required_size
  15: cursive::views::linear_layout::Child::required_size
  16: <cursive::views::linear_layout::LinearLayout as cursive::view::View>::required_size::{{closure}}
  17: core::ops::impls::<impl core::ops::FnOnce<A> for &'a mut F>::call_once
  18: <core::option::Option<T>>::map
  19: <core::iter::Map<I, F> as core::iter::iterator::Iterator>::next
  20: <collections::vec::Vec<T> as collections::vec::SpecExtend<T, I>>::spec_extend
  21: <collections::vec::Vec<T> as collections::vec::SpecExtend<T, I>>::from_iter
  22: <collections::vec::Vec<T> as core::iter::traits::FromIterator<T>>::from_iter
  23: core::iter::iterator::Iterator::collect
  24: <cursive::views::linear_layout::LinearLayout as cursive::view::View>::required_size
  25: cursive::view::view_wrapper::ViewWrapper::wrap_required_size::{{closure}}
  26: <cursive::views::layer::Layer<T> as cursive::view::view_wrapper::ViewWrapper>::with_view_mut
  27: cursive::view::view_wrapper::ViewWrapper::wrap_required_size
  28: cursive::view::view_wrapper::<impl cursive::view::View for T>::required_size
  29: <cursive::views::shadow_view::ShadowView<T> as cursive::view::view_wrapper::ViewWrapper>::wrap_required_size
  30: cursive::view::view_wrapper::<impl cursive::view::View for T>::required_size
  31: <cursive::views::stack_view::StackView as cursive::view::View>::layout
  32: cursive::Cursive::layout
  33: cursive::Cursive::step
  34: cursive::Cursive::run
  35: panel_issue_example::main
  36: __rust_maybe_catch_panic
  37: std::rt::lang_start
  38: main

Debug output when instrumenting certain required_size() functions with eprintln!() on a 80x24 terminal:

LinearLayout::required_size(self, req=XY { x: 78, y: 22 })
LinearLayout::required_size(): simple case
linear_layout::Child::required_size(self, req=XY { x: 78, y: 22 })
BoxView::wrap_required_size(self, req=XY { x: 78, y: 22 })
Panel::wrap_required_size(self, req=XY { x: 78, y: 22 })
TextArea::required_size(self="one", constraint=XY { x: 76, y: 20 })
linear_layout::Child::required_size(self, req=XY { x: 78, y: 22 })
BoxView::wrap_required_size(self, req=XY { x: 78, y: 22 })
Panel::wrap_required_size(self, req=XY { x: 78, y: 22 })
TextArea::required_size(self="two", constraint=XY { x: 76, y: 20 })
LinearLayout::required_size(): budget case // budget_req=XY { x: 1, y: 22 }
linear_layout::Child::required_size(self, req=XY { x: 1, y: 22 })
BoxView::wrap_required_size(self, req=XY { x: 1, y: 22 })
Panel::wrap_required_size(self, req=XY { x: 1, y: 22 })
TextArea::required_size(self="one", constraint=XY { x: 0, y: 0 })
@gyscos gyscos closed this as completed in 4b59808 Aug 14, 2017
@gyscos
Copy link
Owner

gyscos commented Aug 14, 2017

Doh, thanks for the report!

There's still a bunch of not-very-safe subtractions... :S
I fixed TextArea for now; I'll have to go through the other views as well some day.

@gyscos
Copy link
Owner

gyscos commented Aug 14, 2017

I did a first run through unchecked subtractions, and fixed most of those to checked_sub/saturating_sub. I also added those two methods to Vec2 to make checks easier. The result should be published as 0.6.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants