Fix double-free in HashTable::clearAndShrink() with zero elements in it

Before this patch HashTable::clearAndShrink() did this:

    destroyElements();
    clearHasZero();

While destroyElements() already destroys the zero element, so it leads
to double free, like here [1].

  [1]: https://s3.amazonaws.com/clickhouse-test-reports/40003/f09e956ddf150c1aceb210348544bc28876a2d11/stateless_tests__asan__[2/4].html

The issue is only for complex types.

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
This commit is contained in:
Azat Khuzhin 2022-12-12 10:20:42 +01:00
parent 980d5ce289
commit 81be20edfe

View File

@ -389,6 +389,11 @@ public:
zeroValue()->~Cell(); zeroValue()->~Cell();
} }
void clearHasZeroFlag()
{
has_zero = false;
}
Cell * zeroValue() { return std::launder(reinterpret_cast<Cell*>(&zero_value_storage)); } Cell * zeroValue() { return std::launder(reinterpret_cast<Cell*>(&zero_value_storage)); }
const Cell * zeroValue() const { return std::launder(reinterpret_cast<const Cell*>(&zero_value_storage)); } const Cell * zeroValue() const { return std::launder(reinterpret_cast<const Cell*>(&zero_value_storage)); }
}; };
@ -399,6 +404,7 @@ struct ZeroValueStorage<false, Cell>
bool hasZero() const { return false; } bool hasZero() const { return false; }
void setHasZero() { throw DB::Exception("HashTable: logical error", DB::ErrorCodes::LOGICAL_ERROR); } void setHasZero() { throw DB::Exception("HashTable: logical error", DB::ErrorCodes::LOGICAL_ERROR); }
void clearHasZero() {} void clearHasZero() {}
void clearHasZeroFlag() {}
Cell * zeroValue() { return nullptr; } Cell * zeroValue() { return nullptr; }
const Cell * zeroValue() const { return nullptr; } const Cell * zeroValue() const { return nullptr; }
@ -652,6 +658,17 @@ protected:
/// [1]: https://github.com/google/sanitizers/issues/854#issuecomment-329661378 /// [1]: https://github.com/google/sanitizers/issues/854#issuecomment-329661378
__msan_unpoison(it.ptr, sizeof(*it.ptr)); __msan_unpoison(it.ptr, sizeof(*it.ptr));
} }
/// Everything had been destroyed in the loop above, reset the flag
/// only, without calling destructor.
this->clearHasZeroFlag();
}
else
{
/// NOTE: it is OK to call dtor for trivially destructible type
/// even the object hadn't been initialized, so no need to has
/// hasZero() check.
this->clearHasZero();
} }
} }
@ -1284,7 +1301,6 @@ public:
Cell::State::read(rb); Cell::State::read(rb);
destroyElements(); destroyElements();
this->clearHasZero();
m_size = 0; m_size = 0;
size_t new_size = 0; size_t new_size = 0;
@ -1308,7 +1324,6 @@ public:
Cell::State::readText(rb); Cell::State::readText(rb);
destroyElements(); destroyElements();
this->clearHasZero();
m_size = 0; m_size = 0;
size_t new_size = 0; size_t new_size = 0;
@ -1342,7 +1357,6 @@ public:
void clear() void clear()
{ {
destroyElements(); destroyElements();
this->clearHasZero();
m_size = 0; m_size = 0;
memset(static_cast<void*>(buf), 0, grower.bufSize() * sizeof(*buf)); memset(static_cast<void*>(buf), 0, grower.bufSize() * sizeof(*buf));
@ -1353,7 +1367,6 @@ public:
void clearAndShrink() void clearAndShrink()
{ {
destroyElements(); destroyElements();
this->clearHasZero();
m_size = 0; m_size = 0;
free(); free();
} }