On Wed, Oct 02, 2024 at 07:56:39PM -0400, Jeff King wrote:
> Good catch. Your analysis looks correct, though I had two questions I
> managed to answer myself:
>
> 1. This seems like an obvious use-after-free problem. Why didn't we
> catch it sooner? I think the answer is that most uses of the
> output_prefix callback happen via diff_line_prefix(), which was not
> broken by 394affd46d. It's only line-log that was affected.
>
> Building with ASan and running:
>
> ./git log --graph -L :diff_line_prefix:diff.c
>
> shows the problem immediately (and that your patch fixes it).
> Should we include a new test in this patch?
I think this would do it if you'd like to squash it in. Note that
showing the actual patch is necessary to trigger the problem (since
that's where we free the prefix buf). Unfortunately it makes the
expected output a lot uglier. :(
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 02d76dca28..950451cf6a 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -337,4 +337,32 @@ test_expect_success 'zero-width regex .* matches any function name' '
test_cmp expect actual
'
+test_expect_success 'show line-log with graph' '
+ qz_to_tab_space >expect <<-EOF &&
+ * $head_oid Modify func2() in file.c
+ |Z
+ | diff --git a/file.c b/file.c
+ | --- a/file.c
+ | +++ b/file.c
+ | @@ -6,4 +6,4 @@
+ | int func2()
+ | {
+ | - return F2;
+ | + return F2 + 2;
+ | }
+ * $root_oid Add func1() and func2() in file.c
+ ZZ
+ diff --git a/file.c b/file.c
+ --- /dev/null
+ +++ b/file.c
+ @@ -0,0 +6,4 @@
+ +int func2()
+ +{
+ + return F2;
+ +}
+ EOF
+ git log --graph --oneline -L:func2:file.c >actual &&
+ test_cmp expect actual
+'
+
test_done
-Peff
