git show loop

git show loop

[Julia Lawall]

In linux, git show 60d9aa7 -U0 goes into an infinite loop.
The commit log starts with:

commit 60d9aa758c00f20ade0cb1951f6a934f628dd2d7
Merge: b2adf0c 2e16cfc
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed Dec 16 10:23:43 2009 -0800

Everything is fine without the -U0 option.

The commit only contains the following @@ lines:

@@@ -21,7 -21,7 +21,8 @@@
@@@ -33,7 -33,7 +33,8 @@@

That is, both have @@@, not @@.

thanks,
julia

Re: git show loop

[Zbigniew Jędrzejewski-Szmek]

On 03/24/2012 11:05 AM, Julia Lawall wrote:
> In linux, git show 60d9aa7 -U0 goes into an infinite loop.
> The commit log starts with:
> 
> commit 60d9aa758c00f20ade0cb1951f6a934f628dd2d7
> Merge: b2adf0c 2e16cfc
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed Dec 16 10:23:43 2009 -0800
> 
> Everything is fine without the -U0 option.
> 
> The commit only contains the following @@ lines:
> 
> @@@ -21,7 -21,7 +21,8 @@@
> @@@ -33,7 -33,7 +33,8 @@@
> 
> That is, both have @@@, not @@.
> 
> thanks,
> julia
> -- 
> 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
> 
Yeah, I can confirm that with git version 1.7.10.rc1.225.gba57e.

This doesn't happen everytime, some there's some race condition.

unsigned long la; /* lookahead */
is either 49 (works fine) or 18446744073709551615 (seems to hang).

Zbyszek

(gdb) bt
#0  make_hunks (sline=0xb20360, cnt=161, num_parent=2, dense=1) at combine-diff.c:426
#1  0x000000000049295a in show_patch_diff (elem=0x8d0020, num_parent=2, dense=1, working_tree_file=0, rev=0x7fffffffd7a0) at combine-diff.c:961
#2  0x00000000004933cd in diff_tree_combined (sha1=0x82fc74 "`٪u\214", parents=0x7fffffffd620, dense=1, rev=0x7fffffffd7a0)
    at combine-diff.c:1185
#3  0x000000000049348f in diff_tree_combined_merge (commit=0x82fc70, dense=1, rev=0x7fffffffd7a0) at combine-diff.c:1209
#4  0x00000000004c6913 in do_diff_combined (opt=0x7fffffffd7a0, commit=0x82fc70) at log-tree.c:731
#5  0x00000000004c6a2f in log_tree_diff (opt=0x7fffffffd7a0, commit=0x82fc70, log=0x7fffffffd6d0) at log-tree.c:764
#6  0x00000000004c6b7f in log_tree_commit (opt=0x7fffffffd7a0, commit=0x82fc70) at log-tree.c:808
#7  0x0000000000444aa3 in cmd_log_walk (rev=0x7fffffffd7a0) at builtin/log.c:300
#8  0x00000000004455ca in cmd_show (argc=3, argv=0x7fffffffe050, prefix=0x0) at builtin/log.c:510
#9  0x0000000000405762 in run_builtin (p=0x795140, argc=3, argv=0x7fffffffe050) at git.c:308
#10 0x00000000004058f5 in handle_internal_command (argc=3, argv=0x7fffffffe050) at git.c:467
#11 0x0000000000405a0f in run_argv (argcp=0x7fffffffdf3c, argv=0x7fffffffdf30) at git.c:513
#12 0x0000000000405b9c in main (argc=3, argv=0x7fffffffe050) at git.c:588

Re: git show loop

[Zbigniew Jędrzejewski-Szmek]

On 03/24/2012 12:50 PM, Zbigniew Jędrzejewski-Szmek wrote:
> On 03/24/2012 11:05 AM, Julia Lawall wrote:
>> In linux, git show 60d9aa7 -U0 goes into an infinite loop.

Bisect points to
commit b810cbbde9232cbe9a3841edccc5b606bbd3a82e (refs/bisect/bad)
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Jul 22 14:48:29 2009 -0700

     diff --cc: a lost line at the beginning of the file is shown 
incorrectly

Zbyszek

> Yeah, I can confirm that with git version 1.7.10.rc1.225.gba57e.
>
> This doesn't happen everytime, some there's some race condition.
>
> unsigned long la; /* lookahead */
> is either 49 (works fine) or 18446744073709551615 (seems to hang).
I don't know the code enough to understand this, but obviously la is set 
to 0, and then decremented in the loop.

> Zbyszek
>
> (gdb) bt
> #0  make_hunks (sline=0xb20360, cnt=161, num_parent=2, dense=1) at combine-diff.c:426
> #1  0x000000000049295a in show_patch_diff (elem=0x8d0020, num_parent=2, dense=1, working_tree_file=0, rev=0x7fffffffd7a0) at combine-diff.c:961
> #2  0x00000000004933cd in diff_tree_combined (sha1=0x82fc74 "`٪u\214", parents=0x7fffffffd620, dense=1, rev=0x7fffffffd7a0)
>      at combine-diff.c:1185
> #3  0x000000000049348f in diff_tree_combined_merge (commit=0x82fc70, dense=1, rev=0x7fffffffd7a0) at combine-diff.c:1209
> #4  0x00000000004c6913 in do_diff_combined (opt=0x7fffffffd7a0, commit=0x82fc70) at log-tree.c:731
> #5  0x00000000004c6a2f in log_tree_diff (opt=0x7fffffffd7a0, commit=0x82fc70, log=0x7fffffffd6d0) at log-tree.c:764
> #6  0x00000000004c6b7f in log_tree_commit (opt=0x7fffffffd7a0, commit=0x82fc70) at log-tree.c:808
> #7  0x0000000000444aa3 in cmd_log_walk (rev=0x7fffffffd7a0) at builtin/log.c:300
> #8  0x00000000004455ca in cmd_show (argc=3, argv=0x7fffffffe050, prefix=0x0) at builtin/log.c:510
> #9  0x0000000000405762 in run_builtin (p=0x795140, argc=3, argv=0x7fffffffe050) at git.c:308
> #10 0x00000000004058f5 in handle_internal_command (argc=3, argv=0x7fffffffe050) at git.c:467
> #11 0x0000000000405a0f in run_argv (argcp=0x7fffffffdf3c, argv=0x7fffffffdf30) at git.c:513
> #12 0x0000000000405b9c in main (argc=3, argv=0x7fffffffe050) at git.c:588
> --
> 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
>

Re: git show loop

[René Scharfe]

Am 24.03.2012 13:42, schrieb Zbigniew Jędrzejewski-Szmek:
> On 03/24/2012 12:50 PM, Zbigniew Jędrzejewski-Szmek wrote:
>> On 03/24/2012 11:05 AM, Julia Lawall wrote:
>>> In linux, git show 60d9aa7 -U0 goes into an infinite loop.
> 
> Bisect points to
> commit b810cbbde9232cbe9a3841edccc5b606bbd3a82e (refs/bisect/bad)
> Author: Junio C Hamano<gitster@pobox.com>
> Date:   Wed Jul 22 14:48:29 2009 -0700
> 
>       diff --cc: a lost line at the beginning of the file is shown
> incorrectly

That's what I found as well, but I don't understand how this relates to
the patch below, which should fix the issue.  Valgrind told me where to
look instead.

-- >8 --
Subject: [PATCH] combine-diff: fix loop index underflow

If both la and context are zero at the start of the loop, la wraps around
and we end up reading from memory far away.  Skip the loop in that case
instead.

Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 combine-diff.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/combine-diff.c b/combine-diff.c
index a2e8dcf..9786680 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -423,7 +423,7 @@ static int make_hunks(struct sline *sline, unsigned long cnt,
 						     hunk_begin, j);
 				la = (la + context < cnt + 1) ?
 					(la + context) : cnt + 1;
-				while (j <= --la) {
+				while (la && j <= --la) {
 					if (sline[la].flag & mark) {
 						contin = 1;
 						break;
-- 
1.7.9.2

Re: git show loop

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Subject: [PATCH] combine-diff: fix loop index underflow
>
> If both la and context are zero at the start of the loop, la wraps around
> and we end up reading from memory far away.  Skip the loop in that case
> instead.
>
> Reported-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---

Thanks. This dates back to 1.3.0.

Will queue.

>  combine-diff.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index a2e8dcf..9786680 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -423,7 +423,7 @@ static int make_hunks(struct sline *sline, unsigned long cnt,
>  						     hunk_begin, j);
>  				la = (la + context < cnt + 1) ?
>  					(la + context) : cnt + 1;
> -				while (j <= --la) {
> +				while (la && j <= --la) {
>  					if (sline[la].flag & mark) {
>  						contin = 1;
>  						break;