diff --git a/sstables/sstables.cc b/sstables/sstables.cc index c628495775..1f629bd5c5 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -79,6 +79,7 @@ #include "tracing/traced_file.hh" #include "kl/reader.hh" #include "mx/reader.hh" +#include "utils/bit_cast.hh" thread_local disk_error_signal_type sstable_read_error; thread_local disk_error_signal_type sstable_write_error; @@ -215,54 +216,35 @@ static void check_buf_size(temporary_buffer& buf, size_t expected) { } } -// Base parser, parses an integer type template -typename std::enable_if_t::value, void> -read_integer(temporary_buffer& buf, T& i) { - auto *nr = reinterpret_cast *>(buf.get()); - i = net::ntoh(*nr); -} - -template -typename std::enable_if_t::value, future<>> -parse(const schema&, sstable_version_types v, random_access_reader& in, T& i) { +requires std::is_integral_v +future<> parse(const schema&, sstable_version_types v, random_access_reader& in, T& i) { return in.read_exactly(sizeof(T)).then([&i] (auto buf) { check_buf_size(buf, sizeof(T)); - - read_integer(buf, i); + i = net::ntoh(read_unaligned(buf.get())); return make_ready_future<>(); }); } - template -typename std::enable_if_t::value, future<>> -parse(const schema& s, sstable_version_types v, random_access_reader& in, T& i) { - return parse(s, v, in, reinterpret_cast::type&>(i)); +requires std::is_enum_v +future<> parse(const schema& s, sstable_version_types v, random_access_reader& in, T& i) { + return in.read_exactly(sizeof(T)).then([&i] (auto buf) { + check_buf_size(buf, sizeof(T)); + i = static_cast(net::ntoh(read_unaligned>(buf.get()))); + return make_ready_future<>(); + }); } future<> parse(const schema& s, sstable_version_types v, random_access_reader& in, bool& i) { return parse(s, v, in, reinterpret_cast(i)); } -template -static inline To convert(From f) { - static_assert(sizeof(To) == sizeof(From), "Sizes must match"); - union { - To to; - From from; - } conv; - - conv.from = f; - return conv.to; -} - future<> parse(const schema&, sstable_version_types, random_access_reader& in, double& d) { return in.read_exactly(sizeof(double)).then([&d] (auto buf) { check_buf_size(buf, sizeof(double)); - - auto *nr = reinterpret_cast *>(buf.get()); - d = convert(net::ntoh(*nr)); + unsigned long nr = read_unaligned(buf.get()); + d = bit_cast(net::ntoh(nr)); return make_ready_future<>(); }); } @@ -372,9 +354,8 @@ parse(const schema&, sstable_version_types, random_access_reader& in, Size& len, return in.read_exactly(now * sizeof(Members)).then([&arr, len, now, done] (auto buf) { check_buf_size(buf, now * sizeof(Members)); - auto *nr = reinterpret_cast *>(buf.get()); for (size_t i = 0; i < now; ++i) { - arr.push_back(net::ntoh(nr[i])); + arr.push_back(net::ntoh(read_unaligned(buf.get() + i * sizeof(Members)))); } *done += now; return make_ready_future(*done == len ? stop_iteration::yes : stop_iteration::no); @@ -693,10 +674,9 @@ future<> parse(const schema& s, sstable_version_types v, random_access_reader& i check_buf_size(buf, length * type_size); return do_with(size_t(0), std::move(buf), [&eh, length] (size_t& j, auto& buf) mutable { - auto *nr = reinterpret_cast *>(buf.get()); - return do_until([&eh, length] { return eh.buckets.size() == length; }, [nr, &eh, &j] () mutable { - auto offset = net::ntoh(nr[j++]); - auto bucket = net::ntoh(nr[j++]); + return do_until([&eh, length] { return eh.buckets.size() == length; }, [&eh, &j, &buf] () mutable { + auto offset = net::ntoh(read_unaligned(buf.get() + (j++) * sizeof(uint64_t))); + auto bucket = net::ntoh(read_unaligned(buf.get() + (j++) * sizeof(uint64_t))); if (eh.buckets.size() > 0) { eh.bucket_offsets.push_back(offset); } @@ -714,14 +694,14 @@ void write(sstable_version_types v, file_writer& out, const utils::estimated_his write(v, out, len); struct element { - uint64_t offsets; - uint64_t buckets; + int64_t offsets; + int64_t buckets; }; std::vector elements; elements.reserve(eh.buckets.size()); - auto *offsets_nr = reinterpret_cast *>(eh.bucket_offsets.data()); - auto *buckets_nr = reinterpret_cast *>(eh.buckets.data()); + const int64_t* offsets_nr = eh.bucket_offsets.data(); + const int64_t* buckets_nr = eh.buckets.data(); for (size_t i = 0; i < eh.buckets.size(); i++) { auto offsets = net::hton(offsets_nr[i == 0 ? 0 : i - 1]); auto buckets = net::hton(buckets_nr[i]); @@ -813,9 +793,8 @@ future<> parse(const schema& s, sstable_version_types v, random_access_reader& i return do_until(eoarr, [&in, &c, &len, &offsets] () { auto now = std::min(len - c.offsets.size(), 100000 / sizeof(uint64_t)); return in.read_exactly(now * sizeof(uint64_t)).then([&offsets, now] (auto buf) { - uint64_t value; for (size_t i = 0; i < now; ++i) { - std::copy_n(buf.get() + i * sizeof(uint64_t), sizeof(uint64_t), reinterpret_cast(&value)); + uint64_t value = read_unaligned(buf.get() + i * sizeof(uint64_t)); offsets.push_back(net::ntoh(value)); } }); diff --git a/sstables/writer.hh b/sstables/writer.hh index 02c83dfb7e..a48a4d8bd7 100644 --- a/sstables/writer.hh +++ b/sstables/writer.hh @@ -31,6 +31,7 @@ #include "version.hh" #include "counters.hh" #include "service/storage_service.hh" +#include "utils/bit_cast.hh" namespace sstables { @@ -262,10 +263,8 @@ template requires Writer inline typename std::enable_if_t::value, void> write(sstable_version_types v, W& out, T i) { - auto *nr = reinterpret_cast *>(&i); - i = net::hton(*nr); - auto p = reinterpret_cast(&i); - out.write(p, sizeof(T)); + i = net::hton(i); + out.write(reinterpret_cast(&i), sizeof(T)); } template @@ -283,10 +282,8 @@ inline void write(sstable_version_types v, W& out, bool i) { } inline void write(sstable_version_types v, file_writer& out, double d) { - auto *nr = reinterpret_cast *>(&d); - auto tmp = net::hton(*nr); - auto p = reinterpret_cast(&tmp); - out.write(p, sizeof(unsigned long)); + unsigned long tmp = net::hton(bit_cast(d)); + out.write(reinterpret_cast(&tmp), sizeof(unsigned long)); } diff --git a/transport/server.cc b/transport/server.cc index 1426cdd6a1..4637a3265b 100644 --- a/transport/server.cc +++ b/transport/server.cc @@ -67,6 +67,7 @@ #include "types/user.hh" #include "transport/cql_protocol_extension.hh" +#include "utils/bit_cast.hh" namespace cql_transport { @@ -379,8 +380,8 @@ cql_server::connection::parse_frame(temporary_buffer buf) const { switch (_version) { case 1: case 2: { - auto raw = reinterpret_cast(buf.get()); - auto cooked = net::ntoh(*raw); + cql_binary_frame_v1 raw = read_unaligned(buf.get()); + auto cooked = net::ntoh(raw); v3.version = cooked.version; v3.flags = cooked.flags; v3.opcode = cooked.opcode; @@ -390,7 +391,8 @@ cql_server::connection::parse_frame(temporary_buffer buf) const { } case 3: case 4: { - v3 = net::ntoh(*reinterpret_cast(buf.get())); + cql_binary_frame_v3 raw = read_unaligned(buf.get()); + v3 = net::ntoh(raw); break; } default: diff --git a/types.cc b/types.cc index 28873b72f8..726718fb9d 100644 --- a/types.cc +++ b/types.cc @@ -175,7 +175,7 @@ integer_type_impl::integer_type_impl( template static bytes decompose_value(T v) { bytes b(bytes::initialized_later(), sizeof(v)); - *reinterpret_cast(b.begin()) = (T)net::hton(v); + write_unaligned(b.begin(), net::hton(v)); return b; } diff --git a/types.hh b/types.hh index 9963207eb3..2c9612e234 100644 --- a/types.hh +++ b/types.hh @@ -50,6 +50,7 @@ #include "utils/fragmented_temporary_buffer.hh" #include "utils/exceptions.hh" #include "utils/managed_bytes.hh" +#include "utils/bit_cast.hh" class tuple_type_impl; class big_decimal; @@ -1165,7 +1166,7 @@ T read_simple(bytes_view& v) { } auto p = v.begin(); v.remove_prefix(sizeof(T)); - return net::ntoh(*reinterpret_cast*>(p)); + return net::ntoh(read_unaligned(p)); } template @@ -1174,7 +1175,7 @@ T read_simple_exactly(bytes_view v) { throw_with_backtrace(format("read_simple_exactly - size mismatch (expected {:d}, got {:d})", sizeof(T), v.size())); } auto p = v.begin(); - return net::ntoh(*reinterpret_cast*>(p)); + return net::ntoh(read_unaligned(p)); } inline diff --git a/utils/bit_cast.hh b/utils/bit_cast.hh new file mode 100644 index 0000000000..d2a046d685 --- /dev/null +++ b/utils/bit_cast.hh @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2021 ScyllaDB + */ + +/* + * This file is part of Scylla. + * + * Scylla is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Scylla is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Scylla. If not, see . + */ + +#pragma once + +#include +#include +#include + +template concept Trivial = std::is_trivial_v; +template concept TriviallyCopyable = std::is_trivially_copyable_v; + +// C++20 has std::bit_cast, which does the same thing, +// (but better, because it supports constexpr through compiler magic) +// but it hasn't made its way to our libstdc++ yet. +template +requires (sizeof(To) == sizeof(From)) +To bit_cast(const From& src) noexcept { + To dst; + std::memcpy(&dst, &src, sizeof(To)); + return dst; +} + +template +To read_unaligned(const void* src) { + To dst; + std::memcpy(&dst, src, sizeof(To)); + return dst; +} + +template +void write_unaligned(void* dst, const From& src) { + std::memcpy(dst, &src, sizeof(From)); +} diff --git a/utils/crc.hh b/utils/crc.hh index ed041e17d3..cd8988fb1f 100644 --- a/utils/crc.hh +++ b/utils/crc.hh @@ -105,12 +105,12 @@ public: --size; } if ((reinterpret_cast(in) & 3) && size >= 2) { - process_le(*reinterpret_cast(in)); + process_le(seastar::read_le(reinterpret_cast(in))); in += 2; size -= 2; } if ((reinterpret_cast(in) & 7) && size >= 4) { - process_le(*reinterpret_cast(in)); + process_le(seastar::read_le(reinterpret_cast(in))); in += 4; size -= 4; } @@ -146,17 +146,17 @@ public: } while (size >= 8) { - process_le(*reinterpret_cast(in)); + process_le(seastar::read_le(reinterpret_cast(in))); in += 8; size -= 8; } if (size >= 4) { - process_le(*reinterpret_cast(in)); + process_le(seastar::read_le(reinterpret_cast(in))); in += 4; size -= 4; } if (size >= 2) { - process_le(*reinterpret_cast(in)); + process_le(seastar::read_le(reinterpret_cast(in))); in += 2; size -= 2; } diff --git a/utils/fragment_range.hh b/utils/fragment_range.hh index 3971c4bacf..68820e412d 100644 --- a/utils/fragment_range.hh +++ b/utils/fragment_range.hh @@ -29,6 +29,7 @@ #include "marshal_exception.hh" #include "bytes.hh" +#include "utils/bit_cast.hh" enum class mutable_view { no, yes, }; @@ -346,7 +347,7 @@ T read_simple_native(View& v) { if (v.current_fragment().size() >= sizeof(T)) [[likely]] { auto p = v.current_fragment().data(); v.remove_prefix(sizeof(T)); - return *reinterpret_cast*>(p); + return read_unaligned(p); } else if (v.size_bytes() >= sizeof(T)) { T buf; read_fragmented(v, sizeof(T), reinterpret_cast(&buf)); @@ -361,7 +362,7 @@ T read_simple(View& v) { if (v.current_fragment().size() >= sizeof(T)) [[likely]] { auto p = v.current_fragment().data(); v.remove_prefix(sizeof(T)); - return net::ntoh(*reinterpret_cast*>(p)); + return net::ntoh(read_unaligned(p)); } else if (v.size_bytes() >= sizeof(T)) { T buf; read_fragmented(v, sizeof(T), reinterpret_cast(&buf)); @@ -375,7 +376,7 @@ template T read_simple_exactly(View v) { if (v.current_fragment().size() == sizeof(T)) [[likely]] { auto p = v.current_fragment().data(); - return net::ntoh(*reinterpret_cast*>(p)); + return net::ntoh(read_unaligned(p)); } else if (v.size_bytes() == sizeof(T)) { T buf; read_fragmented(v, sizeof(T), reinterpret_cast(&buf)); diff --git a/utils/linearizing_input_stream.hh b/utils/linearizing_input_stream.hh index 083d2b098b..1a985d84ad 100644 --- a/utils/linearizing_input_stream.hh +++ b/utils/linearizing_input_stream.hh @@ -27,6 +27,7 @@ #include #include "utils/fragment_range.hh" +#include "utils/bit_cast.hh" namespace utils { @@ -112,7 +113,7 @@ public: requires std::is_trivial_v Type read_trivial() { auto [bv, linearized] = do_read(sizeof(Type)); - auto ret = net::ntoh(*reinterpret_cast*>(bv.begin())); + auto ret = net::ntoh(::read_unaligned>(bv.begin())); if (linearized) { _linearized_values.pop_back(); }