Le 11 nov. 2025 à 17:37, Jeff King [off-list ref] a écrit :

Given a set of attribute macros like:

  [attr]a1 a2
  [attr]a2 a3
  ...
  [attr]a300000 -text
  file a1

expanding the attributes for "file" requires expanding "a1" to "a2",
"a2" to "a3", and so on until hitting a non-macro expansion ("-text", in
this case). We implement this via recursion: fill_one() calls
macroexpand_one(), which then recurses back to fill_one(). As a result,
very deep macro chains like the one above can run out of stack space and
cause us to segfault.

The required stack space is fairly small; I needed on the order of
200,000 entries to get a segfault on Linux. So it's unlikely anybody
would hit this accidentally, leaving only malicious inputs. There you
can easily construct a repo which will segfault on clone (we look at
attributes during the checkout step, but you'd see the same trying to do
other operations, like diff in a bare repo). It's mostly harmless, since
anybody constructing such a repo is only preventing victims from cloning
their evil garbage, but it could be a nuisance for hosting sites.

One option to prevent this is to limit the depth of recursion we'll
allow. This is conceptually easy to implement, but it raises other
questions: what should the limit be, and do we need a configuration knob
for it?

The recursion here is simple enough that we can avoid those questions by
just converting it to iteration instead. Rather than iterate over the
states of a match_attr in fill_one(), we'll put them all in a queue, and
the expansion of each can add to the queue rather than recursing. Note
that this is a LIFO queue in order to keep the same depth-first order we
did with the recursive implementation. I've avoided using the word
"stack" in the code because the term is already heavily used to refer to
the stack of .gitattribute files that matches the tree structure of the
repository.

Worth catching, and I agree with your choice of in-memory iteration over tunable depth.

My knowledge on memory models is a bit weak and I didn’t check directly, but are we implicitly assuming that we are less likely to run out of heap memory in such an evil case? In effect I suppose we’re turning a stack overflow segfault into an OOM error?

That seems like a fine assumption to me (I’m used to languages where the call stack lives more or less efficiently on the heap), just wanted to check my understanding. 

The memory use has to go somewhere ;) presuming there’s no good way to only keep the relevant entries in memory, since I can of course find a large example that also uses each intermediate macro, so the code would need to get a lot smarter to collapse equivalence classes, prune unused paths, etc., which seems like a poor investment for what AFAICT is a little-used feature*.

*I love it, and use it for custom diff drivers and the baked in hunk headers. But the diff definitions aren’t easily shareable, and I have to be aware of when to turn them off, so I end up not sharing the corresponding attributes either.