Commit 0c66e13e authored by Nathan Bronson's avatar Nathan Bronson Committed by Facebook Github Bot

extend FOLLY_F14_PERTURB_INSERTION_ORDER to all F14FastMap/Set

Summary:
Previously the debug-build randomization of F14 iteration order
was applied only to F14ValueMap/Set, F14NodeMap/Set, and F14FastMap/Set
that uses the value storage strategy.  This extends the behavior to
F14FastMap/Set that use the vector storage strategy, which are those
instances where sizeof(value_type) >= 24.

F14FastMap/Set using the vector storage strategy must move items
to randomize, so this reordering will also expose cases that assume
reference or iterator stability across multiple inserts without a call
to .reserve().

Reviewed By: yfeldblum

Differential Revision: D13305818

fbshipit-source-id: 178a1f7b707998728a0451af34269e735bf063f3
parent 3772b70b
......@@ -1106,14 +1106,16 @@ template <
typename Mapped,
typename Hasher,
typename KeyEqual,
typename Alloc>
typename Alloc,
typename EligibleForPerturbedInsertionOrder>
class F14VectorMapImpl : public F14BasicMap<MapPolicyWithDefaults<
VectorContainerPolicy,
Key,
Mapped,
Hasher,
KeyEqual,
Alloc>> {
Alloc,
EligibleForPerturbedInsertionOrder>> {
protected:
using Policy = MapPolicyWithDefaults<
VectorContainerPolicy,
......@@ -1121,7 +1123,8 @@ class F14VectorMapImpl : public F14BasicMap<MapPolicyWithDefaults<
Mapped,
Hasher,
KeyEqual,
Alloc>;
Alloc,
EligibleForPerturbedInsertionOrder>;
private:
using Super = F14BasicMap<Policy>;
......@@ -1257,11 +1260,15 @@ template <
typename Hasher,
typename KeyEqual,
typename Alloc>
class F14VectorMap
: public f14::detail::
F14VectorMapImpl<Key, Mapped, Hasher, KeyEqual, Alloc> {
using Super =
f14::detail::F14VectorMapImpl<Key, Mapped, Hasher, KeyEqual, Alloc>;
class F14VectorMap : public f14::detail::F14VectorMapImpl<
Key,
Mapped,
Hasher,
KeyEqual,
Alloc,
std::false_type> {
using Super = f14::detail::
F14VectorMapImpl<Key, Mapped, Hasher, KeyEqual, Alloc, std::false_type>;
public:
using typename Super::const_iterator;
......@@ -1366,15 +1373,26 @@ template <
typename Hasher,
typename KeyEqual,
typename Alloc>
class F14FastMap
: public std::conditional_t<
class F14FastMap : public std::conditional_t<
sizeof(std::pair<Key const, Mapped>) < 24,
F14ValueMap<Key, Mapped, Hasher, KeyEqual, Alloc>,
f14::detail::F14VectorMapImpl<Key, Mapped, Hasher, KeyEqual, Alloc>> {
f14::detail::F14VectorMapImpl<
Key,
Mapped,
Hasher,
KeyEqual,
Alloc,
std::true_type>> {
using Super = std::conditional_t<
sizeof(std::pair<Key const, Mapped>) < 24,
F14ValueMap<Key, Mapped, Hasher, KeyEqual, Alloc>,
f14::detail::F14VectorMapImpl<Key, Mapped, Hasher, KeyEqual, Alloc>>;
f14::detail::F14VectorMapImpl<
Key,
Mapped,
Hasher,
KeyEqual,
Alloc,
std::true_type>>;
public:
using typename Super::value_type;
......
......@@ -853,20 +853,27 @@ bool operator!=(
namespace f14 {
namespace detail {
template <typename Key, typename Hasher, typename KeyEqual, typename Alloc>
template <
typename Key,
typename Hasher,
typename KeyEqual,
typename Alloc,
typename EligibleForPerturbedInsertionOrder>
class F14VectorSetImpl : public F14BasicSet<SetPolicyWithDefaults<
VectorContainerPolicy,
Key,
Hasher,
KeyEqual,
Alloc>> {
Alloc,
EligibleForPerturbedInsertionOrder>> {
protected:
using Policy = SetPolicyWithDefaults<
VectorContainerPolicy,
Key,
Hasher,
KeyEqual,
Alloc>;
Alloc,
EligibleForPerturbedInsertionOrder>;
private:
using Super = F14BasicSet<Policy>;
......@@ -1012,8 +1019,10 @@ class F14VectorSetImpl : public F14BasicSet<SetPolicyWithDefaults<
template <typename Key, typename Hasher, typename KeyEqual, typename Alloc>
class F14VectorSet
: public f14::detail::F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc> {
using Super = f14::detail::F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc>;
: public f14::detail::
F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc, std::false_type> {
using Super = f14::detail::
F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc, std::false_type>;
public:
using typename Super::const_iterator;
......@@ -1116,11 +1125,13 @@ class F14FastSet
: public std::conditional_t<
sizeof(Key) < 24,
F14ValueSet<Key, Hasher, KeyEqual, Alloc>,
f14::detail::F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc>> {
f14::detail::
F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc, std::true_type>> {
using Super = std::conditional_t<
sizeof(Key) < 24,
F14ValueSet<Key, Hasher, KeyEqual, Alloc>,
f14::detail::F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc>>;
f14::detail::
F14VectorSetImpl<Key, Hasher, KeyEqual, Alloc, std::true_type>>;
public:
using typename Super::value_type;
......
......@@ -576,9 +576,8 @@ class ValueContainerPolicy : public BasePolicy<
return std::move(item);
}
template <typename... Args>
void
constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) {
template <typename Table, typename... Args>
void constructValueAtItem(Table&&, Item* itemAddr, Args&&... args) {
Alloc& a = this->alloc();
// GCC < 6 doesn't use the fact that itemAddr came from a reference
// to avoid a null-check in the placement new. folly::assume-ing it
......@@ -825,9 +824,8 @@ class NodeContainerPolicy
return std::move(*item);
}
template <typename... Args>
void
constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) {
template <typename Table, typename... Args>
void constructValueAtItem(Table&&, Item* itemAddr, Args&&... args) {
Alloc& a = this->alloc();
// TODO(T31574848): clean up assume-s used to optimize placement new
assume(itemAddr != nullptr);
......@@ -898,7 +896,8 @@ template <
typename MappedTypeOrVoid,
typename HasherOrVoid,
typename KeyEqualOrVoid,
typename AllocOrVoid>
typename AllocOrVoid,
typename EligibleForPerturbedInsertionOrder>
class VectorContainerPolicy;
template <typename ValuePtr>
......@@ -963,7 +962,13 @@ class VectorContainerIterator : public BaseIter<ValuePtr, uint32_t> {
return current_ - lowest_;
}
template <typename K, typename M, typename H, typename E, typename A>
template <
typename K,
typename M,
typename H,
typename E,
typename A,
typename P>
friend class VectorContainerPolicy;
template <typename P>
......@@ -979,7 +984,8 @@ template <
typename MappedTypeOrVoid,
typename HasherOrVoid,
typename KeyEqualOrVoid,
typename AllocOrVoid>
typename AllocOrVoid,
typename EligibleForPerturbedInsertionOrder>
class VectorContainerPolicy : public BasePolicy<
Key,
MappedTypeOrVoid,
......@@ -1157,22 +1163,52 @@ class VectorContainerPolicy : public BasePolicy<
return std::move(values_[item]);
}
template <typename Table>
void constructValueAtItem(
std::size_t /*size*/,
Table&&,
Item* itemAddr,
VectorContainerIndexSearch arg) {
*itemAddr = arg.index_;
}
template <typename... Args>
void constructValueAtItem(std::size_t size, Item* itemAddr, Args&&... args) {
template <typename Table, typename... Args>
void constructValueAtItem(Table&& table, Item* itemAddr, Args&&... args) {
Alloc& a = this->alloc();
std::size_t size = table.size();
FOLLY_SAFE_DCHECK(size < std::numeric_limits<InternalSizeType>::max(), "");
*itemAddr = static_cast<InternalSizeType>(size);
auto dst = std::addressof(values_[size]);
// TODO(T31574848): clean up assume-s used to optimize placement new
assume(dst != nullptr);
AllocTraits::construct(a, dst, std::forward<Args>(args)...);
constexpr bool perturb = FOLLY_F14_PERTURB_INSERTION_ORDER;
if (EligibleForPerturbedInsertionOrder::value && perturb &&
!tlsPendingSafeInserts()) {
// Pick a random victim. We have to do this post-construction
// because the item and tag are already set in the table before
// calling constructValueAtItem, so if there is a tag collision
// find may evaluate values_[size] during the search.
auto i = tlsMinstdRand(size + 1);
if (i != size) {
auto& lhsItem = *itemAddr;
auto rhsIter = table.find(
VectorContainerIndexSearch{static_cast<InternalSizeType>(i)});
FOLLY_SAFE_DCHECK(!rhsIter.atEnd(), "");
auto& rhsItem = rhsIter.item();
FOLLY_SAFE_DCHECK(lhsItem == size, "");
FOLLY_SAFE_DCHECK(rhsItem == i, "");
aligned_storage_for_t<Value> tmp;
Value* tmpValue = static_cast<Value*>(static_cast<void*>(&tmp));
transfer(a, std::addressof(values_[i]), tmpValue, 1);
transfer(
a, std::addressof(values_[size]), std::addressof(values_[i]), 1);
transfer(a, tmpValue, std::addressof(values_[size]), 1);
lhsItem = i;
rhsItem = size;
}
}
}
void moveItemDuringRehash(Item* itemAddr, Item& src) {
......@@ -1461,31 +1497,37 @@ class VectorContainerPolicy : public BasePolicy<
};
template <
template <typename, typename, typename, typename, typename> class Policy,
template <typename, typename, typename, typename, typename, typename...>
class Policy,
typename Key,
typename Mapped,
typename Hasher,
typename KeyEqual,
typename Alloc>
typename Alloc,
typename... Args>
using MapPolicyWithDefaults = Policy<
Key,
Mapped,
VoidDefault<Hasher, DefaultHasher<Key>>,
VoidDefault<KeyEqual, DefaultKeyEqual<Key>>,
VoidDefault<Alloc, DefaultAlloc<std::pair<Key const, Mapped>>>>;
VoidDefault<Alloc, DefaultAlloc<std::pair<Key const, Mapped>>>,
Args...>;
template <
template <typename, typename, typename, typename, typename> class Policy,
template <typename, typename, typename, typename, typename, typename...>
class Policy,
typename Key,
typename Hasher,
typename KeyEqual,
typename Alloc>
typename Alloc,
typename... Args>
using SetPolicyWithDefaults = Policy<
Key,
void,
VoidDefault<Hasher, DefaultHasher<Key>>,
VoidDefault<KeyEqual, DefaultKeyEqual<Key>>,
VoidDefault<Alloc, DefaultAlloc<Key>>>;
VoidDefault<Alloc, DefaultAlloc<Key>>,
Args...>;
} // namespace detail
} // namespace f14
......
......@@ -34,17 +34,32 @@ void F14LinkCheck<getF14IntrinsicsMode()>::check() noexcept {}
EmptyTagVectorType kEmptyTagVector = {};
#endif
FOLLY_F14_TLS_IF_ASAN std::size_t asanPendingSafeInserts = 0;
std::size_t tlsMinstdRand(std::size_t n) {
FOLLY_SAFE_DCHECK(n > 0, "");
//// Debug and ASAN stuff
#if defined(FOLLY_TLS) && (!defined(NDEBUG) || FOLLY_ASAN_ENABLED)
static FOLLY_TLS uint32_t state = 0;
#define FOLLY_F14_DETAIL_TLS_SIZE_T FOLLY_TLS std::size_t
#else
static std::atomic<uint32_t> state{0};
#define FOLLY_F14_DETAIL_TLS_SIZE_T std::atomic<std::size_t>
#endif
uint32_t s = state;
bool tlsPendingSafeInserts(std::ptrdiff_t delta) {
static FOLLY_F14_DETAIL_TLS_SIZE_T value{0};
FOLLY_SAFE_DCHECK(delta >= -1, "");
std::size_t v = value;
if (delta > 0 || (delta == -1 && v > 0)) {
v += delta;
v = std::min(std::numeric_limits<std::size_t>::max() / 2, v);
value = v;
}
return v != 0;
}
std::size_t tlsMinstdRand(std::size_t n) {
FOLLY_SAFE_DCHECK(n > 0, "");
static FOLLY_F14_DETAIL_TLS_SIZE_T state{0};
uint32_t s = static_cast<uint32_t>(state);
if (s == 0) {
uint64_t seed = static_cast<uint64_t>(
std::chrono::steady_clock::now().time_since_epoch().count());
......
......@@ -135,6 +135,9 @@ struct F14LinkCheck<getF14IntrinsicsMode()> {
static void check() noexcept;
};
bool tlsPendingSafeInserts(std::ptrdiff_t delta = 0);
std::size_t tlsMinstdRand(std::size_t n);
#if defined(_LIBCPP_VERSION)
template <typename K, typename V, typename H>
......@@ -330,10 +333,6 @@ using EmptyTagVectorType = std::aligned_storage_t<
extern EmptyTagVectorType kEmptyTagVector;
extern FOLLY_F14_TLS_IF_ASAN std::size_t asanPendingSafeInserts;
std::size_t tlsMinstdRand(std::size_t n);
template <unsigned BitCount>
struct FullMask {
static constexpr MaskType value =
......@@ -1596,7 +1595,7 @@ class F14Table : public Policy {
void insertAtBlank(ItemIter pos, HashPair hp, Args&&... args) {
try {
auto dst = pos.itemAddr();
this->constructValueAtItem(size(), dst, std::forward<Args>(args)...);
this->constructValueAtItem(*this, dst, std::forward<Args>(args)...);
} catch (...) {
eraseBlank(pos, hp);
throw;
......@@ -2008,32 +2007,24 @@ class F14Table : public Policy {
success = true;
}
FOLLY_ALWAYS_INLINE void asanOnReserve(std::size_t capacity) {
if (kIsSanitizeAddress && capacity > size()) {
asanPendingSafeInserts += capacity - size();
}
}
// Randomization to help expose bugs when running tests in debug or
// sanitizer builds
FOLLY_ALWAYS_INLINE bool asanShouldAddExtraRehash() {
if (!kIsSanitizeAddress) {
return false;
} else if (asanPendingSafeInserts > 0) {
--asanPendingSafeInserts;
return false;
} else if (size() <= 1) {
return size() > 0;
} else {
return tlsMinstdRand(size()) == 0;
FOLLY_ALWAYS_INLINE void debugModeOnReserve(std::size_t capacity) {
if (kIsSanitizeAddress || kIsDebug) {
if (capacity > size()) {
tlsPendingSafeInserts(static_cast<std::ptrdiff_t>(capacity - size()));
}
}
}
void asanExtraRehash() {
void debugModeSpuriousRehash() {
auto cc = chunkMask_ + 1;
auto bc = bucket_count();
rehashImpl(cc, bc, cc, bc);
}
FOLLY_ALWAYS_INLINE void asanOnInsert() {
FOLLY_ALWAYS_INLINE void debugModeBeforeInsert() {
// When running under ASAN, we add a spurious rehash with 1/size()
// probability before every insert. This means that finding reference
// stability problems for F14Value and F14Vector is much more likely.
......@@ -2044,17 +2035,24 @@ class F14Table : public Policy {
// One way to fix this is to call map.reserve(N) before such a
// sequence, where N is the number of keys that might be inserted
// within the section that retains references plus the existing size.
if (asanShouldAddExtraRehash()) {
asanExtraRehash();
if (kIsSanitizeAddress && !tlsPendingSafeInserts() && size() > 0 &&
tlsMinstdRand(size()) == 0) {
debugModeSpuriousRehash();
}
}
FOLLY_ALWAYS_INLINE void perturbInsertOrder(
FOLLY_ALWAYS_INLINE void debugModeAfterInsert() {
if (kIsSanitizeAddress || kIsDebug) {
tlsPendingSafeInserts(-1);
}
}
FOLLY_ALWAYS_INLINE void debugModePerturbSlotInsertOrder(
ChunkPtr chunk,
std::size_t& itemIndex) {
FOLLY_SAFE_DCHECK(!chunk->occupied(itemIndex), "");
constexpr bool perturb = FOLLY_F14_PERTURB_INSERTION_ORDER;
if (perturb) {
constexpr bool perturbSlot = FOLLY_F14_PERTURB_INSERTION_ORDER;
if (perturbSlot && !tlsPendingSafeInserts()) {
std::size_t e = chunkMask_ == 0 ? bucket_count() : Chunk::kCapacity;
std::size_t i = itemIndex + tlsMinstdRand(e - itemIndex);
if (!chunk->occupied(i)) {
......@@ -2073,7 +2071,7 @@ class F14Table : public Policy {
void reserve(std::size_t capacity) {
// We want to support the pattern
// map.reserve(2); auto& r1 = map[k1]; auto& r2 = map[k2];
asanOnReserve(capacity);
debugModeOnReserve(capacity);
reserveImpl(
std::max<std::size_t>(capacity, size()),
chunkMask_ + 1,
......@@ -2103,7 +2101,7 @@ class F14Table : public Policy {
}
}
asanOnInsert();
debugModeBeforeInsert();
reserveForInsert();
......@@ -2123,13 +2121,16 @@ class F14Table : public Policy {
}
std::size_t itemIndex = firstEmpty.index();
perturbInsertOrder(chunk, itemIndex);
debugModePerturbSlotInsertOrder(chunk, itemIndex);
chunk->setTag(itemIndex, hp.second);
ItemIter iter{chunk, itemIndex};
// insertAtBlank will clear the tag if the constructor throws
insertAtBlank(iter, hp, std::forward<Args>(args)...);
debugModeAfterInsert();
return std::make_pair(iter, true);
}
......@@ -2447,4 +2448,14 @@ class F14Table : public Policy {
#endif // FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
namespace f14 {
namespace test {
inline void disableInsertOrderRandomization() {
if (kIsSanitizeAddress || kIsDebug) {
detail::tlsPendingSafeInserts(static_cast<std::ptrdiff_t>(
(std::numeric_limits<std::size_t>::max)() / 2));
}
}
} // namespace test
} // namespace f14
} // namespace folly
......@@ -1566,15 +1566,22 @@ TEST(F14FastMap, disabledDoubleTransparent) {
EXPECT_TRUE(map.find(C{20}) == map.end());
}
TEST(F14ValueMap, randomInsertOrder) {
template <typename M>
void runRandomInsertOrderTest() {
if (FOLLY_F14_PERTURB_INSERTION_ORDER) {
std::string prev;
bool diffFound = false;
for (int tries = 0; tries < 100; ++tries) {
F14ValueMap<char, char> m;
for (char x = '0'; x <= '9'; ++x) {
m[x] = x;
}
M m;
for (char x = '0'; x <= '7'; ++x) {
m.try_emplace(x);
}
m.reserve(10);
auto it = m.try_emplace('8').first;
auto addr = &*it;
m.try_emplace('9');
EXPECT_TRUE(it == m.find('8'));
EXPECT_TRUE(addr = &*m.find('8'));
std::string s;
for (auto&& e : m) {
s.push_back(e.first);
......@@ -1593,6 +1600,12 @@ TEST(F14ValueMap, randomInsertOrder) {
}
}
TEST(F14Map, randomInsertOrder) {
runRandomInsertOrderTest<F14ValueMap<char, char>>();
runRandomInsertOrderTest<F14FastMap<char, char>>();
runRandomInsertOrderTest<F14FastMap<char, std::string>>();
}
///////////////////////////////////
#endif // FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
///////////////////////////////////
......@@ -1243,6 +1243,42 @@ TEST(F14ValueSet, valueSize) {
RunAllValueSizeTests<F14ValueSet, 32>{}();
}
template <typename S, typename F>
void runRandomInsertOrderTest(F&& func) {
if (FOLLY_F14_PERTURB_INSERTION_ORDER) {
std::string prev;
bool diffFound = false;
for (int tries = 0; tries < 100; ++tries) {
S set;
for (char x = '0'; x <= '9'; ++x) {
set.emplace(func(x));
}
std::string s;
for (auto&& e : set) {
s += e;
}
LOG(INFO) << s << "\n";
if (prev.empty()) {
prev = s;
continue;
}
if (prev != s) {
diffFound = true;
break;
}
}
EXPECT_TRUE(diffFound) << "no randomness found in insert order";
}
}
TEST(F14Set, randomInsertOrder) {
runRandomInsertOrderTest<F14ValueSet<char>>([](char x) { return x; });
runRandomInsertOrderTest<F14FastSet<char>>([](char x) { return x; });
runRandomInsertOrderTest<F14FastSet<std::string>>([](char x) {
return std::string{std::size_t{1}, x};
});
}
///////////////////////////////////
#endif // FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
///////////////////////////////////
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment