From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 07 Oct 2026 05:29:26 GMT
Subject: Re: [PATCH v2 2/2] packfile: fix corruption due to stale delta base cache entries
Message-ID: <asXYtkkRs6kvEjBJ@pks.im>
In-Reply-To: <xmqqse2id2kq.fsf@gitster.g>

On Tue, Oct 06, 2026 at 12:57:41PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Note that the added test reliably reproduces the above bug on my machine
> > that uses NixOS at c59305bab206 (cosmic-applets: add missing runtime
> > dependency (#566040), 2026-10-01) with glibc 2.44-25. But as we rely on
> > specific allocation behaviour of glibc it is very likely that the test
> > will not work on other platforms.
> 
> In other words, the test will not detect the bug, when the fix is
> reverted, unless the glibc allocator is used?

It is specific to memory allocation patterns and thus to the platform,
yes. But I just confirmed that the test also fails on for example Alpine
Linux, so it even reproduces with musl libc. I haven't tested any other
platforms though.

> Adding an unreliable reproducer for a bug that is already fixed may be
> of dubious value.  However, even if the test is unreliable (since
> other allocators might hide the bug when the fix is reverted), it may
> be OK as long as it catches the bug on widely used configurations and
> does not trigger false positives.

Yeah, it at least catches the bug on some systems. And I think even if
it eventually didn't anymore, it exercises a part of our system (doing
submodule merges across many submodules) that wasn't previously
exercised, I think. So it would still have some value there.

> On the other hand, the earlier suggestion to write custom low-level
> code to simulate a colliding allocation address somehow smells like a
> maintenance burden to me.

Agreed. It simply is too much boilerplate for too specific a failure, if
you ask me.

Patrick