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

Message ID 20190807213945.10464-1-carenas@gmail.com
Headers show
  • grep: no leaks or crashes (windows testing needed)
Related show


Carlo Marcelo Arenas Belón Aug. 7, 2019, 9:39 p.m. UTC
This series is a candidate reroll for cb/pcre2-chartables-leakfix, that
hopefully addresses the root cause of the problem reported by Dscho in
Windows, where the PCRE2 library wasn't aware of the custom allocator and
was returning a pointer created with the system malloc but passing it to
NED's free, resulting in a segfault.

The reason why it was triggered by the original leak fix is the layering
violation reported by René and that is exclusive to PCRE2 (hence why it
hasn't been reported with PCRE1).  Additional work might be available
in a future release of PCRE2 to address that as detailed in the upstream
bug[1] report.

Eitherway, since I am unable to replicate the original bug or take
performance numbers in a representative environment without Windows
this is only published as an RFC,

Changes since v3 (mostly in patch 2):

* git log also calls the "destructor" for grep API
* no more "bug" being triggered by `make test`, sorry René
* hopefully no more crashes in windows (I was expecting at most a BUG)

Future work (other than the needed refactoring explained in the
second patch) and adjacent bugs, includes:

* tracking more possible users of the grep API that might need to call
* completely moving PCRE2 to use NED (as is done with PCRE1 and was
  proposed on the original patch[2] this is based on
* build on top of the new API so that other work could be shared
  (for example the chartables that started this whole mess)

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 allocator
  grep: avoid leak of chartables in PCRE2

 Makefile       |  2 +-
 builtin/grep.c |  1 +
 builtin/log.c  |  1 +
 grep.c         | 77 ++++++++++++++++++++++++++++++++++++++++++++------
 grep.h         |  2 ++
 5 files changed, 73 insertions(+), 10 deletions(-)

[1] https://bugs.exim.org/show_bug.cgi?id=2429
[2] https://public-inbox.org/git/3397e6797f872aedd18c6d795f4976e1c579514b.1565005867.git.gitgitgadget@gmail.com/
[3] https://public-inbox.org/git/7ec60d57-9940-35f2-f7b5-c87d4dc7cdd5@web.de/

base-commit: 51cf315870bbb7254ddf06c84fe03b41bc48eebd