Re: [BUG] git diff --no-index segfaults on large files (NULL object database)
- From
Jeff King <peff@peff.net>
- Date
- Apr 4, 2026, 23:09 UTC
- Message-ID
- <20260404230939.GA1360412@coredump.intra.peff.net>
- In-Reply-To
- <4be492cf-347b-4fa5-9bdd-83e7ea8abd92@malon.dev>
On Sun, Apr 05, 2026 at 01:07:27AM +0800, Tian Yuchen wrote:
Show 30 quoted lines
> On 4/5/26 00:53, Luca Stefani wrote:
> > Thanks for looking into it.
> > Locally, I simply check against null storage and it works just fine,
> > flags is always 0 in my experiments so a check against
> > INDEX_WRITE_OBJECT also worked.
> >
> > diff --git a/object-file.c b/object-file.c
> > index f0b029ff0b..68303aa99c 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -1654,7 +1654,8 @@ int index_fd(struct index_state *istate, struct
> > object_id *oid,
> > } else if ((st->st_size >= 0 &&
> > (size_t)st->st_size <=
> > repo_settings_get_big_file_threshold(istate->repo)) ||
> > type != OBJ_BLOB ||
> > - (path && would_convert_to_git(istate, path))) {
> > + (path && would_convert_to_git(istate, path)) ||
> > + !(flags & INDEX_WRITE_OBJECT)) {
> > ret = index_core(istate, oid, fd, xsize_t(st->st_size),
> > type, path, flags);
> > } else {
> >
> > Luca.
>
> That looks good, almost exactly what I was about to send. I was mistaken—
> there isn’t a hash_write_object flag after all ;-)
>
> It looks like this is your first time posting on the Git mailing list. Would
> you consider contributing this (as a patch)?sAlternatively, should the odb transaction system be more forgiving here, and act as a noop when there is no odb?
Bisecting the segfault yields ce1661f9da (odb: add transaction interface, 2025-09-16). Before then, we passed around the object_database itself, saw that its transaction field was NULL, and returned immediately. After that commit, we pass the object_databse to odb_transaction_begin(), which narrows it to odb->sources (which is NULL) while passing to object_file_transaction_begin(). And then that function looks at source->odb to go back to the object_database! But the source being NULL, it segfaults.
Immediately after that commit, the switch from taking an odb to a source is not helpful, though I think eventually it is used to set transaction->base.source. But should the whole thing check for a NULL source and return early? Or otherwise establish some kind of noop transaction?
I haven't thought about the implications (nor even really looked at odb transaction code before). But doing it that way would fix not only this bug, but also other potential bugs throughout the code base when callers start a noop transaction.
+cc Justin (author of ce1661f9da) for any thoughts.
-Peff