Show 40 quoted lines
> Le 11 nov. 2025 à 17:37, Jeff King <peff@peff.net> 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.
Ben Knoble <ben.knoble@gmail.com>