From: "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org> To: Carlo Arenas <email@example.com> Cc: firstname.lastname@example.org, email@example.com Subject: Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE Date: Mon, 29 Jul 2019 14:38:39 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <CAPUEspgQNCENviPYP6X790DvSgj_RpJVo2KP_39voLQnVc65pQ@mail.gmail.com> On Mon, Jul 29 2019, Carlo Arenas wrote: > On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason > <email@example.com> wrote: >> >> On Mon, Jul 29 2019, Carlo Marcelo Arenas Belón wrote: >> >> > PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never >> > had one, forcing the use of JIT if -P was requested. >> >> What's that PCRE1 compile-time flag? > > NO_LIBPCRE1_JIT at GIT compile time (regardless of JIT support in the > PCRE1 library you are using) Ah of course, I was reading this as "regexp compile-time". I.e. something like (*NO_JIT). No *such* thing exists for PCRE v1 JIT AFAIK as exposed by git-grep. >> > After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15) >> > the PCRE2 engine will be used more broadly and therefore adding this >> > knob will give users a fallback for situations like the one observed >> > in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions: >> > >> > $ git grep 'foo bar' >> > fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48' >> > $ git grep -G 'foo bar' >> > fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48' >> > $ git grep -E 'foo bar' >> > fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48' >> > $ git grep -F 'foo bar' >> > fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48' >> >> Yeah that obviously sucks more with ab/no-kwset, but that seems like a >> case where -P would have been completely broken before, and therefore I >> can't imagine the package ever passed "make test". Or is W^X also >> exposed as some run-time option on OpenBSD? > > ironically, you could use PCRE1 since that is not using the JIT fast > path and therefore will fallback automatically to the interpreter ...because OpenBSD PCRE v1 was compiled with --disable-jit before, but their v2 package has --enable-jit, it just doesn't work at all? Is this your custom built git + OpenBSD packages of PCRE coming with the OS? I don't use OpenBSD, but isn't this their recipe? Seems they use "make test", and don't compile with PCRE at all if I'm reading it right: https://github.com/openbsd/ports/blob/master/devel/git/Makefile > there is also a convoluted way to make your binary work by moving > it into a mount point that has been specially exempted from that W^X > restriction. > >> I.e. aside from the merits of such a setting in general these examples >> seem like just working around something that should be fixed at make >> all/test time, or maybe I'm missing something. > > 1) before you could just avoid using -P and still be able to grep > 2) there is no way to tell PCRE2 to get out of the way even if you are > not using -P Right, no arguments at all about ab/no-kwset making this worse (re: your #1). I just really prefer not to expose/document config for what *should* be something purely internal if the X-Y problem is a bug being exposed that we should just fix. Particularly because I think it's a losing battle to provide run-time options for what are surely a *lot* of "make test" failures. If it really is unavoidable to detect this until runtime in some common configurations I have no problem with it, I just haven't encountered that so far. > you are right though that this is not a new problem and was reported > before with patches and the last comment saying a configuration > should be provided. patches = your recent https://firstname.lastname@example.org/ or something earlier? That patch seems sane without having tested it. Seems like the equivalent of what we do with v1 with PCRE2_JIT_COMPLETE. I *am* curious if there's setups where fixing the code for PCRE v1 isn't purely an academic exercise. Is there a reason for why these platforms can't just move to PCRE v2 in principle (dumpster fires in "next" non-withstanding)? >> To the extent that we'd want to make this sort of thing configurable, I >> wonder if a continuation of my (*NO_JIT) patch isn't better, i.e. just >> adding the ability to configure some string we'd inject at the start of >> every pattern. > > looking at the number of lines of code, it would seem the configuration > approach is simpler. > >> That would allow for setting any other number of options in >> pcre2syntax(3) without us needing to carry config for each one, >> e.g. (*LIMIT_HEAP=d), (*LIMIT_DEPTH=d) etc. It does present a larger >> foot-gun surface though... > > the parameters I suspect users might need are not really accessible through > that (ex: jit stacksize). > > it is important to note that currently we are not preventing any user to use > those flags themselves in their patterns either.
next prev parent reply other threads:[~2019-07-29 12:38 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-28 23:54 Carlo Marcelo Arenas Belón 2019-07-29 0:09 ` Carlo Arenas 2019-07-29 4:57 ` Junio C Hamano 2019-07-29 5:29 ` Carlo Arenas 2019-07-29 8:55 ` Ævar Arnfjörð Bjarmason 2019-07-29 10:26 ` Carlo Arenas 2019-07-29 12:38 ` Ævar Arnfjörð Bjarmason [this message] 2019-07-30 13:01 ` Carlo Arenas 2019-07-29 10:59 ` [RFC PATCH v2] " Carlo Marcelo Arenas Belón 2019-07-29 11:33 ` Carlo Arenas 2019-07-29 15:11 ` René Scharfe 2019-07-29 17:47 ` Junio C Hamano 2019-07-30 0:49 ` Carlo Arenas 2019-07-30 17:55 ` René Scharfe 2019-07-31 12:36 ` Johannes Schindelin 2019-07-31 16:18 ` Junio C Hamano 2019-07-31 12:32 ` Johannes Schindelin 2019-07-31 14:57 ` Ævar Arnfjörð Bjarmason 2019-08-04 0:25 ` Carlo Arenas 2019-08-04 3:14 ` [RFC PATCH v3] grep: treat PCRE2 jit compilation memory error as non fatal Carlo Marcelo Arenas Belón 2019-08-04 7:43 ` Carlo Arenas 2019-08-05 20:16 ` Junio C Hamano 2019-07-31 12:24 ` [RFC PATCH] grep: allow for run time disabling of JIT in PCRE Johannes Schindelin
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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE' \ /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
Code repositories for project(s) associated with this 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).