term: scrolling: hopefully fix all selection/scrolling related crashes

When scrolling, there are a couple of cases where an existing
selection must be canceled because we cannot meaningfully represent it
after scrolling.

These are when the selection is (partly) inside:

* The top scrolling region
* The bottom scrolling region
* The new lines scrolled in. I.e. re-used lines

For the scrolling regions, the real problem is when the selection
crosses the scrolling region boundary; a selection that is completely
inside a scrolling regions _might_ be possible to keep, but we would
need to translate the selection coordinates to the new scrolling
region lines.

For simplicity, we cancel the selection if it touches the scrolling
region. Period.

The last item, newly scrolled in lines is when the selection covers
very old lines and we're now wrapping around the scrollback history.

Then there's a fourth problem case: when the user has started a
selection, but hasn't yet moved the cursor. In this case, we have no
end point.

What's more problematic is that when the user (after scrolling) moves
the cursor, we try to create a huge selection that covers mostly
empty (NULL) rows, causing us to crash.

This can happen e.g. when reverse scrolling in such a way that we wrap
around the scrollback history.

The actual viewport in this case is something like `-n - m`. But the
selection we'll end up trying to create will be `m - (rows - n)`. This
range may very well contain NULL rows.

To deal with this, we simply cancel the selection.

CHANGELOG.md

@@ -36,6 +36,8 @@
   when the mouse button is released - not as soon as `shift` is
   released.
 * Selected cells did not appear selected if modified.
+* Very rare crash when beginning a selection at the same time the
+  terminal content was scrolled.
 
 ### Security
 

selection.c

@@ -30,31 +30,39 @@ selection_enabled(const struct terminal *term)
 }
 
 bool
-selection_on_rows_in_view(const struct terminal *term, int row_start, int row_end)
+selection_on_rows(const struct terminal *term, int row_start, int row_end)
 {
-    LOG_DBG("selection: %d-%d, range: %d-%d (view=%d)",
+    LOG_DBG("on rows: %d-%d, range: %d-%d (offset=%d)",
             term->selection.start.row, term->selection.end.row,
-            row_start, row_end, term->grid->view);
+            row_start, row_end, term->grid->offset);
 
-    if (term->selection.start.row == -1 || term->selection.end.row == -1)
+    if (term->selection.end.row == -1)
         return false;
 
+    assert(term->selection.start.row != -1);
+
+    row_start += term->grid->offset;
+    row_end += term->grid->offset;
+
     const struct coord *start = &term->selection.start;
     const struct coord *end = &term->selection.end;
 
+    if ((row_start <= start->row && row_end >= start->row) ||
+        (row_start <= end->row && row_end >= end->row))
+    {
+        /* The range crosses one of the selection boundaries */
+        return true;
+    }
+
+    /* For the last check we must ensure start <= end */
     if (start->row > end->row) {
         const struct coord *tmp = start;
         start = end;
         end = tmp;
     }
 
-    row_start += term->grid->view;
-    row_end += term->grid->view;
-
-    if ((row_start <= start->row && row_end >= start->row) ||
-        (row_start <= end->row && row_end >= end->row) ||
-        (row_start >= start->row && row_end <= end->row))
-    {
+    if (row_start >= start->row && row_end <= end->row) {
+        LOG_INFO("ON ROWS");
         return true;
     }
 

selection.h

@@ -17,7 +17,7 @@ void selection_dirty_cells(struct terminal *term);
 void selection_cancel(struct terminal *term);
 void selection_extend(struct terminal *term, int col, int row, uint32_t serial);
 
-bool selection_on_rows_in_view(const struct terminal *term, int start, int end);
+bool selection_on_rows(const struct terminal *term, int start, int end);
 
 void selection_mark_word(struct terminal *term, int col, int row,
                          bool spaces_only, uint32_t serial);

terminal.c

@@ -1722,6 +1722,22 @@ term_cursor_blink_restart(struct terminal *term)
     }
 }
 
+static bool
+selection_on_top_region(const struct terminal *term,
+                        struct scroll_region region)
+{
+    return region.start > 0 &&
+        selection_on_rows(term, 0, region.start - 1);
+}
+
+static bool
+selection_on_bottom_region(const struct terminal *term,
+                           struct scroll_region region)
+{
+    return region.end < term->rows &&
+        selection_on_rows(term, region.end, term->rows - 1);
+}
+
 void
 term_scroll_partial(struct terminal *term, struct scroll_region region, int rows)
 {
@@ -1731,8 +1747,32 @@ term_scroll_partial(struct terminal *term, struct scroll_region region, int rows
     /* Clamp scroll amount */
     rows = min(rows, region.end - region.start);
 
-    if (selection_on_rows_in_view(term, region.end - rows, region.end))
-        selection_cancel(term);
+    /* Cancel selections that cannot be scrolled */
+    if (term->selection.start.row != -1) {
+        if (term->selection.end.row != -1) {
+            /*
+             * Selection is (partly) inside either the top or bottom
+             * scrolling regions, or on (at least one) of the lines
+             * scrolled in (i.e. re-used lines).
+             */
+            if (selection_on_top_region(term, region) ||
+                selection_on_bottom_region(term, region) ||
+                selection_on_rows(term, region.end - rows, region.end - 1))
+            {
+                selection_cancel(term);
+            }
+        } else {
+            /*
+             * User started a selection, but didn't move the
+             * cursor.
+             *
+             * Not 100% sure this is needed for forward scrolling, but
+             * let's keep it here, for consistency with reverse
+             * scrolling.
+             */
+            selection_cancel(term);
+        }
+    }
 
     bool view_follows = term->grid->view == term->grid->offset;
     term->grid->offset += rows;
@@ -1779,9 +1819,31 @@ term_scroll_reverse_partial(struct terminal *term,
     /* Clamp scroll amount */
     rows = min(rows, region.end - region.start);
 
-    /* Does the selection cover re-used, newly scrolled in lines? */
-    if (selection_on_rows_in_view(term, region.start, region.start + rows - 1))
-        selection_cancel(term);
+    /* Cancel selections that cannot be scrolled */
+    if (term->selection.start.row != -1) {
+        if (term->selection.end.row != -1) {
+            /*
+             * Selection is (partly) inside either the top or bottom
+             * scrolling regions, or on (at least one) of the lines
+             * scrolled in (i.e. re-used lines).
+             */
+            if (selection_on_top_region(term, region) ||
+                selection_on_bottom_region(term, region) ||
+                selection_on_rows(term, region.start, region.start + rows - 1))
+            {
+                selection_cancel(term);
+            }
+        } else {
+            /*
+             * User started a selection, but didn't move the
+             * cursor.
+             *
+             * Since we're scrolling in reverse, the result may be a
+             * *huge* selection that covers empty (NULL) lines.
+             */
+            selection_cancel(term);
+        }
+    }
 
     bool view_follows = term->grid->view == term->grid->offset;
     term->grid->offset -= rows;