All of lore.kernel.org
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] fix segfault with git log -c --follow
Date: Tue, 28 May 2013 00:49:57 +0200	[thread overview]
Message-ID: <20130527224957.GA7492@ecki> (raw)

In diff_tree_combined we make a copy of diffopts. In
try_to_follow_renames, called via diff_tree_sha1, we free and
re-initialize diffopts->pathspec->items. Since we did not make a deep
copy of diffopts in diff_tree_combined, the original diffopts does not
get the update. By the time we return from diff_tree_combined,
rev->diffopt->pathspec->items points to an invalid memory address. We
get a segfault next time we try to access that pathspec.

Instead, along with the copy of diffopts, make a copy pathspec->items as
well.

We would also have to make a copy of pathspec->raw to keep it consistent
with pathspec->items, but nobody seems to rely on that.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

I wonder why I get a segfault from this so reliably, since it's not
actually a null-pointer dereference. Maybe this is gcc 4.8 doing
something different from previous versions?

Also, I have absolutely no confidence in my understanding of this code.
This is the first solution that came to mind, and could be totally
wrong. I just figured a patch is better than no patch.

Clemens

 combine-diff.c |  3 +++
 t/t4202-log.sh | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/combine-diff.c b/combine-diff.c
index 77d7872..8825604 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1302,6 +1302,7 @@ void diff_tree_combined(const unsigned char *sha1,
 	int i, num_paths, needsep, show_log_first, num_parent = parents->nr;
 
 	diffopts = *opt;
+	diff_tree_setup_paths(diffopts.pathspec.raw, &diffopts);
 	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	DIFF_OPT_SET(&diffopts, RECURSIVE);
 	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
@@ -1372,6 +1373,8 @@ void diff_tree_combined(const unsigned char *sha1,
 		paths = paths->next;
 		free(tmp);
 	}
+
+	diff_tree_release_paths(&diffopts);
 }
 
 void diff_tree_combined_merge(const struct commit *commit, int dense,
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 9243a97..cb03d28 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -530,6 +530,20 @@ test_expect_success 'show added path under "--follow -M"' '
 	)
 '
 
+test_expect_success 'git log -c --follow' '
+	test_create_repo follow-c &&
+	(
+		cd follow-c &&
+		test_commit initial file original &&
+		git rm file &&
+		test_commit rename file2 original &&
+		git reset --hard initial &&
+		test_commit modify file foo &&
+		git merge -m merge rename &&
+		git log -c --follow file2
+	)
+'
+
 cat >expect <<\EOF
 *   commit COMMIT_OBJECT_NAME
 |\  Merge: MERGE_PARENTS
-- 
1.8.2.3

             reply	other threads:[~2013-05-27 23:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-27 22:49 Clemens Buchacher [this message]
2013-05-28 17:22 ` [PATCH] fix segfault with git log -c --follow Junio C Hamano
2013-05-28 22:54   ` Clemens Buchacher
2013-05-28 23:24     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130527224957.GA7492@ecki \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.