Memory corruption when rebasing with git version 1.8.1.5 on arch

Memory corruption when rebasing with git version 1.8.1.5 on arch

[Bernhard Posselt]

Hi im running arch linux and core/glibc 2.17-3

When i try to rebase with:

git clonehttps://github.com/owncloud/core.git
cd core/
git pull --rebasehttps://github.com/PatrickHeller/core.git  master

I'm getting:

$ git pull --rebasehttps://github.com/PatrickHeller/core.git  master
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 3 (delta 2), reused 3 (delta 2)
Unpacking objects: 100% (3/3), done.
  Fromhttps://github.com/PatrickHeller/core
   * branch            master     -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: distinguish between touch and write
Applying: remove debug output
*** Error in `git': malloc(): memory corruption: 0x0000000000be14e0 ***


Using valgrind gives me:

$ valgrind /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git  master
==5995== Memcheck, a memory error detector
==5995== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5995== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5995== Command: /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git  master
==5995==
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 3 (delta 2), reused 3 (delta 2)
Unpacking objects: 100% (3/3), done.
  Fromhttps://github.com/PatrickHeller/core
   * branch            master     -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: distinguish between touch and write
Applying: remove debug output
*** Error in `git': malloc(): memory corruption: 0x00000000027f14e0 ***

^C==5995==
==5995== HEAP SUMMARY:
==5995==     in use at exit: 1,076 bytes in 11 blocks
==5995==   total heap usage: 53 allocs, 42 frees, 11,038 bytes allocated
==5995==
==5995== LEAK SUMMARY:
==5995==    definitely lost: 0 bytes in 0 blocks
==5995==    indirectly lost: 0 bytes in 0 blocks
==5995==      possibly lost: 0 bytes in 0 blocks

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King]

On Fri, Mar 08, 2013 at 01:19:57PM +0100, Bernhard Posselt wrote:

> Using valgrind gives me:
> 
> $ valgrind /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git  master
> ==5995== Memcheck, a memory error detector
> ==5995== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
> ==5995== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
> ==5995== Command: /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git  master
> ==5995==
> remote: Counting objects: 5, done.
> remote: Compressing objects: 100% (1/1), done.
> remote: Total 3 (delta 2), reused 3 (delta 2)
> Unpacking objects: 100% (3/3), done.
>  Fromhttps://github.com/PatrickHeller/core
>   * branch            master     -> FETCH_HEAD
> First, rewinding head to replay your work on top of it...
> Applying: distinguish between touch and write
> Applying: remove debug output
> *** Error in `git': malloc(): memory corruption: 0x00000000027f14e0 ***

The problem is likely happening in a sub-command of git-pull, so
valgrind isn't reporting it. Can you try re-running with
"valgrind --trace-children=yes", or alternatively narrow down the
problematic command by setting GIT_TRACE=1 in the environment?

-Peff

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Bernhard Posselt]

On 03/08/2013 10:28 PM, Jeff King wrote:
> On Fri, Mar 08, 2013 at 01:19:57PM +0100, Bernhard Posselt wrote:
>
>> Using valgrind gives me:
>>
>> $ valgrind /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git  master
>> ==5995== Memcheck, a memory error detector
>> ==5995== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
>> ==5995== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
>> ==5995== Command: /usr/bin/git pull --rebasehttps://github.com/PatrickHeller/core.git  master
>> ==5995==
>> remote: Counting objects: 5, done.
>> remote: Compressing objects: 100% (1/1), done.
>> remote: Total 3 (delta 2), reused 3 (delta 2)
>> Unpacking objects: 100% (3/3), done.
>>   Fromhttps://github.com/PatrickHeller/core
>>    * branch            master     -> FETCH_HEAD
>> First, rewinding head to replay your work on top of it...
>> Applying: distinguish between touch and write
>> Applying: remove debug output
>> *** Error in `git': malloc(): memory corruption: 0x00000000027f14e0 ***
> The problem is likely happening in a sub-command of git-pull, so
> valgrind isn't reporting it. Can you try re-running with
> "valgrind --trace-children=yes", or alternatively narrow down the
> problematic command by setting GIT_TRACE=1 in the environment?
>
> -Peff
Thanks for the reply!

Heres the output with GIT_TRACE=1, the valgrind log has 4000 lines. If 
you should still require the valgrind log, please tell me.

$ git pull --rebase https://github.com/PatrickHeller/core.git master
trace: exec: 'git-pull' '--rebase' 
'https://github.com/PatrickHeller/core.git' 'master'
trace: run_command: 'git-pull' '--rebase' 
'https://github.com/PatrickHeller/core.git' 'master'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--is-bare-repository'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'ls-files' '-u'
trace: built-in: git 'symbolic-ref' '-q' 'HEAD'
trace: built-in: git 'config' '--bool' 'branch.master.rebase'
trace: built-in: git 'config' '--bool' 'pull.rebase'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'rev-parse' '--verify' 'HEAD'
trace: built-in: git 'update-index' '-q' '--ignore-submodules' '--refresh'
trace: built-in: git 'diff-files' '--quiet' '--ignore-submodules'
trace: built-in: git 'diff-index' '--cached' '--quiet' 
'--ignore-submodules' 'HEAD' '--'
trace: built-in: git 'rev-parse' '-q' '--git-dir'
trace: built-in: git 'rev-parse' '-q' '--verify' 
'refs/remotes/https://github.com/PatrickHeller/core.git/master'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'fetch' '--update-head-ok' 
'https://github.com/PatrickHeller/core.git' 'master'
trace: run_command: 'git-remote-https' 
'https://github.com/PatrickHeller/core.git' 
'https://github.com/PatrickHeller/core.git'
trace: run_command: 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: run_command: 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/PatrickHeller/core.git/'
trace: exec: 'git' 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/PatrickHeller/core.git/'
trace: built-in: git 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/PatrickHeller/core.git/'
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (1/1), done.
remote: Total 3 (delta 2), reused 3 (delta 2)
trace: run_command: 'unpack-objects' '--pack_header=2,3'
trace: exec: 'git' 'unpack-objects' '--pack_header=2,3'
trace: built-in: git 'unpack-objects' '--pack_header=2,3'
Unpacking objects: 100% (3/3), done.
trace: run_command: 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: exec: 'git' 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: built-in: git 'rev-list' '--objects' '--stdin' '--not' '--all'
 From https://github.com/PatrickHeller/core
  * branch            master     -> FETCH_HEAD
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'show-branch' '--merge-base' 'refs/heads/master' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'fmt-merge-msg'
trace: built-in: git 'rev-parse' '--parseopt' '--' '--onto' 
'd686039828089d53fb42e42046d7a9a3992a0507' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--is-bare-repository'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'config' '--bool' 'rebase.stat'
trace: built-in: git 'config' '--bool' 'rebase.autosquash'
trace: built-in: git 'rev-parse' '--verify' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'rev-parse' '--verify' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'symbolic-ref' '-q' 'HEAD'
trace: built-in: git 'rev-parse' '--verify' 'master^0'
trace: built-in: git 'rev-parse' '--verify' 'HEAD'
trace: built-in: git 'update-index' '-q' '--ignore-submodules' '--refresh'
trace: built-in: git 'diff-files' '--quiet' '--ignore-submodules'
trace: built-in: git 'diff-index' '--cached' '--quiet' 
'--ignore-submodules' 'HEAD' '--'
trace: built-in: git 'merge-base' 
'd686039828089d53fb42e42046d7a9a3992a0507' 
'0525bbd73c9015499ba92d1ac654b980aaca35b2'
First, rewinding head to replay your work on top of it...
trace: built-in: git 'checkout' '-q' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'update-ref' 'ORIG_HEAD' 
'0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: exec: 'git-am' '--rebasing' '--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: run_command: 'git-am' '--rebasing' '--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: built-in: git 'format-patch' '-k' '--stdout' '--full-index' 
'--ignore-if-in-upstream' '--src-prefix=a/' '--dst-prefix=b/' 
'--no-renames' 
'd686039828089d53fb42e42046d7a9a3992a0507..0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: built-in: git 'rev-parse' '--parseopt' '--' '--rebasing' 
'--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--show-prefix'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'var' 'GIT_COMMITTER_IDENT'
trace: built-in: git 'rev-parse' '--verify' '-q' 'HEAD'
trace: built-in: git 'config' '--bool' '--get' 'am.keepcr'
trace: built-in: git 'mailsplit' '-d4' 
'-o/srv/http/owncloud/.git/rebase-apply' '-b' '--'
trace: built-in: git 'update-index' '-q' '--refresh'
trace: built-in: git 'diff-index' '--cached' '--name-only' 'HEAD' '--'
trace: built-in: git 'cat-file' '-t' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'cat-file' 'commit' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'config' 'i18n.commitencoding'
trace: built-in: git 'show' '-s' '--pretty=raw' '--encoding=UTF-8' 
'48bb53030c657e1133da47765c7c778a069af665' '--'
trace: built-in: git 'diff-tree' '--root' '--binary' '--full-index' 
'48bb53030c657e1133da47765c7c778a069af665'
Applying: distinguish between touch and write
trace: built-in: git 'write-tree'
trace: built-in: git 'rev-parse' '--verify' '-q' 'HEAD'
trace: built-in: git 'commit-tree' 
'd785d568d8b4649dfdcc01e03d6c8e87b036ea5a' '-p' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'update-ref' '-m' 'pull --rebase 
https://github.com/PatrickHeller/core.git master: distinguish between 
touch and write' 'HEAD' '2726978d8cbd9b0a3367fd3d62b64b4c78438e79'
trace: built-in: git 'cat-file' '-t' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'cat-file' 'commit' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'config' 'i18n.commitencoding'
trace: built-in: git 'show' '-s' '--pretty=raw' '--encoding=UTF-8' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24' '--'
trace: built-in: git 'diff-tree' '--root' '--binary' '--full-index' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
Applying: remove debug output
*** Error in `git': malloc(): memory corruption: 0x000000000139f4e0 ***

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King]

On Sat, Mar 09, 2013 at 01:08:32AM +0100, Bernhard Posselt wrote:

> >The problem is likely happening in a sub-command of git-pull, so
> >valgrind isn't reporting it. Can you try re-running with
> >"valgrind --trace-children=yes", or alternatively narrow down the
> >problematic command by setting GIT_TRACE=1 in the environment?
>
> Heres the output with GIT_TRACE=1, the valgrind log has 4000 lines.
> If you should still require the valgrind log, please tell me.

Hmm, the GIT_TRACE output was less clear than I had hoped; it's unclear
to me which git program is actually dying (my guess is "git apply", and
we are squelching stderr, which is where the GIT_TRACE output is going).

Can you try it once again with something like GIT_TRACE=/tmp/foo.out,
which will make sure we record the trace directly, even if stderr ends
up redirected?

Also, I can almost reproduce here, as PatrickHeller/core.git is public.
However, I suspect the problem is particular to your work built on top,
which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
Is it possible for you to make that commit available on a temporary
branch?

-Peff

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Bernhard Posselt]

On 03/09/2013 05:48 AM, Jeff King wrote:
> On Sat, Mar 09, 2013 at 01:08:32AM +0100, Bernhard Posselt wrote:
>
>>> The problem is likely happening in a sub-command of git-pull, so
>>> valgrind isn't reporting it. Can you try re-running with
>>> "valgrind --trace-children=yes", or alternatively narrow down the
>>> problematic command by setting GIT_TRACE=1 in the environment?
>> Heres the output with GIT_TRACE=1, the valgrind log has 4000 lines.
>> If you should still require the valgrind log, please tell me.
> Hmm, the GIT_TRACE output was less clear than I had hoped; it's unclear
> to me which git program is actually dying (my guess is "git apply", and
> we are squelching stderr, which is where the GIT_TRACE output is going).
>
> Can you try it once again with something like GIT_TRACE=/tmp/foo.out,
> which will make sure we record the trace directly, even if stderr ends
> up redirected?
>
> Also, I can almost reproduce here, as PatrickHeller/core.git is public.
> However, I suspect the problem is particular to your work built on top,
> which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
> Is it possible for you to make that commit available on a temporary
> branch?
>
> -Peff
 > commit available on a temporary branch?
What do you mean exactly by that?

I've made copies of both repositories on github.

Heres a copy of the basic repo: 
https://github.com/Raydiation/memorycorruption
Heres my clone of the repo that i pull from: 
https://github.com/Raydiation/core

Basically:

git clone https://github.com/Raydiation/memorycorruption
cd memorycorruption
git pull --rebase https://github.com/Raydiation/core

Heres the output of the GIT_TRACE file

trace: built-in: git 'branch'
trace: built-in: git 'branch' '--no-color'
trace: built-in: git 'status'
trace: exec: 'git-pull' '--rebase' 'https://github.com/Raydiation/core' 
'master'
trace: run_command: 'git-pull' '--rebase' 
'https://github.com/Raydiation/core' 'master'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--is-bare-repository'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'ls-files' '-u'
trace: built-in: git 'symbolic-ref' '-q' 'HEAD'
trace: built-in: git 'config' '--bool' 'branch.master.rebase'
trace: built-in: git 'config' '--bool' 'pull.rebase'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'rev-parse' '--verify' 'HEAD'
trace: built-in: git 'update-index' '-q' '--ignore-submodules' '--refresh'
trace: built-in: git 'diff-files' '--quiet' '--ignore-submodules'
trace: built-in: git 'diff-index' '--cached' '--quiet' 
'--ignore-submodules' 'HEAD' '--'
trace: built-in: git 'rev-parse' '-q' '--git-dir'
trace: built-in: git 'rev-parse' '-q' '--verify' 
'refs/remotes/https://github.com/Raydiation/core/master'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'fetch' '--update-head-ok' 
'https://github.com/Raydiation/core' 'master'
trace: run_command: 'git-remote-https' 
'https://github.com/Raydiation/core' 'https://github.com/Raydiation/core'
trace: run_command: 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: exec: 'git' 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: built-in: git 'rev-list' '--objects' '--stdin' '--not' '--all' 
'--quiet'
trace: run_command: 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/Raydiation/core/'
trace: exec: 'git' 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/Raydiation/core/'
trace: built-in: git 'fetch-pack' '--stateless-rpc' '--stdin' 
'--lock-pack' '--thin' 'https://github.com/Raydiation/core/'
trace: run_command: 'unpack-objects' '--pack_header=2,3'
trace: exec: 'git' 'unpack-objects' '--pack_header=2,3'
trace: built-in: git 'unpack-objects' '--pack_header=2,3'
trace: run_command: 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: exec: 'git' 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: built-in: git 'rev-list' '--objects' '--stdin' '--not' '--all'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD'
trace: built-in: git 'show-branch' '--merge-base' 'refs/heads/master' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'fmt-merge-msg'
trace: built-in: git 'rev-parse' '--parseopt' '--' '--onto' 
'd686039828089d53fb42e42046d7a9a3992a0507' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--is-bare-repository'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'config' '--bool' 'rebase.stat'
trace: built-in: git 'config' '--bool' 'rebase.autosquash'
trace: built-in: git 'rev-parse' '--verify' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'rev-parse' '--verify' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'symbolic-ref' '-q' 'HEAD'
trace: built-in: git 'rev-parse' '--verify' 'master^0'
trace: built-in: git 'rev-parse' '--verify' 'HEAD'
trace: built-in: git 'update-index' '-q' '--ignore-submodules' '--refresh'
trace: built-in: git 'diff-files' '--quiet' '--ignore-submodules'
trace: built-in: git 'diff-index' '--cached' '--quiet' 
'--ignore-submodules' 'HEAD' '--'
trace: built-in: git 'merge-base' 
'd686039828089d53fb42e42046d7a9a3992a0507' 
'0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: built-in: git 'checkout' '-q' 
'd686039828089d53fb42e42046d7a9a3992a0507^0'
trace: built-in: git 'update-ref' 'ORIG_HEAD' 
'0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: built-in: git 'format-patch' '-k' '--stdout' '--full-index' 
'--ignore-if-in-upstream' '--src-prefix=a/' '--dst-prefix=b/' 
'--no-renames' 
'd686039828089d53fb42e42046d7a9a3992a0507..0525bbd73c9015499ba92d1ac654b980aaca35b2'
trace: exec: 'git-am' '--rebasing' '--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: run_command: 'git-am' '--rebasing' '--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: built-in: git 'rev-parse' '--parseopt' '--' '--rebasing' 
'--resolvemsg=
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase 
--abort".
'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--show-prefix'
trace: built-in: git 'rev-parse' '--is-inside-work-tree'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'var' 'GIT_COMMITTER_IDENT'
trace: built-in: git 'rev-parse' '--verify' '-q' 'HEAD'
trace: built-in: git 'config' '--bool' '--get' 'am.keepcr'
trace: built-in: git 'mailsplit' '-d4' 
'-o/srv/http/owncloud/.git/rebase-apply' '-b' '--'
trace: built-in: git 'update-index' '-q' '--refresh'
trace: built-in: git 'diff-index' '--cached' '--name-only' 'HEAD' '--'
trace: built-in: git 'cat-file' '-t' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'cat-file' 'commit' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'config' 'i18n.commitencoding'
trace: built-in: git 'show' '-s' '--pretty=raw' '--encoding=UTF-8' 
'48bb53030c657e1133da47765c7c778a069af665' '--'
trace: built-in: git 'diff-tree' '--root' '--binary' '--full-index' 
'48bb53030c657e1133da47765c7c778a069af665'
trace: built-in: git 'apply' '--index' 
'/srv/http/owncloud/.git/rebase-apply/patch'
trace: built-in: git 'write-tree'
trace: built-in: git 'rev-parse' '--verify' '-q' 'HEAD'
trace: built-in: git 'commit-tree' 
'd785d568d8b4649dfdcc01e03d6c8e87b036ea5a' '-p' 
'd686039828089d53fb42e42046d7a9a3992a0507'
trace: built-in: git 'update-ref' '-m' 'pull --rebase 
https://github.com/Raydiation/core master: distinguish between touch and 
write' 'HEAD' '3b7aa1e847993b2afdbaf19cd8ed50b81d37fc5b'
trace: built-in: git 'cat-file' '-t' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'cat-file' 'commit' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'config' 'i18n.commitencoding'
trace: built-in: git 'show' '-s' '--pretty=raw' '--encoding=UTF-8' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24' '--'
trace: built-in: git 'diff-tree' '--root' '--binary' '--full-index' 
'45869afa5ac718e11c3d2e3bccdb501a022cfc24'
trace: built-in: git 'apply' '--index' 
'/srv/http/owncloud/.git/rebase-apply/patch'

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King]

On Sat, Mar 09, 2013 at 11:54:36AM +0100, Bernhard Posselt wrote:

> >Also, I can almost reproduce here, as PatrickHeller/core.git is public.
> >However, I suspect the problem is particular to your work built on top,
> >which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
> >Is it possible for you to make that commit available on a temporary
> >branch?

> What do you mean exactly by that?

I just meant to push the work from your local repository somewhere where
I could access it to try to replicate the issue. What you did here:

> git clone https://github.com/Raydiation/memorycorruption
> cd memorycorruption
> git pull --rebase https://github.com/Raydiation/core

...should be plenty. Unfortunately, I'm not able to reproduce the
segfault.  All of the patches apply fine, both normally and when run
under valgrind.

> Heres the output of the GIT_TRACE file
> [...]
> trace: built-in: git 'apply' '--index' '/srv/http/owncloud/.git/rebase-apply/patch'

This confirms my suspicion that the problem is in "git apply".

You had mentioned before that the valgrind log was very long.  If you're
still able to reproduce, could you try running it with valgrind like
this:

  valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
    git pull --rebase https://github.com/Raydiation/core

Logging to a file instead of stderr should mean we still get output for
commands that are invoked with their stderr redirected (which is the
case for the "git apply" in question), and using "-q" should eliminate
the uninteresting cruft from the log.

-Peff

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Bernhard Posselt]

On 03/10/2013 08:05 AM, Jeff King wrote:
> On Sat, Mar 09, 2013 at 11:54:36AM +0100, Bernhard Posselt wrote:
>
>>> Also, I can almost reproduce here, as PatrickHeller/core.git is public.
>>> However, I suspect the problem is particular to your work built on top,
>>> which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
>>> Is it possible for you to make that commit available on a temporary
>>> branch?
>> What do you mean exactly by that?
> I just meant to push the work from your local repository somewhere where
> I could access it to try to replicate the issue. What you did here:
>
>> git clone https://github.com/Raydiation/memorycorruption
>> cd memorycorruption
>> git pull --rebase https://github.com/Raydiation/core
> ...should be plenty. Unfortunately, I'm not able to reproduce the
> segfault.  All of the patches apply fine, both normally and when run
> under valgrind.
>
>> Heres the output of the GIT_TRACE file
>> [...]
>> trace: built-in: git 'apply' '--index' '/srv/http/owncloud/.git/rebase-apply/patch'
> This confirms my suspicion that the problem is in "git apply".
>
> You had mentioned before that the valgrind log was very long.  If you're
> still able to reproduce, could you try running it with valgrind like
> this:
>
>    valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
>      git pull --rebase https://github.com/Raydiation/core
>
> Logging to a file instead of stderr should mean we still get output for
> commands that are invoked with their stderr redirected (which is the
> case for the "git apply" in question), and using "-q" should eliminate
> the uninteresting cruft from the log.
>
> -Peff
Do you need debug symbols?

==2395== Invalid write of size 1
==2395==    at 0x4C2DB93: memcpy@@GLIBC_2.14 (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395==    by 0x4076B1: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40A60F: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40C29F: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40CC35: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40F584: ??? (in /usr/lib/git-core/git)
==2395==    by 0x4058E7: ??? (in /usr/lib/git-core/git)
==2395==    by 0x404DD1: ??? (in /usr/lib/git-core/git)
==2395==    by 0x58F3A14: (below main) (in /usr/lib/libc-2.17.so)
==2395==  Address 0x5f245c0 is 0 bytes after a block of size 384 alloc'd
==2395==    at 0x4C2C04B: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395==    by 0x4C2C2FF: realloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395==    by 0x4F057B: ??? (in /usr/lib/git-core/git)
==2395==    by 0x4DDF9F: ??? (in /usr/lib/git-core/git)
==2395==    by 0x409E9C: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40C29F: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40CC35: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40F584: ??? (in /usr/lib/git-core/git)
==2395==    by 0x4058E7: ??? (in /usr/lib/git-core/git)
==2395==    by 0x404DD1: ??? (in /usr/lib/git-core/git)
==2395==    by 0x58F3A14: (below main) (in /usr/lib/libc-2.17.so)
==2395==
==2395== Invalid read of size 1
==2395==    at 0x4C2DCB4: memcpy@@GLIBC_2.14 (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2395==    by 0x40B0D5: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40C29F: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40CC35: ??? (in /usr/lib/git-core/git)
==2395==    by 0x40F584: ??? (in /usr/lib/git-core/git)
==2395==    by 0x4058E7: ??? (in /usr/lib/git-core/git)
==2395==    by 0x404DD1: ??? (in /usr/lib/git-core/git)
==2395==    by 0x58F3A14: (below main) (in /usr/lib/libc-2.17.so)
==2395==  Address 0x5f245e1 is not stack'd, malloc'd or (recently) free'd
==2395==

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Bernhard Posselt]

On 03/10/2013 08:05 AM, Jeff King wrote:
> On Sat, Mar 09, 2013 at 11:54:36AM +0100, Bernhard Posselt wrote:
>
>>> Also, I can almost reproduce here, as PatrickHeller/core.git is public.
>>> However, I suspect the problem is particular to your work built on top,
>>> which looks like it is at commit 0525bbd73c9015499ba92d1ac654b980aaca35b2.
>>> Is it possible for you to make that commit available on a temporary
>>> branch?
>> What do you mean exactly by that?
> I just meant to push the work from your local repository somewhere where
> I could access it to try to replicate the issue. What you did here:
>
>> git clone https://github.com/Raydiation/memorycorruption
>> cd memorycorruption
>> git pull --rebase https://github.com/Raydiation/core
> ...should be plenty. Unfortunately, I'm not able to reproduce the
> segfault.  All of the patches apply fine, both normally and when run
> under valgrind.
>
>> Heres the output of the GIT_TRACE file
>> [...]
>> trace: built-in: git 'apply' '--index' '/srv/http/owncloud/.git/rebase-apply/patch'
> This confirms my suspicion that the problem is in "git apply".
>
> You had mentioned before that the valgrind log was very long.  If you're
> still able to reproduce, could you try running it with valgrind like
> this:
>
>    valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
>      git pull --rebase https://github.com/Raydiation/core
>
> Logging to a file instead of stderr should mean we still get output for
> commands that are invoked with their stderr redirected (which is the
> case for the "git apply" in question), and using "-q" should eliminate
> the uninteresting cruft from the log.
>
> -Peff
First time I've used Archlinux ABS and build from source :)

The log file was empty and it seemed to apply everything nice when 
running valgrind. When i tried to run it without valgrind it failed with 
memory corruption.
Heres the output with debug symbols, fetched with tail -f:

==22291== Invalid write of size 1
==22291==    at 0x4C2DB93: memcpy@@GLIBC_2.14 (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291==    by 0x4076B1: update_pre_post_images (in /usr/lib/git-core/git)
==22291==    by 0x40A60F: apply_fragments (in /usr/lib/git-core/git)
==22291==    by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
==22291==    by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
==22291==    by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
==22291==    by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
==22291==    by 0x404DD1: main (in /usr/lib/git-core/git)
==22291==  Address 0x5f245c0 is 0 bytes after a block of size 384 alloc'd
==22291==    at 0x4C2C04B: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291==    by 0x4C2C2FF: realloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291==    by 0x4F057B: xrealloc (in /usr/lib/git-core/git)
==22291==    by 0x4DDF9F: strbuf_grow (in /usr/lib/git-core/git)
==22291==    by 0x409E9C: apply_fragments (in /usr/lib/git-core/git)
==22291==    by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
==22291==    by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
==22291==    by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
==22291==    by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
==22291==    by 0x404DD1: main (in /usr/lib/git-core/git)
==22291==
==22291== Invalid read of size 1
==22291==    at 0x4C2DCB4: memcpy@@GLIBC_2.14 (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22291==    by 0x40B0D5: apply_fragments (in /usr/lib/git-core/git)
==22291==    by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
==22291==    by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
==22291==    by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
==22291==    by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
==22291==    by 0x404DD1: main (in /usr/lib/git-core/git)
==22291==  Address 0x5f245e1 is not stack'd, malloc'd or (recently) free'd
==22291==

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King]

On Sun, Mar 10, 2013 at 12:45:43PM +0100, Bernhard Posselt wrote:

> >   valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
> >     git pull --rebase https://github.com/Raydiation/core
> 
> The log file was empty and it seemed to apply everything nice when
> running valgrind. When i tried to run it without valgrind it failed
> with memory corruption.

Thanks, we are maybe getting closer. It's weird that it works OK with
valgrind. If the valgrind log is empty, though, I'm confused about where
the output you pasted below came from.

> ==22291== Invalid write of size 1
> ==22291==    at 0x4C2DB93: memcpy@@GLIBC_2.14 (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==22291==    by 0x4076B1: update_pre_post_images (in /usr/lib/git-core/git)
> ==22291==    by 0x40A60F: apply_fragments (in /usr/lib/git-core/git)
> ==22291==    by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
> ==22291==    by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
> ==22291==    by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
> ==22291==    by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
> ==22291==    by 0x404DD1: main (in /usr/lib/git-core/git)

Hmm, it would be nice to have line numbers. Can you try compiling with
"-g -O0"?

The function where the problem is deals with whitespace munging. Just a
guess, but do you have any whitespace config options set (e.g.,
apply.whitespace)?

-Peff

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Bernhard Posselt]

Ok, sorrry for not responsding for quite a while, we had the 5.0 release and had too much to do.

I found out why it segfaults: I had a .gitconfig file (sry must have somehow missed that no idea why actually) with the following contents:

http://dpaste.com/1027662/

it seems that the memory corruption does not happen anymore when i change

[apply]
         whitespace = fix

to

[apply]
         #whitespace = fix


so fixing whitespaces may be the culprit

On 03/11/2013 06:18 AM, Jeff King wrote:
> On Sun, Mar 10, 2013 at 12:45:43PM +0100, Bernhard Posselt wrote:
>
>>>    valgrind -q --trace-children=yes --log-file=/tmp/valgrind.out \
>>>      git pull --rebase https://github.com/Raydiation/core
>> The log file was empty and it seemed to apply everything nice when
>> running valgrind. When i tried to run it without valgrind it failed
>> with memory corruption.
> Thanks, we are maybe getting closer. It's weird that it works OK with
> valgrind. If the valgrind log is empty, though, I'm confused about where
> the output you pasted below came from.
>
>> ==22291== Invalid write of size 1
>> ==22291==    at 0x4C2DB93: memcpy@@GLIBC_2.14 (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==22291==    by 0x4076B1: update_pre_post_images (in /usr/lib/git-core/git)
>> ==22291==    by 0x40A60F: apply_fragments (in /usr/lib/git-core/git)
>> ==22291==    by 0x40C29F: check_patch_list (in /usr/lib/git-core/git)
>> ==22291==    by 0x40CC35: apply_patch (in /usr/lib/git-core/git)
>> ==22291==    by 0x40F584: cmd_apply (in /usr/lib/git-core/git)
>> ==22291==    by 0x4058E7: handle_internal_command (in /usr/lib/git-core/git)
>> ==22291==    by 0x404DD1: main (in /usr/lib/git-core/git)
> Hmm, it would be nice to have line numbers. Can you try compiling with
> "-g -O0"?
>
> The function where the problem is deals with whitespace munging. Just a
> guess, but do you have any whitespace config options set (e.g.,
> apply.whitespace)?
>
> -Peff

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King]

On Tue, Mar 19, 2013 at 11:42:45AM +0100, Bernhard Posselt wrote:

> it seems that the memory corruption does not happen anymore when i change
> 
> [apply]
>         whitespace = fix
> 
> to
> 
> [apply]
>         #whitespace = fix
> 
> so fixing whitespaces may be the culprit

Thanks, I'm able to reproduce with the config you showed. The other key
element seems to be using tab-in-indent.  I am not too familiar with
this code, but I was able to get a much smaller reproduction recipe:

-- >8 --
# make tabs more obvious by using "Q" instead
q_to_tab() {
  perl -lpe 's/Q/\t/g'
}

q_to_tab >preimage <<\EOF
QQa
QQb
QQc
                d
QQe
QQf
QQg
EOF

q_to_tab >patch <<\EOF
diff --git a/preimage b/preimage
--- a/preimage
+++ b/preimage
@@ -1,7 +1,6 @@ public static function store($filename) {
 QQa
 QQb
 QQc
-QQd
 QQe
 QQf
 QQg
EOF

valgrind \
git -c core.whitespace=tab-in-indent apply --whitespace=fix patch
-- 8< --

which yields:

==7112== Invalid write of size 2
==7112==    at 0x4C2C023: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7112==    by 0x40C365: update_pre_post_images (apply.c:2165)
==7112==    by 0x40CC52: match_fragment (apply.c:2402)
[...]
==7112==  Address 0x6e57a5e is 0 bytes after a block of size 94 alloc'd
==7112==    at 0x4C2A26B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7112==    by 0x4C2A51F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7112==    by 0x535193: xrealloc (wrapper.c:100)
==7112==    by 0x51C322: strbuf_grow (strbuf.c:74)
==7112==    by 0x51C10C: strbuf_init (strbuf.c:34)
==7112==    by 0x40D329: apply_one_fragment (apply.c:2602)
[...]

and so on. I haven't quite figured out what is going on. It looks like
we call update_pre_post_images with postlen==0, which causes it to just
write the fixed postimage into the existing buffer. But of course the
fixed version is bigger, because we are expanding the tabs into 8
spaces (but it _doesn't_ break if each line starts with only one tab,
which confuses me).

I'm not too familiar with this code. Maybe Junio can say more.

-Peff

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> and so on. I haven't quite figured out what is going on. It looks like
> we call update_pre_post_images with postlen==0, which causes it to just
> write the fixed postimage into the existing buffer. But of course the
> fixed version is bigger, because we are expanding the tabs into 8
> spaces (but it _doesn't_ break if each line starts with only one tab,
> which confuses me).

I used to be intimately familiar with the update_pre_post_images()
function, but the version after 86c91f91794c (git apply: option to
ignore whitespace differences, 2009-08-04), I won't vouch for it
doing anything sensible.  We recently had to do 5de7166d46d2
(apply.c:update_pre_post_images(): the preimage can be truncated,
2012-10-12) to fix one of its corner cases but I would not be
surprised if there are other cases the function gets it all wrong.

I'd come back to the topic after I finish other tasks on my plate,
so if somebody is inclined please go ahead digging this a bit
further; I won't have much head start to begin with in this code
X-<.

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> and so on. I haven't quite figured out what is going on. It looks like
>> we call update_pre_post_images with postlen==0, which causes it to just
>> write the fixed postimage into the existing buffer. But of course the
>> fixed version is bigger, because we are expanding the tabs into 8
>> spaces (but it _doesn't_ break if each line starts with only one tab,
>> which confuses me).
>
> I used to be intimately familiar with the update_pre_post_images()
> function, but the version after 86c91f91794c (git apply: option to
> ignore whitespace differences, 2009-08-04), I won't vouch for it
> doing anything sensible.  We recently had to do 5de7166d46d2
> (apply.c:update_pre_post_images(): the preimage can be truncated,
> 2012-10-12) to fix one of its corner cases but I would not be
> surprised if there are other cases the function gets it all wrong.
>
> I'd come back to the topic after I finish other tasks on my plate,
> so if somebody is inclined please go ahead digging this a bit
> further; I won't have much head start to begin with in this code
> X-<.

This may be sufficient.  In the olden days, we relied on that all
whitespace fixing rules made the result shorter and took advantage
of it in update-pre-post-images to rewrite the images in place.  The
oddball tab-in-indent (aka Python), however, can grow the result by
expanding tabs (deemed "incorrect") in the input into runs of spaces
(deemed "kosher").

Fortunately, we already support its more generalized form "match
while ignoring whitespace differences" that can lengthen the result;
as long as we correctly count the number of bytes needed to hold
rewritten postimage, the existing logic in update_pre_post_images
should be able to do the rest for us.

 builtin/apply.c          | 16 ++++++++++------
 t/t4124-apply-ws-rule.sh | 26 ++++++++++++++++++++++++++
 t/t4150-am.sh            |  2 +-
 t/test-lib-functions.sh  |  4 ++--
 4 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 080ce2e..cad4e4f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2117,10 +2117,10 @@ static void update_pre_post_images(struct image *preimage,
 
 	/*
 	 * Adjust the common context lines in postimage. This can be
-	 * done in-place when we are just doing whitespace fixing,
-	 * which does not make the string grow, but needs a new buffer
-	 * when ignoring whitespace causes the update, since in this case
-	 * we could have e.g. tabs converted to multiple spaces.
+	 * done in-place when we are shrinking it with whitespace
+	 * fixing, but needs a new buffer when ignoring whitespace or
+	 * expanding leading tabs to spaces.
+	 *
 	 * We trust the caller to tell us if the update can be done
 	 * in place (postlen==0) or not.
 	 */
@@ -2185,7 +2185,7 @@ static int match_fragment(struct image *img,
 	int i;
 	char *fixed_buf, *buf, *orig, *target;
 	struct strbuf fixed;
-	size_t fixed_len;
+	size_t fixed_len, postlen;
 	int preimage_limit;
 
 	if (preimage->nr + try_lno <= img->nr) {
@@ -2335,6 +2335,7 @@ static int match_fragment(struct image *img,
 	strbuf_init(&fixed, preimage->len + 1);
 	orig = preimage->buf;
 	target = img->buf + try;
+	postlen = 0;
 	for (i = 0; i < preimage_limit; i++) {
 		size_t oldlen = preimage->line[i].len;
 		size_t tgtlen = img->line[try_lno + i].len;
@@ -2362,6 +2363,7 @@ static int match_fragment(struct image *img,
 		match = (tgtfix.len == fixed.len - fixstart &&
 			 !memcmp(tgtfix.buf, fixed.buf + fixstart,
 					     fixed.len - fixstart));
+		postlen += tgtfix.len;
 
 		strbuf_release(&tgtfix);
 		if (!match)
@@ -2399,8 +2401,10 @@ static int match_fragment(struct image *img,
 	 * hunk match.  Update the context lines in the postimage.
 	 */
 	fixed_buf = strbuf_detach(&fixed, &fixed_len);
+	if (postlen < postimage->len)
+		postlen = 0;
 	update_pre_post_images(preimage, postimage,
-			       fixed_buf, fixed_len, 0);
+			       fixed_buf, fixed_len, postlen);
 	return 1;
 
  unmatch_exit:
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 6f6ee88..0bbcf06 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -486,4 +486,30 @@ test_expect_success 'same, but with CR-LF line endings && cr-at-eol unset' '
 	test_cmp one expect
 '
 
+test_expect_success 'whitespace=fix to expand' '
+	qz_to_tab_space >preimage <<-\EOF &&
+	QQa
+	QQb
+	QQc
+	ZZZZZZZZZZZZZZZZd
+	QQe
+	QQf
+	QQg
+	EOF
+	qz_to_tab_space >patch <<-\EOF &&
+	diff --git a/preimage b/preimage
+	--- a/preimage
+	+++ b/preimage
+	@@ -1,7 +1,6 @@
+	 QQa
+	 QQb
+	 QQc
+	-QQd
+	 QQe
+	 QQf
+	 QQg
+	EOF
+	git -c core.whitespace=tab-in-indent apply --whitespace=fix patch
+'
+
 test_done
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index cdafd7e..12f6b02 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -17,7 +17,7 @@ test_expect_success 'setup: messages' '
 	vero eos et accusam et justo duo dolores et ea rebum.
 
 	EOF
-	q_to_tab <<-\EOF >>msg &&
+	qz_to_tab_space <<-\EOF >>msg &&
 	QDuis autem vel eum iriure dolor in hendrerit in vulputate velit
 	Qesse molestie consequat, vel illum dolore eu feugiat nulla facilisis
 	Qat vero eros et accumsan et iusto odio dignissim qui blandit
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fa62d01..349f3b8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -87,8 +87,8 @@ q_to_cr () {
 	tr Q '\015'
 }
 
-q_to_tab () {
-	tr Q '\011'
+qz_to_tab_space () {
+	tr QZ '\011\040'
 }
 
 append_cr () {

Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King]

On Fri, Mar 22, 2013 at 11:08:59AM -0700, Junio C Hamano wrote:

> This may be sufficient.  In the olden days, we relied on that all
> whitespace fixing rules made the result shorter and took advantage
> of it in update-pre-post-images to rewrite the images in place.  The
> oddball tab-in-indent (aka Python), however, can grow the result by
> expanding tabs (deemed "incorrect") in the input into runs of spaces
> (deemed "kosher").
> 
> Fortunately, we already support its more generalized form "match
> while ignoring whitespace differences" that can lengthen the result;
> as long as we correctly count the number of bytes needed to hold
> rewritten postimage, the existing logic in update_pre_post_images
> should be able to do the rest for us.

Yeah, your patch looks right to me. I do wonder if the optimization
here:

> @@ -2399,8 +2401,10 @@ static int match_fragment(struct image *img,
>  	 * hunk match.  Update the context lines in the postimage.
>  	 */
>  	fixed_buf = strbuf_detach(&fixed, &fixed_len);
> +	if (postlen < postimage->len)
> +		postlen = 0;
>  	update_pre_post_images(preimage, postimage,
> -			       fixed_buf, fixed_len, 0);
> +			       fixed_buf, fixed_len, postlen);

should simply go into update_pre_post_images (i.e., let it decide on
whether to do it inline or with a new allocation, rather than making
postlen==0 special). That would let the ignore-whitespace code path use
the optimization, too (when it's possible).

By the way, I notice that when update_pre_post_images does allocate, the
old value of postimage->buf is lost. It looks like that is not leaked,
because it was pointing to a strbuf (newlines in apply_one_fragment)
that we are going to release anyway afterwards. But that means nobody is
freeing postimage->buf, which means that our newly malloc'd version is
getting leaked.

-Peff