All of lore.kernel.org
 help / color / mirror / Atom feed
* Segmentation fault in git apply
@ 2015-01-14 18:20 Michael Blume
  2015-01-14 18:40 ` Michael Blume
  2015-01-14 18:50 ` Segmentation fault in git apply Junio C Hamano
  0 siblings, 2 replies; 26+ messages in thread
From: Michael Blume @ 2015-01-14 18:20 UTC (permalink / raw)
  To: Git List

This is a mac with a fresh build of git from pu branch, commit 53b80d0.

With my gitconfig looking like

[user]
    email = blume.mike@gmail.com
    name = Michael Blume
[apply]
    whitespace = fix
[core]
    whitespace = fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4

If I run
git clone git@github.com:MichaelBlume/clojure.git
cd clojure
git checkout origin/rebase-start
git rebase origin/rebase-base

I get

src/jvm/clojure/lang/Compiler.java                          |  26
+++++++++++++++++---------
 test/clojure/test_clojure/compilation.clj                   |  33
++++++++++++++++++++++++++++++++-
 test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+), 10 deletions(-)
 create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
First, rewinding head to replay your work on top of it...
Applying: CLJ-1603 - add reducible cycle, iterate, repeat
Applying: CLJ-1515 Reify range
Applying: CLJ-1499 Direct iterators for PersistentHashMap,
APersistentSet, PersistentQueue, and PersistentStructMap, and records.
Added new IMapIterable interface for key and val iterators.
Applying: CLJ-1602 Make keys and vals return Iterable result
Applying: fix AOT bug preventing overriding of clojure.core functions
Applying: catch multiple rest forms
Applying: zipmap using iterators and transient maps
Applying: Define merge/merge-with after reduce has loaded
Applying: very simple test of the merge function
Applying: Support get on arbitrary java.util.List instances
Applying: CLJ-1451 add take-until
Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
Applying: add unrolled vector implementation
Applying: add transient? predicate
Applying: fix emitted line numbers
Using index info to reconstruct a base tree...
M src/jvm/clojure/lang/Compiler.java
Falling back to patching base and 3-way merge...
Auto-merging src/jvm/clojure/lang/Compiler.java
Applying: just use a not
Applying: trailing whitespace
Applying: don't mix tabs/spaces in clojure.xml/emit-element
Applying: avoid mixing tabs with spaces in clojure core code
Applying: don't optimize for defrecord lookup if keyword is namespaced
Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
Applying: unrolled impls for maps
Applying: CLJ-703: Remove flush and sync calls when writing class files.
Applying: CLJ-1078: Add queue and queue? to clojure.core
Applying: make RT.boundedLength lazier
Applying: first try for adding compare
Applying: Fix for #CLJ-1565
Applying: CLJ-1074: Read +/- Infinity and NaN
Applying: Fix CLJ-1074 for EdnReader too, see
eaeda2e7bf2697e565decdf14a8a99fbf8588c57
Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
Applying: if test expr of an if statement is a literal, don't emit the
runtime test
Applying: evaluate def symbol metadata only once
Applying: CLJ-1295: Speed up dissoc on array-maps
Applying: some throwing
Applying: don't pass offset to ArrayChunk
Applying: make EMPTY accessible
Applying: add handy create methods
Applying: regenerate
Applying: regenerate
/Users/michael.blume/libexec/git-core/git-am: line 854: 92059
Segmentation fault: 11  git apply --index "$dotest/patch" > /dev/null
2>&1
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
in indent.
   IPersistentVector v = (IPersistentVector) asTransient().conj(val)
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
in indent.
   .persistent();
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:15: tab
in indent.
   return (IPersistentVector) ((IObj) v).withMeta(meta);
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:25: tab
in indent.
ITransientCollection coll = PersistentVector.EMPTY
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:27: tab
in indent.
return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
warning: squelched 1 whitespace error
warning: 6 lines add whitespace errors.
Using index info to reconstruct a base tree...
M src/jvm/clojure/lang/PersistentUnrolledVector.java
<stdin>:13: tab in indent.
   IPersistentVector v = (IPersistentVector) asTransient().conj(val)
<stdin>:14: tab in indent.
   .persistent();
<stdin>:15: tab in indent.
   return (IPersistentVector) ((IObj) v).withMeta(meta);
<stdin>:25: tab in indent.
ITransientCollection coll = PersistentVector.EMPTY
<stdin>:27: tab in indent.
return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
warning: squelched 1 whitespace error
warning: 6 lines applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
fatal: Unable to create
'/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.

If no other git process is currently running, this probably means a
git process crashed in this repository earlier. Make sure no other git
process is running and remove the file manually to continue.
Failed to merge in the changes.
Patch failed at 0041 regenerate
The copy of the patch that failed is found in:
   /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch

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".

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 18:20 Segmentation fault in git apply Michael Blume
@ 2015-01-14 18:40 ` Michael Blume
  2015-01-14 18:44   ` Michael Blume
  2015-01-14 18:50 ` Segmentation fault in git apply Junio C Hamano
  1 sibling, 1 reply; 26+ messages in thread
From: Michael Blume @ 2015-01-14 18:40 UTC (permalink / raw)
  To: Git List

On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com> wrote:
> This is a mac with a fresh build of git from pu branch, commit 53b80d0.
>
> With my gitconfig looking like
>
> [user]
>     email = blume.mike@gmail.com
>     name = Michael Blume
> [apply]
>     whitespace = fix
> [core]
>     whitespace = fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4
>
> If I run
> git clone git@github.com:MichaelBlume/clojure.git
> cd clojure
> git checkout origin/rebase-start
> git rebase origin/rebase-base
>
> I get
>
> src/jvm/clojure/lang/Compiler.java                          |  26
> +++++++++++++++++---------
>  test/clojure/test_clojure/compilation.clj                   |  33
> ++++++++++++++++++++++++++++++++-
>  test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 170 insertions(+), 10 deletions(-)
>  create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
> First, rewinding head to replay your work on top of it...
> Applying: CLJ-1603 - add reducible cycle, iterate, repeat
> Applying: CLJ-1515 Reify range
> Applying: CLJ-1499 Direct iterators for PersistentHashMap,
> APersistentSet, PersistentQueue, and PersistentStructMap, and records.
> Added new IMapIterable interface for key and val iterators.
> Applying: CLJ-1602 Make keys and vals return Iterable result
> Applying: fix AOT bug preventing overriding of clojure.core functions
> Applying: catch multiple rest forms
> Applying: zipmap using iterators and transient maps
> Applying: Define merge/merge-with after reduce has loaded
> Applying: very simple test of the merge function
> Applying: Support get on arbitrary java.util.List instances
> Applying: CLJ-1451 add take-until
> Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
> Applying: add unrolled vector implementation
> Applying: add transient? predicate
> Applying: fix emitted line numbers
> Using index info to reconstruct a base tree...
> M src/jvm/clojure/lang/Compiler.java
> Falling back to patching base and 3-way merge...
> Auto-merging src/jvm/clojure/lang/Compiler.java
> Applying: just use a not
> Applying: trailing whitespace
> Applying: don't mix tabs/spaces in clojure.xml/emit-element
> Applying: avoid mixing tabs with spaces in clojure core code
> Applying: don't optimize for defrecord lookup if keyword is namespaced
> Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
> Applying: unrolled impls for maps
> Applying: CLJ-703: Remove flush and sync calls when writing class files.
> Applying: CLJ-1078: Add queue and queue? to clojure.core
> Applying: make RT.boundedLength lazier
> Applying: first try for adding compare
> Applying: Fix for #CLJ-1565
> Applying: CLJ-1074: Read +/- Infinity and NaN
> Applying: Fix CLJ-1074 for EdnReader too, see
> eaeda2e7bf2697e565decdf14a8a99fbf8588c57
> Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
> Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
> Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
> Applying: if test expr of an if statement is a literal, don't emit the
> runtime test
> Applying: evaluate def symbol metadata only once
> Applying: CLJ-1295: Speed up dissoc on array-maps
> Applying: some throwing
> Applying: don't pass offset to ArrayChunk
> Applying: make EMPTY accessible
> Applying: add handy create methods
> Applying: regenerate
> Applying: regenerate
> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
> Segmentation fault: 11  git apply --index "$dotest/patch" > /dev/null
> 2>&1
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
> in indent.
>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
> in indent.
>    .persistent();
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:15: tab
> in indent.
>    return (IPersistentVector) ((IObj) v).withMeta(meta);
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:25: tab
> in indent.
> ITransientCollection coll = PersistentVector.EMPTY
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:27: tab
> in indent.
> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
> warning: squelched 1 whitespace error
> warning: 6 lines add whitespace errors.
> Using index info to reconstruct a base tree...
> M src/jvm/clojure/lang/PersistentUnrolledVector.java
> <stdin>:13: tab in indent.
>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
> <stdin>:14: tab in indent.
>    .persistent();
> <stdin>:15: tab in indent.
>    return (IPersistentVector) ((IObj) v).withMeta(meta);
> <stdin>:25: tab in indent.
> ITransientCollection coll = PersistentVector.EMPTY
> <stdin>:27: tab in indent.
> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
> warning: squelched 1 whitespace error
> warning: 6 lines applied after fixing whitespace errors.
> Falling back to patching base and 3-way merge...
> fatal: Unable to create
> '/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.
>
> If no other git process is currently running, this probably means a
> git process crashed in this repository earlier. Make sure no other git
> process is running and remove the file manually to continue.
> Failed to merge in the changes.
> Patch failed at 0041 regenerate
> The copy of the patch that failed is found in:
>    /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch
>
> 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".

Same problem, exists on master, checking 2.2.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 18:40 ` Michael Blume
@ 2015-01-14 18:44   ` Michael Blume
  2015-01-14 18:48     ` Michael Blume
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Blume @ 2015-01-14 18:44 UTC (permalink / raw)
  To: Git List

On Wed, Jan 14, 2015 at 10:40 AM, Michael Blume <blume.mike@gmail.com> wrote:
> On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com> wrote:
>> This is a mac with a fresh build of git from pu branch, commit 53b80d0.
>>
>> With my gitconfig looking like
>>
>> [user]
>>     email = blume.mike@gmail.com
>>     name = Michael Blume
>> [apply]
>>     whitespace = fix
>> [core]
>>     whitespace = fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4
>>
>> If I run
>> git clone git@github.com:MichaelBlume/clojure.git
>> cd clojure
>> git checkout origin/rebase-start
>> git rebase origin/rebase-base
>>
>> I get
>>
>> src/jvm/clojure/lang/Compiler.java                          |  26
>> +++++++++++++++++---------
>>  test/clojure/test_clojure/compilation.clj                   |  33
>> ++++++++++++++++++++++++++++++++-
>>  test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 170 insertions(+), 10 deletions(-)
>>  create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
>> First, rewinding head to replay your work on top of it...
>> Applying: CLJ-1603 - add reducible cycle, iterate, repeat
>> Applying: CLJ-1515 Reify range
>> Applying: CLJ-1499 Direct iterators for PersistentHashMap,
>> APersistentSet, PersistentQueue, and PersistentStructMap, and records.
>> Added new IMapIterable interface for key and val iterators.
>> Applying: CLJ-1602 Make keys and vals return Iterable result
>> Applying: fix AOT bug preventing overriding of clojure.core functions
>> Applying: catch multiple rest forms
>> Applying: zipmap using iterators and transient maps
>> Applying: Define merge/merge-with after reduce has loaded
>> Applying: very simple test of the merge function
>> Applying: Support get on arbitrary java.util.List instances
>> Applying: CLJ-1451 add take-until
>> Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
>> Applying: add unrolled vector implementation
>> Applying: add transient? predicate
>> Applying: fix emitted line numbers
>> Using index info to reconstruct a base tree...
>> M src/jvm/clojure/lang/Compiler.java
>> Falling back to patching base and 3-way merge...
>> Auto-merging src/jvm/clojure/lang/Compiler.java
>> Applying: just use a not
>> Applying: trailing whitespace
>> Applying: don't mix tabs/spaces in clojure.xml/emit-element
>> Applying: avoid mixing tabs with spaces in clojure core code
>> Applying: don't optimize for defrecord lookup if keyword is namespaced
>> Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
>> Applying: unrolled impls for maps
>> Applying: CLJ-703: Remove flush and sync calls when writing class files.
>> Applying: CLJ-1078: Add queue and queue? to clojure.core
>> Applying: make RT.boundedLength lazier
>> Applying: first try for adding compare
>> Applying: Fix for #CLJ-1565
>> Applying: CLJ-1074: Read +/- Infinity and NaN
>> Applying: Fix CLJ-1074 for EdnReader too, see
>> eaeda2e7bf2697e565decdf14a8a99fbf8588c57
>> Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
>> Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
>> Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
>> Applying: if test expr of an if statement is a literal, don't emit the
>> runtime test
>> Applying: evaluate def symbol metadata only once
>> Applying: CLJ-1295: Speed up dissoc on array-maps
>> Applying: some throwing
>> Applying: don't pass offset to ArrayChunk
>> Applying: make EMPTY accessible
>> Applying: add handy create methods
>> Applying: regenerate
>> Applying: regenerate
>> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
>> Segmentation fault: 11  git apply --index "$dotest/patch" > /dev/null
>> 2>&1
>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
>> in indent.
>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
>> in indent.
>>    .persistent();
>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:15: tab
>> in indent.
>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:25: tab
>> in indent.
>> ITransientCollection coll = PersistentVector.EMPTY
>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:27: tab
>> in indent.
>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>> warning: squelched 1 whitespace error
>> warning: 6 lines add whitespace errors.
>> Using index info to reconstruct a base tree...
>> M src/jvm/clojure/lang/PersistentUnrolledVector.java
>> <stdin>:13: tab in indent.
>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>> <stdin>:14: tab in indent.
>>    .persistent();
>> <stdin>:15: tab in indent.
>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>> <stdin>:25: tab in indent.
>> ITransientCollection coll = PersistentVector.EMPTY
>> <stdin>:27: tab in indent.
>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>> warning: squelched 1 whitespace error
>> warning: 6 lines applied after fixing whitespace errors.
>> Falling back to patching base and 3-way merge...
>> fatal: Unable to create
>> '/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.
>>
>> If no other git process is currently running, this probably means a
>> git process crashed in this repository earlier. Make sure no other git
>> process is running and remove the file manually to continue.
>> Failed to merge in the changes.
>> Patch failed at 0041 regenerate
>> The copy of the patch that failed is found in:
>>    /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch
>>
>> 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".
>
> Same problem, exists on master, checking 2.2.2
Exists in 2.2.2, checking 2.0.0

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 18:44   ` Michael Blume
@ 2015-01-14 18:48     ` Michael Blume
  2015-01-14 18:58       ` Michael Blume
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Blume @ 2015-01-14 18:48 UTC (permalink / raw)
  To: Git List

On Wed, Jan 14, 2015 at 10:44 AM, Michael Blume <blume.mike@gmail.com> wrote:
> On Wed, Jan 14, 2015 at 10:40 AM, Michael Blume <blume.mike@gmail.com> wrote:
>> On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com> wrote:
>>> This is a mac with a fresh build of git from pu branch, commit 53b80d0.
>>>
>>> With my gitconfig looking like
>>>
>>> [user]
>>>     email = blume.mike@gmail.com
>>>     name = Michael Blume
>>> [apply]
>>>     whitespace = fix
>>> [core]
>>>     whitespace = fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4
>>>
>>> If I run
>>> git clone git@github.com:MichaelBlume/clojure.git
>>> cd clojure
>>> git checkout origin/rebase-start
>>> git rebase origin/rebase-base
>>>
>>> I get
>>>
>>> src/jvm/clojure/lang/Compiler.java                          |  26
>>> +++++++++++++++++---------
>>>  test/clojure/test_clojure/compilation.clj                   |  33
>>> ++++++++++++++++++++++++++++++++-
>>>  test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 170 insertions(+), 10 deletions(-)
>>>  create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
>>> First, rewinding head to replay your work on top of it...
>>> Applying: CLJ-1603 - add reducible cycle, iterate, repeat
>>> Applying: CLJ-1515 Reify range
>>> Applying: CLJ-1499 Direct iterators for PersistentHashMap,
>>> APersistentSet, PersistentQueue, and PersistentStructMap, and records.
>>> Added new IMapIterable interface for key and val iterators.
>>> Applying: CLJ-1602 Make keys and vals return Iterable result
>>> Applying: fix AOT bug preventing overriding of clojure.core functions
>>> Applying: catch multiple rest forms
>>> Applying: zipmap using iterators and transient maps
>>> Applying: Define merge/merge-with after reduce has loaded
>>> Applying: very simple test of the merge function
>>> Applying: Support get on arbitrary java.util.List instances
>>> Applying: CLJ-1451 add take-until
>>> Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
>>> Applying: add unrolled vector implementation
>>> Applying: add transient? predicate
>>> Applying: fix emitted line numbers
>>> Using index info to reconstruct a base tree...
>>> M src/jvm/clojure/lang/Compiler.java
>>> Falling back to patching base and 3-way merge...
>>> Auto-merging src/jvm/clojure/lang/Compiler.java
>>> Applying: just use a not
>>> Applying: trailing whitespace
>>> Applying: don't mix tabs/spaces in clojure.xml/emit-element
>>> Applying: avoid mixing tabs with spaces in clojure core code
>>> Applying: don't optimize for defrecord lookup if keyword is namespaced
>>> Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
>>> Applying: unrolled impls for maps
>>> Applying: CLJ-703: Remove flush and sync calls when writing class files.
>>> Applying: CLJ-1078: Add queue and queue? to clojure.core
>>> Applying: make RT.boundedLength lazier
>>> Applying: first try for adding compare
>>> Applying: Fix for #CLJ-1565
>>> Applying: CLJ-1074: Read +/- Infinity and NaN
>>> Applying: Fix CLJ-1074 for EdnReader too, see
>>> eaeda2e7bf2697e565decdf14a8a99fbf8588c57
>>> Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
>>> Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
>>> Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
>>> Applying: if test expr of an if statement is a literal, don't emit the
>>> runtime test
>>> Applying: evaluate def symbol metadata only once
>>> Applying: CLJ-1295: Speed up dissoc on array-maps
>>> Applying: some throwing
>>> Applying: don't pass offset to ArrayChunk
>>> Applying: make EMPTY accessible
>>> Applying: add handy create methods
>>> Applying: regenerate
>>> Applying: regenerate
>>> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
>>> Segmentation fault: 11  git apply --index "$dotest/patch" > /dev/null
>>> 2>&1
>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
>>> in indent.
>>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
>>> in indent.
>>>    .persistent();
>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:15: tab
>>> in indent.
>>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:25: tab
>>> in indent.
>>> ITransientCollection coll = PersistentVector.EMPTY
>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:27: tab
>>> in indent.
>>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>>> warning: squelched 1 whitespace error
>>> warning: 6 lines add whitespace errors.
>>> Using index info to reconstruct a base tree...
>>> M src/jvm/clojure/lang/PersistentUnrolledVector.java
>>> <stdin>:13: tab in indent.
>>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>>> <stdin>:14: tab in indent.
>>>    .persistent();
>>> <stdin>:15: tab in indent.
>>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>>> <stdin>:25: tab in indent.
>>> ITransientCollection coll = PersistentVector.EMPTY
>>> <stdin>:27: tab in indent.
>>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>>> warning: squelched 1 whitespace error
>>> warning: 6 lines applied after fixing whitespace errors.
>>> Falling back to patching base and 3-way merge...
>>> fatal: Unable to create
>>> '/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.
>>>
>>> If no other git process is currently running, this probably means a
>>> git process crashed in this repository earlier. Make sure no other git
>>> process is running and remove the file manually to continue.
>>> Failed to merge in the changes.
>>> Patch failed at 0041 regenerate
>>> The copy of the patch that failed is found in:
>>>    /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch
>>>
>>> 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".
>>
>> Same problem, exists on master, checking 2.2.2
> Exists in 2.2.2, checking 2.0.0
Ok, problem exists in 2.0.0 but not in 1.8.0, bisecting...

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 18:20 Segmentation fault in git apply Michael Blume
  2015-01-14 18:40 ` Michael Blume
@ 2015-01-14 18:50 ` Junio C Hamano
  1 sibling, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-01-14 18:50 UTC (permalink / raw)
  To: Michael Blume; +Cc: Git List

Michael Blume <blume.mike@gmail.com> writes:

> This is a mac with a fresh build of git from pu branch, commit 53b80d0.

Hmm, I do not see anything suspicious between master and pu.  Is it
possible to bisect?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 18:48     ` Michael Blume
@ 2015-01-14 18:58       ` Michael Blume
  2015-01-14 19:09         ` Michael Blume
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Blume @ 2015-01-14 18:58 UTC (permalink / raw)
  To: Git List

On Wed, Jan 14, 2015 at 10:48 AM, Michael Blume <blume.mike@gmail.com> wrote:
> On Wed, Jan 14, 2015 at 10:44 AM, Michael Blume <blume.mike@gmail.com> wrote:
>> On Wed, Jan 14, 2015 at 10:40 AM, Michael Blume <blume.mike@gmail.com> wrote:
>>> On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com> wrote:
>>>> This is a mac with a fresh build of git from pu branch, commit 53b80d0.
>>>>
>>>> With my gitconfig looking like
>>>>
>>>> [user]
>>>>     email = blume.mike@gmail.com
>>>>     name = Michael Blume
>>>> [apply]
>>>>     whitespace = fix
>>>> [core]
>>>>     whitespace = fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4
>>>>
>>>> If I run
>>>> git clone git@github.com:MichaelBlume/clojure.git
>>>> cd clojure
>>>> git checkout origin/rebase-start
>>>> git rebase origin/rebase-base
>>>>
>>>> I get
>>>>
>>>> src/jvm/clojure/lang/Compiler.java                          |  26
>>>> +++++++++++++++++---------
>>>>  test/clojure/test_clojure/compilation.clj                   |  33
>>>> ++++++++++++++++++++++++++++++++-
>>>>  test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 170 insertions(+), 10 deletions(-)
>>>>  create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
>>>> First, rewinding head to replay your work on top of it...
>>>> Applying: CLJ-1603 - add reducible cycle, iterate, repeat
>>>> Applying: CLJ-1515 Reify range
>>>> Applying: CLJ-1499 Direct iterators for PersistentHashMap,
>>>> APersistentSet, PersistentQueue, and PersistentStructMap, and records.
>>>> Added new IMapIterable interface for key and val iterators.
>>>> Applying: CLJ-1602 Make keys and vals return Iterable result
>>>> Applying: fix AOT bug preventing overriding of clojure.core functions
>>>> Applying: catch multiple rest forms
>>>> Applying: zipmap using iterators and transient maps
>>>> Applying: Define merge/merge-with after reduce has loaded
>>>> Applying: very simple test of the merge function
>>>> Applying: Support get on arbitrary java.util.List instances
>>>> Applying: CLJ-1451 add take-until
>>>> Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
>>>> Applying: add unrolled vector implementation
>>>> Applying: add transient? predicate
>>>> Applying: fix emitted line numbers
>>>> Using index info to reconstruct a base tree...
>>>> M src/jvm/clojure/lang/Compiler.java
>>>> Falling back to patching base and 3-way merge...
>>>> Auto-merging src/jvm/clojure/lang/Compiler.java
>>>> Applying: just use a not
>>>> Applying: trailing whitespace
>>>> Applying: don't mix tabs/spaces in clojure.xml/emit-element
>>>> Applying: avoid mixing tabs with spaces in clojure core code
>>>> Applying: don't optimize for defrecord lookup if keyword is namespaced
>>>> Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
>>>> Applying: unrolled impls for maps
>>>> Applying: CLJ-703: Remove flush and sync calls when writing class files.
>>>> Applying: CLJ-1078: Add queue and queue? to clojure.core
>>>> Applying: make RT.boundedLength lazier
>>>> Applying: first try for adding compare
>>>> Applying: Fix for #CLJ-1565
>>>> Applying: CLJ-1074: Read +/- Infinity and NaN
>>>> Applying: Fix CLJ-1074 for EdnReader too, see
>>>> eaeda2e7bf2697e565decdf14a8a99fbf8588c57
>>>> Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
>>>> Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
>>>> Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
>>>> Applying: if test expr of an if statement is a literal, don't emit the
>>>> runtime test
>>>> Applying: evaluate def symbol metadata only once
>>>> Applying: CLJ-1295: Speed up dissoc on array-maps
>>>> Applying: some throwing
>>>> Applying: don't pass offset to ArrayChunk
>>>> Applying: make EMPTY accessible
>>>> Applying: add handy create methods
>>>> Applying: regenerate
>>>> Applying: regenerate
>>>> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
>>>> Segmentation fault: 11  git apply --index "$dotest/patch" > /dev/null
>>>> 2>&1
>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
>>>> in indent.
>>>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
>>>> in indent.
>>>>    .persistent();
>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:15: tab
>>>> in indent.
>>>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:25: tab
>>>> in indent.
>>>> ITransientCollection coll = PersistentVector.EMPTY
>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:27: tab
>>>> in indent.
>>>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>>>> warning: squelched 1 whitespace error
>>>> warning: 6 lines add whitespace errors.
>>>> Using index info to reconstruct a base tree...
>>>> M src/jvm/clojure/lang/PersistentUnrolledVector.java
>>>> <stdin>:13: tab in indent.
>>>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>>>> <stdin>:14: tab in indent.
>>>>    .persistent();
>>>> <stdin>:15: tab in indent.
>>>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>>>> <stdin>:25: tab in indent.
>>>> ITransientCollection coll = PersistentVector.EMPTY
>>>> <stdin>:27: tab in indent.
>>>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>>>> warning: squelched 1 whitespace error
>>>> warning: 6 lines applied after fixing whitespace errors.
>>>> Falling back to patching base and 3-way merge...
>>>> fatal: Unable to create
>>>> '/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.
>>>>
>>>> If no other git process is currently running, this probably means a
>>>> git process crashed in this repository earlier. Make sure no other git
>>>> process is running and remove the file manually to continue.
>>>> Failed to merge in the changes.
>>>> Patch failed at 0041 regenerate
>>>> The copy of the patch that failed is found in:
>>>>    /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch
>>>>
>>>> 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".
>>>
>>> Same problem, exists on master, checking 2.2.2
>> Exists in 2.2.2, checking 2.0.0
> Ok, problem exists in 2.0.0 but not in 1.8.0, bisecting...
Hmm, sorry, no, this is strange, problem actually does exist in 1.8.0
but waits until a later commit and has somewhat different output but
still looks like a crash in git-apply...

First, rewinding head to replay your work on top of it...
Applying: CLJ-1603 - add reducible cycle, iterate, repeat
Applying: CLJ-1515 Reify range
Applying: CLJ-1499 Direct iterators for PersistentHashMap,
APersistentSet, PersistentQueue, and PersistentStructMap, and records.
Added new IMapIterable interface for key and val iterators.
Applying: CLJ-1602 Make keys and vals return Iterable result
Applying: fix AOT bug preventing overriding of clojure.core functions
Applying: catch multiple rest forms
Applying: zipmap using iterators and transient maps
Applying: Define merge/merge-with after reduce has loaded
Applying: very simple test of the merge function
Applying: Support get on arbitrary java.util.List instances
Applying: CLJ-1451 add take-until
Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
Applying: add unrolled vector implementation
Applying: add transient? predicate
Applying: fix emitted line numbers
Using index info to reconstruct a base tree...
M src/jvm/clojure/lang/Compiler.java
Falling back to patching base and 3-way merge...
Auto-merging src/jvm/clojure/lang/Compiler.java
Applying: just use a not
Applying: trailing whitespace
Applying: don't mix tabs/spaces in clojure.xml/emit-element
Applying: avoid mixing tabs with spaces in clojure core code
Applying: don't optimize for defrecord lookup if keyword is namespaced
Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
Applying: unrolled impls for maps
Applying: CLJ-703: Remove flush and sync calls when writing class files.
Applying: CLJ-1078: Add queue and queue? to clojure.core
Applying: make RT.boundedLength lazier
Applying: first try for adding compare
Applying: Fix for #CLJ-1565
Applying: CLJ-1074: Read +/- Infinity and NaN
Applying: Fix CLJ-1074 for EdnReader too, see
eaeda2e7bf2697e565decdf14a8a99fbf8588c57
Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
Applying: if test expr of an if statement is a literal, don't emit the
runtime test
Applying: evaluate def symbol metadata only once
Applying: CLJ-1295: Speed up dissoc on array-maps
Applying: some throwing
Applying: don't pass offset to ArrayChunk
Applying: make EMPTY accessible
Applying: add handy create methods
Applying: regenerate
Applying: regenerate
Applying: fix handling of key meta in assoc
Applying: don't check mapequivalence
Applying: add editablemap
Applying: use an arraymap in persistenthashset
Applying: no, use an unrolledmap
Applying: use the new stuff
Applying: CLJ-1613: evaluate :or defaults in enclosing scope in map
destructuring
Applying: CLJ-1615 ensure transient set "keys" and "values" have
consistent metadata
Applying: fix the unrolledmap transient entryat thing
/Users/michael.blume/libexec/git-core/git-am: line 811: 59793 Abort
trap: 6           git apply --index "$dotest/patch" > /dev/null 2>&1
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:10: tab
in indent.
IMapEntry doEntryAt(Object key) {
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:11: tab
in indent.
   int h = Util.hasheq(key);
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:12: tab
in indent.
   int idx = indexOf(h, key);
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
in indent.
   switch (idx) {
/Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
in indent.
   case 0:
warning: squelched 15 whitespace errors
warning: 20 lines add whitespace errors.
Using index info to reconstruct a base tree...
M src/jvm/clojure/lang/PersistentUnrolledMap.java
<stdin>:10: tab in indent.
IMapEntry doEntryAt(Object key) {
<stdin>:11: tab in indent.
   int h = Util.hasheq(key);
<stdin>:12: tab in indent.
   int idx = indexOf(h, key);
<stdin>:13: tab in indent.
   switch (idx) {
<stdin>:14: tab in indent.
   case 0:
warning: squelched 15 whitespace errors
warning: 20 lines applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
fatal: Unable to create
'/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.

If no other git process is currently running, this probably means a
git process crashed in this repository earlier. Make sure no other git
process is running and remove the file manually to continue.
Failed to merge in the changes.
Patch failed at 0050 fix the unrolledmap transient entryat thing
The copy of the patch that failed is found in:
   /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch

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".

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 18:58       ` Michael Blume
@ 2015-01-14 19:09         ` Michael Blume
  2015-01-15  8:26           ` Kyle J. McKay
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Blume @ 2015-01-14 19:09 UTC (permalink / raw)
  To: Git List

On Wed, Jan 14, 2015 at 10:58 AM, Michael Blume <blume.mike@gmail.com> wrote:
> On Wed, Jan 14, 2015 at 10:48 AM, Michael Blume <blume.mike@gmail.com> wrote:
>> On Wed, Jan 14, 2015 at 10:44 AM, Michael Blume <blume.mike@gmail.com> wrote:
>>> On Wed, Jan 14, 2015 at 10:40 AM, Michael Blume <blume.mike@gmail.com> wrote:
>>>> On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com> wrote:
>>>>> This is a mac with a fresh build of git from pu branch, commit 53b80d0.
>>>>>
>>>>> With my gitconfig looking like
>>>>>
>>>>> [user]
>>>>>     email = blume.mike@gmail.com
>>>>>     name = Michael Blume
>>>>> [apply]
>>>>>     whitespace = fix
>>>>> [core]
>>>>>     whitespace = fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4
>>>>>
>>>>> If I run
>>>>> git clone git@github.com:MichaelBlume/clojure.git
>>>>> cd clojure
>>>>> git checkout origin/rebase-start
>>>>> git rebase origin/rebase-base
>>>>>
>>>>> I get
>>>>>
>>>>> src/jvm/clojure/lang/Compiler.java                          |  26
>>>>> +++++++++++++++++---------
>>>>>  test/clojure/test_clojure/compilation.clj                   |  33
>>>>> ++++++++++++++++++++++++++++++++-
>>>>>  test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 170 insertions(+), 10 deletions(-)
>>>>>  create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
>>>>> First, rewinding head to replay your work on top of it...
>>>>> Applying: CLJ-1603 - add reducible cycle, iterate, repeat
>>>>> Applying: CLJ-1515 Reify range
>>>>> Applying: CLJ-1499 Direct iterators for PersistentHashMap,
>>>>> APersistentSet, PersistentQueue, and PersistentStructMap, and records.
>>>>> Added new IMapIterable interface for key and val iterators.
>>>>> Applying: CLJ-1602 Make keys and vals return Iterable result
>>>>> Applying: fix AOT bug preventing overriding of clojure.core functions
>>>>> Applying: catch multiple rest forms
>>>>> Applying: zipmap using iterators and transient maps
>>>>> Applying: Define merge/merge-with after reduce has loaded
>>>>> Applying: very simple test of the merge function
>>>>> Applying: Support get on arbitrary java.util.List instances
>>>>> Applying: CLJ-1451 add take-until
>>>>> Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
>>>>> Applying: add unrolled vector implementation
>>>>> Applying: add transient? predicate
>>>>> Applying: fix emitted line numbers
>>>>> Using index info to reconstruct a base tree...
>>>>> M src/jvm/clojure/lang/Compiler.java
>>>>> Falling back to patching base and 3-way merge...
>>>>> Auto-merging src/jvm/clojure/lang/Compiler.java
>>>>> Applying: just use a not
>>>>> Applying: trailing whitespace
>>>>> Applying: don't mix tabs/spaces in clojure.xml/emit-element
>>>>> Applying: avoid mixing tabs with spaces in clojure core code
>>>>> Applying: don't optimize for defrecord lookup if keyword is namespaced
>>>>> Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
>>>>> Applying: unrolled impls for maps
>>>>> Applying: CLJ-703: Remove flush and sync calls when writing class files.
>>>>> Applying: CLJ-1078: Add queue and queue? to clojure.core
>>>>> Applying: make RT.boundedLength lazier
>>>>> Applying: first try for adding compare
>>>>> Applying: Fix for #CLJ-1565
>>>>> Applying: CLJ-1074: Read +/- Infinity and NaN
>>>>> Applying: Fix CLJ-1074 for EdnReader too, see
>>>>> eaeda2e7bf2697e565decdf14a8a99fbf8588c57
>>>>> Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
>>>>> Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
>>>>> Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
>>>>> Applying: if test expr of an if statement is a literal, don't emit the
>>>>> runtime test
>>>>> Applying: evaluate def symbol metadata only once
>>>>> Applying: CLJ-1295: Speed up dissoc on array-maps
>>>>> Applying: some throwing
>>>>> Applying: don't pass offset to ArrayChunk
>>>>> Applying: make EMPTY accessible
>>>>> Applying: add handy create methods
>>>>> Applying: regenerate
>>>>> Applying: regenerate
>>>>> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
>>>>> Segmentation fault: 11  git apply --index "$dotest/patch" > /dev/null
>>>>> 2>&1
>>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
>>>>> in indent.
>>>>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
>>>>> in indent.
>>>>>    .persistent();
>>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:15: tab
>>>>> in indent.
>>>>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:25: tab
>>>>> in indent.
>>>>> ITransientCollection coll = PersistentVector.EMPTY
>>>>> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:27: tab
>>>>> in indent.
>>>>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>>>>> warning: squelched 1 whitespace error
>>>>> warning: 6 lines add whitespace errors.
>>>>> Using index info to reconstruct a base tree...
>>>>> M src/jvm/clojure/lang/PersistentUnrolledVector.java
>>>>> <stdin>:13: tab in indent.
>>>>>    IPersistentVector v = (IPersistentVector) asTransient().conj(val)
>>>>> <stdin>:14: tab in indent.
>>>>>    .persistent();
>>>>> <stdin>:15: tab in indent.
>>>>>    return (IPersistentVector) ((IObj) v).withMeta(meta);
>>>>> <stdin>:25: tab in indent.
>>>>> ITransientCollection coll = PersistentVector.EMPTY
>>>>> <stdin>:27: tab in indent.
>>>>> return (ITransientVector) coll.conj(e0).conj(e1).conj(e2)
>>>>> warning: squelched 1 whitespace error
>>>>> warning: 6 lines applied after fixing whitespace errors.
>>>>> Falling back to patching base and 3-way merge...
>>>>> fatal: Unable to create
>>>>> '/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.
>>>>>
>>>>> If no other git process is currently running, this probably means a
>>>>> git process crashed in this repository earlier. Make sure no other git
>>>>> process is running and remove the file manually to continue.
>>>>> Failed to merge in the changes.
>>>>> Patch failed at 0041 regenerate
>>>>> The copy of the patch that failed is found in:
>>>>>    /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch
>>>>>
>>>>> 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".
>>>>
>>>> Same problem, exists on master, checking 2.2.2
>>> Exists in 2.2.2, checking 2.0.0
>> Ok, problem exists in 2.0.0 but not in 1.8.0, bisecting...
> Hmm, sorry, no, this is strange, problem actually does exist in 1.8.0
> but waits until a later commit and has somewhat different output but
> still looks like a crash in git-apply...
>
> First, rewinding head to replay your work on top of it...
> Applying: CLJ-1603 - add reducible cycle, iterate, repeat
> Applying: CLJ-1515 Reify range
> Applying: CLJ-1499 Direct iterators for PersistentHashMap,
> APersistentSet, PersistentQueue, and PersistentStructMap, and records.
> Added new IMapIterable interface for key and val iterators.
> Applying: CLJ-1602 Make keys and vals return Iterable result
> Applying: fix AOT bug preventing overriding of clojure.core functions
> Applying: catch multiple rest forms
> Applying: zipmap using iterators and transient maps
> Applying: Define merge/merge-with after reduce has loaded
> Applying: very simple test of the merge function
> Applying: Support get on arbitrary java.util.List instances
> Applying: CLJ-1451 add take-until
> Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
> Applying: add unrolled vector implementation
> Applying: add transient? predicate
> Applying: fix emitted line numbers
> Using index info to reconstruct a base tree...
> M src/jvm/clojure/lang/Compiler.java
> Falling back to patching base and 3-way merge...
> Auto-merging src/jvm/clojure/lang/Compiler.java
> Applying: just use a not
> Applying: trailing whitespace
> Applying: don't mix tabs/spaces in clojure.xml/emit-element
> Applying: avoid mixing tabs with spaces in clojure core code
> Applying: don't optimize for defrecord lookup if keyword is namespaced
> Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
> Applying: unrolled impls for maps
> Applying: CLJ-703: Remove flush and sync calls when writing class files.
> Applying: CLJ-1078: Add queue and queue? to clojure.core
> Applying: make RT.boundedLength lazier
> Applying: first try for adding compare
> Applying: Fix for #CLJ-1565
> Applying: CLJ-1074: Read +/- Infinity and NaN
> Applying: Fix CLJ-1074 for EdnReader too, see
> eaeda2e7bf2697e565decdf14a8a99fbf8588c57
> Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
> Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
> Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
> Applying: if test expr of an if statement is a literal, don't emit the
> runtime test
> Applying: evaluate def symbol metadata only once
> Applying: CLJ-1295: Speed up dissoc on array-maps
> Applying: some throwing
> Applying: don't pass offset to ArrayChunk
> Applying: make EMPTY accessible
> Applying: add handy create methods
> Applying: regenerate
> Applying: regenerate
> Applying: fix handling of key meta in assoc
> Applying: don't check mapequivalence
> Applying: add editablemap
> Applying: use an arraymap in persistenthashset
> Applying: no, use an unrolledmap
> Applying: use the new stuff
> Applying: CLJ-1613: evaluate :or defaults in enclosing scope in map
> destructuring
> Applying: CLJ-1615 ensure transient set "keys" and "values" have
> consistent metadata
> Applying: fix the unrolledmap transient entryat thing
> /Users/michael.blume/libexec/git-core/git-am: line 811: 59793 Abort
> trap: 6           git apply --index "$dotest/patch" > /dev/null 2>&1
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:10: tab
> in indent.
> IMapEntry doEntryAt(Object key) {
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:11: tab
> in indent.
>    int h = Util.hasheq(key);
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:12: tab
> in indent.
>    int idx = indexOf(h, key);
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:13: tab
> in indent.
>    switch (idx) {
> /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch:14: tab
> in indent.
>    case 0:
> warning: squelched 15 whitespace errors
> warning: 20 lines add whitespace errors.
> Using index info to reconstruct a base tree...
> M src/jvm/clojure/lang/PersistentUnrolledMap.java
> <stdin>:10: tab in indent.
> IMapEntry doEntryAt(Object key) {
> <stdin>:11: tab in indent.
>    int h = Util.hasheq(key);
> <stdin>:12: tab in indent.
>    int idx = indexOf(h, key);
> <stdin>:13: tab in indent.
>    switch (idx) {
> <stdin>:14: tab in indent.
>    case 0:
> warning: squelched 15 whitespace errors
> warning: 20 lines applied after fixing whitespace errors.
> Falling back to patching base and 3-way merge...
> fatal: Unable to create
> '/Users/michael.blume/workspace/clojure/.git/index.lock': File exists.
>
> If no other git process is currently running, this probably means a
> git process crashed in this repository earlier. Make sure no other git
> process is running and remove the file manually to continue.
> Failed to merge in the changes.
> Patch failed at 0050 fix the unrolledmap transient entryat thing
> The copy of the patch that failed is found in:
>    /Users/michael.blume/workspace/clojure/.git/rebase-apply/patch
>
> 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".
Ok, looks like this is not purely a mac bug, that's good I guess:

[11:08][ubuntu@saucy-server:~/dev/clojure((f70fdba...))]$ git rebase
origin/rebase-base
 src/jvm/clojure/lang/Compiler.java                          |  26
+++++++++++++++++---------
 test/clojure/test_clojure/compilation.clj                   |  33
++++++++++++++++++++++++++++++++-
 test/clojure/test_clojure/compilation/examples_clj_1561.clj | 121
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+), 10 deletions(-)
 create mode 100644 test/clojure/test_clojure/compilation/examples_clj_1561.clj
First, rewinding head to replay your work on top of it...
Applying: CLJ-1603 - add reducible cycle, iterate, repeat
Applying: CLJ-1515 Reify range
Applying: CLJ-1499 Direct iterators for PersistentHashMap,
APersistentSet, PersistentQueue, and PersistentStructMap, and records.
Added new IMapIterable interface for key and val iterators.
Applying: CLJ-1602 Make keys and vals return Iterable result
Applying: fix AOT bug preventing overriding of clojure.core functions
Applying: catch multiple rest forms
Applying: zipmap using iterators and transient maps
Applying: Define merge/merge-with after reduce has loaded
Applying: very simple test of the merge function
Applying: Support get on arbitrary java.util.List instances
Applying: CLJ-1451 add take-until
Applying: CLJ-1606 - complete eduction's xform without completing outer rfn
Applying: add unrolled vector implementation
Applying: add transient? predicate
Applying: fix emitted line numbers
Using index info to reconstruct a base tree...
M src/jvm/clojure/lang/Compiler.java
Falling back to patching base and 3-way merge...
Auto-merging src/jvm/clojure/lang/Compiler.java
Applying: just use a not
Applying: trailing whitespace
Applying: don't mix tabs/spaces in clojure.xml/emit-element
Applying: avoid mixing tabs with spaces in clojure core code
Applying: don't optimize for defrecord lookup if keyword is namespaced
Applying: CLJ-1572 - Extend CollReduce to IReduceInit for supported arity
Applying: unrolled impls for maps
Applying: CLJ-703: Remove flush and sync calls when writing class files.
Applying: CLJ-1078: Add queue and queue? to clojure.core
Applying: make RT.boundedLength lazier
Applying: first try for adding compare
Applying: Fix for #CLJ-1565
Applying: CLJ-1074: Read +/- Infinity and NaN
Applying: Fix CLJ-1074 for EdnReader too, see
eaeda2e7bf2697e565decdf14a8a99fbf8588c57
Applying: add get-and-set! to expose AtomicReference.getAndSet() for atoms
Applying: CLJ-1472 Locking macro without explicit monitor-enter, monitor-exit
Applying: CLJ-1449: Add starts-with? ends-with? contains? to clojure.string
Applying: if test expr of an if statement is a literal, don't emit the
runtime test
Applying: evaluate def symbol metadata only once
Applying: CLJ-1295: Speed up dissoc on array-maps
Applying: some throwing
Applying: don't pass offset to ArrayChunk
Applying: make EMPTY accessible
Applying: add handy create methods
Applying: regenerate
Applying: regenerate
*** Error in `git': malloc(): memory corruption: 0x0000000001d690a0 ***

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-14 19:09         ` Michael Blume
@ 2015-01-15  8:26           ` Kyle J. McKay
  2015-01-15  9:10             ` Kyle J. McKay
  0 siblings, 1 reply; 26+ messages in thread
From: Kyle J. McKay @ 2015-01-15  8:26 UTC (permalink / raw)
  To: Michael Blume; +Cc: Git List, Junio C Hamano

On Jan 14, 2015, at 11:09, Michael Blume wrote:
> On Wed, Jan 14, 2015 at 10:58 AM, Michael Blume  
> <blume.mike@gmail.com> wrote:
>> On Wed, Jan 14, 2015 at 10:48 AM, Michael Blume  
>> <blume.mike@gmail.com> wrote:
>>> On Wed, Jan 14, 2015 at 10:44 AM, Michael Blume <blume.mike@gmail.com 
>>> > wrote:
>>>> On Wed, Jan 14, 2015 at 10:40 AM, Michael Blume <blume.mike@gmail.com 
>>>> > wrote:
>>>>> On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com 
>>>>> > wrote:
>>>>>> This is a mac with a fresh build of git from pu branch, commit  
>>>>>> 53b80d0.
>>>>>>
>>>>>> With my gitconfig looking like
>>>>>>
>>>>>> [user]
>>>>>>    email = blume.mike@gmail.com
>>>>>>    name = Michael Blume
>>>>>> [apply]
>>>>>>    whitespace = fix
>>>>>> [core]
>>>>>>    whitespace = fix,trailing-space,space-before-tab, tab-in- 
>>>>>> indent, tabwidth=4
>>>>>>
>>>>>> If I run
>>>>>> git clone git@github.com:MichaelBlume/clojure.git
>>>>>> cd clojure
>>>>>> git checkout origin/rebase-start
>>>>>> git rebase origin/rebase-base
>>>>>>
>>>>>> I get
[...]
>>>>>> Applying: CLJ-1295: Speed up dissoc on array-maps
>>>>>> Applying: some throwing
>>>>>> Applying: don't pass offset to ArrayChunk
>>>>>> Applying: make EMPTY accessible
>>>>>> Applying: add handy create methods
>>>>>> Applying: regenerate
>>>>>> Applying: regenerate
>>>>>> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
>>>>>> Segmentation fault: 11  git apply --index "$dotest/patch" > / 
>>>>>> dev/null
>>>>>> 2>&1

I can reproduce in a 64-bit v2.1.4 as well, but not in a 32-bit v2.1.4  
build.

My recipe is slightly different to facilitate automation:

cd /tmp
git clone git://github.com/MichaelBlume/clojure.git
cd clojure
git config user.email "blume.mike@gmail.com"
git config user.name "Michael Blume"
git config apply.whitespace fix
git config core.whitespace \
   "fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4"
git checkout origin/rebase-start
git rebase origin/rebase-base

Looks like v1.7.6.6 64-bit works okay.

Running git bisect run...

5782..2890..1445..722..361..179..91..44..23..13..7..3..1..0

And the winner is (first appearing in v1.8.2.2):

commit 250b3c6c992b3cb04e756eb33bed99442fc55193
Author: Junio C Hamano <gitster@pobox.com>
Date:   Fri Mar 22 11:10:03 2013 -0700

     apply --whitespace=fix: avoid running over the postimage buffer

     Originally update-pre-post-images could assume that any whitespace
     fixing will make the result only shorter by unexpanding runs of
     leading SPs into HTs and removing trailing whitespaces at the end  
of
     lines.  Updating the post-image we read from the patch to match the
     actual result can be performed in-place under this assumption.
     These days, however, we have tab-in-indent (aka Python) rule whose
     result can be longer than the original, and we do need to allocate
     a larger buffer than the input and replace the result.

     Fortunately the support for lengthening rewrite was already added
     when we began supporting "match while ignoring whitespace
     differences" mode in 86c91f91794c (git apply: option to ignore
     whitespace differences, 2009-08-04).  We only need to correctly
     count the number of bytes necessary to hold the updated result and
     tell the function to allocate a new buffer.

     Signed-off-by: Junio C Hamano <gitster@pobox.com>

And just to confirm, building with 250b3c6c^ (which also happens to be  
v1.8.0.3) does not fail.

And the stack trace from the crash dump of a debug build of 250b3c6c is:

Thread 0 Crashed:
0   libSystem.B.dylib  0x00007fff8290242a szone_free + 1222
1   git                0x0000000100009fe9 apply_one_fragment + 2164  
(apply.c:2816)
2   git                0x000000010000a760 apply_fragments + 195  
(apply.c:2959)
3   git                0x000000010000b62d apply_data + 96 (apply.c:3340)
4   git                0x000000010000c0b1 check_patch + 869 (apply.c: 
3559)
5   git                0x000000010000c157 check_patch_list + 83  
(apply.c:3574)
6   git                0x000000010000dc70 apply_patch + 646 (apply.c: 
4189)
7   git                0x000000010000ea3a cmd_apply + 2700 (apply.c: 
4418)
8   git                0x0000000100001ae8 run_builtin + 402 (git.c:306)
9   git                0x0000000100001c9a handle_internal_command +  
181 (git.c:467)
10  git                0x0000000100001dab run_argv + 41 (git.c:516)
11  git                0x0000000100001ede main + 258 (git.c:588)
12  git                0x0000000100000ee8 start + 52

And the gdb backtrace from the core file:

#0  0x00007fff8290242a at szone_free + 1222
#1  0x0000000100009fe9 in apply_one_fragment (img=0x7fff5fbfe640,  
frag=0x100400a60, inaccurate_eof=0, ws_rule=3268, nth_fragment=1) at  
builtin/apply.c:2815
#2  0x000000010000a760 in apply_fragments (img=0x7fff5fbfe640,  
patch=0x1004005e0) at builtin/apply.c:2959
#3  0x000000010000b62d in apply_data (patch=0x1004005e0,  
st=0x7fff5fbfe6b0, ce=0x1004072e0) at builtin/apply.c:3340
#4  0x000000010000c0b1 in check_patch (patch=0x1004005e0) at builtin/ 
apply.c:3559
#5  0x000000010000c157 in check_patch_list (patch=0x1004005e0) at  
builtin/apply.c:3574
#6  0x000000010000dc70 in apply_patch (fd=3, filename=0x7fff5fbff33a "/ 
private/tmp/clojure/.git/rebase-apply/patch", options=0) at builtin/ 
apply.c:4189
#7  0x000000010000ea3a in cmd_apply (argc=1, argv=0x7fff5fbff178,  
prefix_=0x0) at builtin/apply.c:4418
#8  0x0000000100001ae8 in run_builtin (p=0x1001a7070, argc=3,  
argv=0x7fff5fbff178) at git.c:306
#9  0x0000000100001c9a in handle_internal_command (argc=3,  
argv=0x7fff5fbff178) at git.c:467
#10 0x0000000100001dab in run_argv (argcp=0x7fff5fbff13c,  
argv=0x7fff5fbff130) at git.c:513
#11 0x0000000100001ede in main (argc=3, argv=0x7fff5fbff178) at git.c: 
588

The crashing line at apply.c:2815 is:

   free(oldlines);

And oldlines appears to be valid (it has normal program text in it).

Running with various MallocCheckHeap and MallocErrorAbort settings  
leads to:

git(12926) malloc: *** error for object 0x10040be80: incorrect  
checksum for freed object - object was probably modified after being  
freed.

And a new backtrace from the core file:

#0  0x00007fff82962da6 at __kill + 10
#1  0x00007fff829c5af8 at szone_error + 476
#2  0x00007fff829c7218 at szone_check + 637
#3  0x00007fff829caaf8 at malloc_zone_check + 42
#4  0x00007fff829cb11d at internal_check + 14
#5  0x00007fff828fc939 at malloc_zone_malloc + 60
#6  0x00007fff828fc8e0 at malloc + 44
#7  0x0000000100131ae4 in xmalloc (size=47378) at wrapper.c:50
#8  0x000000010000950b in update_image (img=0x7fff5fbfe4a0,  
applied_pos=1569, preimage=0x7fff5fbfe340, postimage=0x7fff5fbfe310)  
at builtin/apply.c:2533
#9  0x0000000100009fa7 in apply_one_fragment (img=0x7fff5fbfe4a0,  
frag=0x100400a60, inaccurate_eof=0, ws_rule=3268, nth_fragment=1) at  
builtin/apply.c:2808
#10 0x000000010000a760 in apply_fragments (img=0x7fff5fbfe4a0,  
patch=0x1004005e0) at builtin/apply.c:2959
#11 0x000000010000b62d in apply_data (patch=0x1004005e0,  
st=0x7fff5fbfe510, ce=0x1004072e0) at builtin/apply.c:3340
#12 0x000000010000c0b1 in check_patch (patch=0x1004005e0) at builtin/ 
apply.c:3559
#13 0x000000010000c157 in check_patch_list (patch=0x1004005e0) at  
builtin/apply.c:3574
#14 0x000000010000dc70 in apply_patch (fd=9, filename=0x7fff5fbff1e2 "/ 
private/tmp/clojure/.git/rebase-apply/patch", options=0) at builtin/ 
apply.c:4189
#15 0x000000010000ea3a in cmd_apply (argc=1, argv=0x7fff5fbfefe0,  
prefix_=0x0) at builtin/apply.c:4418
#16 0x0000000100001ae8 in run_builtin (p=0x1001a7070, argc=3,  
argv=0x7fff5fbfefe0) at git.c:306
#17 0x0000000100001c9a in handle_internal_command (argc=3,  
argv=0x7fff5fbfefe0) at git.c:467
#18 0x0000000100001dab in run_argv (argcp=0x7fff5fbfef9c,  
argv=0x7fff5fbfef90) at git.c:513
#19 0x0000000100001ede in main (argc=3, argv=0x7fff5fbfefe0) at git.c: 
588

I looked at the code a bit, but a fix does not just jump out at me.   
 From the debug info it seems pretty clear that some memory's being  
stepped on.

-Kyle

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-15  8:26           ` Kyle J. McKay
@ 2015-01-15  9:10             ` Kyle J. McKay
  2015-01-16 19:58               ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Kyle J. McKay @ 2015-01-15  9:10 UTC (permalink / raw)
  To: Michael Blume; +Cc: Git List, Junio C Hamano

On Jan 15, 2015, at 00:26, Kyle J. McKay wrote:

> On Jan 14, 2015, at 11:09, Michael Blume wrote:
>> On Wed, Jan 14, 2015 at 10:58 AM, Michael Blume  
>> <blume.mike@gmail.com> wrote:
>>> On Wed, Jan 14, 2015 at 10:48 AM, Michael Blume <blume.mike@gmail.com 
>>> > wrote:
>>>> On Wed, Jan 14, 2015 at 10:44 AM, Michael Blume <blume.mike@gmail.com 
>>>> > wrote:
>>>>> On Wed, Jan 14, 2015 at 10:40 AM, Michael Blume <blume.mike@gmail.com 
>>>>> > wrote:
>>>>>> On Wed, Jan 14, 2015 at 10:20 AM, Michael Blume <blume.mike@gmail.com 
>>>>>> > wrote:
>>>>>>> This is a mac with a fresh build of git from pu branch, commit  
>>>>>>> 53b80d0.
>>>>>>>
>>>>>>> With my gitconfig looking like
>>>>>>>
>>>>>>> [user]
>>>>>>>   email = blume.mike@gmail.com
>>>>>>>   name = Michael Blume
>>>>>>> [apply]
>>>>>>>   whitespace = fix
>>>>>>> [core]
>>>>>>>   whitespace = fix,trailing-space,space-before-tab, tab-in- 
>>>>>>> indent, tabwidth=4
>>>>>>>
>>>>>>> If I run
>>>>>>> git clone git@github.com:MichaelBlume/clojure.git
>>>>>>> cd clojure
>>>>>>> git checkout origin/rebase-start
>>>>>>> git rebase origin/rebase-base
>>>>>>>
>>>>>>> I get
> [...]
>>>>>>> Applying: CLJ-1295: Speed up dissoc on array-maps
>>>>>>> Applying: some throwing
>>>>>>> Applying: don't pass offset to ArrayChunk
>>>>>>> Applying: make EMPTY accessible
>>>>>>> Applying: add handy create methods
>>>>>>> Applying: regenerate
>>>>>>> Applying: regenerate
>>>>>>> /Users/michael.blume/libexec/git-core/git-am: line 854: 92059
>>>>>>> Segmentation fault: 11  git apply --index "$dotest/patch" > / 
>>>>>>> dev/null
>>>>>>> 2>&1
>
> I can reproduce in a 64-bit v2.1.4 as well, but not in a 32-bit  
> v2.1.4 build.
>
> My recipe is slightly different to facilitate automation:
>
> cd /tmp
> git clone git://github.com/MichaelBlume/clojure.git
> cd clojure
> git config user.email "blume.mike@gmail.com"
> git config user.name "Michael Blume"
> git config apply.whitespace fix
> git config core.whitespace \
>  "fix,trailing-space,space-before-tab, tab-in-indent, tabwidth=4"
> git checkout origin/rebase-start
> git rebase origin/rebase-base
>
> Looks like v1.7.6.6 64-bit works okay.
>
> Running git bisect run...
>
> 5782..2890..1445..722..361..179..91..44..23..13..7..3..1..0
>
> And the winner is (first appearing in v1.8.2.2):
>
> commit 250b3c6c992b3cb04e756eb33bed99442fc55193
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Fri Mar 22 11:10:03 2013 -0700
>
>    apply --whitespace=fix: avoid running over the postimage buffer
[...]
> And just to confirm, building with 250b3c6c^ (which also happens to  
> be v1.8.0.3) does not fail.
[...]
> Running with various MallocCheckHeap and MallocErrorAbort settings  
> leads to:
>
> git(12926) malloc: *** error for object 0x10040be80: incorrect  
> checksum for freed object - object was probably modified after being  
> freed.
>
> And a new backtrace from the core file:
>
> #0  0x00007fff82962da6 at __kill + 10
> #1  0x00007fff829c5af8 at szone_error + 476
> #2  0x00007fff829c7218 at szone_check + 637
> #3  0x00007fff829caaf8 at malloc_zone_check + 42
> #4  0x00007fff829cb11d at internal_check + 14
> #5  0x00007fff828fc939 at malloc_zone_malloc + 60
> #6  0x00007fff828fc8e0 at malloc + 44
> #7  0x0000000100131ae4 in xmalloc (size=47378) at wrapper.c:50
> #8  0x000000010000950b in update_image (img=0x7fff5fbfe4a0,  
> applied_pos=1569, preimage=0x7fff5fbfe340, postimage=0x7fff5fbfe310)  
> at builtin/apply.c:2533
> #9  0x0000000100009fa7 in apply_one_fragment (img=0x7fff5fbfe4a0,  
> frag=0x100400a60, inaccurate_eof=0, ws_rule=3268, nth_fragment=1) at  
> builtin/apply.c:2808
> #10 0x000000010000a760 in apply_fragments (img=0x7fff5fbfe4a0,  
> patch=0x1004005e0) at builtin/apply.c:2959
> #11 0x000000010000b62d in apply_data (patch=0x1004005e0,  
> st=0x7fff5fbfe510, ce=0x1004072e0) at builtin/apply.c:3340
> #12 0x000000010000c0b1 in check_patch (patch=0x1004005e0) at builtin/ 
> apply.c:3559
> #13 0x000000010000c157 in check_patch_list (patch=0x1004005e0) at  
> builtin/apply.c:3574
> #14 0x000000010000dc70 in apply_patch (fd=9, filename=0x7fff5fbff1e2  
> "/private/tmp/clojure/.git/rebase-apply/patch", options=0) at  
> builtin/apply.c:4189
> #15 0x000000010000ea3a in cmd_apply (argc=1, argv=0x7fff5fbfefe0,  
> prefix_=0x0) at builtin/apply.c:4418
> #16 0x0000000100001ae8 in run_builtin (p=0x1001a7070, argc=3,  
> argv=0x7fff5fbfefe0) at git.c:306
> #17 0x0000000100001c9a in handle_internal_command (argc=3,  
> argv=0x7fff5fbfefe0) at git.c:467
> #18 0x0000000100001dab in run_argv (argcp=0x7fff5fbfef9c,  
> argv=0x7fff5fbfef90) at git.c:513
> #19 0x0000000100001ede in main (argc=3, argv=0x7fff5fbfefe0) at  
> git.c:588
>
> I looked at the code a bit, but a fix does not just jump out at me.   
> From the debug info it seems pretty clear that some memory's being  
> stepped on.

If I make this change on top of 250b3c6c:

diff --git a/builtin/apply.c b/builtin/apply.c
index df773c75..8795e830 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2390,6 +2390,8 @@ static int match_fragment(struct image *img,
  	fixed_buf = strbuf_detach(&fixed, &fixed_len);
  	if (postlen < postimage->len)
  		postlen = 0;
+	if (postlen)
+		postlen = 2 * postimage->len;
  	update_pre_post_images(preimage, postimage,
  			       fixed_buf, fixed_len, postlen);
  	return 1;

Then the problem goes away.  That seems to suggest that postlen is  
being computed incorrectly, but someone more familiar with bulitin/ 
apply.c is going to need to look at it.

-Kyle

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: Segmentation fault in git apply
  2015-01-15  9:10             ` Kyle J. McKay
@ 2015-01-16 19:58               ` Junio C Hamano
  2015-01-16 23:54                 ` [PATCH] apply: count the size of postimage correctly Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2015-01-16 19:58 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Michael Blume, Git List

"Kyle J. McKay" <mackyle@gmail.com> writes:

> If I make this change on top of 250b3c6c:
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index df773c75..8795e830 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -2390,6 +2390,8 @@ static int match_fragment(struct image *img,
>  	fixed_buf = strbuf_detach(&fixed, &fixed_len);
>  	if (postlen < postimage->len)
>  		postlen = 0;
> +	if (postlen)
> +		postlen = 2 * postimage->len;
>  	update_pre_post_images(preimage, postimage,
>  			       fixed_buf, fixed_len, postlen);
>  	return 1;
>
> Then the problem goes away.  That seems to suggest that postlen is
> being computed incorrectly, but someone more familiar with
> bulitin/apply.c is going to need to look at it.

Indeed, with this, the test case detects under-counting in the
caller (the caller counts 262 bytes but the expansion consumes 273
bytes).

-- >8 --
Subject: apply: make update_pre_post_images() sanity check the given postlen

---
 builtin/apply.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/builtin/apply.c b/builtin/apply.c
index 622ee16..18b7997 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2174,6 +2174,10 @@ static void update_pre_post_images(struct image *preimage,
 	/* Fix the length of the whole thing */
 	postimage->len = new - postimage->buf;
 	postimage->nr -= reduced;
+
+	if (postlen && postlen < (new - postimage->buf))
+		die("BUG: postlen = %d, used = %d",
+		    (int)postlen, (int)(new - postimage->buf));
 }
 
 static int match_fragment(struct image *img,
-- 
2.3.0-rc0-149-g0286818

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH] apply: count the size of postimage correctly
  2015-01-16 19:58               ` Junio C Hamano
@ 2015-01-16 23:54                 ` Junio C Hamano
  2015-01-18 10:49                   ` [PATCH] test: add git apply whitespace expansion tests Kyle J. McKay
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2015-01-16 23:54 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Michael Blume, Git List

Under --whitespace=fix option, match_fragment() function examines
the preimage (the common context and the removed lines in the patch)
and the file being patched and checks if they match after correcting
all whitespace errors.  When they are found to match, the common
context lines in the preimage is replaced with the fixed copy,
because these lines will then be copied to the corresponding place
in the postimage by a later call to update_pre_post_images().  Lines
that are added in the postimage, under --whitespace=fix, have their
whitespace errors already fixed when apply_one_fragment() prepares
the preimage and the postimage, so in the end, application of the
patch can be done by replacing the block of text in the file being
patched that matched the preimage with what is in the postimage that
was updated by update_pre_post_images().

In the earlier days, fixing whitespace errors always resulted in
reduction of size, either collapsing runs of spaces in the indent to
a tab or removing the trailing whitespaces.  These days, however,
some whitespace error fix results in extending the size.

250b3c6c (apply --whitespace=fix: avoid running over the postimage
buffer, 2013-03-22) tried to compute the final postimage size but
its math was flawed.  It counted the size of the block of text in
the original being patched after fixing the whitespace errors on its
lines that correspond to the preimage.  That number does not have
much to do with how big the final postimage would be.

Instead count (1) the added lines in the postimage, whose size is
the same as in the final patch result because their whitespace
errors have already been corrected, and (2) the fixed size of the
lines that are common.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This seems to fix the clojure test case without breaking existing
   tests.  We would need a test case for this, though.

 builtin/apply.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 622ee16..8e79510 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2330,6 +2330,23 @@ static int match_fragment(struct image *img,
 	 * ignore whitespace, we were asked to correct whitespace
 	 * errors, so let's try matching after whitespace correction.
 	 *
+	 * While checking the preimage against the target, whitespace
+	 * errors in both fixed, we count how large the corresponding
+	 * postimage needs to be.  The postimage prepared by
+	 * apply_one_fragment() has whitespace errors fixed on added
+	 * lines already, but the common lines were propagated as-is,
+	 * which may become longer when their whitespace errors are
+	 * fixed.
+	 */
+
+	/* First count added lines in postimage */
+	postlen = 0;
+	for (i = 0; i < postimage->nr; i++) {
+		if (!(postimage->line[i].flag & LINE_COMMON))
+			postlen += postimage->line[i].len;
+	}
+
+	/*
 	 * The preimage may extend beyond the end of the file,
 	 * but in this loop we will only handle the part of the
 	 * preimage that falls within the file.
@@ -2337,7 +2354,6 @@ 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;
@@ -2365,7 +2381,10 @@ 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;
+
+		/* Add the length if this is common with the postimage */
+		if (preimage->line[i].flag & LINE_COMMON)
+			postlen += tgtfix.len;
 
 		strbuf_release(&tgtfix);
 		if (!match)
-- 
2.3.0-rc0-157-g96da9ba

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH] test: add git apply whitespace expansion tests
  2015-01-16 23:54                 ` [PATCH] apply: count the size of postimage correctly Junio C Hamano
@ 2015-01-18 10:49                   ` Kyle J. McKay
  2015-01-18 22:11                     ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Kyle J. McKay @ 2015-01-18 10:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git mailing list, Michael Blume

When git apply fixes whitespace, the result can end up being
longer than the initial text if whitespace ends up being expanded
(such as with the tab-in-indent option).

Since 250b3c6c (apply --whitespace=fix: avoid running over the
postimage buffer, 2013-03-22) an attempt has been made to compute
the correct final length in such a case.

These tests all stress the whitespace-expansion-during-apply
condition and can result in core dump failures when the final
length is not computed correctly.

Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---

* Here's some tests.  With "apply: make update_pre_post_images() sanity
  check the given postlen" but not "apply: count the size of postimage
  correctly" test 1/4 and 4/4 trigger the 'die("BUG: postlen...' but
  test 2/4 and 3/4 do not although they fail because git apply generates
  garbage.

* After applying "apply: count the size of postimage correctly" all 4
  tests pass whereas they all fail without that.  It's interesting that
  the "BUG: postlen" check does not trigger for 2/4 or 3/4 but the output
  is garbage in those cases without the fix.

* Theses tests can easily trigger core dumps.  It seems to depend on how
  the git binary was built and what exactly ends up getting stepped on as
  I have several different Git builds and some of them core dump on tests
  that others pass or just produce garbage for, but none of them passes
  2/4 or 3/4 without the "count postimage size correctly" fix.

 t/t4138-apply-ws-expansion.sh | 121 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)
 create mode 100755 t/t4138-apply-ws-expansion.sh

diff --git a/t/t4138-apply-ws-expansion.sh b/t/t4138-apply-ws-expansion.sh
new file mode 100755
index 00000000..140ed9ac
--- /dev/null
+++ b/t/t4138-apply-ws-expansion.sh
@@ -0,0 +1,121 @@
+#!/bin/sh
+#
+# Copyright (C) 2015 Kyle J. McKay
+#
+
+# NOTE: To facilitate separate testing, this script can be run
+# standalone to just create the test files and do nothing else
+# by first setting the environment variable MAKE_PATCHES=1.
+
+test_description='git apply test patches with whitespace expansion.'
+
+[ -n "$MAKE_PATCHES" ] || \
+. ./test-lib.sh
+
+#
+## create test-N, patchN.patch, expect-N files
+#
+
+# test 1
+printf '\t%s\n' 1 2 3 4 5 6 > before
+printf '\t%s\n' 1 2 3 > after
+printf '%64s\n' a b c $x >> after
+printf '\t%s\n' 4 5 6 >> after
+git diff --no-index before after | \
+sed -e 's/before/test-1/' -e 's/after/test-1/' > patch1.patch
+printf '%64s\n' 1 2 3 4 5 6 > test-1
+printf '%64s\n' 1 2 3 a b c 4 5 6 > expect-1
+
+# test 2
+printf '\t%s\n' a b c d e f > before
+printf '\t%s\n' a b c > after
+n=10
+x=1
+while [ $x -lt $n ]; do
+	printf '%63s%d\n' '' $x >> after
+	x=$(( $x + 1 ))
+done
+printf '\t%s\n' d e f >> after
+git diff --no-index before after | \
+sed -e 's/before/test-2/' -e 's/after/test-2/' > patch2.patch
+printf '%64s\n' a b c d e f > test-2
+printf '%64s\n' a b c > expect-2
+x=1
+while [ $x -lt $n ]; do
+	printf '%63s%d\n' '' $x >> expect-2
+	x=$(( $x + 1 ))
+done
+printf '%64s\n' d e f >> expect-2
+
+# test 3
+printf '\t%s\n' a b c d e f > before
+printf '\t%s\n' a b c > after
+n=100
+x=0
+while [ $x -lt $n ]; do
+	printf '%63s%02d\n' '' $x >> after
+	x=$(( $x + 1 ))
+done
+printf '\t%s\n' d e f >> after
+git diff --no-index before after | \
+sed -e 's/before/test-3/' -e 's/after/test-3/' > patch3.patch
+printf '%64s\n' a b c d e f > test-3
+printf '%64s\n' a b c > expect-3
+x=0
+while [ $x -lt $n ]; do
+	printf '%63s%02d\n' '' $x >> expect-3
+	x=$(( $x + 1 ))
+done
+printf '%64s\n' d e f >> expect-3
+
+# test 4
+> before
+x=0
+while [ $x -lt 50 ]; do
+	printf '\t%02d\n' $x >> before
+	x=$(( $x + 1 ))
+done
+cat before > after
+printf '%64s\n' a b c >> after
+while [ $x -lt 100 ]; do
+	printf '\t%02d\n' $x >> before
+	printf '\t%02d\n' $x >> after
+	x=$(( $x + 1 ))
+done
+git diff --no-index before after | \
+sed -e 's/before/test-4/' -e 's/after/test-4/' > patch4.patch
+> test-4
+x=0
+while [ $x -lt 50 ]; do
+	printf '%63s%02d\n' '' $x >> test-4
+	x=$(( $x + 1 ))
+done
+cat test-4 > expect-4
+printf '%64s\n' a b c >> expect-4
+while [ $x -lt 100 ]; do
+	printf '%63s%02d\n' '' $x >> test-4
+	printf '%63s%02d\n' '' $x >> expect-4
+	x=$(( $x + 1 ))
+done
+
+# cleanup
+rm before after
+
+[ -z "$MAKE_PATCHES" ] || exit 0
+
+#
+## Run the tests
+#
+
+# Note that `patch` can successfully apply all patches when run
+# with the --ignore-whitespace option.
+
+for t in 1 2 3 4; do
+	test_expect_success "apply with ws expansion (t=$t)" '
+		git -c core.whitespace=tab-in-indent,tabwidth=63 \
+			apply --whitespace=fix patch$t.patch &&
+		test_cmp test-$t expect-$t
+	'
+done
+
+test_done
--

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] test: add git apply whitespace expansion tests
  2015-01-18 10:49                   ` [PATCH] test: add git apply whitespace expansion tests Kyle J. McKay
@ 2015-01-18 22:11                     ` Junio C Hamano
  2015-01-19  3:54                       ` Kyle J. McKay
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2015-01-18 22:11 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Git mailing list, Michael Blume

On Sun, Jan 18, 2015 at 2:49 AM, Kyle J. McKay <mackyle@gmail.com> wrote:
> * Here's some tests.  With "apply: make update_pre_post_images() sanity
>   check the given postlen" but not "apply: count the size of postimage
>   correctly" test 1/4 and 4/4 trigger the 'die("BUG: postlen...' but
>   test 2/4 and 3/4 do not although they fail because git apply generates
>   garbage.

Do the failing cases that do not trigger the new postlen check fail
because the original (mis)counting thinks (incorrectly) that the
rewritten result _should_ fit within the original postlen (hence we
allow an in-place rewrite by passing postlen=0 to the helper), but
in fact after rewriting postimage->len ends up being longer due to
the miscounting?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] test: add git apply whitespace expansion tests
  2015-01-18 22:11                     ` Junio C Hamano
@ 2015-01-19  3:54                       ` Kyle J. McKay
  2015-01-21 22:33                         ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Kyle J. McKay @ 2015-01-19  3:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git mailing list, Michael Blume

On Jan 18, 2015, at 14:11, Junio C Hamano wrote:

> On Sun, Jan 18, 2015 at 2:49 AM, Kyle J. McKay <mackyle@gmail.com>  
> wrote:
>> * Here's some tests.  With "apply: make update_pre_post_images()  
>> sanity
>>  check the given postlen" but not "apply: count the size of postimage
>>  correctly" test 1/4 and 4/4 trigger the 'die("BUG: postlen...' but
>>  test 2/4 and 3/4 do not although they fail because git apply  
>> generates
>>  garbage.
>
> Do the failing cases that do not trigger the new postlen check fail
> because the original (mis)counting thinks (incorrectly) that the
> rewritten result _should_ fit within the original postlen (hence we
> allow an in-place rewrite by passing postlen=0 to the helper), but
> in fact after rewriting postimage->len ends up being longer due to
> the miscounting?

I'm not 100%, but I think so because:

Before 250b3c6c (apply --whitespace=fix: avoid running over the  
postimage buffer, 2013-03-22), tests 1 and 4 tend to easily cause seg  
faults whereas 2 and 3 just give garbage.

After 250b3c6c (apply --whitespace=fix: avoid running over the  
postimage buffer, 2013-03-22), tests 1 and 4 may pass without seg  
faulting although clearly there's some memory corruption going on  
because after "apply: make update_pre_post_images() sanity check the  
given postlen" they always die with the BUG message.

I created the tests after reading your description in "apply: count  
the size of postimage correctly".  I made a guess about what would  
trigger the problem -- I do not have a deep understanding of the  
builtin/apply.c code though.  Tests 2 and 3 were attempts to make the  
discrepancy between counted and needed (assuming the "apply: count the  
size of postimage correctly" fix has not been applied) progressively  
worse and instead I ended up with a different kind of failure.  Test 4  
was then an alternate attempt to create a very large discrepancy and  
it ended up with BUG: values not that dissimilar from test 1.

FYI, without the counting fix, test 1 causes "BUG: postlen = 390, used  
= 585" and test 4 causes "BUG: postlen = 396, used = 591".  So while I  
did manage to increase the discrepancy a bit from the values you  
reported for clojure (postlen = 262, used = 273), I was actually  
aiming for a difference big enough to pretty much always guarantee a  
core dump.

The garbage tests 2 and 3 produce without the counting fix is  
reminiscent of what you get when you use memcpy instead of memmove for  
an overlapping memory copy operation.

A slightly modified version of these 4 tests can be run as far back a  
v1.7.4 (when apply --whitespace=fix started fixing tab-in-indent  
errors) and you get core dumps or garbage there too for all 4.

So since I've not been able to get test 2 or 3 to core dump (even  
before 250b3c6c) I tend to believe you are correct in that the code  
thinks (incorrectly) that the result should fit within the buffer.  I  
say buffer because the test 3 patch inserts 100 lines into a 6 line  
file and yet it never seems to cause a core dump (even in v1.7.4), so  
the buffer size must be based on the patch, not the original -- I'm  
sure that would make sense if I understood what's going on in the  
apply code.

I did manage to create a test 5 that causes "BUG: postlen = 3036, used  
= 3542", but its verbose output has unfriendly long lines and it's  
fixed by the same counting fix as the others so it doesn't seem  
worthwhile to include it.

-Kyle

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] test: add git apply whitespace expansion tests
  2015-01-19  3:54                       ` Kyle J. McKay
@ 2015-01-21 22:33                         ` Junio C Hamano
  2015-01-22  6:55                           ` Kyle J. McKay
  2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
  0 siblings, 2 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-01-21 22:33 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Git mailing list, Michael Blume

"Kyle J. McKay" <mackyle@gmail.com> writes:

> So since I've not been able to get test 2 or 3 to core dump (even
> before 250b3c6c) I tend to believe you are correct in that the code
> thinks (incorrectly) that the result should fit within the buffer.

Thanks; let me steal your tests when I reroll.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] test: add git apply whitespace expansion tests
  2015-01-21 22:33                         ` Junio C Hamano
@ 2015-01-22  6:55                           ` Kyle J. McKay
  2015-01-22 19:23                             ` Junio C Hamano
  2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
  1 sibling, 1 reply; 26+ messages in thread
From: Kyle J. McKay @ 2015-01-22  6:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git mailing list, Michael Blume

On Jan 21, 2015, at 14:33, Junio C Hamano wrote:

> "Kyle J. McKay" <mackyle@gmail.com> writes:
>
>> So since I've not been able to get test 2 or 3 to core dump (even
>> before 250b3c6c) I tend to believe you are correct in that the code
>> thinks (incorrectly) that the result should fit within the buffer.
>
> Thanks; let me steal your tests when I reroll.

Awesome. :)

But please squash in this tiny change if using the tests verbatim:

On Jan 18, 2015, at 02:49, Kyle J. McKay wrote:

> +#
> +## create test-N, patchN.patch, expect-N files
> +#
> +
> +# test 1
> +printf '\t%s\n' 1 2 3 4 5 6 > before
> +printf '\t%s\n' 1 2 3 > after
> +printf '%64s\n' a b c $x >> after

This line ^ in test 1 should not have a '$x' in it.  It should just be:

> +printf '%64s\n' a b c >> after

The test runs fine currently, but if somehow x should get defined  
before running the tests, test 1 would fail.  All the other '$x' in  
the other tests are correct.

> +printf '\t%s\n' 4 5 6 >> after
> +git diff --no-index before after | \
> +sed -e 's/before/test-1/' -e 's/after/test-1/' > patch1.patch
> +printf '%64s\n' 1 2 3 4 5 6 > test-1
> +printf '%64s\n' 1 2 3 a b c 4 5 6 > expect-1
> +
> +# test 2

-Kyle

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] test: add git apply whitespace expansion tests
  2015-01-22  6:55                           ` Kyle J. McKay
@ 2015-01-22 19:23                             ` Junio C Hamano
  2015-01-23  0:12                               ` Kyle J. McKay
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 19:23 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Git mailing list, Michael Blume

"Kyle J. McKay" <mackyle@gmail.com> writes:

> On Jan 21, 2015, at 14:33, Junio C Hamano wrote:
>
>> "Kyle J. McKay" <mackyle@gmail.com> writes:
>>
>>> So since I've not been able to get test 2 or 3 to core dump (even
>>> before 250b3c6c) I tend to believe you are correct in that the code
>>> thinks (incorrectly) that the result should fit within the buffer.
>>
>> Thanks; let me steal your tests when I reroll.
>
> Awesome. :)
>
> But please squash in this tiny change if using the tests verbatim:

Thanks.  I actually have a question wrt the need for $MAKE_PATCHES.

It would have been more natural to do something like:

test_expect_success 'setup' '
	printf "\t%s\n" 1 2 3 4 5 6 >before &&
	printf "\t%s\n" 1 2 3 >after &&
	printf "%64s\n" a b c >>after &&
	printf "\t%s\n" 4 5 6 >>after &&
	git diff --no-index before after |
	sed -e "s/before/test-1/" -e "s/after/test-1/" >patch1.patch &&
	printf "%64s\n" 1 2 3 4 5 6 >test-1 &&
	printf "%64s\n" 1 2 3 a b c 4 5 6 >expect-1 &&

	printf "\t%s\n" a b c d e f >before &&
	printf "\t%s\n" a b c >after &&
        ...
	cat test-4 >expect-4 &&
	printf "%64s\n" a b c >>expect-4 &&
	while test $x -lt 100
	do
		printf "%63s%02d\n" "" $x >>test-4
		printf "%63s%02d\n" "" $x >>expect-4
		x=$(( $x + 1 ))
	done &&

	git config core.whitespace tab-in-indent,tabwidth=63 &&
        git config apply.whitespace fix
'

test_expect_success 'apply with ws expansion (1)' '
	git apply patch1.patch &&
        test_cmp test-1 expect-1
'

and if you want test files you can just skip tests #2 and later,
without introducing an ad-hoc mechanism like you did.

Was there something more than that that you wanted from
$MAKE_PATCHES?

In any case, here is an update to that sanity check patch to catch
the two cases the BUG did not trigger.

Sometimes the caller under-counted the size of the result but
thought that it would still fit within the original (hence allowing
us to update in-place by passing postlen==0) but the actual result
was larger than the space we have allocated in the postimage,
clobbering the piece of memory after the postimage->buf.


diff --git a/builtin/apply.c b/builtin/apply.c
index 31f8733..3b7ba63 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct image *preimage,
 		ctx++;
 	}
 
+	if (postlen
+	    ? postlen < new - postimage->buf
+	    : postimage->len < new - postimage->buf)
+		die("BUG: caller miscounted postlen: asked %d, orig = %d, used = %d",
+		    (int)postlen, (int) postimage->len, (int)(new - postimage->buf));
+
 	/* Fix the length of the whole thing */
 	postimage->len = new - postimage->buf;
 	postimage->nr -= reduced;

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix
  2015-01-21 22:33                         ` Junio C Hamano
  2015-01-22  6:55                           ` Kyle J. McKay
@ 2015-01-22 22:58                           ` Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 1/4] apply.c: typofix Junio C Hamano
                                               ` (3 more replies)
  1 sibling, 4 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 22:58 UTC (permalink / raw)
  To: git

"git apply --whitespace=fix" used to be able to assume that fixing
errors will always reduce the size by e.g. stripping whitespaces at
the end of lines or collapsing runs of spaces into tabs at the
beginning of lines.  An update to accomodate fixes that lengthens
the result by e.g. expanding leading tabs into spaces were made long
time ago but the logic miscounted the necessary space after such
whitespace fixes, leading to either under-allocation or over-usage
of already allocated space.

The second patch in this series is to illustrate this with a runtime
sanity-check to protect us from future breakage (this is a reroll of
a "how about this" weatherbaloon patch $gmane/262579, with Kyle's
test script).

The third patch corrects the under-counting and makes the new test
pass.

The fourth patch makes us report the fact that we corrected
whitespace errors in the common-context part.  When asked to correct
whitespace errors, and given a patch that has whitespace errors in
the common context (i.e. the lines prefixed with " ") either in the
patch itself or the corresponding part in the file we are patching,
we have been correcting the whitespace errors "while at it" since
around c1beba5b (git-apply --whitespace=fix: fix whitespace fuzz
introduced by previous run, 2008-01-30), but we were not reporting
that we saw a need to fix them.

This is not about the "buffer corruption fix", which is the primary
focus of this series, but it is here because it is related to
whitespace fix.


Junio C Hamano (4):
  typofix
  apply: make update_pre_post_images() sanity check the given postlen
  apply: count the size of postimage correctly
  apply: detect and mark whitespace errors in context lines when fixing

 builtin/apply.c               |  34 ++++++++++--
 t/t4138-apply-ws-expansion.sh | 121 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 3 deletions(-)
 create mode 100755 t/t4138-apply-ws-expansion.sh

-- 
2.3.0-rc1-116-g84c5016

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 1/4] apply.c: typofix
  2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
@ 2015-01-22 22:58                             ` Junio C Hamano
  2015-01-22 23:17                               ` Stefan Beller
  2015-01-22 22:58                             ` [PATCH v2 2/4] apply: make update_pre_post_images() sanity check the given postlen Junio C Hamano
                                               ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 22:58 UTC (permalink / raw)
  To: git

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/apply.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 622ee16..31f8733 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2326,7 +2326,7 @@ static int match_fragment(struct image *img,
 
 	/*
 	 * The hunk does not apply byte-by-byte, but the hash says
-	 * it might with whitespace fuzz. We haven't been asked to
+	 * it might with whitespace fuzz. We weren't asked to
 	 * ignore whitespace, we were asked to correct whitespace
 	 * errors, so let's try matching after whitespace correction.
 	 *
-- 
2.3.0-rc1-116-g84c5016

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 2/4] apply: make update_pre_post_images() sanity check the given postlen
  2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 1/4] apply.c: typofix Junio C Hamano
@ 2015-01-22 22:58                             ` Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 3/4] apply: count the size of postimage correctly Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 4/4] apply: detect and mark whitespace errors in context lines when fixing Junio C Hamano
  3 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 22:58 UTC (permalink / raw)
  To: git

"git apply --whitespace=fix" used to be able to assume that fixing
errors will always reduce the size by e.g. stripping whitespaces at
the end of lines or collapsing runs of spaces into tabs at the
beginning of lines.  An update to accomodate fixes that lengthens
the result by e.g. expanding leading tabs into spaces were made long
time ago but the logic miscounted the necessary space after such
whitespace fixes, leading to either under-allocation or over-usage
of already allocated space.

Illustrate this with a runtime sanity-check to protect us from
future breakage.  The test was stolen from Kyle McKay who helped
to identify the problem.

Helped-by: "Kyle J. McKay" <mackyle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/apply.c               |   6 +++
 t/t4138-apply-ws-expansion.sh | 121 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100755 t/t4138-apply-ws-expansion.sh

diff --git a/builtin/apply.c b/builtin/apply.c
index 31f8733..da6fb35 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct image *preimage,
 		ctx++;
 	}
 
+	if (postlen
+	    ? postlen < new - postimage->buf
+	    : postimage->len < new - postimage->buf)
+		die("BUG: caller miscounted postlen: asked %d, orig = %d, used = %d",
+		    (int)postlen, (int) postimage->len, (int)(new - postimage->buf));
+
 	/* Fix the length of the whole thing */
 	postimage->len = new - postimage->buf;
 	postimage->nr -= reduced;
diff --git a/t/t4138-apply-ws-expansion.sh b/t/t4138-apply-ws-expansion.sh
new file mode 100755
index 0000000..0ffe33f
--- /dev/null
+++ b/t/t4138-apply-ws-expansion.sh
@@ -0,0 +1,121 @@
+#!/bin/sh
+#
+# Copyright (C) 2015 Kyle J. McKay
+#
+
+test_description='git apply test patches with whitespace expansion.'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	#
+	## create test-N, patchN.patch, expect-N files
+	#
+
+	# test 1
+	printf "\t%s\n" 1 2 3 4 5 6 >before &&
+	printf "\t%s\n" 1 2 3 >after &&
+	printf "%64s\n" a b c >>after &&
+	printf "\t%s\n" 4 5 6 >>after &&
+	git diff --no-index before after |
+		sed -e "s/before/test-1/" -e "s/after/test-1/" >patch1.patch &&
+	printf "%64s\n" 1 2 3 4 5 6 >test-1 &&
+	printf "%64s\n" 1 2 3 a b c 4 5 6 >expect-1 &&
+
+	# test 2
+	printf "\t%s\n" a b c d e f >before &&
+	printf "\t%s\n" a b c >after &&
+	n=10 &&
+	x=1 &&
+	while test $x -lt $n
+	do
+		printf "%63s%d\n" "" $x >>after
+		x=$(( $x + 1 ))
+	done &&
+	printf "\t%s\n" d e f >>after &&
+	git diff --no-index before after |
+		sed -e "s/before/test-2/" -e "s/after/test-2/" >patch2.patch &&
+	printf "%64s\n" a b c d e f >test-2 &&
+	printf "%64s\n" a b c >expect-2 &&
+	x=1 &&
+	while test $x -lt $n
+	do
+		printf "%63s%d\n" "" $x >>expect-2
+		x=$(( $x + 1 ))
+	done &&
+	printf "%64s\n" d e f >>expect-2 &&
+
+	# test 3
+	printf "\t%s\n" a b c d e f >before &&
+	printf "\t%s\n" a b c >after &&
+	n=100 &&
+	x=0 &&
+	while test $x -lt $n
+	do
+		printf "%63s%02d\n" "" $x >>after
+		x=$(( $x + 1 ))
+	done &&
+	printf "\t%s\n" d e f >>after &&
+	git diff --no-index before after |
+	sed -e "s/before/test-3/" -e "s/after/test-3/" >patch3.patch &&
+	printf "%64s\n" a b c d e f >test-3 &&
+	printf "%64s\n" a b c >expect-3 &&
+	x=0 &&
+	while test $x -lt $n
+	do
+		printf "%63s%02d\n" "" $x >>expect-3
+		x=$(( $x + 1 ))
+	done &&
+	printf "%64s\n" d e f >>expect-3 &&
+
+	# test 4
+	>before &&
+	x=0 &&
+	while test $x -lt 50
+	do
+		printf "\t%02d\n" $x >>before
+		x=$(( $x + 1 ))
+	done &&
+	cat before >after &&
+	printf "%64s\n" a b c >>after &&
+	while test $x -lt 100
+	do
+		printf "\t%02d\n" $x >>before
+		printf "\t%02d\n" $x >>after
+		x=$(( $x + 1 ))
+	done &&
+	git diff --no-index before after |
+	sed -e "s/before/test-4/" -e "s/after/test-4/" >patch4.patch &&
+	>test-4 &&
+	x=0 &&
+	while test $x -lt 50
+	do
+		printf "%63s%02d\n" "" $x >>test-4
+		x=$(( $x + 1 ))
+	done &&
+	cat test-4 >expect-4 &&
+	printf "%64s\n" a b c >>expect-4 &&
+	while test $x -lt 100
+	do
+		printf "%63s%02d\n" "" $x >>test-4
+		printf "%63s%02d\n" "" $x >>expect-4
+		x=$(( $x + 1 ))
+	done &&
+
+	git config core.whitespace tab-in-indent,tabwidth=63 &&
+	git config apply.whitespace fix
+
+'
+
+# Note that `patch` can successfully apply all patches when run
+# with the --ignore-whitespace option.
+
+for t in 1 2 3 4
+do
+	test_expect_success 'apply with ws expansion (t=$t)' '
+		git apply patch$t.patch &&
+		test_cmp test-$t expect-$t
+	'
+done
+
+test_done
-- 
2.3.0-rc1-116-g84c5016

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 3/4] apply: count the size of postimage correctly
  2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 1/4] apply.c: typofix Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 2/4] apply: make update_pre_post_images() sanity check the given postlen Junio C Hamano
@ 2015-01-22 22:58                             ` Junio C Hamano
  2015-01-22 22:58                             ` [PATCH v2 4/4] apply: detect and mark whitespace errors in context lines when fixing Junio C Hamano
  3 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 22:58 UTC (permalink / raw)
  To: git

Under --whitespace=fix option, match_fragment() function examines
the preimage (the common context and the removed lines in the patch)
and the file being patched and checks if they match after correcting
all whitespace errors.  When they are found to match, the common
context lines in the preimage is replaced with the fixed copy,
because these lines will then be copied to the corresponding place
in the postimage by a later call to update_pre_post_images().  Lines
that are added in the postimage, under --whitespace=fix, have their
whitespace errors already fixed when apply_one_fragment() prepares
the preimage and the postimage, so in the end, application of the
patch can be done by replacing the block of text in the file being
patched that matched the preimage with what is in the postimage that
was updated by update_pre_post_images().

In the earlier days, fixing whitespace errors always resulted in
reduction of size, either collapsing runs of spaces in the indent to
a tab or removing the trailing whitespaces.  These days, however,
some whitespace error fix results in extending the size.

250b3c6c (apply --whitespace=fix: avoid running over the postimage
buffer, 2013-03-22) tried to compute the final postimage size but
its math was flawed.  It counted the size of the block of text in
the original being patched after fixing the whitespace errors on its
lines that correspond to the preimage.  That number does not have
much to do with how big the final postimage would be.

Instead count (1) the added lines in the postimage, whose size is
the same as in the final patch result because their whitespace
errors have already been corrected, and (2) the fixed size of the
lines that are common.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/apply.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index da6fb35..e895340 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2336,6 +2336,23 @@ static int match_fragment(struct image *img,
 	 * ignore whitespace, we were asked to correct whitespace
 	 * errors, so let's try matching after whitespace correction.
 	 *
+	 * While checking the preimage against the target, whitespace
+	 * errors in both fixed, we count how large the corresponding
+	 * postimage needs to be.  The postimage prepared by
+	 * apply_one_fragment() has whitespace errors fixed on added
+	 * lines already, but the common lines were propagated as-is,
+	 * which may become longer when their whitespace errors are
+	 * fixed.
+	 */
+
+	/* First count added lines in postimage */
+	postlen = 0;
+	for (i = 0; i < postimage->nr; i++) {
+		if (!(postimage->line[i].flag & LINE_COMMON))
+			postlen += postimage->line[i].len;
+	}
+
+	/*
 	 * The preimage may extend beyond the end of the file,
 	 * but in this loop we will only handle the part of the
 	 * preimage that falls within the file.
@@ -2343,7 +2360,6 @@ 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;
@@ -2371,7 +2387,10 @@ 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;
+
+		/* Add the length if this is common with the postimage */
+		if (preimage->line[i].flag & LINE_COMMON)
+			postlen += tgtfix.len;
 
 		strbuf_release(&tgtfix);
 		if (!match)
-- 
2.3.0-rc1-116-g84c5016

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 4/4] apply: detect and mark whitespace errors in context lines when fixing
  2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
                                               ` (2 preceding siblings ...)
  2015-01-22 22:58                             ` [PATCH v2 3/4] apply: count the size of postimage correctly Junio C Hamano
@ 2015-01-22 22:58                             ` Junio C Hamano
  3 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 22:58 UTC (permalink / raw)
  To: git

When the incoming patch has whitespace errors in a common context
line (i.e. a line that is expected to be found and is not modified
by the patch), "apply --whitespace=fix" corrects the whitespace
errors the line has, in addition to the whitespace error on a line
that is updated by the patch.  However, we did not count and report
that we fixed whitespace errors on such lines.

[jc: This is iffy.  What if the whitespace error has been fixed in
the target since the patch was written?  A common context line we
see in the patch has errors, and it matches a line in the target
that has the errors already corrected, resulting in no change, which
we may not want to count after all.  On the other hand, we are
reporting whitespace errors _in_ the incoming patch, so...]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/apply.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/apply.c b/builtin/apply.c
index e895340..a51a1b0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1606,6 +1606,9 @@ static int parse_fragment(const char *line, unsigned long size,
 			if (!deleted && !added)
 				leading++;
 			trailing++;
+			if (!apply_in_reverse &&
+			    ws_error_action == correct_ws_error)
+				check_whitespace(line, len, patch->ws_rule);
 			break;
 		case '-':
 			if (apply_in_reverse &&
-- 
2.3.0-rc1-116-g84c5016

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/4] apply.c: typofix
  2015-01-22 22:58                             ` [PATCH v2 1/4] apply.c: typofix Junio C Hamano
@ 2015-01-22 23:17                               ` Stefan Beller
  2015-01-22 23:42                                 ` Junio C Hamano
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Beller @ 2015-01-22 23:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jan 22, 2015 at 2:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/apply.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 622ee16..31f8733 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -2326,7 +2326,7 @@ static int match_fragment(struct image *img,
>
>         /*
>          * The hunk does not apply byte-by-byte, but the hash says
> -        * it might with whitespace fuzz. We haven't been asked to
> +        * it might with whitespace fuzz. We weren't asked to

(not a native speaker):
A quick websearch reveals "We haven't been asked to ..."
is quite commonly used in the web. So it's more of a grammar fix or a
rewording of a comment instead of a typofix(which I assume are miss
spellings)

>          * ignore whitespace, we were asked to correct whitespace
>          * errors, so let's try matching after whitespace correction.
>          *
> --
> 2.3.0-rc1-116-g84c5016
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/4] apply.c: typofix
  2015-01-22 23:17                               ` Stefan Beller
@ 2015-01-22 23:42                                 ` Junio C Hamano
  2015-01-22 23:48                                   ` Stefan Beller
  0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2015-01-22 23:42 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

Stefan Beller <sbeller@google.com> writes:

>> -        * it might with whitespace fuzz. We haven't been asked to
>> +        * it might with whitespace fuzz. We weren't asked to
>
> (not a native speaker):
> A quick websearch reveals "We haven't been asked to ..."
> is quite commonly used in the web. So it's more of a grammar fix or a
> rewording of a comment instead of a typofix(which I assume are miss
> spellings)

Correct; it is not grammo or typo fix, but is a rephrasing to match
the tense with what follows (i.e. we weren't asked to ignore; we
were asked to fix).

>
>>          * ignore whitespace, we were asked to correct whitespace
>>          * errors, so let's try matching after whitespace correction.
>>          *
>> --
>> 2.3.0-rc1-116-g84c5016
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/4] apply.c: typofix
  2015-01-22 23:42                                 ` Junio C Hamano
@ 2015-01-22 23:48                                   ` Stefan Beller
  0 siblings, 0 replies; 26+ messages in thread
From: Stefan Beller @ 2015-01-22 23:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jan 22, 2015 at 3:42 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> -        * it might with whitespace fuzz. We haven't been asked to
>>> +        * it might with whitespace fuzz. We weren't asked to
>>
>> (not a native speaker):
>> A quick websearch reveals "We haven't been asked to ..."
>> is quite commonly used in the web. So it's more of a grammar fix or a
>> rewording of a comment instead of a typofix(which I assume are miss
>> spellings)
>
> Correct; it is not grammo or typo fix, but is a rephrasing to match
> the tense with what follows (i.e. we weren't asked to ignore; we
> were asked to fix).
>

Ok, I realized I missed my conclusion in the first mail: The commit message
and what the patch actually does, do not match.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] test: add git apply whitespace expansion tests
  2015-01-22 19:23                             ` Junio C Hamano
@ 2015-01-23  0:12                               ` Kyle J. McKay
  0 siblings, 0 replies; 26+ messages in thread
From: Kyle J. McKay @ 2015-01-23  0:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git mailing list, Michael Blume

On Jan 22, 2015, at 11:23, Junio C Hamano wrote:
> "Kyle J. McKay" <mackyle@gmail.com> writes:
>
>> On Jan 21, 2015, at 14:33, Junio C Hamano wrote:
>>
>>> "Kyle J. McKay" <mackyle@gmail.com> writes:
>>>
>>>> So since I've not been able to get test 2 or 3 to core dump (even
>>>> before 250b3c6c) I tend to believe you are correct in that the code
>>>> thinks (incorrectly) that the result should fit within the buffer.
>>>
>>> Thanks; let me steal your tests when I reroll.
>>
>> Awesome. :)
>>
>> But please squash in this tiny change if using the tests verbatim:
>
> Thanks.  I actually have a question wrt the need for $MAKE_PATCHES.
>
> It would have been more natural to do something like:
>
> test_expect_success 'setup' '
> 	printf "\t%s\n" 1 2 3 4 5 6 >before &&
> 	printf "\t%s\n" 1 2 3 >after &&
> 	printf "%64s\n" a b c >>after &&
> 	printf "\t%s\n" 4 5 6 >>after &&
> 	git diff --no-index before after |
> 	sed -e "s/before/test-1/" -e "s/after/test-1/" >patch1.patch &&
> 	printf "%64s\n" 1 2 3 4 5 6 >test-1 &&
> 	printf "%64s\n" 1 2 3 a b c 4 5 6 >expect-1 &&
>
> 	printf "\t%s\n" a b c d e f >before &&
> 	printf "\t%s\n" a b c >after &&
>        ...
> 	cat test-4 >expect-4 &&
> 	printf "%64s\n" a b c >>expect-4 &&
> 	while test $x -lt 100
> 	do
> 		printf "%63s%02d\n" "" $x >>test-4
> 		printf "%63s%02d\n" "" $x >>expect-4
> 		x=$(( $x + 1 ))
> 	done &&
>
> 	git config core.whitespace tab-in-indent,tabwidth=63 &&
>        git config apply.whitespace fix
> '
>
> test_expect_success 'apply with ws expansion (1)' '
> 	git apply patch1.patch &&
>        test_cmp test-1 expect-1
> '
>
> and if you want test files you can just skip tests #2 and later,
> without introducing an ad-hoc mechanism like you did.
>
> Was there something more than that that you wanted from
> $MAKE_PATCHES?

Well, see I found t/t4135/make-patches that makes patches for use by  
t4135-apply-weird-filenames.sh and thought perhaps that was the  
approved way to do things.

But then it seemed overkill since making the patches takes so little  
time it didn't seem to warrant a separate directory.  But the ability  
to just make the patch files without requiring any external scripts or  
test framework seemed nice so I added those two extra lines to make it  
possible.

I don't have any strong feelings about it.  The setup test plus 4  
explicit tests looks fine.

> In any case, here is an update to that sanity check patch to catch
> the two cases the BUG did not trigger.
>
> Sometimes the caller under-counted the size of the result but
> thought that it would still fit within the original (hence allowing
> us to update in-place by passing postlen==0) but the actual result
> was larger than the space we have allocated in the postimage,
> clobbering the piece of memory after the postimage->buf.
>
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 31f8733..3b7ba63 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct  
> image *preimage,
> 		ctx++;
> 	}
>
> +	if (postlen
> +	    ? postlen < new - postimage->buf
> +	    : postimage->len < new - postimage->buf)
> +		die("BUG: caller miscounted postlen: asked %d, orig = %d, used =  
> %d",
> +		    (int)postlen, (int) postimage->len, (int)(new - postimage- 
> >buf));
> +
> 	/* Fix the length of the whole thing */
> 	postimage->len = new - postimage->buf;
> 	postimage->nr -= reduced;

Nice.  No more of those bogus results can slip through that somehow  
evade evoking a core dump.

-Kyle

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-01-23  0:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-14 18:20 Segmentation fault in git apply Michael Blume
2015-01-14 18:40 ` Michael Blume
2015-01-14 18:44   ` Michael Blume
2015-01-14 18:48     ` Michael Blume
2015-01-14 18:58       ` Michael Blume
2015-01-14 19:09         ` Michael Blume
2015-01-15  8:26           ` Kyle J. McKay
2015-01-15  9:10             ` Kyle J. McKay
2015-01-16 19:58               ` Junio C Hamano
2015-01-16 23:54                 ` [PATCH] apply: count the size of postimage correctly Junio C Hamano
2015-01-18 10:49                   ` [PATCH] test: add git apply whitespace expansion tests Kyle J. McKay
2015-01-18 22:11                     ` Junio C Hamano
2015-01-19  3:54                       ` Kyle J. McKay
2015-01-21 22:33                         ` Junio C Hamano
2015-01-22  6:55                           ` Kyle J. McKay
2015-01-22 19:23                             ` Junio C Hamano
2015-01-23  0:12                               ` Kyle J. McKay
2015-01-22 22:58                           ` [PATCH v2 0/4] apply --whitespace=fix buffer corruption fix Junio C Hamano
2015-01-22 22:58                             ` [PATCH v2 1/4] apply.c: typofix Junio C Hamano
2015-01-22 23:17                               ` Stefan Beller
2015-01-22 23:42                                 ` Junio C Hamano
2015-01-22 23:48                                   ` Stefan Beller
2015-01-22 22:58                             ` [PATCH v2 2/4] apply: make update_pre_post_images() sanity check the given postlen Junio C Hamano
2015-01-22 22:58                             ` [PATCH v2 3/4] apply: count the size of postimage correctly Junio C Hamano
2015-01-22 22:58                             ` [PATCH v2 4/4] apply: detect and mark whitespace errors in context lines when fixing Junio C Hamano
2015-01-14 18:50 ` Segmentation fault in git apply Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.