Commit 44e30c29 authored by Nathan Bronson's avatar Nathan Bronson Committed by Facebook Github Bot

randomize folly::dynamic and most F14 map and set insert order in debug builds

Summary:
It's easy to accidentally rely on the relationship between
insert and iteration order in unordered containers.  This diff adds some
randomization to the insertion order within an F14 chunk when compiled in
debug mode.  This isn't full randomization of the order, but is enough to
break tests that rely on the iteration order of F14 maps and sets as small
as two elements.

This diff applies randomization to folly::dynamic object, F14NodeSet,
F14NodeMap, F14ValueSet, F14ValueMap, F14FastSet for small (< 24 bytes)
value_type, and F14FastSet for small value_type.

The long-term plan is to randomize insertion order for all F14FastSet
and F14FastMap, leaving iteration order deterministic only for explicit
uses of F14VectorSet and F14VectorMap.

Reviewed By: yfeldblum

Differential Revision: D13099681

fbshipit-source-id: 35b3174a8afbc6cef27d2080adc5a01e85154dcd
parent 140f6df7
...@@ -84,6 +84,29 @@ table's key_type. F14 maps understand all possible forms that can be ...@@ -84,6 +84,29 @@ table's key_type. F14 maps understand all possible forms that can be
used to construct the underlying std::pair<key_type const, value_type), used to construct the underlying std::pair<key_type const, value_type),
so heterogeneous keys can be used even with insert and emplace. so heterogeneous keys can be used even with insert and emplace.
## RANDOMIZED BEHAVIOR IN DEBUG BUILDS
F14 introduces randomness into its behavior in debug builds and when
the address sanitizer (ASAN) is in use. This randomness is designed to
expose bugs during test that might otherwise only occur in production.
Bugs are exposed probabilistically, they may appear only some of the time.
In debug builds F14ValueMap and F14NodeMap randomize the relationship
between insertion and iteration order. This means that adding the same
k1 and k2 to two empty maps (or the same map twice after clearing it)
can produce the iteration order k1,k2 or k2,k1. Unit tests will
fail if they assume that the iteration order is the same between
identically constructed maps, even in the same process. This also
affects folly::dynamic's object mode.
When the address sanitizer is enabled all of the F14 variants perform some
randomized extra rehashes on insert, which exposes iterator and reference
stability issues. If reserve(size()+n) (or a non-zero initialCapacity)
is called then the following n insertions are exempt from the spurious
failures. Tracking is per-thread rather than per-table so this heuristic
could lead to false positives, although we haven't seen any yet.
(Please let us know if you encounter this problem.)
## WHY CHUNKS? ## WHY CHUNKS?
Assuming that you have a magic wand that lets you search all of the keys Assuming that you have a magic wand that lets you search all of the keys
......
...@@ -16,6 +16,9 @@ ...@@ -16,6 +16,9 @@
#include <folly/container/detail/F14Table.h> #include <folly/container/detail/F14Table.h>
#include <atomic>
#include <chrono>
namespace folly { namespace folly {
namespace f14 { namespace f14 {
namespace detail { namespace detail {
...@@ -32,7 +35,26 @@ EmptyTagVectorType kEmptyTagVector = {}; ...@@ -32,7 +35,26 @@ EmptyTagVectorType kEmptyTagVector = {};
#endif #endif
FOLLY_F14_TLS_IF_ASAN std::size_t asanPendingSafeInserts = 0; FOLLY_F14_TLS_IF_ASAN std::size_t asanPendingSafeInserts = 0;
FOLLY_F14_TLS_IF_ASAN std::size_t asanRehashState = 0;
std::size_t tlsMinstdRand(std::size_t n) {
FOLLY_SAFE_DCHECK(n > 0, "");
#if defined(FOLLY_TLS) && (!defined(NDEBUG) || FOLLY_ASAN_ENABLED)
static FOLLY_TLS uint32_t state = 0;
#else
static std::atomic<uint32_t> state{0};
#endif
uint32_t s = state;
if (s == 0) {
uint64_t seed = static_cast<uint64_t>(
std::chrono::steady_clock::now().time_since_epoch().count());
s = hash::twang_32from64(seed);
}
s = static_cast<uint32_t>((s * uint64_t{48271}) % uint64_t{2147483647});
state = s;
return std::size_t{s} % n;
}
} // namespace detail } // namespace detail
} // namespace f14 } // namespace f14
......
...@@ -77,6 +77,10 @@ ...@@ -77,6 +77,10 @@
#endif #endif
#ifndef FOLLY_F14_PERTURB_INSERTION_ORDER
#define FOLLY_F14_PERTURB_INSERTION_ORDER folly::kIsDebug
#endif
namespace folly { namespace folly {
struct F14TableStats { struct F14TableStats {
...@@ -327,7 +331,8 @@ using EmptyTagVectorType = std::aligned_storage_t< ...@@ -327,7 +331,8 @@ using EmptyTagVectorType = std::aligned_storage_t<
extern EmptyTagVectorType kEmptyTagVector; extern EmptyTagVectorType kEmptyTagVector;
extern FOLLY_F14_TLS_IF_ASAN std::size_t asanPendingSafeInserts; extern FOLLY_F14_TLS_IF_ASAN std::size_t asanPendingSafeInserts;
extern FOLLY_F14_TLS_IF_ASAN std::size_t asanRehashState;
std::size_t tlsMinstdRand(std::size_t n);
template <unsigned BitCount> template <unsigned BitCount>
struct FullMask { struct FullMask {
...@@ -2006,13 +2011,13 @@ class F14Table : public Policy { ...@@ -2006,13 +2011,13 @@ class F14Table : public Policy {
success = true; success = true;
} }
void asanOnReserve(std::size_t capacity) { FOLLY_ALWAYS_INLINE void asanOnReserve(std::size_t capacity) {
if (kIsSanitizeAddress && capacity > size()) { if (kIsSanitizeAddress && capacity > size()) {
asanPendingSafeInserts += capacity - size(); asanPendingSafeInserts += capacity - size();
} }
} }
bool asanShouldAddExtraRehash() { FOLLY_ALWAYS_INLINE bool asanShouldAddExtraRehash() {
if (!kIsSanitizeAddress) { if (!kIsSanitizeAddress) {
return false; return false;
} else if (asanPendingSafeInserts > 0) { } else if (asanPendingSafeInserts > 0) {
...@@ -2021,9 +2026,7 @@ class F14Table : public Policy { ...@@ -2021,9 +2026,7 @@ class F14Table : public Policy {
} else if (size() <= 1) { } else if (size() <= 1) {
return size() > 0; return size() > 0;
} else { } else {
constexpr std::size_t kBigPrime = 4294967291U; return tlsMinstdRand(size()) == 0;
auto s = (asanRehashState += kBigPrime);
return (s % size()) == 0;
} }
} }
...@@ -2033,7 +2036,7 @@ class F14Table : public Policy { ...@@ -2033,7 +2036,7 @@ class F14Table : public Policy {
rehashImpl(cc, bc, cc, bc); rehashImpl(cc, bc, cc, bc);
} }
void asanOnInsert() { FOLLY_ALWAYS_INLINE void asanOnInsert() {
// When running under ASAN, we add a spurious rehash with 1/size() // When running under ASAN, we add a spurious rehash with 1/size()
// probability before every insert. This means that finding reference // probability before every insert. This means that finding reference
// stability problems for F14Value and F14Vector is much more likely. // stability problems for F14Value and F14Vector is much more likely.
...@@ -2043,12 +2046,26 @@ class F14Table : public Policy { ...@@ -2043,12 +2046,26 @@ class F14Table : public Policy {
// //
// One way to fix this is to call map.reserve(N) before such a // 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 // sequence, where N is the number of keys that might be inserted
// within the section that retains references. // within the section that retains references plus the existing size.
if (asanShouldAddExtraRehash()) { if (asanShouldAddExtraRehash()) {
asanExtraRehash(); asanExtraRehash();
} }
} }
FOLLY_ALWAYS_INLINE void perturbInsertOrder(
ChunkPtr chunk,
std::size_t& itemIndex) {
FOLLY_SAFE_DCHECK(!chunk->occupied(itemIndex), "");
constexpr bool perturb = FOLLY_F14_PERTURB_INSERTION_ORDER;
if (perturb) {
std::size_t e = chunkMask_ == 0 ? bucket_count() : Chunk::kCapacity;
std::size_t i = itemIndex + tlsMinstdRand(e - itemIndex);
if (!chunk->occupied(i)) {
itemIndex = i;
}
}
}
public: public:
// user has no control over max_load_factor // user has no control over max_load_factor
...@@ -2108,7 +2125,8 @@ class F14Table : public Policy { ...@@ -2108,7 +2125,8 @@ class F14Table : public Policy {
chunk->adjustHostedOverflowCount(Chunk::kIncrHostedOverflowCount); chunk->adjustHostedOverflowCount(Chunk::kIncrHostedOverflowCount);
} }
std::size_t itemIndex = firstEmpty.index(); std::size_t itemIndex = firstEmpty.index();
FOLLY_SAFE_DCHECK(!chunk->occupied(itemIndex), "");
perturbInsertOrder(chunk, itemIndex);
chunk->setTag(itemIndex, hp.second); chunk->setTag(itemIndex, hp.second);
ItemIter iter{chunk, itemIndex}; ItemIter iter{chunk, itemIndex};
......
...@@ -1566,6 +1566,33 @@ TEST(F14FastMap, disabledDoubleTransparent) { ...@@ -1566,6 +1566,33 @@ TEST(F14FastMap, disabledDoubleTransparent) {
EXPECT_TRUE(map.find(C{20}) == map.end()); EXPECT_TRUE(map.find(C{20}) == map.end());
} }
TEST(F14ValueMap, randomInsertOrder) {
if (kIsDebug) {
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;
}
std::string s;
for (auto&& e : m) {
s.push_back(e.first);
}
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";
}
}
/////////////////////////////////// ///////////////////////////////////
#endif // FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE #endif // FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE
/////////////////////////////////// ///////////////////////////////////
...@@ -31,6 +31,72 @@ bool compareJson(StringPiece json1, StringPiece json2) { ...@@ -31,6 +31,72 @@ bool compareJson(StringPiece json1, StringPiece json2) {
return obj1 == obj2; return obj1 == obj2;
} }
bool compareJsonWithNestedJson(
StringPiece json1,
StringPiece json2,
unsigned strNestingDepth) {
auto obj1 = parseJson(json1);
auto obj2 = parseJson(json2);
return compareDynamicWithNestedJson(obj1, obj2, strNestingDepth);
}
bool compareDynamicWithNestedJson(
dynamic const& obj1,
dynamic const& obj2,
unsigned strNestingDepth) {
if (obj1 == obj2) {
return true;
}
if (strNestingDepth == 0) {
return false;
}
if (obj1.type() != obj2.type()) {
return false;
}
switch (obj1.type()) {
case dynamic::Type::ARRAY:
if (obj1.size() != obj2.size()) {
return false;
}
for (auto i1 = obj1.begin(), i2 = obj2.begin(); i1 != obj1.end();
++i1, ++i2) {
if (!compareDynamicWithNestedJson(*i1, *i2, strNestingDepth)) {
return false;
}
}
return true;
case dynamic::Type::OBJECT:
if (obj1.size() != obj2.size()) {
return false;
}
return std::all_of(
obj1.items().begin(), obj1.items().end(), [&](const auto& item) {
const auto& value1 = item.second;
const auto value2 = obj2.get_ptr(item.first);
return value2 &&
compareDynamicWithNestedJson(value1, *value2, strNestingDepth);
});
case dynamic::Type::STRING:
if (obj1.getString().find('{') != std::string::npos &&
obj2.getString().find('{') != std::string::npos) {
try {
auto nested1 = parseJson(obj1.getString());
auto nested2 = parseJson(obj2.getString());
return compareDynamicWithNestedJson(
nested1, nested2, strNestingDepth - 1);
} catch (...) {
// parse error, rely on basic equality check already failed
}
}
return false;
default:
return false;
}
assume_unreachable();
}
namespace { namespace {
bool isClose(double x, double y, double tolerance) { bool isClose(double x, double y, double tolerance) {
...@@ -87,7 +153,7 @@ bool compareDynamicWithTolerance( ...@@ -87,7 +153,7 @@ bool compareDynamicWithTolerance(
compareDynamicWithTolerance(value1, *value2, tolerance); compareDynamicWithTolerance(value1, *value2, tolerance);
}); });
case dynamic::Type::STRING: case dynamic::Type::STRING:
return obj1.asString() == obj2.asString(); return obj1.getString() == obj2.getString();
} }
assume_unreachable(); assume_unreachable();
......
...@@ -16,23 +16,45 @@ ...@@ -16,23 +16,45 @@
#pragma once #pragma once
#include <iostream>
#include <string>
#include <folly/Range.h> #include <folly/Range.h>
#include <folly/dynamic.h> #include <folly/dynamic.h>
#include <folly/portability/GMock.h>
namespace folly { namespace folly {
/** /**
* Compares two JSON strings and returns whether they represent the * Compares two JSON strings and returns whether they represent the
* same document (thus ignoring things like object ordering or * same document (thus ignoring things like object ordering or multiple
* multiple representations of the same number). * representations of the same number).
* *
* This is implemented by deserializing both strings into dynamic, so * This is implemented by deserializing both strings into dynamic, so it
* it is not efficient and it is meant to only be used in tests. * is not efficient and it is meant to only be used in tests.
* *
* It will throw an exception if any of the inputs is invalid. * It will throw an exception if any of the inputs is invalid.
*/ */
bool compareJson(StringPiece json1, StringPiece json2); bool compareJson(StringPiece json1, StringPiece json2);
/**
* Like compareJson, but if strNestingDepth > 0 then contained strings that
* are valid JSON will be compared using compareJsonWithNestedJson(str1,
* str2, strNestingDepth - 1).
*/
bool compareJsonWithNestedJson(
StringPiece json1,
StringPiece json2,
unsigned strNestingDepth);
/**
* Like compareJson, but with dynamic instances.
*/
bool compareDynamicWithNestedJson(
dynamic const& obj1,
dynamic const& obj2,
unsigned strNestingDepth);
/** /**
* Like compareJson, but allows for the given tolerance when comparing * Like compareJson, but allows for the given tolerance when comparing
* numbers. * numbers.
...@@ -59,6 +81,57 @@ bool compareDynamicWithTolerance( ...@@ -59,6 +81,57 @@ bool compareDynamicWithTolerance(
const dynamic& obj2, const dynamic& obj2,
double tolerance); double tolerance);
namespace detail {
template <typename T>
class JsonEqMatcher : public ::testing::MatcherInterface<T> {
public:
explicit JsonEqMatcher(std::string expected, std::string prefixBeforeJson)
: expected_(std::move(expected)),
prefixBeforeJson_(std::move(prefixBeforeJson)) {}
virtual bool MatchAndExplain(
T const& actual,
::testing::MatchResultListener* /*listener*/) const override {
StringPiece sp{actual};
if (!sp.startsWith(prefixBeforeJson_)) {
return false;
}
return compareJson(sp.subpiece(prefixBeforeJson_.size()), expected_);
}
virtual void DescribeTo(::std::ostream* os) const override {
*os << "JSON is equivalent to " << expected_;
if (prefixBeforeJson_.size() > 0) {
*os << " after removing prefix '" << prefixBeforeJson_ << "'";
}
}
virtual void DescribeNegationTo(::std::ostream* os) const override {
*os << "JSON is not equivalent to " << expected_;
if (prefixBeforeJson_.size() > 0) {
*os << " after removing prefix '" << prefixBeforeJson_ << "'";
}
}
private:
std::string expected_;
std::string prefixBeforeJson_;
};
} // namespace detail
/**
* GMock matcher that uses compareJson, expecting exactly a type T as an
* argument. Popular choices for T are std::string const&, StringPiece, and
* std::string. prefixBeforeJson should not be present in expected.
*/
template <typename T>
::testing::Matcher<T> JsonEq(
std::string expected,
std::string prefixBeforeJson = "") {
return ::testing::MakeMatcher(new detail::JsonEqMatcher<T>(
std::move(expected), std::move(prefixBeforeJson)));
}
} // namespace folly } // namespace folly
/** /**
...@@ -68,5 +141,8 @@ bool compareDynamicWithTolerance( ...@@ -68,5 +141,8 @@ bool compareDynamicWithTolerance(
#define FOLLY_EXPECT_JSON_EQ(json1, json2) \ #define FOLLY_EXPECT_JSON_EQ(json1, json2) \
EXPECT_PRED2(::folly::compareJson, json1, json2) EXPECT_PRED2(::folly::compareJson, json1, json2)
#define FOLLY_EXPECT_JSON_WITH_NESTED_JSON_EQ(json1, json2) \
EXPECT_PRED3(::folly::compareJsonWithNestedJson, json1, json2, 1)
#define FOLLY_EXPECT_JSON_NEAR(json1, json2, tolerance) \ #define FOLLY_EXPECT_JSON_NEAR(json1, json2, tolerance) \
EXPECT_PRED3(::folly::compareJsonWithTolerance, json1, json2, tolerance) EXPECT_PRED3(::folly::compareJsonWithTolerance, json1, json2, tolerance)
...@@ -31,6 +31,18 @@ TEST(CompareJson, Simple) { ...@@ -31,6 +31,18 @@ TEST(CompareJson, Simple) {
EXPECT_FALSE(compareJson(json1, json3)); EXPECT_FALSE(compareJson(json1, json3));
} }
TEST(CompareJson, Nested) {
constexpr StringPiece json1 = R"({"a": "{\"A\": 1, \"B\": 2}", "b": 2})";
LOG(INFO) << json1 << "\n";
constexpr StringPiece json2 = R"({"b": 2, "a": "{\"B\": 2, \"A\": 1}"})";
EXPECT_FALSE(compareJsonWithNestedJson(json1, json2, 0));
EXPECT_TRUE(compareJsonWithNestedJson(json1, json2, 1));
constexpr StringPiece json3 = R"({"b": 2, "a": "{\"B\": 3, \"A\": 1}"})";
EXPECT_FALSE(compareJsonWithNestedJson(json1, json3, 0));
EXPECT_FALSE(compareJsonWithNestedJson(json1, json3, 1));
}
TEST(CompareJson, Malformed) { TEST(CompareJson, Malformed) {
constexpr StringPiece json1 = R"({"a": 1, "b": 2})"; constexpr StringPiece json1 = R"({"a": 1, "b": 2})";
constexpr StringPiece json2 = R"({"b": 2, "a": 1)"; constexpr StringPiece json2 = R"({"b": 2, "a": 1)";
...@@ -71,3 +83,10 @@ TEST(CompareJsonWithTolerance, Simple) { ...@@ -71,3 +83,10 @@ TEST(CompareJsonWithTolerance, Simple) {
FOLLY_EXPECT_JSON_NEAR( FOLLY_EXPECT_JSON_NEAR(
R"({"a": 1, "b": 2})", R"({"b": 2.01, "a": 1.05})", 0.1); R"({"a": 1, "b": 2})", R"({"b": 2.01, "a": 1.05})", 0.1);
} }
TEST(JsonEqMock, Simple) {
EXPECT_THAT(StringPiece{"1"}, JsonEq<StringPiece>("1"));
EXPECT_THAT(std::string{"1"}, JsonEq<std::string>("1"));
EXPECT_THAT(std::string{"1"}, JsonEq<std::string const&>("1"));
EXPECT_THAT(std::string{"^J1"}, JsonEq<std::string const&>("1", "^J"));
}
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