[PATCH v2 3/3] grep: avoid leak of chartables in PCRE2

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= 94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced a small memory leak visible with valgrind in t7813. Complete the creation of a PCRE2 specific variable that was missing from the original change and free the

[PATCH v2 2/3] grep: make PCRE2 aware of custom allocator

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= 94da9193a6 (grep: add support for PCRE v2, 2017-06-01) didn't include a way to override the system allocator, and so it is incompatible with custom allocators (e.g. nedmalloc). This problem became obvious when we tried to plug a memory

[PATCH v2 1/3] grep: make PCRE1 aware of custom allocator

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) didn't include a way to override the system alocator, and so it is incompatible with USE_NED_ALLOCATOR as reported by Dscho[1] (in similar code from PCRE2) Note that nedmal

[PATCH 1/3] grep: make PCRE1 aware of custom allocator

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) didn't include a way to override the system alocator, and so it is incompatible with USE_NED_ALLOCATOR as reported by Dscho[1] (in similar code from PCRE2) Note that nedmal

[PATCH 2/3] grep: make PCRE2 aware of custom allocator

From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= 94da9193a6 (grep: add support for PCRE v2, 2017-06-01) didn't include a way to override the system allocator, and so it is incompatible with custom allocators (e.g. nedmalloc). This problem became obvious when we tried to plug a memory

Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator

y new shiny windows dev box hasn't seen much action. will update here and in github shortly (which might mean up to this weekend, being realistic), but should be better code (since it is mostly Rene's) and better tested that way and hopefully won't cause more breakage (specially in Windows, sorry Dscho) Carlo

Re: [PATCH v2] grep: skip UTF8 checks explicitly

ping any feedback on code/approach highly appreciated Carlo On Wed, Aug 28, 2019 at 9:57 AM Carlo Arenas wrote: > > FWIW, the changes in grep.h are IMHO optional and hadn't been really > tested as I couldn't find a version of PCRE1 old enough to trigger it > but I am ho

Re: Git in Outreachy December 2019?

e same level both technically and resource wise (after all she wrote the documentation on how to contribute and I only found a silly bug on her code that will prevent really obsolete systems to compile and didn't even wrote the patch) Carlo

Re: Git in Outreachy December 2019?

I'm interested to mentor/help too, but I am definitely not a (some people would even argue against "reliable") contributor but I might be better than nothing and could pass my "lessons learned" along, so hopefully next contributors are less of a pain to deal with than I am Carlo

Re: [PATCH v2] grep: skip UTF8 checks explicitly

st a non JIT enabled PCRE1): $ ./git-grep -P 'Nguyễn Thái.Ngọc' .mailmap:Nguyễn Thái Ngọc Duy fatal: pcre_exec failed with error code -10 Carlo [0] 685668faaa (grep: stop using a custom JIT stack with PCRE v1, 2019-07-26) [1] https://public-inbox.org/git/87lfwn70nb@evledraar.gmail.com/ [2] https://github.com/carenas/git/tree/pcre1-cleanup

[PATCH v2] grep: skip UTF8 checks explicitly

thout the risks) instead in the future Helped-by: Johannes Schindelin Helped-by: Eric Sunshine Helped-by: Ævar Arnfjörð Bjarmason Suggested-by: Junio C Hamano Signed-off-by: Carlo Marcelo Arenas Belón --- V2: * drop PCRE2 support * add backward compatibility define grep.c | 4 ++-- grep.h |

Re: What's cooking in git.git (Aug 2019, #06; Fri, 23)

rsions of v6 that could be used as a reroll if you would rather do that, but will still not address the full set of issues that will be required to stabilize it into next. Carlo Carlo

Re: [RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator

RC (my preference) or dropping this series from pu if that would help clear the ugliness of pu for windows hopefully this won't be repeated now that I am aware of github's integration and have my own (albeit very slow) windows environment as well. Carlo [0] https://github.com/git/git/co

Re: [PATCH 1/2] grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1

On Mon, Aug 26, 2019 at 11:54 AM Junio C Hamano wrote: > > Carlo Marcelo Arenas Belón writes: > > > e87de7cab4 ("grep: un-break building with PCRE < 8.32", 2017-05-25) > > added a restriction for JIT support that is no longer needed after > > pcre_

Re: [PATCH] grep: under --debug, show whether PCRE JIT is enabled

On Mon, Aug 26, 2019 at 9:02 AM Junio C Hamano wrote: > > Carlo Arenas writes: > > > ... but > > ab/pcre-jit-fixes and UTF-8 validation are likely to make that more > > difficult (even if it is a mostly self inflicted wound AFAIK) > > Hmm, in what way? Do yo

Re: [PATCH] grep: under --debug, show whether PCRE JIT is enabled

On Mon, Aug 26, 2019 at 7:29 AM Johannes Schindelin wrote: > > On Sat, 24 Aug 2019, Carlo Arenas wrote: > > > On Mon, Aug 19, 2019 at 3:23 PM Junio C Hamano wrote: > > > > > > There may be others I am missing. > > > > should we still support PCRE1

[PATCH 0/2] PCRE1 cleanup

and to make sure the solution from 2fff1e196d (grep: fix NO_LIBPCRE1_JIT to fully disable JIT, 2017-11-12) is working as expected. Carlo Marcelo Arenas Belón (2): grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1 grep: refactor and simplify PCRE1 support Makefile | 9 ++--- grep.c |

[PATCH 1/2] grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1

d reliably to enforce JIT doesn't get used. Signed-off-by: Carlo Marcelo Arenas Belón --- Makefile | 9 ++--- grep.h | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index f58bf14c7b..3f78ef942f 100644 --- a/Makefile +++ b/Makefile @@ -

[PATCH 2/2] grep: refactor and simplify PCRE1 support

pcre_study with the right parameter after JIT support has been confirmed and unless it was requested to be disabled with NO_LIBPCRE1_JIT Signed-off-by: Carlo Marcelo Arenas Belón --- grep.c | 16 ++-- grep.h | 9 - 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a

Re: [PATCH] grep: under --debug, show whether PCRE JIT is enabled

t would be part of a reroll from that series. Carlo [1] https://public-inbox.org/git/CAPUEspgStVxL=0soag82vxrmrglsekdhrt-xq6ncw1snq7n...@mail.gmail.com/ [2] https://public-inbox.org/git/20190726202642.7986-1-care...@gmail.com/ [3] https://public-inbox.org/git/20190721194052.15440-1-care...@gmail.com/

[PATCH v3] http: use xmalloc with cURL

USE_NED_ALLOCATOR=YesPlease is used (most likely in Windows) Signed-off-by: Carlo Marcelo Arenas Belón --- v3: proper use of #elif (Thanks Junio) v2: keep all curl_global_init ifdefs together (as suggested by Junio) http.h | 4 1 file changed, 4 insertions(+) diff --git a/http.h b/http.h index b429f1cf04

[PATCH v2] http: use xmalloc with cURL

USE_NED_ALLOCATOR=YesPlease is used (most likely in Windows) Signed-off-by: Carlo Marcelo Arenas Belón --- Notes: v2: keep all global_init ifdefs together (as suggested by Junio) http.h | 4 1 file changed, 4 insertions(+) diff --git a/http.h b/http.h index b429f1cf04..20a2030c94 100644 --- a/http.h

[PATCH] http: use xmalloc with cURL

USE_NED_ALLOCATOR=YesPlease is used (most likely in Windows) Signed-off-by: Carlo Marcelo Arenas Belón --- This doesn't conflict with anything and was originally based on maint (so it applies cleanly also to master and next), but is now rebased on top of jk/drop-release-pack-memory so the final product wou

Re: What's cooking in git.git (Aug 2019, #04; Wed, 14)

t; + grep: fix worktree case in submodules > > There is a new version of this patch here[1], addressing the comments > you and Christian made. since it is already in next, would be better to submit a patch on top of the current topic instead (more details in Documentation/SubmittingPatches). Carlo

Re: [PATCH] SQUASH

sted[1] as can be seen in github where Dscho integration did most of the heavy lifting with Windows Carlo [1] https://github.com/git/git/pull/627

Re: [PATCH] SQUASH

reported[1] before but wasn't picked up and that ironically might explain the last segfaults from my old V4 Carlo [1] https://public-inbox.org/git/20190721194052.15440-1-care...@gmail.com/

Re: [RFC PATCH] http: use xmalloc with cURL

On Sun, Aug 11, 2019 at 4:20 AM Johannes Schindelin wrote: > On Sat, 10 Aug 2019, Carlo Marcelo Arenas Belón wrote: > > cURL is very strict about its allocator being thread safe and so that might > > be an issue to look for. > > Looks good to me. it is not ready yet for usi

Re: [RFC PATCH] http: use xmalloc with cURL

On Sat, Aug 10, 2019 at 3:17 PM Daniel Stenberg wrote: > > On Sat, 10 Aug 2019, Carlo Marcelo Arenas Belón wrote: > > > tested in macOS 10.14.6 with the system provided cURL (7.54.0) > > and latest (7.65.3) and while the API used should be added starting around > > 7.

[RFC PATCH] http: use xmalloc with cURL

ismatch is unlikely to be found while testing because of that. cURL is very strict about its allocator being thread safe and so that might be an issue to look for. Signed-off-by: Carlo Marcelo Arenas Belón --- http.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/http.h b/http.h index

Re: [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes

On Sat, Aug 10, 2019 at 12:57 AM René Scharfe wrote: > > Am 10.08.19 um 05:05 schrieb Carlo Arenas: > > in macOS (obviously testing without NED) the following is the output > > of (a hacked version) of p7801 for maint (against chromium's > > repository), with René&#

Re: [PATCH] SQUASH

On Sat, Aug 10, 2019 at 12:57 AM René Scharfe wrote: > > Am 10.08.19 um 05:03 schrieb Carlo Marcelo Arenas Belón: > > Make using a general context (that is only needed with NED) to depend > > on NED being selected at compile time. > > A custom general context is needed

Re: [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes

with low number of cores), which is why testing for performance regressions in windows is strongly encouraged, regardless Carlo [1] https://public-inbox.org/git/capuespgh1v1zo7smzqwcv4rx9pkvklv84gdsfcpdt7lffqx...@mail.gmail.com/ [2] https://public-inbox.org/git/20190810030315.7519-1-care...@gmail.com/

[PATCH] SQUASH

Make using a general context (that is only needed with NED) to depend on NED being selected at compile time. the compile_context could be also make conditional but it gets ugly really fasts with #ifdef --- Makefile | 2 +- grep.c | 4 2 files changed, 5 insertions(+), 1 deletion(-) diff -

Re: [RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes

might be a red herring as well, but would be nice if someone with windows would definitely get some numbers so we can't be sure there were truly no regressions Carlo [1] https://github.com/carenas/git/tree/pcre2-chartables-leakfix

Re: [RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed)

On Thu, Aug 8, 2019 at 1:21 PM Johannes Schindelin wrote: > On Tue, 6 Aug 2019, Carlo Marcelo Arenas Belón wrote: > > Eitherway, since I am unable to replicate the original bug or take > > performance numbers in a representative environment without Windows > > this is onl

[RFC PATCH v5 3/3] grep: avoid leak of chartables in PCRE2

94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced a small memory leak visible with valgrind in t7813. Complete the creation of a PCRE2 specific variable that was missing from the original change and free the generated table just like it is done for PCRE1. Signed-off

[RFC PATCH v5 2/3] grep: make PCRE2 aware of custom allocator

t of the users of it added later. Helped-by: René Scharfe Reported-by: Johannes Schindelin Signed-off-by: Carlo Marcelo Arenas Belón --- builtin/grep.c | 1 + grep.c | 56 +- grep.h | 1 + 3 files changed, 57 insertions(+), 1 delet

[RFC PATCH v5 0/3] grep: almost no more leaks, hopefully no crashes

@gmail.com/ [3] https://public-inbox.org/git/7ec60d57-9940-35f2-f7b5-c87d4dc7c...@web.de/ Carlo Marcelo Arenas Belón (3): grep: make PCRE1 aware of custom allocator grep: make PCRE2 aware of custom allocator grep: avoid leak of chartables in PCRE2 Makefile | 2 +- bu

[RFC PATCH v5 1/3] grep: make PCRE1 aware of custom allocator

be good idea to test it in a platform where NED might have a positive impact (ex: Windows 7) [1] https://public-inbox.org/git/pull.306.git.gitgitgad...@gmail.com Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- Makefile | 2 +- grep.c | 10 ++ 2 files changed

Re: [PATCH 1/3] grep: make PCRE1 aware of custom allocator

On Thu, Aug 8, 2019 at 6:55 AM Johannes Schindelin wrote: > On Tue, 6 Aug 2019, Carlo Marcelo Arenas Belón wrote: > > > 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) didn't include a way > > to override the system allocator, and so it is incompatible with >

Re: [PATCH 2/3] grep: make PCRE2 aware of custom allocator

On Thu, Aug 8, 2019 at 6:57 AM Johannes Schindelin wrote: > On Tue, 6 Aug 2019, Carlo Marcelo Arenas Belón wrote: > > Suggested-by: Johannes Schindelin > > Actually not so much suggested by me, as your patch still causes > crashes (mine didn't): the "equivalent"

Re: [RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator

On Thu, Aug 8, 2019 at 12:07 AM René Scharfe wrote: > > Am 08.08.19 um 04:35 schrieb Carlo Arenas: > > On Wed, Aug 7, 2019 at 6:03 AM René Scharfe wrote: > >> > >> Am 07.08.19 um 11:49 schrieb Carlo Arenas: > >>> was hoping will perform better but it se

Re: [RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator

On Wed, Aug 7, 2019 at 6:03 AM René Scharfe wrote: > > Am 07.08.19 um 11:49 schrieb Carlo Arenas: > > was hoping will perform better but it seems that testing can be done > > only in windows > > nedmalloc works on other platforms as well. I meant[1] it works reliably

[RFC PATCH v4 1/3] grep: make PCRE1 aware of custom allocator

be good idea to test it in a platform where NED might have a positive impact (ex: Windows 7) [1] https://public-inbox.org/git/pull.306.git.gitgitgad...@gmail.com Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- Makefile | 2 +- grep.c | 10 ++ 2 files changed

[RFC PATCH v4 3/3] grep: avoid leak of chartables in PCRE2

94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced a small memory leak visible with valgrind in t7813. Complete the creation of a PCRE2 specific variable that was missing from the original change and free the generated table just like it is done for PCRE1. Signed-off

[RFC PATCH v4 2/3] grep: make PCRE2 aware of custom allocator

rest of the users of it added later. Helped-by: René Scharfe Reported-by: Johannes Schindelin Signed-off-by: Carlo Marcelo Arenas Belón --- V4: * use xmalloc instead as suggested by René and Junio * "fix" for regression in t7816 as reported by René builtin/grep.c | 1 + builtin/log.

[RFC PATCH v4 0/3] grep: no leaks or crashes (windows testing needed)

) or (hopefully not) * ignore the original leak (maybe with an UNLEAK) as René suggested [3] * discard this work and just use Dscho's fix (probably with some improvements) Carlo Marcelo Arenas Belón (3): grep: make PCRE1 aware of custom allocator grep: make PCRE2 aware of custom alloc

Re: [PATCH] cc5e1bf992 (gettext: avoid initialization if the locale dir is not present, 2018-04-21) changed the way the gettext initialization is done skipping most of it for performance reasons if th

d up doing that more often, because of this. Carlo PS. apologize for the badly formatted patch, don't think this needs to be fixed in maint, eventhough the patch is based on it with the most likely to be affected being developers (or automated tests)

[PATCH] cc5e1bf992 (gettext: avoid initialization if the locale dir is not present, 2018-04-21) changed the way the gettext initialization is done skipping most of it for performance reasons if the lo

re keeping most of the performance improvement. Signed-off-by: Carlo Marcelo Arenas Belón --- gettext.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/gettext.c b/gettext.c index d4021d690c..3ecf456f74 100644 --- a/gettext.c +++ b/gettext.c @@ -69,7 +69,14 @@

Re: [RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator

On Tue, Aug 6, 2019 at 10:38 PM René Scharfe wrote: > > Am 06.08.19 um 18:36 schrieb Carlo Marcelo Arenas Belón: > > Move some of the logic that was before done per thread (in the workers) > > into an earlier phase to avoid degrading performance > > Which logic is moved?

[RFC PATCH v3 3/3] grep: avoid leak of chartables in PCRE2

94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced a small memory leak visible with valgrind in t7813. Complete the creation of a PCRE2 specific variable that was missing from the original change and free the generated table just like it is done for PCRE1. Signed-off

[RFC PATCH v3 1/3] grep: make PCRE1 aware of custom allocator

be good idea to test it in a platform where NED might have a positive impact (ex: Windows 7) [1] https://public-inbox.org/git/pull.306.git.gitgitgad...@gmail.com Signed-off-by: Carlo Marcelo Arenas Belón --- Makefile | 2 +- grep.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-

[RFC PATCH v3 2/3] grep: make PCRE2 aware of custom allocator

riginal code[1] this work was based on. [1] https://public-inbox.org/git/3397e6797f872aedd18c6d795f4976e1c579514b.1565005867.git.gitgitgad...@gmail.com/ Reported-by: Johannes Schindelin Signed-off-by: Carlo Marcelo Arenas Belón --- builtin/grep.c | 1 +

[RFC PATCH v3 0/3] grep: no leaks or crashes (windows testing needed)

been tested and considered mostly complete. Junio, could you comment in my assumption that the use of grep in revision.c doesn't require initializing a PCRE2 global context and therefore not doing the cleanup? Carlo Marcelo Arenas Belón (3): grep: make PCRE1 aware of custom allocator grep: make P

[PATCH 2/3] grep: make PCRE2 aware of custom allocator

Most of the code stolen from[1] to easy on comparison and including the deficiency of setting the global context even for patterns that won't need it. Ideally, the call from grep_init could be moved to a place where it could be set without needing a lock and at least with this approach we have a p

[PATCH 3/3] grep: avoid leak of chartables in PCRE2

94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced a small memory leak visible with valgrind in t7813. Complete the creation of a PCRE2 specific variable that was missing from the original change and free the generated table just like it is done for PCRE1. Signed-off

[PATCH 0/3] grep: no leaks (WIP)

there are still more things to consider as explained there The third patch is the original leak patch rebased on top. Carlo Marcelo Arenas Belón (3): grep: make PCRE1 aware of custom allocator grep: make PCRE2 aware of custom allocator grep: avoid leak of chartables in PCRE2 Makefile |

[PATCH 1/3] grep: make PCRE1 aware of custom allocator

https://public-inbox.org/git/pull.306.git.gitgitgad...@gmail.com Signed-off-by: Carlo Marcelo Arenas Belón --- Makefile | 2 +- grep.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index bd246f2989..4b384f3759 100644 --- a/Makefile

Re: [PATCH 1/1] pcre2: allow overriding the system allocator

On Mon, Aug 5, 2019 at 2:53 PM Junio C Hamano wrote: > > Carlo Arenas writes: > > > LGTM except from the suggestion below that might make the code more > > "standard" > > and probably be a good base for a similar PCRE1 fix > >> > >>

Re: [PATCH 1/1] pcre2: allow overriding the system allocator

And forgot to mention that technically these are not UTF-8 tables but single byte tables like for example the ones used with en_US.ISO8859-1 Carlo

Re: [PATCH 1/1] pcre2: allow overriding the system allocator

itionally to being more consistent will avoid creating the global context for the most common case (when the locale is either C/POSIX or UTF-8) and therefore have a smaller impact on performance. Carlo

[RFC PATCH 1/2] p7810: add more grep performance relevant cases

Add a baseline for a matching regex and make clear the distinction between fixed (now using kwset) and a real simple expression. Signed-off-by: Carlo Marcelo Arenas Belón --- t/perf/p7810-grep.sh | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/t

[RFC PATCH 2/2] grep: make default number of threads reflect runtime

(23.28+0.09) -1.5% Signed-off-by: Carlo Marcelo Arenas Belón --- Documentation/git-grep.txt | 2 +- builtin/grep.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index 2d27969057..5d72e03b2e 100644 --- a

[RFC PATCH 0/2] grep: make threading smarter

mance issue in HP-UX[1] Lastly the performance numbers point to deficiencies in kwset and the compat/regex code that will need to be addressed independently. Carlo Marcelo Arenas Belón (2): p7810: add more grep performance relevant cases grep: make default number of threads reflect ru

Re: [RFC PATCH v3] grep: treat PCRE2 jit compilation memory error as non fatal

PROs: * it works (only for PCRE2) and tested in OpenBSD, NetBSD, macOS, Linux (Debian) * it applies everywhere (even pu) without conflicts * it doesn't introduce any regressions in tests (tested in Debian with SElinux in enforcing mode) * it is simple CONs: * HardenedBSD still segfaults (bugfix pr

[RFC PATCH v3] grep: treat PCRE2 jit compilation memory error as non fatal

warning so that corrective action could be taken. [1] https://bugs.exim.org/show_bug.cgi?id=1749 Signed-off-by: Carlo Marcelo Arenas Belón --- grep.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/grep.c b/grep.c index f7c3a5803e..593a1cb7a0 100644 --- a/grep.c +++ b/grep.c

Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE

On Wed, Jul 31, 2019 at 7:57 AM Ævar Arnfjörð Bjarmason wrote: > What hasn't been supported is all of that saying "yes, I support JIT" > and the feature then fail whaling. I had not encountered that before. > > So far that seems like because Carlo just built a complete

Re: [PATCH v2] grep: avoid leak of chartables in PCRE2

Junio Thanks for fixing the conflicts in pu as well; apologize if I could had made it easier by doings things differently Carlo

[PATCH v2] grep: avoid leak of chartables in PCRE2

leanup use free as there is no global context defined when it was created (pcre2_maketables is passed a NULL pointer) but if that gets ever changed will need to be updated in tandem. Signed-off-by: Carlo Marcelo Arenas Belón --- V2: * better document why free is used as suggested by René * avo

Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE

On Mon, Jul 29, 2019 at 5:38 AM Ævar Arnfjörð Bjarmason wrote: > On Mon, Jul 29 2019, Carlo Arenas wrote: > > On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason > > wrote: > >> > >> On Mon, Jul 29 2019, Carlo Marcelo Arenas Belón wrote: > >> >

Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE

e, allowing stack-size control at runtime and / or making JIT optional at runtime." making JIT optional at runtime is therefore the title of this patch and as I mentioned in some other thread it might be even useful to us for our own tests. Carlo [1] https://public-inb

Re: [PATCH 3/3] grep: plug leak of pcre chartables in PCRE2

On Mon, Jul 29, 2019 at 1:35 PM René Scharfe wrote: > > Am 28.07.19 um 03:41 schrieb Carlo Arenas: > > On Sat, Jul 27, 2019 at 4:48 PM Ævar Arnfjörð Bjarmason > > wrote: > >>> + free((void *)p->pcre_tables); > >> > >> Is the cast really n

Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE

kernels) and macOS 10.13.6 but haven't yet tested them. HardenedBSD will likely segfault unless pcre.jit=0 as described in the original report[2] Testing with SElinux and PAX enabled for Linux encouraged Carlo [1] https://public-inbox.org/git/20190726202642.7986-1-care...@gmail.com/ [2]

[RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE

when a JIT failure consistent with known security restrictions is found at regex compilation time. $ git grep 'foo bar' fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48' Signed-off-by: Carlo Marcelo Arenas Belón --- V2: add command line to grep as suggest

Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE

On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason 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

Re: [PATCH] grep: print the pcre2_jit_on value

the former if > available, without any UI to change Git's behavior in that respect), so > I would be surprised if this patch wasn't applicable after Ævar's > patches. the PCRE1 changes are significant enough that would break the current check and might need a more convoluted check

Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE

licts with other in fly features too (some not even in pu), which is why now is based on pu and depends on ab/pcre-jit-fixes Carlo

Re: [PATCH v2 6/8] grep: stess test PCRE v2 on invalid UTF-8 data

pcre2syntax(3) the syntax. Alternativelly; using `git -c pcre.jit=false grep ...` on top of [1], might be cleaner Carlo [1] https://public-inbox.org/git/20190728235427.41425-1-care...@gmail.com/

Re: [PATCH v2 4/8] grep: consistently use "p->fixed" in compile_regexp()

ine. that at least will give us a logical way to fix the pattern reported in [1] and that currently requires the user to know git's grep internals and know he can skip the "is_fixed" optimization by doing something like : $ git grep 'foo[ ]bar' Carlo [1] https://public-inbox.org/git/20190728235427.41425-1-care...@gmail.com/

Re: [PATCH v2 3/8] grep: stop using a custom JIT stack with PCRE v1

led at compile time and the logic is less messy also appreciated. Let me know also how you want to keep it on sync, as IMHO makes more sense inside your branch instead of as an independent topic. Carlo CC +Brian [1] https://public-inbox.org/git/20190615191514.gd8...@genre.crustyt

Re: [PATCH v2 2/8] grep: stop "using" a custom JIT stack with PCRE v2

ruct grep_pat *p, > const struct grep_opt *opt > p->pcre2_jit_on = 0; > return; this return and brackets no longer needed Carlo

Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE

On Sun, Jul 28, 2019 at 4:54 PM Carlo Marcelo Arenas Belón wrote: > @@ -125,6 +126,12 @@ int grep_config(const char *var, const char *value, void > *cb) > return 0; > } > > + if (!strcmp(var, "pcre.jit")) { > + int

[RFC PATCH] grep: allow for run time disabling of JIT in PCRE

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 '-4

Re: What's cooking in git.git (Jul 2019, #06; Thu, 25)

pics was published in: https://public-inbox.org/git/20190728200724.35630-1-care...@gmail.com/ apologies for keeping Peff's patches hostage otherwise Carlo

[PATCH v2 0/5] system header cleanup

stems (except for the 3rd one) and that has since shown to also be needed in Alpine Linux (because of _XOPEN_SOURCE redefinition). The last 2 patches are new to the series and just cleanup the dependency list in xdiff. Carlo Marcelo Arenas Belón (3): xdiff: drop system includes in xutils.c

[PATCH v2 3/5] xdiff: drop system includes in xutils.c

e_tests.h:231:0: note: this is the location of the previous definition #define _FILE_OFFSET_BITS 32 Make sure git-compat-util.h is the first header (through xinclude.h) Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- V2: reword commit with feedback from Johannes x

[PATCH v2 1/5] verify-tag: drop signal.h include

From: Jeff King There's no reason verify-tag.c needs to include signal.h. It's already in git-compat-util.h, which we properly include as the first header. And there doesn't seem to be a particular reason for this include; it's just an artifact from the file creation in 2ae68fcb78 (Make verify-ta

[PATCH v2 4/5] xdiff: remove duplicate headers from xhistogram.c

8c912eea94 ("teach --histogram to diff", 2011-07-12) included them, but were already part of xinclude.h Signed-off-by: Carlo Marcelo Arenas Belón --- xdiff/xhistogram.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index ec85f5992b..

[PATCH v2 2/5] wt-status.h: drop stdio.h include

From: Jeff King We started including stdio.h to pick up the declaration of "FILE" in f26a001226 (Enable wt-status output to a given FILE pointer., 2007-09-17). But there's no need, since headers can assume that git-compat-util.h has been included, which covers stdio. This should just be redundan

[PATCH v2 5/5] xdiff: remove duplicate headers from xpatience.c

92b7de93fb (Implement the patience diff algorithm, 2009-01-07) added them but were already part of xinclude.h Signed-off-by: Carlo Marcelo Arenas Belón --- xdiff/xpatience.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index f3573d9f00..3c5601b602

Re: [PATCH 1/3] grep: make pcre1_tables version agnostic

On Sat, Jul 27, 2019 at 4:47 PM Ævar Arnfjörð Bjarmason wrote: > > On Sat, Jul 27 2019, Carlo Marcelo Arenas Belón wrote: > > > 6d4b5747f0 ("grep: change internal *pcre* variable & function names > > to be *pcre1*", 2017-05-25), renamed most variables to be

Re: [PATCH 3/3] grep: plug leak of pcre chartables in PCRE2

the const qualifier (-Wdiscarded-qualifiers) and another for mismatching parameter to free(): note: expected 'void *' but argument is of type 'const uint8_t *' {aka 'const unsigned char *'} Carlo

[PATCH 1/3] grep: make pcre1_tables version agnostic

t unsigned char* vs const uint8_t*) to be shared. Revert that change, as 94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) failed to create an equivalent PCRE2 version. Signed-off-by: Carlo Marcelo Arenas Belón --- grep.c | 6 +++--- grep.h | 2 +- 2 files changed, 4 insertions(+

[PATCH 2/3] grep: use pcre_tables also for PCRE2

red by all functions. Signed-off-by: Carlo Marcelo Arenas Belón --- grep.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/grep.c b/grep.c index cc65f7a987..d04635fad4 100644 --- a/grep.c +++ b/grep.c @@ -488,7 +488,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, con

[PATCH 3/3] grep: plug leak of pcre chartables in PCRE2

Just as it is done with PCRE1, make sure that the allocated chartables get free at cleanup time. This assumes no global context is used (NULL passed when created the tables), but will likely be updated in tandem if that ever changes. Signed-off-by: Carlo Marcelo Arenas Belón --- grep.c | 1

[PATCH 0/3] grep: memory leak in PCRE2

codebase (ex: ab/pcre-jit-fixes) but hopefully the spreading on short simple commits helps with reviewing. Carlo Marcelo Arenas Belón (3): grep: make pcre1_tables version agnostic grep: use pcre_tables also for PCRE2 grep: plug leak of pcre chartables in PCRE2 grep.c | 12

[RFC PATCH 1/2] grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1

d reliably to enforce JIT doesn't get used. Signed-off-by: Carlo Marcelo Arenas Belón --- Makefile | 9 ++--- grep.h | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index 11ccea4071..7e0e6cc129 100644 --- a/Makefile +++ b/Makefile @@ -

[RFC PATCH 2/2] grep: refactor and simplify PCRE1 support

pcre_study with the right parameter after JIT support has been confirmed and unless it was requested to be disabled with NO_LIBPCRE1_JIT Signed-off-by: Carlo Marcelo Arenas Belón --- grep.c | 15 +-- grep.h | 9 - 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a

[RFC PATCH 0/2] PCRE1 cleanup

Sent as an RFC since it was meant to be applied against ab/pcre-jit-fixes but that is likely to change with the reroll of that branch. [PATCH 1/2] grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1 [PATCH 2/2] grep: refactor and simplify PCRE1 support The end result could be squashed together

Re: [PATCH] grep: skip UTF8 checks explicitally

e it segfault when using PCRE. in that line, I am not sure I understand the pushback against making that explicit since it only makes both codepaths behave the same (bugs and risks of burning alike) Carlo

  1   2   3   >