Fix self-move-assignment use-after-destroy in RE2::Set and FilteredRE2 - #654
Open
Alb3e3 wants to merge 1 commit into
Open
Fix self-move-assignment use-after-destroy in RE2::Set and FilteredRE2#654Alb3e3 wants to merge 1 commit into
Alb3e3 wants to merge 1 commit into
Conversation
RE2::Set::operator=(Set&&) and FilteredRE2::operator=(FilteredRE2&&) implement move-assignment as `this->~T(); (void) new (this) T(std::move(other));`. This is undefined behavior on self-move-assignment (this == &other): the destructor releases the object's owned regexes (Regexp::Decref() for Set, delete for FilteredRE2) and the move constructor then reads from the just-destroyed object. Under libstdc++ the object is left empty, so Match()/AllMatches() silently returns wrong results; on other implementations the same UB can manifest as a double-free or crash. Add the standard self-assignment guard to both operators, and extend the existing Set.MoveSemantics and FilteredRE2Test.MoveSemantics tests with a self-move-assignment case (via an aliased reference, to avoid a -Wself-move diagnostic) that asserts the object is unchanged and usable. Fixes google#615.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #615.
RE2::Set::operator=(Set&&)andFilteredRE2::operator=(FilteredRE2&&)implement move-assignment with the destroy-then-placement-new idiom:This is undefined behavior when
this == &other(self-move-assignment): the destructor runs first and releases the object's owned regexes (Regexp::Decref()forSet,deleteforFilteredRE2), and the move constructor then reads from the just-destroyed object. Self-move-assignment can arise through aliased references or library operations that are permitted to self-assign.Observed effect: under libstdc++ the object is left destroyed and empty, so its compiled state is gone and
Match()/AllMatches()silently returns wrong results; on other implementations the same UB can manifest as a double-free or crash.Fix: add the standard self-assignment guard to both operators:
Tests: extend the existing
Set.MoveSemanticsandFilteredRE2Test.MoveSemanticswith a self-move-assignment case, written through an aliased reference to avoid a-Wself-movediagnostic, asserting the object is unchanged and still usable. Without the guard these fail (the object is emptied); with it they pass.