git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Mathias Krause" <minipli@grsecurity.net>,
	git@vger.kernel.org,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Subject: Re: [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails
Date: Mon, 30 Jan 2023 10:49:01 -0800	[thread overview]
Message-ID: <xmqq8rhj504i.fsf@gitster.g> (raw)
In-Reply-To: <230130.867cx4s2j4.gmgdl@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Mon, 30 Jan 2023 11:56:42 +0100")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> If I compile libpcre2 with JIT support I'm expecting Git to use that,
> and not fall back in those cases where the JIT engine would give up.

The thing is, the reason why their Git has JIT enabled pcre2 for
many users is not because they choose to compile their own Git for
themselves because they wanted to play with JIT.  To them, their
distro and/or their employer gave a precompiled Git, in the hope
that with JIT would be faster than without JIT when JIT is usable.

In that context, "Speed is a feature in itself" is correct but
"failing fast, forcing the user to try different things" is not a
"Speed" feature at all.  It may be interesting only for those who
are curious to see what pattern was rejected by JIT.  It is
especially true as (1) we are willing to fall back to interpreter in
the SELinux senario, and (2) for normal users who want to use Git,
and not necessarily interested in playing with JIT, there is no
other recourse than prefixing "I do not want this JITted" to their
pattern ANYWAY.  Why fail fast and force the user to take the only
recourse manually, when the machinery already knows what the user's
only viable alternative is (i.e. falling back to the interpreter)?

> Pathological regexes are pretty much only interesting to anyone in the
> context of DoS attacks where they're being used to cause intentional
> slowdowns.

Exactly.

> Here we're discussing an orthagonal case where the "JIT fails", but
> rather than some pathological pattern it's because SELinux has made it
> not work at runtime, and we're trying to tease the two cases apart.

s/and we're/but you're/.  And I do not think you want to.

> I don't think this is plausible at all per the above, and that we
> shouldn't harm realistic use-cases to satisfy hypothetical ones.

To me, what you are advocating is exactly the hypothetical ones that
harm end-users who did not choose to enable JIT themselves.  When JIT
fails for whatever reason (including the SELinux senario) for them,
they do not need to be told by Git failing, when the interpreter can
give them the correct answer.  Wanting to see the result of the
operation they asked Git to do, while allowing Git to use clever
optimizations WHEN ABLE, is what I see as realistic use-cases.


  reply	other threads:[~2023-01-30 18:49 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-16 12:15 [PATCH] grep: fall back to interpreter mode if JIT fails Mathias Krause
2022-12-16 16:12 ` Ævar Arnfjörð Bjarmason
2022-12-16 19:26   ` Mathias Krause
2022-12-16 23:09     ` Junio C Hamano
2022-12-17  2:50       ` Carlo Arenas
2022-12-19  9:00         ` Ævar Arnfjörð Bjarmason
2022-12-20 19:29           ` Mathias Krause
2022-12-20 21:11             ` Ævar Arnfjörð Bjarmason
2023-01-18 14:22               ` Mathias Krause
2023-01-18 15:44                 ` Ævar Arnfjörð Bjarmason
2023-01-19  9:19                   ` Mathias Krause
2022-12-16 22:52 ` Junio C Hamano
2022-12-20 20:40   ` Mathias Krause
2023-01-27 15:49 ` [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails Mathias Krause
2023-01-27 16:34   ` Junio C Hamano
2023-01-27 17:39     ` Junio C Hamano
2023-01-27 18:46       ` Junio C Hamano
2023-01-29 13:37         ` Mathias Krause
2023-01-29 13:36       ` Mathias Krause
2023-01-29 17:15         ` Junio C Hamano
2023-01-30 10:56           ` Ævar Arnfjörð Bjarmason
2023-01-30 18:49             ` Junio C Hamano [this message]
2023-01-31  8:34               ` Ævar Arnfjörð Bjarmason
2023-01-30 11:08           ` Mathias Krause
2023-01-30 18:54             ` Junio C Hamano
2023-01-30 20:08               ` Junio C Hamano
2023-01-30 21:21                 ` Junio C Hamano
2023-01-30 22:30                   ` Ramsay Jones
2023-01-30 23:27                     ` Junio C Hamano
2023-01-31  7:48                   ` Mathias Krause
2023-01-31 16:41                     ` Junio C Hamano
2023-01-31 18:34                       ` Mathias Krause
2023-01-31  7:30                 ` Mathias Krause
2023-01-29 12:28     ` Mathias Krause
2023-01-31 18:56   ` [PATCH v3] " Mathias Krause
2023-01-31 21:05     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq8rhj504i.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=minipli@grsecurity.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).