git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 08/10] grep/pcre2: actually make pcre2 use custom allocator

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 4, 2021, 21:05 UTC
Message-ID
<20210204210556.25242-9-avarab@gmail.com>
In-Reply-To
<191d3a2280232ff98964fd42bfe0bc85ee3708f5.1571227824.git.gitgitgadget@gmail.com>

Continue work started in 513f2b0bbd4 (grep: make PCRE2 aware of custom allocator, 2019-10-16) and make PCREv2 use our pcre2_{malloc,free}(). functions for allocation. We'll now use it for all PCREv2 allocations.

The reason 513f2b0bbd4 worked as a bugfix for the USE_NED_ALLOCATOR issue is because it managed to target pretty much the allocation freed via free(), as opposed to by a pcre2_*free() function. I.e. the pcre2_maketables() and pcre2_maketables_free() pair. For most of the rest we continued allocating with stock malloc() inside PCREv2 itself, but didn't segfault because we'd use its corresponding free().

In a preceding commit of mine I changed the free() to pcre2_maketables_free() on versions of PCREv2 10.34 and newer. So as far as fixing the segfault goes we could revert 513f2b0bbd4. But then we wouldn't use the desired allocator, let's just use it instead.

Before this patch we'd on e.g.:
    grep --threads=1 -iP æ.*var.*xyz

Only use pcre2_{malloc,free}() for 2 malloc() calls and 2 corresponding free() call. Now it's 12 calls to each. This can be observed with the GREP_PCRE2_DEBUG_MALLOC debug mode.

Reading the history of how this bug got introduced it wasn't present in Johannes's original patch[1] to fix the issue.

My reading of that thread is that the approach the follow-up patches to Johannes's original pursued were based on misunderstanding of how the PCREv2 API works. In particular this part of [2]:

    "most of the time (like when using UTF-8) the chartable (and
    therefore the global context) is not needed (even when using
    alternate allocators)"

That's simply not how PCREv2 memory allocation works. It's easy to see how the misunderstanding came about. It's because (as noted above) the issue was noticed because of our use of free() in our own grep.c for freeing the memory allocated by pcre2_maketables().

Thus the misunderstanding that PCREv2's compile context is something only needed for pcre2_maketables(), and e.g. an aborted earlier attempt[3] to only set it up when we ourselves called pcre2_maketables().

That's not what PCREv2's compile context is. To quote PCREv2's documentation:

    "This context just contains pointers to (and data for) external
    memory management functions that are called from several places in
    the PCRE2 library."

Thus the failed attempts to go down the route of only creating the general context in cases where we ourselves call pcre2_maketables(), before finally settling on the approach 513f2b0bbd4 took of always creating it.

Instead we should always create it, and then pass the general context to those functions that accept it, so that they'll consistently use our preferred memory allocation functions.

1. https://public-inbox.org/git/3397e6797f872aedd18c6d795f4976e1c579514b.1565005867.git.gitgitgadget@gmail.com/
2. https://lore.kernel.org/git/CAPUEsphMh_ZqcH3M7PXC9jHTfEdQN3mhTAK2JDkdvKBp53YBoA@mail.gmail.com/
3. https://lore.kernel.org/git/20190806085014.47776-3-carenas@gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 grep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index c63dbff4b2..0116ff5f09 100644
--- a/grep.c
+++ b/grep.c
@@ -390,7 +390,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 			if (!pcre2_global_context)
 				BUG("pcre2_global_context uninitialized");
 			p->pcre2_tables = pcre2_maketables(pcre2_global_context);
-			p->pcre2_compile_context = pcre2_compile_context_create(NULL);
+			p->pcre2_compile_context = pcre2_compile_context_create(pcre2_global_context);
 			pcre2_set_character_tables(p->pcre2_compile_context,
 							p->pcre2_tables);
 		}
@@ -411,7 +411,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 					 p->pcre2_compile_context);
 
 	if (p->pcre2_pattern) {
-		p->pcre2_match_data = pcre2_match_data_create_from_pattern(p->pcre2_pattern, NULL);
+		p->pcre2_match_data = pcre2_match_data_create_from_pattern(p->pcre2_pattern, pcre2_global_context);
 		if (!p->pcre2_match_data)
 			die("Couldn't allocate PCRE2 match data");
 	} else {
-- 
2.30.0.284.gd98b1dd5eaa7
Previous: Ævar Arnfjörð BjarmasonNext: Johannes Schindelin
Message 34 of 42 in “Revive 'pcre2-chartables-leakfix'”
  1. 0/3 Revive 'pcre2-chartables-leakfix'Johannes Schindelin via GitGitGadget, Oct 16, 2019
  2. 2/3 grep: make PCRE2 aware of custom allocatorCarlo Marcelo Arenas Belón via GitGitGadget, Oct 16, 2019
  3. 1/3 grep: make PCRE1 aware of custom allocatorCarlo Marcelo Arenas Belón via GitGitGadget, Oct 16, 2019
  4. 3/3 grep: avoid leak of chartables in PCRE2Johannes Schindelin via GitGitGadget, Oct 16, 2019
  5. 0/3 Revive 'pcre2-chartables-leakfix'Johannes Schindelin via GitGitGadget, Oct 16, 2019
  6. 1/3 grep: make PCRE1 aware of custom allocatorCarlo Marcelo Arenas Belón via GitGitGadget, Oct 16, 2019
  7. 2/3 grep: make PCRE2 aware of custom allocatorCarlo Marcelo Arenas Belón via GitGitGadget, Oct 16, 2019
  8. Junio C HamanoOct 18, 2019
  9. 00/10 grep/pcre2: memory allocation fixesÆvar Arnfjörð Bjarmason, Feb 4, 2021
  10. Junio C HamanoFeb 10, 2021
  11. 00/10 grep/pcre2: memory allocation fixesÆvar Arnfjörð Bjarmason, Feb 18, 2021
  12. Junio C HamanoMar 4, 2021
  13. 03/10 grep/pcre2: correct reference to grep_init() in commentÆvar Arnfjörð Bjarmason, Feb 18, 2021
  14. 04/10 grep/pcre2: prepare to add debugging to pcre2_malloc()Ævar Arnfjörð Bjarmason, Feb 18, 2021
  15. 02/10 grep/pcre2: drop needless assignment to NULLÆvar Arnfjörð Bjarmason, Feb 18, 2021
  16. 01/10 grep/pcre2: drop needless assignment + assert() on opt->pcre2Ævar Arnfjörð Bjarmason, Feb 18, 2021
  17. 05/10 grep/pcre2: add GREP_PCRE2_DEBUG_MALLOC debug modeÆvar Arnfjörð Bjarmason, Feb 18, 2021
  18. 06/10 grep/pcre2: use compile-time PCREv2 version testÆvar Arnfjörð Bjarmason, Feb 18, 2021
  19. Junio C HamanoMar 4, 2021
  20. 07/10 grep/pcre2: use pcre2_maketables_free() functionÆvar Arnfjörð Bjarmason, Feb 18, 2021
  21. 08/10 grep/pcre2: actually make pcre2 use custom allocatorÆvar Arnfjörð Bjarmason, Feb 18, 2021
  22. Junio C HamanoMar 4, 2021
  23. 10/10 grep/pcre2: move definitions of pcre2_{malloc,free}Ævar Arnfjörð Bjarmason, Feb 18, 2021
  24. Junio C HamanoMar 4, 2021
  25. 09/10 grep/pcre2: move back to thread-only PCREv2 structuresÆvar Arnfjörð Bjarmason, Feb 18, 2021
  26. Junio C HamanoMar 4, 2021
  27. 01/10 grep/pcre2: drop needless assignment + assert() on opt->pcre2Ævar Arnfjörð Bjarmason, Feb 4, 2021
  28. 02/10 grep/pcre2: drop needless assignment to NULLÆvar Arnfjörð Bjarmason, Feb 4, 2021
  29. 03/10 grep/pcre2: correct reference to grep_init() in commentÆvar Arnfjörð Bjarmason, Feb 4, 2021
  30. 04/10 grep/pcre2: prepare to add debugging to pcre2_malloc()Ævar Arnfjörð Bjarmason, Feb 4, 2021
  31. 05/10 grep/pcre2: add GREP_PCRE2_DEBUG_MALLOC debug modeÆvar Arnfjörð Bjarmason, Feb 4, 2021
  32. Johannes SchindelinFeb 10, 2021
  33. 06/10 grep/pcre2: use compile-time PCREv2 version testÆvar Arnfjörð Bjarmason, Feb 4, 2021
  34. 08/10 grep/pcre2: actually make pcre2 use custom allocatorÆvar Arnfjörð Bjarmason, Feb 4, 2021
  35. Johannes SchindelinFeb 10, 2021
  36. 07/10 grep/pcre2: use pcre2_maketables_free() functionÆvar Arnfjörð Bjarmason, Feb 4, 2021
  37. Johannes SchindelinFeb 10, 2021
  38. Junio C HamanoMar 4, 2021
  39. 09/10 grep/pcre2: move back to thread-only PCREv2 structuresÆvar Arnfjörð Bjarmason, Feb 4, 2021
  40. 10/10 grep/pcre2: move definitions of pcre2_{malloc,free}Ævar Arnfjörð Bjarmason, Feb 4, 2021
  41. Johannes SchindelinFeb 10, 2021
  42. 3/3 grep: avoid leak of chartables in PCRE2Carlo Marcelo Arenas Belón via GitGitGadget, Oct 16, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.