Bugfix in iterator emplace
Summary: Subtle bugfix to fbvector Reproduction requirements: - emplace into a vector at a non-end position - the vector has enough capacity for the extra elements - the value type's internal state is poisoned after a move What should happen, explained by picture: INITIAL SETUP abc1234XY____ ^ want to insert two elements here FIRST STEP: uninitialized move-construction x and y are initialized, but could be in an invalid state abc1234xyXY__ SECOND STEP: move-assignment, in reverse order abc123xy4XY__ abc12xy34XY__ abc1xy234XY__ abcxy1234XY__ What actually happens: An indexing error causes the entire tail (##1234xy##) to be move-assigned, instead of the intended head of the tail (##1234##). If the data type's move operator does not invalidate its own data (i.e. move is essentially a copy, as is the case with atomic types) then the move assignment of ##y## onto ##Y## and ##x## onto ##X## is basically a no-op. However, for types that do invalidate their own data when being the source of a move (e.g. non-empty vectors and fbvectors), then the end result will have bad data in place of ##X## and ##Y##. Detection: This bug has lain dormant for a while. Very few people emplace into a vector, and those few who do could be saved by using a copy-movable type like int. The original test case did not use a data type which poisoned itself after being the source of a move, so the tests did not notice any oddities. A new testsuite, which does poison itself, did notice. Test Plan: re-enable the test/stl_test mega-test. Run it. All tests pass. fbconfig -r folly && fbmake runtests_opt Run fbvector through the new test suite (which initially caught the error). It now passes. Reviewed By: robbert@fb.com FB internal diff: D1257258
Showing
Please register or sign in to comment