From b4f666118febfc95bf97e31aa1e39e3df9522dfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 25 Apr 2022 19:57:18 +0200
Subject: [PATCH 1/6] grid: add abs-to-sb and sb-to-abs utility function
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

These functions convert row numbers between absolute coordinates and
“scrollback relative” coordinates.

Absolute row numbers can be used to index into the grid->rows[] array.

Scrollback relative numbers are ordered with the *oldest* row first,
and the *newest* row last. That is, in these coordinates, row 0 is the
*first* (oldest) row in the scrollback history, and row N is the
*last* (newest) row.

Scrollback relative numbers are used when we need to sort things after
their age, when determining if something has scrolled out, or when
limiting an operation to ensure we don’t go past the scrollback
wrap-around.
---
 grid.c | 28 ++++++++++++++++++++++++++++
 grid.h |  5 +++++
 2 files changed, 33 insertions(+)

diff --git a/grid.c b/grid.c
index 5a0b5c7e..4a3995a0 100644
--- a/grid.c
+++ b/grid.c
@@ -15,6 +15,34 @@
 
 #define TIME_REFLOW 0
 
+/*
+ * “sb” (scrollback relative) coordinates
+ *
+ * The scrollback relative row number 0 is the *first*, and *oldest*
+ * row in the scrollback history (and thus the *first* row to be
+ * scrolled out). Thus, a higher number means further *down* in the
+ * scrollback, with the *highest* number being at the bottom of the
+ * screen, where new input appears.
+ */
+int
+grid_row_abs_to_sb(const struct grid *grid, int screen_rows, int abs_row)
+{
+    const int scrollback_start = grid->offset + screen_rows;
+    int rebased_row = abs_row - scrollback_start + grid->num_rows;
+
+    rebased_row &= grid->num_rows - 1;
+    return rebased_row;
+}
+
+int grid_row_sb_to_abs(const struct grid *grid, int screen_rows, int sb_rel_row)
+{
+    const int scrollback_start = grid->offset + screen_rows;
+    int abs_row = sb_rel_row + scrollback_start;
+
+    abs_row &= grid->num_rows - 1;
+    return abs_row;
+}
+
 static void
 ensure_row_has_extra_data(struct row *row)
 {
diff --git a/grid.h b/grid.h
index 7819db4d..22bd76bb 100644
--- a/grid.h
+++ b/grid.h
@@ -21,6 +21,10 @@ void grid_resize_and_reflow(
     size_t tracking_points_count,
     struct coord *const _tracking_points[static tracking_points_count]);
 
+/* Convert row numbers between scrollback-relative and absolute coordinates */
+int grid_row_abs_to_sb(const struct grid *grid, int screen_rows, int abs_row);
+int grid_row_sb_to_abs(const struct grid *grid, int screen_rows, int sb_rel_row);
+
 static inline int
 grid_row_absolute(const struct grid *grid, int row_no)
 {
@@ -33,6 +37,7 @@ grid_row_absolute_in_view(const struct grid *grid, int row_no)
     return (grid->view + row_no) & (grid->num_rows - 1);
 }
 
+
 static inline struct row *
 _grid_row_maybe_alloc(struct grid *grid, int row_no, bool alloc_if_null)
 {

From 6316a5eb0cf2b1fe25ca0e9d60c4e11fce9c22d1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 25 Apr 2022 19:59:23 +0200
Subject: [PATCH 2/6] selection: add start/end coordinate getters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Internally, selection coordinates are *unbounded* (that is, the row
numbers may be larger than grid->num_rows) while a selection is
ongoing. Only after it has been finalized are the coordinates bounded.

This means it isn’t safe to use term->selection.coords.* directly.
---
 selection.c | 24 ++++++++++++++++++++++++
 selection.h |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/selection.c b/selection.c
index 8a18c6bb..091d3ed0 100644
--- a/selection.c
+++ b/selection.c
@@ -38,6 +38,29 @@ static const char *const mime_type_map[] = {
     [DATA_OFFER_MIME_TEXT_UTF8_STRING] = "UTF8_STRING",
 };
 
+static inline struct coord
+bounded(const struct grid *grid, struct coord coord)
+{
+    coord.row &= grid->num_rows - 1;
+    return coord;
+}
+
+struct coord
+selection_get_start(const struct terminal *term)
+{
+    if (term->selection.coords.start.row < 0)
+        return term->selection.coords.start;
+    return bounded(term->grid, term->selection.coords.start);
+}
+
+struct coord
+selection_get_end(const struct terminal *term)
+{
+    if (term->selection.coords.end.row < 0)
+        return term->selection.coords.end;
+    return bounded(term->grid, term->selection.coords.end);
+}
+
 bool
 selection_on_rows(const struct terminal *term, int row_start, int row_end)
 {
@@ -2461,3 +2484,4 @@ const struct zwp_primary_selection_device_v1_listener primary_selection_device_l
     .data_offer = &primary_data_offer,
     .selection = &primary_selection,
 };
+
diff --git a/selection.h b/selection.h
index 295d8d1c..0a6ece91 100644
--- a/selection.h
+++ b/selection.h
@@ -79,3 +79,6 @@ void selection_find_word_boundary_left(
     struct terminal *term, struct coord *pos, bool spaces_only);
 void selection_find_word_boundary_right(
     struct terminal *term, struct coord *pos, bool spaces_only);
+
+struct coord selection_get_start(const struct terminal *term);
+struct coord selection_get_end(const struct terminal *term);

From 1d4e1b921d351da349ccfbc5d587489d7b77e690 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 25 Apr 2022 20:00:14 +0200
Subject: [PATCH 3/6] sixel/terminal: use the new grid and selection APIs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Use grid_row_abs_to_sb() instead of manually “rebasing” row numbers.

Use selection_get_{start,end}() to retrieve the current selection
coordinates.
---
 sixel.c    | 55 ++++++++++++++++++++++++------------------------------
 terminal.c | 25 +++++++++++++------------
 2 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/sixel.c b/sixel.c
index 5316c0ad..0c94117f 100644
--- a/sixel.c
+++ b/sixel.c
@@ -7,8 +7,9 @@
 #define LOG_ENABLE_DBG 0
 #include "log.h"
 #include "debug.h"
-#include "render.h"
+#include "grid.h"
 #include "hsl.h"
+#include "render.h"
 #include "util.h"
 #include "xmalloc.h"
 #include "xsnprintf.h"
@@ -138,25 +139,6 @@ sixel_erase(struct terminal *term, struct sixel *sixel)
     sixel_destroy(sixel);
 }
 
-/*
- * Calculates the scrollback relative row number, given an absolute row number.
- *
- * The scrollback relative row number 0 is the *first*, and *oldest*
- * row in the scrollback history (and thus the *first* row to be
- * scrolled out). Thus, a higher number means further *down* in the
- * scrollback, with the *highest* number being at the bottom of the
- * screen, where new input appears.
- */
-static int
-rebase_row(const struct terminal *term, int abs_row)
-{
-    int scrollback_start = term->grid->offset + term->rows;
-    int rebased_row = abs_row - scrollback_start + term->grid->num_rows;
-
-    rebased_row &= term->grid->num_rows - 1;
-    return rebased_row;
-}
-
 /*
  * Verify the sixels are sorted correctly.
  *
@@ -175,7 +157,8 @@ verify_list_order(const struct terminal *term)
     size_t idx = 0;
 
     tll_foreach(term->grid->sixel_images, it) {
-        int row = rebase_row(term, it->item.pos.row + it->item.rows - 1);
+        int row = grid_row_abs_to_sb(
+            term->grid, term->rows, it->item.pos.row + it->item.rows - 1);
         int col = it->item.pos.col;
         int col_count = it->item.cols;
 
@@ -232,7 +215,8 @@ verify_scrollback_consistency(const struct terminal *term)
 
         int last_row = -1;
         for (int i = 0; i < six->rows; i++) {
-            int row_no = rebase_row(term, six->pos.row + i);
+            int row_no = grid_row_abs_to_sb(
+                term->grid, term->rows, six->pos.row + i);
 
             if (last_row != -1)
                 xassert(last_row < row_no);
@@ -295,10 +279,14 @@ verify_sixels(const struct terminal *term)
 static void
 sixel_insert(struct terminal *term, struct sixel sixel)
 {
-    int end_row = rebase_row(term, sixel.pos.row + sixel.rows - 1);
+    int end_row = grid_row_abs_to_sb(
+        term->grid, term->rows, sixel.pos.row + sixel.rows - 1);
 
     tll_foreach(term->grid->sixel_images, it) {
-        if (rebase_row(term, it->item.pos.row + it->item.rows - 1) < end_row) {
+        int rebased = grid_row_abs_to_sb(
+            term->grid, term->rows, it->item.pos.row + it->item.rows - 1);
+
+        if (rebased < end_row) {
             tll_insert_before(term->grid->sixel_images, it, sixel);
             goto out;
         }
@@ -325,7 +313,7 @@ sixel_scroll_up(struct terminal *term, int rows)
     tll_rforeach(term->grid->sixel_images, it) {
         struct sixel *six = &it->item;
 
-        int six_start = rebase_row(term, six->pos.row);
+        int six_start = grid_row_abs_to_sb(term->grid, term->rows, six->pos.row);
 
         if (six_start < rows) {
             sixel_erase(term, six);
@@ -358,7 +346,8 @@ sixel_scroll_down(struct terminal *term, int rows)
     tll_foreach(term->grid->sixel_images, it) {
         struct sixel *six = &it->item;
 
-        int six_end = rebase_row(term, six->pos.row + six->rows - 1);
+        int six_end = grid_row_abs_to_sb(
+            term->grid, term->rows, six->pos.row + six->rows - 1);
         if (six_end >= term->grid->num_rows - rows) {
             sixel_erase(term, six);
             tll_remove(term->grid->sixel_images, it);
@@ -668,7 +657,8 @@ _sixel_overwrite_by_rectangle(
     /* We should never generate scrollback wrapping sixels */
     xassert(end < term->grid->num_rows);
 
-    const int scrollback_rel_start = rebase_row(term, start);
+    const int scrollback_rel_start = grid_row_abs_to_sb(
+        term->grid, term->rows, start);
 
     bool UNUSED would_have_breaked = false;
 
@@ -677,7 +667,8 @@ _sixel_overwrite_by_rectangle(
 
         const int six_start = six->pos.row;
         const int six_end = (six_start + six->rows - 1);
-        const int six_scrollback_rel_end = rebase_row(term, six_end);
+        const int six_scrollback_rel_end =
+            grid_row_abs_to_sb(term->grid, term->rows, six_end);
 
         /* We should never generate scrollback wrapping sixels */
         xassert(six_end < term->grid->num_rows);
@@ -776,7 +767,7 @@ sixel_overwrite_by_row(struct terminal *term, int _row, int col, int width)
         width = term->grid->num_cols - col;
 
     const int row = (term->grid->offset + _row) & (term->grid->num_rows - 1);
-    const int scrollback_rel_row = rebase_row(term, row);
+    const int scrollback_rel_row = grid_row_abs_to_sb(term->grid, term->rows, row);
 
     tll_foreach(term->grid->sixel_images, it) {
         struct sixel *six = &it->item;
@@ -786,7 +777,8 @@ sixel_overwrite_by_row(struct terminal *term, int _row, int col, int width)
         /* We should never generate scrollback wrapping sixels */
         xassert(six_end >= six_start);
 
-        const int six_scrollback_rel_end = rebase_row(term, six_end);
+        const int six_scrollback_rel_end =
+            grid_row_abs_to_sb(term->grid, term->rows, six_end);
 
         if (six_scrollback_rel_end < scrollback_rel_row) {
             /* All remaining sixels are *before* "our" row */
@@ -888,7 +880,8 @@ sixel_reflow(struct terminal *term)
             int last_row = -1;
 
             for (int j = 0; j < six->rows; j++) {
-                int row_no = rebase_row(term, six->pos.row + j);
+                int row_no = grid_row_abs_to_sb(
+                    term->grid, term->rows, six->pos.row + j);
                 if (last_row != -1 && last_row >= row_no) {
                     sixel_destroy(six);
                     sixel_destroyed = true;
diff --git a/terminal.c b/terminal.c
index f1b05f57..7d230cdf 100644
--- a/terminal.c
+++ b/terminal.c
@@ -2154,18 +2154,18 @@ term_erase(struct terminal *term, int start_row, int start_col,
 void
 term_erase_scrollback(struct terminal *term)
 {
-    const int num_rows = term->grid->num_rows;
+    const struct grid *grid = term->grid;
+    const int num_rows = grid->num_rows;
     const int mask = num_rows - 1;
 
-    const int start = (term->grid->offset + term->rows) & mask;
-    const int end = (term->grid->offset - 1) & mask;
+    const int start = (grid->offset + term->rows) & mask;
+    const int end = (grid->offset - 1) & mask;
 
-    const int scrollback_start = term->grid->offset + term->rows;
-    const int rel_start = (start - scrollback_start + num_rows) & mask;
-    const int rel_end = (end - scrollback_start + num_rows) & mask;
+    const int rel_start = grid_row_abs_to_sb(grid, term->rows, start);
+    const int rel_end = grid_row_abs_to_sb(grid, term->rows, end);
 
-    const int sel_start = term->selection.coords.start.row;
-    const int sel_end = term->selection.coords.end.row;
+    const int sel_start = selection_get_start(term).row;
+    const int sel_end = selection_get_end(term).row;
 
     if (sel_end >= 0) {
         /*
@@ -2183,8 +2183,8 @@ term_erase_scrollback(struct terminal *term)
          * closer to the screen bottom.
          */
 
-        const int rel_sel_start = (sel_start - scrollback_start + num_rows) & mask;
-        const int rel_sel_end = (sel_end - scrollback_start + num_rows) & mask;
+        const int rel_sel_start = grid_row_abs_to_sb(grid, term->rows, sel_start);
+        const int rel_sel_end = grid_row_abs_to_sb(grid, term->rows, sel_end);
 
         if ((rel_sel_start <= rel_start && rel_sel_end >= rel_start) ||
             (rel_sel_start <= rel_end && rel_sel_end >= rel_end) ||
@@ -2196,8 +2196,9 @@ term_erase_scrollback(struct terminal *term)
 
     tll_foreach(term->grid->sixel_images, it) {
         struct sixel *six = &it->item;
-        const int six_start = (six->pos.row - scrollback_start + num_rows) & mask;
-        const int six_end = (six->pos.row + six->rows - 1 - scrollback_start + num_rows) & mask;
+        const int six_start = grid_row_abs_to_sb(grid, term->rows, six->pos.row);
+        const int six_end = grid_row_abs_to_sb(
+            grid, term->rows, six->pos.row + six->rows - 1);
 
         if ((six_start <= rel_start && six_end >= rel_start) ||
             (six_start <= rel_end && six_end >= rel_end) ||

From 5c4ddebc3c57724bd89b959a33f5ad0bd97f482b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 25 Apr 2022 20:00:47 +0200
Subject: [PATCH 4/6] search: fix multiple crashes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* When extending the selection to the next word boundary, ensure the
  row numbers are valid:

  - use selection_get_end() when retrieving the current end
    coordinate. This alone fixes a crash where we previously would
    crash in an out-of-bounds array access in grid->rows[], due to
    term->selection.coords.end being unbounded.

  - ensure the new end coordinate is bounded before and after calling
    selection_find_word_boundary_right().

* When moving the viewport (to ensure a new match is visible), make
  sure we don’t end up with the match outside the new viewport.

  Under certain, unusual, circumstances, the moved viewport _still_
  did not contain the match. This resulted in assertions triggering
  later, that assumed the match(es) are *always* visible.

  It’s fairly easy to trigger this one by running foot with e.g.

    foot -o scrollback.lines=0 --window-size-chars 25x3

  and then hitting enter a couple of times, to fill the scrollback
  history (which should consist of a single row in the example above),
  and the do a scrollback search for (part of) the prompt, and keep
  searching backward until it crashes.

  This would happen if calculated (new) viewport had to be adjusted
  (for example, to ensure it didn’t go past the scrollback end).

  This patch changes the logic used when calculating the new
  viewport. Instead of calculating the wanted viewport (match is
  vertically centered) and then trying to adjust it to ensure the new
  viewport is valid, start with a “safe” new viewport value, and then
  determine how much we can move it, if at all, to center the match.

  This is done by using scrollback relative coordinates. In this
  coordinate system, the new viewport must be

    >= 0, and < grid->num_lines - term->rows

  This makes it very easy to limit the amount by which the viewport is
  adjusted.

  As a side-effect, we can remove all the old re-adjustment logic.

* The match iterator no longer special cases the primary match. This
  was needed before, when the search iterator did not handle
  overlapping matches correctly. Now that we do, the iterator is
  guaranteed to find the primary match, and thus we no longer need to
  special case it.

  This fixes a bug where the primary match was returned twice, due to
  the logic checking if a secondary match is the same as the primary
  match was flawed...

Closes #1036
---
 search.c | 194 ++++++++++++++++++++++---------------------------------
 1 file changed, 76 insertions(+), 118 deletions(-)

diff --git a/search.c b/search.c
index 73f831eb..09b59d50 100644
--- a/search.c
+++ b/search.c
@@ -82,11 +82,7 @@ search_ensure_size(struct terminal *term, size_t wanted_size)
 static bool
 has_wrapped_around(const struct terminal *term, int abs_row_no)
 {
-    const struct grid *grid = term->grid;
-    int scrollback_start = grid->offset + term->rows;
-    int rebased_row = abs_row_no - scrollback_start + grid->num_rows;
-    rebased_row &= grid->num_rows - 1;
-
+    int rebased_row = grid_row_abs_to_sb(term->grid, term->rows, abs_row_no);
     return rebased_row == 0;
 }
 
@@ -182,6 +178,9 @@ search_update_selection(struct terminal *term, const struct range *match)
     int end_row = match->end.row;
     int end_col = match->end.col;
 
+    xassert(start_row >= 0);
+    xassert(start_row < grid->num_rows);
+
     bool move_viewport = true;
 
     int view_end = (grid->view + term->rows - 1) & (grid->num_rows - 1);
@@ -196,24 +195,14 @@ search_update_selection(struct terminal *term, const struct range *match)
     }
 
     if (move_viewport) {
-        int old_view = grid->view;
-        int new_view = start_row - term->rows / 2;
+        int rebased_new_view = grid_row_abs_to_sb(grid, term->rows, start_row);
 
-        while (new_view < 0)
-            new_view += grid->num_rows;
+        rebased_new_view -= term->rows / 2;
+        rebased_new_view =
+            min(max(rebased_new_view, 0), grid->num_rows - term->rows);
 
-        new_view = ensure_view_is_allocated(term, new_view);
-
-        /* Don't scroll past scrollback history */
-        int end = (grid->offset + term->rows - 1) & (grid->num_rows - 1);
-        if (end >= grid->offset) {
-            /* Not wrapped */
-            if (new_view >= grid->offset && new_view <= end)
-                new_view = grid->offset;
-        } else {
-            if (new_view >= grid->offset || new_view <= end)
-                new_view = grid->offset;
-        }
+        const int old_view = grid->view;
+        int new_view = grid_row_sb_to_abs(grid, term->rows, rebased_new_view);
 
 #if defined(_DEBUG)
         /* Verify all to-be-visible rows have been allocated */
@@ -227,23 +216,8 @@ search_update_selection(struct terminal *term, const struct range *match)
             term_damage_view(term);
     }
 
-#if 0
-    /* Selection endpoint is inclusive */
-    if (--end_col < 0) {
-        end_col = term->cols - 1;
-        end_row--;
-    }
-#endif
-
-    /*
-     * Begin a new selection if the start coords changed
-     *
-     * Note: check column against selection.coords, since our “old”
-     * start column isn’t reliable - we modify it when searching
-     * “next” or “prev”.
-     */
     if (start_row != term->search.match.row ||
-        start_col != term->selection.coords.start.col)
+        start_col != term->search.match.col)
     {
         int selection_row = start_row - grid->view + grid->num_rows;
         selection_row &= grid->num_rows - 1;
@@ -516,7 +490,7 @@ search_matches_new_iter(struct terminal *term)
 {
     return (struct search_match_iterator){
         .term = term,
-        .start = {-2, -2},
+        .start = {0, 0},
     };
 }
 
@@ -529,86 +503,63 @@ search_matches_next(struct search_match_iterator *iter)
     if (term->search.match_len == 0)
         goto no_match;
 
-    struct range match;
-    bool found;
-
-    const bool return_primary_match =
-        iter->start.row == -2 && term->selection.coords.end.row >= 0;
-
-    if (return_primary_match) {
-        /* First, return the primary match */
-        match = term->selection.coords;
-        found = true;
-    }
-
-    else if (iter->start.row >= term->rows) {
+    if (iter->start.row >= term->rows)
         goto no_match;
+
+    xassert(iter->start.row >= 0);
+    xassert(iter->start.row < term->rows);
+    xassert(iter->start.col >= 0);
+    xassert(iter->start.col < term->cols);
+
+    struct coord abs_start = iter->start;
+    abs_start.row = grid_row_absolute_in_view(grid, abs_start.row);
+
+    struct coord abs_end = {
+        term->cols - 1,
+        grid_row_absolute_in_view(grid, term->rows - 1)};
+
+    struct range match;
+    bool found = find_next(term, SEARCH_FORWARD, abs_start, abs_end, &match);
+    if (!found)
+        goto no_match;
+
+    LOG_DBG("match at (absolute coordinates) %dx%d-%dx%d",
+            match.start.row, match.start.col,
+            match.end.row, match.end.col);
+
+    /* Convert absolute row numbers back to view relative */
+    match.start.row = match.start.row - grid->view + grid->num_rows;
+    match.start.row &= grid->num_rows - 1;
+    match.end.row = match.end.row - grid->view + grid->num_rows;
+    match.end.row &= grid->num_rows - 1;
+
+    LOG_DBG("match at (view-local coordinates) %dx%d-%dx%d, view=%d",
+            match.start.row, match.start.col,
+            match.end.row, match.end.col, grid->view);
+
+    xassert(match.start.row >= 0);
+    xassert(match.start.row < term->rows);
+    xassert(match.end.row >= 0);
+    xassert(match.end.row < term->rows);
+
+    xassert(match.end.row > match.start.row ||
+            (match.end.row == match.start.row &&
+             match.end.col >= match.start.col));
+
+    /* Continue at next column, next time */
+    iter->start.row = match.start.row;
+    iter->start.col = match.start.col + 1;
+
+    if (iter->start.col >= term->cols) {
+        iter->start.col = 0;
+        iter->start.row++;  /* Overflow is caught in next iteration */
     }
 
-    else {
-        xassert(iter->start.row >= 0);
-        xassert(iter->start.row < term->rows);
-        xassert(iter->start.col >= 0);
-        xassert(iter->start.col < term->cols);
-
-        struct coord abs_start = iter->start;
-        abs_start.row = grid_row_absolute_in_view(grid, abs_start.row);
-
-        struct coord abs_end = {
-            term->cols - 1,
-            grid_row_absolute_in_view(grid, term->rows - 1)};
-
-        found = find_next(term, SEARCH_FORWARD, abs_start, abs_end, &match);
-    }
-
-    if (found) {
-        LOG_DBG("match at %dx%d-%dx%d",
-                match.start.row, match.start.col,
-                match.end.row, match.end.col);
-
-        /* Convert absolute row numbers back to view relative */
-        match.start.row = match.start.row - grid->view + grid->num_rows;
-        match.start.row &= grid->num_rows - 1;
-        match.end.row = match.end.row - grid->view + grid->num_rows;
-        match.end.row &= grid->num_rows - 1;
-
-        xassert(match.start.row >= 0);
-        xassert(match.start.row < term->rows);
-        xassert(match.end.row >= 0);
-        xassert(match.end.row < term->rows);
-
-        xassert(match.end.row > match.start.row ||
-                (match.end.row == match.start.row &&
-                 match.end.col >= match.start.col));
-
-        if (return_primary_match) {
-            iter->start.row = 0;
-            iter->start.col = 0;
-        } else {
-            /* Continue at next column, next time */
-            iter->start.row = match.start.row;
-            iter->start.col = match.start.col + 1;
-
-            if (iter->start.col >= term->cols) {
-                iter->start.col = 0;
-                iter->start.row++;  /* Overflow is caught in next iteration */
-            }
-
-            xassert(iter->start.row >= 0);
-            xassert(iter->start.row <= term->rows);
-            xassert(iter->start.col >= 0);
-            xassert(iter->start.col < term->cols);
-
-            if (match.start.row == term->search.match.row &&
-                match.start.col == term->search.match.col)
-            {
-                /* Primary match is handled explicitly */
-                LOG_DBG("primary match: skipping");
-                return search_matches_next(iter);
-            }
-        }
-        return match;
-    }
+    xassert(iter->start.row >= 0);
+    xassert(iter->start.row <= term->rows);
+    xassert(iter->start.col >= 0);
+    xassert(iter->start.col < term->cols);
+    return match;
 
 no_match:
     iter->start.row = -1;
@@ -663,15 +614,18 @@ search_match_to_end_of_word(struct terminal *term, bool spaces_only)
     if (term->search.match_len == 0)
         return;
 
-    xassert(term->selection.coords.end.row != -1);
+    xassert(term->selection.coords.end.row >= 0);
 
     struct grid *grid = term->grid;
     const bool move_cursor = term->search.cursor == term->search.len;
 
-    const struct coord old_end = term->selection.coords.end;
+    struct coord old_end = selection_get_end(term);
     struct coord new_end = old_end;
     struct row *row = NULL;
 
+    xassert(new_end.row >= 0);
+    xassert(new_end.row < grid->num_rows);
+
     /* Advances a coordinate by one column, to the right. Returns
      * false if we’ve reached the scrollback wrap-around */
 #define advance_pos(coord) __extension__                                \
@@ -691,12 +645,16 @@ search_match_to_end_of_word(struct terminal *term, bool spaces_only)
     if (!advance_pos(new_end))
         return;
 
+    xassert(new_end.row >= 0);
+    xassert(new_end.row < grid->num_rows);
     xassert(grid->rows[new_end.row] != NULL);
 
     /* Find next word boundary */
-    new_end.row -= grid->view;
+    new_end.row -= grid->view + grid->num_rows;
+    new_end.row &= grid->num_rows - 1;
     selection_find_word_boundary_right(term, &new_end, spaces_only);
     new_end.row += grid->view;
+    new_end.row &= grid->num_rows - 1;
 
     struct coord pos = old_end;
     row = grid->rows[pos.row];

From 1b5b1d5d925f296619c511a5b12a10f48717efce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Tue, 26 Apr 2022 17:40:00 +0200
Subject: [PATCH 5/6] changelog: crash when extending selection in search mode

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a7ad4371..a03b4c82 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -50,6 +50,8 @@
   box, that does not result in a grid update (for example, when the
   search criteria did not result in any matches) ([#1040][1040]).
 * foot freezing in scrollback search mode, using 100% CPU ([#1036][1036]).
+* Crash when extending a selection to the next word boundary in
+  scrollback search mode ([#1036][1036]).
 
 [1040]: https://codeberg.org/dnkl/foot/issues/1040
 [1036]: https://codeberg.org/dnkl/foot/issues/1036

From b94f540113dd10e4b34f5dda826f732b29b47217 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Tue, 26 Apr 2022 17:40:20 +0200
Subject: [PATCH 6/6] changelog: search mode not always highlighting all
 matches correctly

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a03b4c82..89c19a0e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -52,6 +52,8 @@
 * foot freezing in scrollback search mode, using 100% CPU ([#1036][1036]).
 * Crash when extending a selection to the next word boundary in
   scrollback search mode ([#1036][1036]).
+* Scrollback search mode not always highlighting all matches
+  correctly.
 
 [1040]: https://codeberg.org/dnkl/foot/issues/1040
 [1036]: https://codeberg.org/dnkl/foot/issues/1036