threads / discuss / 30047

git show loop

Subject: git show loop

## tl;dr

5 messages between Mar 24, 2012 and Mar 26, 2012.

replies: 4people: 4as markdown or json

Julia Lawall· Mar 24, 2012, 10:05 UTC · lore

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

Zbigniew Jędrzejewski-Szmek· Mar 24, 2012, 11:50 UTC · re: Julia Lawall · lore

Re: git show loop

On 03/24/2012 11:05 AM, Julia Lawall wrote:
Show 24 quoted lines
> 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
Zbigniew Jędrzejewski-Szmek· Mar 24, 2012, 12:42 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: git show loop

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
Show 6 quoted lines
> 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.

Show 22 quoted lines
> 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
>
René Scharfe· Mar 24, 2012, 15:18 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: git show loop

Am 24.03.2012 13:42, schrieb Zbigniew Jędrzejewski-Szmek:
Show 11 quoted lines
> 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
Junio C Hamano· Mar 26, 2012, 04:34 UTC · re: René Scharfe · lore

Re: git show loop

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
Show 9 quoted lines
> 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.
Show 16 quoted lines
>  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;

← back to recent threads