From 7fbb3d2cc7c8c3e2f60ddfabc6b178fa8b141a8a Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 10 Dec 2024 11:06:12 -0800 Subject: [PATCH] Make conversion functions for into c++ string types from PtrAndLen. The invariant is that PtrAndLen may hold ptr+len values which are legal for _either_ C++ string_view or Rust slices: constructing one from either Rust or C++ permits the laxest constraints, and care must be taken when converting a PtrAndLen into either type. For "into Rust slice" case to handle is that len=0 ptr must be non-null, so len=0 ptr=null gets turned into an arbitrary non-null pointer as provided by std::ptr::NonNull::dangling() (any preexisting non-null ptr is kept in that direction). For the "into C++ slice" the risk is more obscure that a Rust non-null pointer could potentially be an illegal pointer (such that ptr+0 is not a legal operation in C++), so when going into C++ we map any len=0 cases into ptr=null to avoid this as a possible risk. PiperOrigin-RevId: 704776210 --- rust/cpp_kernel/BUILD | 1 + rust/cpp_kernel/map.cc | 5 ++--- rust/cpp_kernel/strings.cc | 15 ++++++++++++++- rust/cpp_kernel/strings.h | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/rust/cpp_kernel/BUILD b/rust/cpp_kernel/BUILD index f16566b80c4d..ad4580701cf5 100644 --- a/rust/cpp_kernel/BUILD +++ b/rust/cpp_kernel/BUILD @@ -31,6 +31,7 @@ cc_library( "@com_google_absl//absl/functional:overload", "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/log:absl_log", + "@com_google_absl//absl/strings:string_view", ], ) diff --git a/rust/cpp_kernel/map.cc b/rust/cpp_kernel/map.cc index e776599cf829..4e4f78ca042c 100644 --- a/rust/cpp_kernel/map.cc +++ b/rust/cpp_kernel/map.cc @@ -1,6 +1,5 @@ #include "google/protobuf/map.h" -#include #include #include #include @@ -8,7 +7,7 @@ #include #include "absl/functional/overload.h" -#include "absl/log/absl_log.h" +#include "absl/strings/string_view.h" #include "google/protobuf/message.h" #include "google/protobuf/message_lite.h" #include "rust/cpp_kernel/strings.h" @@ -67,7 +66,7 @@ template bool Insert(internal::UntypedMapBase* m, Key key, MapValue value) { internal::NodeBase* node = internal::RustMapHelper::AllocNode(m); if constexpr (std::is_same::value) { - new (node->GetVoidKey()) std::string(key.ptr, key.len); + key.PlacementNewString(node->GetVoidKey()); } else { *static_cast(node->GetVoidKey()) = key; } diff --git a/rust/cpp_kernel/strings.cc b/rust/cpp_kernel/strings.cc index af29ddeaf430..d1af5656b02c 100644 --- a/rust/cpp_kernel/strings.cc +++ b/rust/cpp_kernel/strings.cc @@ -3,12 +3,25 @@ #include #include +#include "absl/strings/string_view.h" #include "rust/cpp_kernel/rust_alloc_for_cpp_api.h" namespace google { namespace protobuf { namespace rust { +std::string PtrAndLen::CopyToString() const { + return len == 0 ? "" : std::string(ptr, len); +} + +absl::string_view PtrAndLen::AsStringView() const { + return absl::string_view(len == 0 ? nullptr : ptr, len); +} + +void PtrAndLen::PlacementNewString(void* location) { + new (location) std::string(len == 0 ? nullptr : ptr, len); +} + RustStringRawParts::RustStringRawParts(std::string src) { if (src.empty()) { data = nullptr; @@ -27,7 +40,7 @@ RustStringRawParts::RustStringRawParts(std::string src) { extern "C" { std::string* proto2_rust_cpp_new_string(google::protobuf::rust::PtrAndLen src) { - return new std::string(src.ptr, src.len); + return new std::string(src.CopyToString()); } void proto2_rust_cpp_delete_string(std::string* str) { delete str; } diff --git a/rust/cpp_kernel/strings.h b/rust/cpp_kernel/strings.h index b3302a191972..124b6395dd8b 100644 --- a/rust/cpp_kernel/strings.h +++ b/rust/cpp_kernel/strings.h @@ -12,16 +12,32 @@ #include #include +#include "absl/strings/string_view.h" + namespace google { namespace protobuf { namespace rust { // Represents an ABI-stable version of &[u8]/string_view (borrowed slice of // bytes) for FFI use only. +// +// Note that the intent is for either Rust or C++ to construct one of these with +// the pointer that they would have from a Rust slice or C++ string_view. +// Notably this means that if len==0, ptr may be any value, including nullptr or +// an invalid pointer, which may be a value incompatible for use with either a +// Rust slice or C++ string_view. +// +// It may be constructed trivially, but use the provided conversion methods +// when converting from this type into any other type to avoid obscure undefined +// behavior. struct PtrAndLen { /// Borrows the memory. const char* ptr; size_t len; + + std::string CopyToString() const; + absl::string_view AsStringView() const; + void PlacementNewString(void* location); }; // Represents an owned string for FFI purposes.