Re: "git am" crash (builtin/apply.c:2108) + small repro

Junio C Hamano [off-list ref] writes:

quoted

I've never been a fan of update_pre_post_images().  I think I know
what is going on (--whitespace=fix).  Will post a fix sometime
later.

It bisects down to 5166714 (apply: Allow blank context lines to
match beyond EOF, 2010-03-06).

I do not like this patch at all (I _think_ it should somehow check
the corresponding postimage is sane when the "ah, we ran out of
preimage because the caller fixed it to reduce trailing blank lines"
case), but at least this should get the ball rolling.

Björn, both of the changes involved are yours, so I am thinking that
you may be a good person to give a sanity check on this.  No need to
hurry, as this is not a recent regression and we are in a pre-release
freeze.

Thanks.

-- >8 --
Subject: apply.c:update_pre_post_images(): the preimage can be truncated

5166714 (apply: Allow blank context lines to match beyond EOF,
2010-03-06) and then later 0c3ef98 (apply: Allow blank *trailing*
context lines to match beyond EOF, 2010-04-08) taught "git apply"
to trim new blank lines at the end in the patch text when matching
the contents being patched and the preimage recorded in the patch,
under --whitespace=fix mode.

When a preimage is modified to match the current contents in
preparation for such a "fixed" patch application, the context lines
in the postimage must be updated to match (otherwise, it would
reintroduce whitespace breakages), and update_pre_post_images()
function is responsible for doing this.  However, this function was
not updated to take into account a case where the removal of
trailing blank lines reduces the number of lines in the preimage,
and triggered an assertion error.

The logic to fix the postimage by copying the corrected context
lines from the preimage was not prepared to handle this case,
either, but it was protected by the assert() and only got exposed
when the assertion is corrected.

Signed-off-by: Junio C Hamano <redacted>
---
 builtin/apply.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git c/builtin/apply.c w/builtin/apply.c
index 156b3ce..6c11e8b 100644
--- c/builtin/apply.c
+++ w/builtin/apply.c

@@ -2095,7 +2095,7 @@ static void update_pre_post_images(struct image *preimage,
                                   char *buf,
                                   size_t len, size_t postlen)
 {
-       int i, ctx;
+       int i, ctx, reduced;
        char *new, *old, *fixed;
        struct image fixed_preimage;

@@ -2105,8 +2105,10 @@ static void update_pre_post_images(struct image *preimage,
         * free "oldlines".
         */
        prepare_image(&fixed_preimage, buf, len, 1);
-       assert(fixed_preimage.nr == preimage->nr);
-       for (i = 0; i < preimage->nr; i++)
+       assert(postlen
+              ? fixed_preimage.nr == preimage->nr
+              : fixed_preimage.nr <= preimage->nr);
+       for (i = 0; i < fixed_preimage.nr; i++)
                fixed_preimage.line[i].flag = preimage->line[i].flag;
        free(preimage->line_allocated);
        *preimage = fixed_preimage;

@@ -2126,7 +2128,8 @@ static void update_pre_post_images(struct image *preimage,
        else
                new = old;
        fixed = preimage->buf;
-       for (i = ctx = 0; i < postimage->nr; i++) {
+
+       for (i = reduced = ctx = 0; i < postimage->nr; i++) {
                size_t len = postimage->line[i].len;
                if (!(postimage->line[i].flag & LINE_COMMON)) {
                        /* an added line -- no counterparts in preimage */

@@ -2145,8 +2148,15 @@ static void update_pre_post_images(struct image *preimage,
                        fixed += preimage->line[ctx].len;
                        ctx++;
                }
-               if (preimage->nr <= ctx)
-                       die(_("oops"));
+
+               /*
+                * preimage is expected to run out, if the caller
+                * fixed addition of trailing blank lines.
+                */
+               if (preimage->nr <= ctx) {
+                       reduced++;
+                       continue;
+               }

                /* and copy it in, while fixing the line length */
                len = preimage->line[ctx].len;

@@ -2159,6 +2169,7 @@ static void update_pre_post_images(struct image *preimage,

        /* Fix the length of the whole thing */
        postimage->len = new - postimage->buf;
+       postimage->nr -= reduced;
 }

 static int match_fragment(struct image *img,