From 4be240789d5b322df9f02b7e19c8651f3ccbf205 Mon Sep 17 00:00:00 2001 From: Paul Wankadia Date: Tue, 29 Nov 2022 06:47:38 -0800 Subject: [PATCH] Avoid "immortal" dynamic memory allocations. Statically allocating the storage and then lazily constructing the objects (in onces) avoids global constructors and the false positives (thanks, Valgrind) about memory leaks at program termination. Fixes #28, #190, #377, #387. Change-Id: I5b3eaf5d743a4723212c5a7461c280d99a0384ac Reviewed-on: https://code-review.googlesource.com/c/re2/+/60850 Reviewed-by: Perry Lorier Reviewed-by: Paul Wankadia --- re2/re2.cc | 49 +++++++++++++++++++++++++++++++++---------------- re2/regexp.cc | 38 ++++++++++++++++++++++++-------------- 2 files changed, 57 insertions(+), 30 deletions(-) diff --git a/re2/re2.cc b/re2/re2.cc index 05e438179..b24c6d66b 100644 --- a/re2/re2.cc +++ b/re2/re2.cc @@ -65,11 +65,30 @@ RE2::Options::Options(RE2::CannedOptions opt) one_line_(false) { } -// static empty objects for use as const references. -// To avoid global constructors, allocated in RE2::Init(). -static const std::string* empty_string; -static const std::map* empty_named_groups; -static const std::map* empty_group_names; +// Empty objects for use as const references. +// Statically allocating the storage and then +// lazily constructing the objects (in a once +// in RE2::Init()) avoids global constructors +// and the false positives (thanks, Valgrind) +// about memory leaks at program termination. +struct EmptyStorage { + std::string empty_string; + std::map empty_named_groups; + std::map empty_group_names; +}; +alignas(EmptyStorage) static char empty_storage[sizeof(EmptyStorage)]; + +static inline std::string* empty_string() { + return &reinterpret_cast(empty_storage)->empty_string; +} + +static inline std::map* empty_named_groups() { + return &reinterpret_cast(empty_storage)->empty_named_groups; +} + +static inline std::map* empty_group_names() { + return &reinterpret_cast(empty_storage)->empty_group_names; +} // Converts from Regexp error code to RE2 error code. // Maybe some day they will diverge. In any event, this @@ -180,17 +199,15 @@ int RE2::Options::ParseFlags() const { void RE2::Init(const StringPiece& pattern, const Options& options) { static std::once_flag empty_once; std::call_once(empty_once, []() { - empty_string = new std::string; - empty_named_groups = new std::map; - empty_group_names = new std::map; + (void) new (empty_storage) EmptyStorage; }); pattern_ = new std::string(pattern); options_.Copy(options); entire_regexp_ = NULL; suffix_regexp_ = NULL; - error_ = empty_string; - error_arg_ = empty_string; + error_ = empty_string(); + error_arg_ = empty_string(); num_captures_ = -1; error_code_ = NoError; @@ -275,15 +292,15 @@ re2::Prog* RE2::ReverseProg() const { } RE2::~RE2() { - if (group_names_ != empty_group_names) + if (group_names_ != empty_group_names()) delete group_names_; - if (named_groups_ != empty_named_groups) + if (named_groups_ != empty_named_groups()) delete named_groups_; delete rprog_; delete prog_; - if (error_arg_ != empty_string) + if (error_arg_ != empty_string()) delete error_arg_; - if (error_ != empty_string) + if (error_ != empty_string()) delete error_; if (suffix_regexp_) suffix_regexp_->Decref(); @@ -369,7 +386,7 @@ const std::map& RE2::NamedCapturingGroups() const { if (re->suffix_regexp_ != NULL) re->named_groups_ = re->suffix_regexp_->NamedCaptures(); if (re->named_groups_ == NULL) - re->named_groups_ = empty_named_groups; + re->named_groups_ = empty_named_groups(); }, this); return *named_groups_; } @@ -380,7 +397,7 @@ const std::map& RE2::CapturingGroupNames() const { if (re->suffix_regexp_ != NULL) re->group_names_ = re->suffix_regexp_->CaptureNames(); if (re->group_names_ == NULL) - re->group_names_ = empty_group_names; + re->group_names_ = empty_group_names(); }, this); return *group_names_; } diff --git a/re2/regexp.cc b/re2/regexp.cc index ca1318b43..74ecb3196 100644 --- a/re2/regexp.cc +++ b/re2/regexp.cc @@ -74,16 +74,27 @@ bool Regexp::QuickDestroy() { return false; } -// Lazily allocated. -static Mutex* ref_mutex; -static std::map* ref_map; +// Similar to EmptyStorage in re2.cc. +struct RefStorage { + Mutex ref_mutex; + std::map ref_map; +}; +alignas(RefStorage) static char ref_storage[sizeof(RefStorage)]; + +static inline Mutex* ref_mutex() { + return &reinterpret_cast(ref_storage)->ref_mutex; +} + +static inline std::map* ref_map() { + return &reinterpret_cast(ref_storage)->ref_map; +} int Regexp::Ref() { if (ref_ < kMaxRef) return ref_; - MutexLock l(ref_mutex); - return (*ref_map)[this]; + MutexLock l(ref_mutex()); + return (*ref_map())[this]; } // Increments reference count, returns object as convenience. @@ -91,18 +102,17 @@ Regexp* Regexp::Incref() { if (ref_ >= kMaxRef-1) { static std::once_flag ref_once; std::call_once(ref_once, []() { - ref_mutex = new Mutex; - ref_map = new std::map; + (void) new (ref_storage) RefStorage; }); // Store ref count in overflow map. - MutexLock l(ref_mutex); + MutexLock l(ref_mutex()); if (ref_ == kMaxRef) { // already overflowed - (*ref_map)[this]++; + (*ref_map())[this]++; } else { // overflowing now - (*ref_map)[this] = kMaxRef; + (*ref_map())[this] = kMaxRef; ref_ = kMaxRef; } return this; @@ -116,13 +126,13 @@ Regexp* Regexp::Incref() { void Regexp::Decref() { if (ref_ == kMaxRef) { // Ref count is stored in overflow map. - MutexLock l(ref_mutex); - int r = (*ref_map)[this] - 1; + MutexLock l(ref_mutex()); + int r = (*ref_map())[this] - 1; if (r < kMaxRef) { ref_ = static_cast(r); - ref_map->erase(this); + ref_map()->erase(this); } else { - (*ref_map)[this] = r; + (*ref_map())[this] = r; } return; }