From cd384a4ab2861efa93f406e5acf1d16c6c837d67 Mon Sep 17 00:00:00 2001 From: Dennis Hezel Date: Fri, 6 Oct 2023 18:26:22 +0200 Subject: [PATCH] fix: Avoid ub when casting function pointer to regular pointer --- src/agrpc/detail/basic_sender.hpp | 11 ++++------- src/agrpc/detail/operation_base.hpp | 18 ++++++++++++++++-- .../detail/repeatedly_request_context.hpp | 4 ++-- .../detail/sender_implementation_operation.hpp | 2 +- src/agrpc/detail/server_write_reactor.hpp | 10 +++++----- 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/agrpc/detail/basic_sender.hpp b/src/agrpc/detail/basic_sender.hpp index 638bc467..47f293c2 100644 --- a/src/agrpc/detail/basic_sender.hpp +++ b/src/agrpc/detail/basic_sender.hpp @@ -159,7 +159,7 @@ class BasicSenderRunningOperation : public detail::BaseForSenderImplementationTy template void set_on_complete() noexcept { - detail::OperationBaseAccess::get_on_complete(*this) = &do_complete; + detail::OperationBaseAccess::set_on_complete(*this, &do_complete); } template @@ -169,17 +169,14 @@ class BasicSenderRunningOperation : public detail::BaseForSenderImplementationTy detail::satisfy_receiver(static_cast(receiver()), static_cast(args)...); } - void put_into_scratch_space(void* ptr) noexcept - { - detail::OperationBaseAccess::get_on_complete(*this) = reinterpret_cast(ptr); - } + void put_into_scratch_space(void* ptr) noexcept { detail::OperationBaseAccess::set_scratch_space(*this, ptr); } [[nodiscard]] void* get_scratch_space() const noexcept { - return reinterpret_cast(detail::OperationBaseAccess::get_on_complete(*this)); + return detail::OperationBaseAccess::get_scratch_space(*this); } - void restore_scratch_space() noexcept { detail::OperationBaseAccess::get_on_complete(*this) = &do_complete; } + void restore_scratch_space() noexcept { detail::OperationBaseAccess::set_on_complete(*this, &do_complete); } private: detail::CompressedPair, Implementation> impl_; diff --git a/src/agrpc/detail/operation_base.hpp b/src/agrpc/detail/operation_base.hpp index 39553771..000e4fb9 100644 --- a/src/agrpc/detail/operation_base.hpp +++ b/src/agrpc/detail/operation_base.hpp @@ -53,7 +53,11 @@ class OperationBase private: friend detail::OperationBaseAccess; - OperationOnComplete on_complete_; + union + { + OperationOnComplete on_complete_; + void* scratch_space_; + }; }; class QueueableOperationBase : public detail::OperationBase @@ -71,9 +75,19 @@ class QueueableOperationBase : public detail::OperationBase struct OperationBaseAccess { - static auto& get_on_complete(detail::OperationBase& operation) noexcept { return operation.on_complete_; } + static void set_on_complete(detail::OperationBase& operation, OperationOnComplete on_complete) noexcept + { + operation.on_complete_ = on_complete; + } static auto get_on_complete(const detail::OperationBase& operation) noexcept { return operation.on_complete_; } + + static void set_scratch_space(detail::OperationBase& operation, void* ptr) noexcept + { + operation.scratch_space_ = ptr; + } + + static void* get_scratch_space(const detail::OperationBase& operation) noexcept { return operation.scratch_space_; } }; [[nodiscard]] constexpr bool is_ok(OperationResult result) noexcept { return result == OperationResult::OK; } diff --git a/src/agrpc/detail/repeatedly_request_context.hpp b/src/agrpc/detail/repeatedly_request_context.hpp index 24df41e1..9947398f 100644 --- a/src/agrpc/detail/repeatedly_request_context.hpp +++ b/src/agrpc/detail/repeatedly_request_context.hpp @@ -107,8 +107,8 @@ class RepeatedlyRequestOperation : public detail::QueueableOperationBase, void add_completing_operation(agrpc::GrpcContext& grpc_context) { - detail::OperationBaseAccess::get_on_complete(*this) = - detail::DO_COMPLETE_NO_ARG_HANDLER; + detail::OperationBaseAccess::set_on_complete(*this, + detail::DO_COMPLETE_NO_ARG_HANDLER); detail::GrpcContextImplementation::add_local_operation(grpc_context, this); } diff --git a/src/agrpc/detail/sender_implementation_operation.hpp b/src/agrpc/detail/sender_implementation_operation.hpp index eda40692..9b47ffef 100644 --- a/src/agrpc/detail/sender_implementation_operation.hpp +++ b/src/agrpc/detail/sender_implementation_operation.hpp @@ -105,7 +105,7 @@ struct SenderImplementationOperation : public detail::BaseForSenderImplementatio template void set_on_complete() noexcept { - detail::OperationBaseAccess::get_on_complete(*this) = &do_complete; + detail::OperationBaseAccess::set_on_complete(*this, &do_complete); } template diff --git a/src/agrpc/detail/server_write_reactor.hpp b/src/agrpc/detail/server_write_reactor.hpp index 1ad2cc03..a67d3139 100644 --- a/src/agrpc/detail/server_write_reactor.hpp +++ b/src/agrpc/detail/server_write_reactor.hpp @@ -63,7 +63,7 @@ class ServerWriteReactor : public detail::ServerWriteReactorStepBase, public det void write(const Response& response) { - get_on_complete() = &ServerWriteReactor::do_write_done; + set_on_complete(&ServerWriteReactor::do_write_done); writer_.Write(response, static_cast(this)); } @@ -71,7 +71,7 @@ class ServerWriteReactor : public detail::ServerWriteReactorStepBase, public det void finish(const grpc::Status& status) { - get_on_complete() = &ServerWriteReactor::do_finish_done; + set_on_complete(&ServerWriteReactor::do_finish_done); writer_.Finish(status, static_cast(this)); } @@ -80,9 +80,9 @@ class ServerWriteReactor : public detail::ServerWriteReactorStepBase, public det private: auto get_allocator() const noexcept { return grpc_context_->get_allocator(); } - auto& get_on_complete() noexcept + void set_on_complete(detail::OperationOnComplete on_complete) noexcept { - return detail::OperationBaseAccess::get_on_complete(*static_cast(this)); + detail::OperationBaseAccess::set_on_complete(*static_cast(this), on_complete); } auto get_on_complete() const noexcept @@ -90,7 +90,7 @@ class ServerWriteReactor : public detail::ServerWriteReactorStepBase, public det return detail::OperationBaseAccess::get_on_complete(*static_cast(this)); } - void set_step_done() noexcept { get_on_complete() = nullptr; } + void set_step_done() noexcept { set_on_complete(nullptr); } bool is_finishing_or_writing() const noexcept { return nullptr != get_on_complete(); }