On Mon, Jul 29 2019, Carlo Arenas wrote: > On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason > <avarab@xxxxxxxxx> 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://public-inbox.org/git/20181209230024.43444-2-carenas@xxxxxxxxx/ 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.
