[Bug libstdc++/91263] New: unordered_map and unordered_set operator== double key comparison causes exponential behavior

2019-07-25 Thread terrelln at fb dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91263

Bug ID: 91263
   Summary: unordered_map and unordered_set operator== double key
comparison causes exponential behavior
   Product: gcc
   Version: 10.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: terrelln at fb dot com
  Target Milestone: ---

When unordered_map or unordered_set has a nested unordered_{map,set} as a key,
operator== becomes exponential in the depth of the nesting. This is because
unordered_{map,set}::operator== does two key comparisons, one in find() using
key_eq() and one when comparing the key-value pairs using the key’s operator==.
These two comparisons can theoretically be different but are almost always the
same in practice. unordered_{map,set} should only do one key comparison in
operator== to avoid this. It is also generally more efficient even in the
non-nesting cases.

Nathan Bronson helped create a minimal repro below that demonstrates this
exponential behavior.

#include 
#include 
#include 

struct Nested;

struct NestedHash {
  std::size_t operator()(Nested const& n) const;
};

struct Nested {
  std::unique_ptr> set;

  explicit Nested(int depth)
  : set(std::make_unique>()) {
if (depth > 0) {
  set->emplace(depth - 1);
}
  }
};

std::size_t NestedHash::operator()(Nested const& n) const {
  std::size_t rv = 1;
  for (auto& k : *n.set) {
rv += operator()(k);
  }
  return rv;
}

bool operator==(Nested const& lhs, Nested const& rhs) {
  return *lhs.set == *rhs.set;
}

bool operator!=(Nested const& lhs, Nested const& rhs) {
  return !(lhs == rhs);
}

void testNestedSetEquality() {
  auto n1 = Nested(100);
  auto n2 = Nested(100);
  auto n3 = Nested(99);
  assert(n1 == n1);
  assert(n1 == n2);
  assert(n1 != n3);
}

This is a contrived example, but this shows up in practice in
folly::dynamic::operator== [1] as a worst case exponential algorithm. No one
will create objects like this intentionally, but they can be produced with
folly::parseJson() [2] when non-string keys are allowed. If an attacker
controlled the input to parseJson() they could easily DDOS the machine with a
payload like "{0:0}:0}:0}:0}:0}" but nested 100 layers deep instead of 5.

unordered_map and unordered_set should only do one key comparison per item in
operator==. This is possible [3] because it is undefined behavior to call the
containers operator== if the key’s operator== isn’t a refinement of key_eq().
Fixing this bug in unordered_map and unordered_set avoids the exponential
behavior with nested keys, and it is generally more efficient even in the
non-nesting cases. An implementation of set and map equality with only one key
comparison is included below, thanks to Nathan Bronson.

template 
bool eqSet(S const& lhs, S const& rhs) {
  if (lhs.size() != rhs.size()) {
return false;
  }
  for (auto& v : lhs) {
auto slot = rhs.bucket(v);
auto b = rhs.begin(slot);
auto e = rhs.end(slot);
while (b != e && !(*b == v)) {
  ++b;
}
if (b == e) {
  return false;
}
  }
  return true;
}

template 
bool eqMap(M const& lhs, M const& rhs) {
  if (lhs.size() != rhs.size()) {
return false;
  }
  for (auto& v : lhs) {
auto slot = rhs.bucket(v.first);
auto b = rhs.begin(slot);
auto e = rhs.end(slot);
while (b != e && !(b->first == v.first)) {
  ++b;
}
if (b == e || !(b->second == v.second)) {
  return false;
}
  }
  return true;
}

[1] https://github.com/facebook/folly/blob/master/folly/dynamic.cpp#L113
[2] https://github.com/facebook/folly/blob/master/folly/json.h#L194
[3]
https://github.com/facebook/folly/commit/11855c21b21fb46fe1004de8c0412dd94eb5c45f

[Bug tree-optimization/81388] New: Incorrect code generation with -O1 -fno-strict-overflow

2017-07-10 Thread terrelln at fb dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81388

Bug ID: 81388
   Summary: Incorrect code generation with -O1
-fno-strict-overflow
   Product: gcc
   Version: 7.1.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: tree-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: terrelln at fb dot com
  Target Milestone: ---

gcc 7.1 on x86 produces incorrect output for correct code. gcc 6.3 produced
correct assembly. See https://godbolt.org/g/mLyxGz or below for the code and
generated assembly.

code:
.
// -O1 -fno-strict-overflow

void bar();
void foo(char *dst)
{
char *const end = dst;
do {
bar();
dst += 2;
} while (dst < end);
}
`

assembly:
.
foo(char*):
pushrbx
movabs  rbx, -9223372036854775808
.L2:
callbar()
sub rbx, 1
jne .L2
pop rbx
ret
`

[Bug tree-optimization/81388] Incorrect code generation with -O1 -fno-strict-overflow

2017-07-10 Thread terrelln at fb dot com
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81388

--- Comment #4 from Nick Terrell  ---
Good point Marc.

The generated assembly is wrong because the loop should be run once, but it is
run 2^63  times. Changing the amount dst is incremented by to 4 makes it run
2^62 times, and changing it to 8 makes it run 2^61 times.