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

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

[Alexey Spiridonov]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

This reproduces in trunk, 1.7.8.4, and 1.7.9.5.

I suspect this has to do with a whitespace + no trailing newline
issues. The patch was generated by 1.7.9.5. I mangled it by hand to
get it to be small, but the initial crash happened on a large,
real-world output of "git format-patch".

Error messages:

~/GIT-AM-CRASH$ ../git/git am crashy.patch
Applying: Git crash bug
git: builtin/apply.c:2108: update_pre_post_images: Assertion
`fixed_preimage.nr == preimage->nr' failed.
/home/lesha/GIT-AM-CRASH/../git/git-am: line 811: 23819 Aborted
         git apply --index "$dotest/patch"
Patch failed at 0001 Git crash bug
The copy of the patch that failed is found in:
   /home/lesha/GIT-AM-CRASH/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Repro steps:

mkdir GIT-AM-CRASH
cd GIT-AM-CRASH
# download files into this directory
git init .
mkdir -p meep/spork
mv __init__.py meep/spork
git add meep/spork/__init__.py
git ci -am 'moo'
git am crashy.patch

Hope this helps!

Alexey

[-- Attachment #2: crashy.patch --]
[-- Type: application/octet-stream, Size: 504 bytes --]

From ab376a1d76b1605443b4d7eb9197be333b037f16 Mon Sep 17 00:00:00 2001
From: lesha <snarkmaster@gmail.com>
Date: Wed, 26 Sep 2012 10:58:43 -0700
Subject: Git crash bug

---

diff --git a/meep/spork/__init__.py b/meep/spork/__init__.py
index 9d869c9..ef120c7 100644
--- a/meep/spork/__init__.py
+++ b/meep/spork/__init__.py
@@ -1,7 +1,9 @@
 from __future__ import absolute_import
 
+from bar import Boo
+
 from .baz import Bork
 
-Boo.mooFooCow(Bork)
+Boo.mooCow(Bork)
   
   
\ No newline at end of file

[-- Attachment #3: __init__.py --]
[-- Type: application/octet-stream, Size: 84 bytes --]

from __future__ import absolute_import

from .baz import Bork

Boo.mooFooCow(Bork)

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

[Nguyen Thai Ngoc Duy]

On Tue, Oct 2, 2012 at 1:33 AM, Alexey Spiridonov <snarkmaster@gmail.com> wrote:
> This reproduces in trunk, 1.7.8.4, and 1.7.9.5.

fwiw, I cannot reproduce it (git-apply does not crash). I tried both
versions and 1.8.0-rc1. Just in case the attached files are somehow
corrupted, this is sha1sum from my side:

3d4711cd15d9617e0d3a52bbcd7def898c12c328  crashy.patch
fd63cc32338823f216a6684ce5118a69113968c8  meep/spork/__init__.py

> I suspect this has to do with a whitespace + no trailing newline
> issues. The patch was generated by 1.7.9.5. I mangled it by hand to
> get it to be small, but the initial crash happened on a large,
> real-world output of "git format-patch".
>
> Error messages:
>
> ~/GIT-AM-CRASH$ ../git/git am crashy.patch
> Applying: Git crash bug
> git: builtin/apply.c:2108: update_pre_post_images: Assertion
> `fixed_preimage.nr == preimage->nr' failed.
> /home/lesha/GIT-AM-CRASH/../git/git-am: line 811: 23819 Aborted
>          git apply --index "$dotest/patch"
> Patch failed at 0001 Git crash bug
> The copy of the patch that failed is found in:
>    /home/lesha/GIT-AM-CRASH/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Repro steps:
>
> mkdir GIT-AM-CRASH
> cd GIT-AM-CRASH
> # download files into this directory
> git init .
> mkdir -p meep/spork
> mv __init__.py meep/spork
> git add meep/spork/__init__.py
> git ci -am 'moo'
> git am crashy.patch
>
> Hope this helps!
>
> Alexey
-- 
Duy

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

[Alexey Spiridonov]

Thanks a lot for trying this.

My hashes match. I just re-reproduced it on two flavors of Linux (64
and 32-bit), with two different Git versions (see below). What
platform are you using?


lesha@buryonka-ubuntu32:~$ mkdir GIT-AM-CRASH
lesha@buryonka-ubuntu32:~$ cd GIT-AM-CRASH
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ mv ../crashy.patch ../__init__.py .
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ git init .
Initialized empty Git repository in /home/lesha/GIT-AM-CRASH/.git/
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ mkdir -p meep/spork
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ mv __init__.py meep/spork
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ git add meep/spork/__init__.py
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ git ci -am 'moo'
[master (root-commit) fa8f8fd] moo
 1 file changed, 6 insertions(+)
 create mode 100644 meep/spork/__init__.py
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ git am crashy.patch
Applying: Git crash bug
git: builtin/apply.c:1990: update_pre_post_images: Assertion
`fixed_preimage.nr == preimage->nr' failed.
Aborted (core dumped)
Patch failed at 0001 Git crash bug
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ sha1
sha1pass  sha1sum
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ sha1sum crashy.patch
meep/spork/__init__.py
3d4711cd15d9617e0d3a52bbcd7def898c12c328  crashy.patch
fd63cc32338823f216a6684ce5118a69113968c8  meep/spork/__init__.py
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ git --version
git version 1.7.9.5
lesha@buryonka-ubuntu32:~/GIT-AM-CRASH$ uname -a
Linux buryonka-ubuntu32 3.2.0-31-generic #50-Ubuntu SMP Fri Sep 7
16:17:36 UTC 2012 i686 i686 i386 GNU/Linux


[lesha@dev037 ~/GIT-AM-CRASH] git init .
Initialized empty Git repository in /home/lesha/GIT-AM-CRASH/.git/
[lesha@dev037 ~/GIT-AM-CRASH] mkdir -p meep/spork
[lesha@dev037 ~/GIT-AM-CRASH] mv __init__.py meep/spork
[lesha@dev037 ~/GIT-AM-CRASH] git add meep/spork/__init__.py
[lesha@dev037 ~/GIT-AM-CRASH] git ci -am 'moo'
[master (root-commit) 4c3fe5f] moo
 1 files changed, 6 insertions(+), 0 deletions(-)
 create mode 100644 meep/spork/__init__.py
[lesha@dev037 ~/GIT-AM-CRASH] git am crashy.patch
Applying: Git crash bug
git: builtin/apply.c:1989: update_pre_post_images: Assertion
`fixed_preimage.nr == preimage->nr' failed.
/usr/libexec/git-core/git-am: line 789: 32074 Aborted
git apply --index "$dotest/patch"
Patch failed at 0001 Git crash bug
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
[lesha@dev037 ~/GIT-AM-CRASH] git --version
git version 1.7.8.4
[lesha@dev037 ~/GIT-AM-CRASH] uname -a
Linux dev037 2.6.38.4 #63 SMP Mon Feb 13 16:22:45 PST 2012 x86_64
x86_64 x86_64 GNU/Linux
[lesha@dev037 ~/GIT-AM-CRASH] sha1sum crashy.patch meep/spork/__init__.py
3d4711cd15d9617e0d3a52bbcd7def898c12c328  crashy.patch
fd63cc32338823f216a6684ce5118a69113968c8  meep/spork/__init__.py

-a

On Wed, Oct 3, 2012 at 4:27 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Tue, Oct 2, 2012 at 1:33 AM, Alexey Spiridonov <snarkmaster@gmail.com> wrote:
>> This reproduces in trunk, 1.7.8.4, and 1.7.9.5.
>
> fwiw, I cannot reproduce it (git-apply does not crash). I tried both
> versions and 1.8.0-rc1. Just in case the attached files are somehow
> corrupted, this is sha1sum from my side:
>
> 3d4711cd15d9617e0d3a52bbcd7def898c12c328  crashy.patch
> fd63cc32338823f216a6684ce5118a69113968c8  meep/spork/__init__.py
>
>> I suspect this has to do with a whitespace + no trailing newline
>> issues. The patch was generated by 1.7.9.5. I mangled it by hand to
>> get it to be small, but the initial crash happened on a large,
>> real-world output of "git format-patch".
>>
>> Error messages:
>>
>> ~/GIT-AM-CRASH$ ../git/git am crashy.patch
>> Applying: Git crash bug
>> git: builtin/apply.c:2108: update_pre_post_images: Assertion
>> `fixed_preimage.nr == preimage->nr' failed.
>> /home/lesha/GIT-AM-CRASH/../git/git-am: line 811: 23819 Aborted
>>          git apply --index "$dotest/patch"
>> Patch failed at 0001 Git crash bug
>> The copy of the patch that failed is found in:
>>    /home/lesha/GIT-AM-CRASH/.git/rebase-apply/patch
>> When you have resolved this problem, run "git am --resolved".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>>
>> Repro steps:
>>
>> mkdir GIT-AM-CRASH
>> cd GIT-AM-CRASH
>> # download files into this directory
>> git init .
>> mkdir -p meep/spork
>> mv __init__.py meep/spork
>> git add meep/spork/__init__.py
>> git ci -am 'moo'
>> git am crashy.patch
>>
>> Hope this helps!
>>
>> Alexey
> --
> Duy

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

[Nguyen Thai Ngoc Duy]

On Wed, Oct 3, 2012 at 10:44 PM, Alexey Spiridonov
<snarkmaster@gmail.com> wrote:
> Thanks a lot for trying this.
>
> My hashes match. I just re-reproduced it on two flavors of Linux (64
> and 32-bit), with two different Git versions (see below). What
> platform are you using?

x86, 32 bit. Perhaps it has something to do with your configuration
(config files, hooks, attributes). Assuming you use standard
repository templates, you create new repo in your test so hooks and
attributes are out. Is there anything suspicuous in "git config -l"?
-- 
Duy

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

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Wed, Oct 3, 2012 at 10:44 PM, Alexey Spiridonov
> <snarkmaster@gmail.com> wrote:
>> Thanks a lot for trying this.
>>
>> My hashes match. I just re-reproduced it on two flavors of Linux (64
>> and 32-bit), with two different Git versions (see below). What
>> platform are you using?
>
> x86, 32 bit. Perhaps it has something to do with your configuration
> (config files, hooks, attributes). Assuming you use standard
> repository templates, you create new repo in your test so hooks and
> attributes are out. Is there anything suspicuous in "git config -l"?

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.

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

[Alexey Spiridonov]

Thanks for looking into this, guys!

I seem to run into this with some regularity, but my setting is
apply.whitespace=strip rather than 'fix'.

Is there an obvious workaround?

Here are my remaining settings, sanitized for file paths and URLs:

svn.rmdir=true
push.default=upstream
color.ui=auto
color.diff=auto
color.status=auto
color.branch=auto
color.interactive=auto
color.branch.current=white blue bold
color.branch.local=blue
color.branch.remote=green
color.diff.plain=white
color.diff.meta=yellow bold
color.diff.frag=magenta bold
color.diff.old=red
color.diff.new=green
color.diff.whitespace=red blink
color.status.added=yellow
color.status.changed=green
color.status.untracked=cyan
svn.followparent=true
log.date=relative
blame.date=short
diff.renames=copies
diff.copies=true
diff.mnemonicprefix=true
apply.whitespace=strip
merge.tool=emerge
status.relativepaths=true
web.browser=lynx
rebase.stat=true
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
rerere.enabled=true
branch.autosetuprebase=always

-a

On Fri, Oct 12, 2012 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> 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 <gitster@pobox.com>
> ---
>  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,

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

[Junio C Hamano]

Alexey Spiridonov <snarkmaster@gmail.com> writes:

> Thanks for looking into this, guys!
>
> I seem to run into this with some regularity, but my setting is
> apply.whitespace=strip rather than 'fix'.

'strip' is an old synonym for 'fix'.