Merge pull request #49595 from ClickHouse/remove-dangerous-code

Remove dangerous code (stringstream)
This commit is contained in:
Alexey Milovidov 2023-05-07 04:51:22 +03:00 committed by GitHub
commit 8323c59742
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 29 additions and 67 deletions

View File

@ -26,9 +26,7 @@ using namespace DB;
template <typename T>
void checkColumn(
const WeakHash32::Container & hash,
const PaddedPODArray<T> & eq_class,
size_t allowed_collisions = 0,
size_t max_collisions_to_print = 10)
const PaddedPODArray<T> & eq_class)
{
ASSERT_EQ(hash.size(), eq_class.size());
@ -52,41 +50,6 @@ void checkColumn(
}
}
}
/// Check have not many collisions.
{
std::unordered_map<UInt32, T> map;
size_t num_collisions = 0;
std::stringstream collisions_str; // STYLE_CHECK_ALLOW_STD_STRING_STREAM
collisions_str.exceptions(std::ios::failbit);
for (size_t i = 0; i < eq_class.size(); ++i)
{
auto & val = eq_class[i];
auto it = map.find(hash[i]);
if (it == map.end())
map[hash[i]] = val;
else if (it->second != val)
{
++num_collisions;
if (num_collisions <= max_collisions_to_print)
{
collisions_str << "Collision:\n";
}
if (num_collisions > allowed_collisions)
{
std::cerr << collisions_str.rdbuf();
break;
}
}
}
ASSERT_LE(num_collisions, allowed_collisions);
}
}
TEST(WeakHash32, ColumnVectorU8)
@ -374,10 +337,7 @@ TEST(WeakHash32, ColumnString2)
WeakHash32 hash(col->size());
col->updateWeakHash32(hash);
/// Now there is single collision between 'k' * 544 and 'q' * 2512 (which is calculated twice)
size_t allowed_collisions = 4;
checkColumn(hash.getData(), data, allowed_collisions);
checkColumn(hash.getData(), data);
}
TEST(WeakHash32, ColumnString3)
@ -717,8 +677,7 @@ TEST(WeakHash32, ColumnTupleUInt64String)
WeakHash32 hash(col_tuple->size());
col_tuple->updateWeakHash32(hash);
size_t allowed_collisions = 8;
checkColumn(hash.getData(), eq, allowed_collisions);
checkColumn(hash.getData(), eq);
}
TEST(WeakHash32, ColumnTupleUInt64FixedString)
@ -803,10 +762,5 @@ TEST(WeakHash32, ColumnTupleUInt64Array)
WeakHash32 hash(col_tuple->size());
col_tuple->updateWeakHash32(hash);
/// There are 2 collisions right now (repeated 2 times each):
/// (0, [array of size 1212 with values 7]) vs (0, [array of size 2265 with values 17])
/// (0, [array of size 558 with values 5]) vs (1, [array of size 879 with values 21])
size_t allowed_collisions = 8;
checkColumn(hash.getData(), eq_data, allowed_collisions);
checkColumn(hash.getData(), eq_data);
}

View File

@ -9,6 +9,10 @@
#include <Common/MemorySanitizer.h>
#include <Common/SymbolIndex.h>
#include <IO/WriteBufferFromString.h>
#include <IO/WriteHelpers.h>
#include <IO/Operators.h>
#include <atomic>
#include <filesystem>
#include <map>
@ -340,8 +344,6 @@ toStringEveryLineImpl([[maybe_unused]] bool fatal, const StackTraceRefTriple & s
return callback("<Empty trace>");
#if defined(__ELF__) && !defined(OS_FREEBSD)
std::stringstream out; // STYLE_CHECK_ALLOW_STD_STRING_STREAM
out.exceptions(std::ios::failbit);
using enum DB::Dwarf::LocationInfoMode;
const auto mode = fatal ? FULL_WITH_INLINE : FAST;
@ -358,6 +360,7 @@ toStringEveryLineImpl([[maybe_unused]] bool fatal, const StackTraceRefTriple & s
uintptr_t virtual_offset = object ? uintptr_t(object->address_begin) : 0;
const void * physical_addr = reinterpret_cast<const void *>(uintptr_t(virtual_addr) - virtual_offset);
DB::WriteBufferFromOwnString out;
out << i << ". ";
if (std::error_code ec; object && std::filesystem::exists(object->name, ec) && !ec)
@ -376,7 +379,10 @@ toStringEveryLineImpl([[maybe_unused]] bool fatal, const StackTraceRefTriple & s
out << "?";
if (shouldShowAddress(physical_addr))
out << " @ " << physical_addr;
{
out << " @ ";
DB::writePointerHex(physical_addr, out);
}
out << " in " << (object ? object->name : "?");
@ -393,7 +399,6 @@ toStringEveryLineImpl([[maybe_unused]] bool fatal, const StackTraceRefTriple & s
}
callback(out.str());
out.str({});
}
#else
for (size_t i = stack_trace.offset; i < stack_trace.size; ++i)
@ -431,8 +436,7 @@ String toStringCached(const StackTrace::FramePointers & pointers, size_t offset,
return it->second;
else
{
std::ostringstream out; // STYLE_CHECK_ALLOW_STD_STRING_STREAM
out.exceptions(std::ios::failbit);
DB::WriteBufferFromOwnString out;
toStringEveryLineImpl(false, key, [&](std::string_view str) { out << str << '\n'; });
return cache.emplace(StackTraceTriple{pointers, offset, size}, out.str()).first->second;

View File

@ -6,6 +6,9 @@
#include <sstream>
#include <stdexcept>
#include <IO/WriteBufferFromString.h>
#include <IO/Operators.h>
namespace
{
@ -153,19 +156,17 @@ std::pair<bool, std::string> StudentTTest::compareAndReport(size_t confidence_le
double mean_confidence_interval = table_value * t_statistic;
std::stringstream ss; // STYLE_CHECK_ALLOW_STD_STRING_STREAM
ss.exceptions(std::ios::failbit);
DB::WriteBufferFromOwnString out;
if (mean_difference > mean_confidence_interval && (mean_difference - mean_confidence_interval > 0.0001)) /// difference must be more than 0.0001, to take into account connection latency.
{
ss << "Difference at " << confidence_level[confidence_level_index] << "% confidence: ";
ss << std::fixed << std::setprecision(8) << "mean difference is " << mean_difference << ", but confidence interval is " << mean_confidence_interval;
return {false, ss.str()};
out << "Difference at " << confidence_level[confidence_level_index] << "% confidence: ";
out << "mean difference is " << mean_difference << ", but confidence interval is " << mean_confidence_interval;
return {false, out.str()};
}
else
{
ss << "No difference proven at " << confidence_level[confidence_level_index] << "% confidence";
return {true, ss.str()};
out << "No difference proven at " << confidence_level[confidence_level_index] << "% confidence";
return {true, out.str()};
}
}

View File

@ -358,10 +358,13 @@ private:
/// NOTE: This still require memory allocations and mutex lock inside logger.
/// BTW we can also print it to stderr using write syscalls.
std::stringstream bare_stacktrace; // STYLE_CHECK_ALLOW_STD_STRING_STREAM
bare_stacktrace << "Stack trace:";
DB::WriteBufferFromOwnString bare_stacktrace;
DB::writeString("Stack trace:", bare_stacktrace);
for (size_t i = stack_trace.getOffset(); i < stack_trace.getSize(); ++i)
bare_stacktrace << ' ' << stack_trace.getFramePointers()[i];
{
DB::writeChar(' ', bare_stacktrace);
DB::writePointerHex(stack_trace.getFramePointers()[i], bare_stacktrace);
}
LOG_FATAL(log, fmt::runtime(bare_stacktrace.str()));
}