Re: [PATCH v2] line-range: Fix infinite loop bug with degenerate regex
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Dec 7, 2022, 04:52 UTC
- Message-ID
- <221207.86tu27aju3.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20221205193625.2424202-1-lars@oddbit.com>
On Mon, Dec 05 2022, Lars Kellogg-Stedman wrote:
Show 42 quoted lines
> When the -L argument to "git log" is passed the degenerate regular
> expression "$" (as in "-L :$:line-range.c"), this results in an
> infinite loop in find_funcname_matching_regexp() (the function
> iterates through the file correctly, but when it reaches the end of
> the file it matches $ against the empty string, "", and at that points
> loops forever).
>
> Modify the loop condition from while (1) to while (*start) so that the
> loop exits when start is the empty string. In this case, "git log" exits
> with the error:
>
> fatal: -L parameter '$' starting at line 1: no match
>
> Originally reported in <https://stackoverflow.com/q/74690545/147356>.
>
> Signed-off-by: Lars Kellogg-Stedman <lars@oddbit.com>
> ---
> line-range.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/line-range.c b/line-range.c
> index 955a8a9535..bdcb810485 100644
> --- a/line-range.c
> +++ b/line-range.c
> @@ -135,7 +135,7 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
> {
> int reg_error;
> regmatch_t match[1];
> - while (1) {
> + while (*start) {
> const char *bol, *eol;
> reg_error = regexec(regexp, start, 1, match, 0);
> if (reg_error == REG_NOMATCH)
> @@ -161,6 +161,8 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char
> return bol;
> start = eol;
> }
> +
> + return NULL;
> }
>
> static const char *parse_range_funcname(We really should fix this, but why not just count this as a match, rather than erroring out?
That we're mixing up whether '$' always matches here with our iteration loop is our own internal bug, we shouldn't error out on a '$'. It's just a regex that happens to match everything.
It's perfectly OK to provide a zero-width regex in general, e.g. try:
git config --get-regexp '$'
Or for a non-git tool:
grep '$' FILE
I think the real bug here is that we are pointing our regexec() at the haystack of multiple lines as an optimization.
Then in "determine extent of line matched" and "is it a funcname line" assuming that a positive match must be non-zero-width. But that's just because the optimization is leaky.