Add rs_std::SliceRef type This is needed by Crubit for translating between C++ spans and Rust's slices. PiperOrigin-RevId: 661409789 Change-Id: I30284a2588c2486892e712f507f59176da3b2959
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs index 3b24687..b23e2bc 100644 --- a/cc_bindings_from_rs/bindings.rs +++ b/cc_bindings_from_rs/bindings.rs
@@ -31,7 +31,9 @@ use rustc_middle::ty::{self, Ty, TyCtxt}; // See <internal link>/ty.html#import-conventions use rustc_span::def_id::{DefId, LocalDefId, LOCAL_CRATE}; use rustc_span::symbol::{kw, sym, Symbol}; -use rustc_target::abi::{Abi, FieldsShape, Integer, Layout, Primitive, Scalar}; +use rustc_target::abi::{ + Abi, AddressSpace, FieldsShape, Integer, Layout, Pointer, Primitive, Scalar, +}; use rustc_target::spec::PanicStrategy; use rustc_trait_selection::infer::InferCtxtExt; use rustc_type_ir::RegionKind; @@ -494,6 +496,28 @@ Ok(CcSnippet { prereqs, tokens: quote! { #tokens #const_qualifier #pointer_sigil } }) } +/// Checks that `ty` has the same ABI as `rs_std::SliceRef`. +fn check_slice_layout<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) { + // Check the assumption from `rust_builtin_type_abi_assumptions.md` that Rust's + // slice has the same ABI as `rs_std::SliceRef`. + let layout = tcx + .layout_of(ty::ParamEnv::empty().and(ty)) + .expect("`layout_of` is expected to succeed for `{ty}` type") + .layout; + assert_eq!(8, layout.align().abi.bytes()); + assert_eq!(16, layout.size().bytes()); + assert!(matches!( + layout.abi(), + Abi::ScalarPair( + Scalar::Initialized { value: Pointer(AddressSpace(_)), .. }, + Scalar::Initialized { + value: Primitive::Int(Integer::I64, /* signedness = */ false), + .. + } + ) + )); +} + /// Formats `ty` into a `CcSnippet` that represents how the type should be /// spelled in a C++ declaration of a function parameter or field. fn format_ty_for_cc<'tcx>( @@ -647,6 +671,9 @@ } ty::TyKind::RawPtr(pointee_mid, mutbl) => { + if let ty::TyKind::Slice(_) = pointee_mid.kind() { + check_slice_layout(db.tcx(), ty.mid()); + } let mut pointee_hir = None; if let Some(hir) = ty.hir(db) { if let rustc_hir::TyKind::Ptr(mut_p) = hir.kind { @@ -660,6 +687,9 @@ } ty::TyKind::Ref(region, referent_mid, mutability) => { + if let ty::TyKind::Slice(_) = referent_mid.kind() { + check_slice_layout(db.tcx(), ty.mid()); + } let mut referent_hir = None; if let Some(hir) = ty.hir(db) { if let rustc_hir::TyKind::Ref(_, mut_p, ..) = &hir.kind { @@ -7673,6 +7703,8 @@ "The following Rust type is not supported yet: [i32; 42]", ), ( + // Check that the failure for slices is about not being supported and not failed + // asserts about ABI and layout. "&'static [i32]", // TyKind::Slice (nested underneath TyKind::Ref) "Failed to format the referent of the reference type `&'static [i32]`: \ The following Rust type is not supported yet: [i32]",
diff --git a/docs/overview/rust_builtin_type_abi_assumptions.md b/docs/overview/rust_builtin_type_abi_assumptions.md index 01b08d6..a6c94d6 100644 --- a/docs/overview/rust_builtin_type_abi_assumptions.md +++ b/docs/overview/rust_builtin_type_abi_assumptions.md
@@ -32,7 +32,7 @@ `crubit/support/rs_std/rs_char.h`). The assumptions are verified by assertions that verify the properties of the -target achitecture when `cc_bindings_from_rs` runs (`layout.align()`, +target architecture when `cc_bindings_from_rs` runs (`layout.align()`, `layout.size()`, and `layout.abi()` assertions in `format_ty_for_cc` in `cc_bindings_from_rs/bindings.rs`). Similar assertions are verified on C++ side in `support/rs_std/rs_char_test.cc`. These assertions seem unlikely to fail, but @@ -51,10 +51,14 @@ Rust does *not* document the ABI of slice references (i.e. if the pointer comes before or after the length in memory). `cc_bindings_from_rs` assumes that `&[T]` -has the same ABI as (future) `rs_std::slice<T>` - a C++ struct with 2 fields: a -`T*` pointer, and the `size_t` number of slice elements. TODO: Add runtime -assertions to `bindings.rs` to further verify these assumptions. TODO: Specify a -plan of action when the assertions fail. +has the same layout as `rs_std::slice<T>` - a C++ struct with 2 fields: a `T*` +pointer, and the `size_t` number of slice elements. `check_slice_layout` inside +`crubit/cc_bindings_from_rs/bindings.rs` verifies the assumptions on the Rust +slice each time a reference or a pointer to a slice is processed by +`format_ty_for_cc`. Similar assertions are verified on C++ side in +`support/rs_std/slice_ref_test.cc`. The response to the unlikely failure of +these assertions can be changing the types from which `rs_std::SliceRef` is +built to match the Rust side again. `cc_bindings_from_rs` does *not* assume that `&[T]` and `rs_std::slice<T>` have the same ABI as
diff --git a/support/rs_std/BUILD b/support/rs_std/BUILD index 6dd2497..5bc0537 100644 --- a/support/rs_std/BUILD +++ b/support/rs_std/BUILD
@@ -32,3 +32,35 @@ "@com_google_googletest//:gtest_main", ], ) + +cc_library( + name = "slice_ref", + hdrs = ["slice_ref.h"], + # Enable bidirectional bindings (via crubit_internal_rust_type). + aspect_hints = ["//features:experimental"], + visibility = [ + "//visibility:public", + ], + + # Try to avoid unnecessary dependencies, so that Crubit users have to pull in as little as + # possible. Using mature Abseil APIs seems okay - we should be able to assume that Crubit users + # have a version of Abseil that is relatively recent (although we can't rely on an exact version + # and/or exact absl/base/options.h). + deps = [ + "//support/internal:bindings_support", + "@abseil-cpp//absl/base:core_headers", + "@abseil-cpp//absl/types:span", + ], +) + +crubit_cc_test( + name = "slice_ref_test", + srcs = ["slice_ref_test.cc"], + deps = [ + ":slice_ref", + "//testing/base/public:gmock_no_google3", + "//testing/fuzzing:fuzztest_gunit_main_no_google3", + "//testing/fuzzing:fuzztest_no_google3", + "@abseil-cpp//absl/types:span", + ], +)
diff --git a/support/rs_std/slice_ref.h b/support/rs_std/slice_ref.h new file mode 100644 index 0000000..47cfe1b --- /dev/null +++ b/support/rs_std/slice_ref.h
@@ -0,0 +1,71 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef THIRD_PARTY_CRUBIT_SUPPORT_RS_STD_SLICEREF_H_ +#define THIRD_PARTY_CRUBIT_SUPPORT_RS_STD_SLICEREF_H_ + +#include <cstddef> +#include <cstdint> +#include <span> // NOLINT(build/c++20); <internal link> + +#include "absl/base/attributes.h" +#include "absl/types/span.h" +#include "support/internal/attribute_macros.h" + +namespace rs_std { + +// `rs_std::SliceRef` is a C++ representation of a pointer or reference to a +// Rust slice. `SliceRef<int const>` is like a `&[c_int]` or `*const [c_int]`, +// while `SliceRef<int>` is like a `&[c_int]` or `*mut [c_int]`. `SliceRef` is +// trivially destructible, copyable, and moveable. +// `rust_builtin_type_abi_assumptions.md` documents the ABI compatibility of +// these types. +template <typename T> +class CRUBIT_INTERNAL_RUST_TYPE("&[]") + ABSL_ATTRIBUTE_TRIVIAL_ABI SliceRef final { + public: + // Creates a default `SliceRef` - one that represents an empty slice. + // To mirror slices in Rust, the data pointer is not null. + constexpr SliceRef() noexcept : ptr_(alignof(T)), size_(0) {} + + // This constructor cannot be constexpr because it calls `reinterpret_cast`. + // The `reinterpret_cast`, in turn, is needed if we want to keep `ptr_` an + // `uintptr_t` instead of a `T*`, which we do to avoid storing dangling + // pointers. The `reinterpret_cast` preserves both invariants required for + // `ptr_`, because the resulting value comes from a valid, non-null pointer. + // NOLINTNEXTLINE(google-explicit-constructor) + SliceRef(absl::Span<T> span) noexcept + : ptr_(span.empty() ? alignof(T) + : reinterpret_cast<uintptr_t>(span.data())), + size_(span.size()) {} + + // NOLINTNEXTLINE(google-explicit-constructor) + constexpr operator std::span<T>() const { + return std::span<T>(size_ > 0 ? static_cast<T*>(ptr_) : nullptr, size_); + } + + constexpr SliceRef(const SliceRef&) = default; + constexpr SliceRef& operator=(const SliceRef&) = default; + constexpr SliceRef(SliceRef&&) = default; + constexpr SliceRef& operator=(SliceRef&&) = default; + ~SliceRef() = default; + + // The `reinterpret_cast` is safe thanks to invariant (2) (see the definition + // of `ptr_`). + T* data() const { return size_ > 0 ? reinterpret_cast<T*>(ptr_) : nullptr; } + size_t size() const { return size_; } + + absl::Span<T> to_span() const { return absl::Span<T>(data(), size()); } + + private: + // Stick to the following invariants when changing the data member values: + // (1) `ptr_` is never 0 (to mirror slices in Rust). + // (2) if `size_ > 0` then `reinterpret_cast<T*>(ptr_)` is a valid pointer. + uintptr_t ptr_; + size_t size_; +}; + +} // namespace rs_std + +#endif // THIRD_PARTY_CRUBIT_SUPPORT_RS_STD_SLICEREF_H_
diff --git a/support/rs_std/slice_ref_test.cc b/support/rs_std/slice_ref_test.cc new file mode 100644 index 0000000..dbdcd5e --- /dev/null +++ b/support/rs_std/slice_ref_test.cc
@@ -0,0 +1,145 @@ +// Part of the Crubit project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "support/rs_std/slice_ref.h" + +#include <stdint.h> + +#include <array> +#include <bit> +#include <concepts> +#include <cstddef> +#include <type_traits> +#include <vector> + +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "testing/fuzzing/fuzztest.h" +#include "absl/types/span.h" + +namespace { +using ::testing::IsNull; +using ::testing::Not; + +// Check that `rs_std::SliceRef` is trivially destructible, copyable, and +// moveable. It is not trivial because it has a non-trivial constructor, and +// that's because its default constructor needs to be user-defined to make sure +// that `ptr_` is never null. The latter is needed so that interpreting the data +// as a slice in Rust won't result in a (in Rust) forbidden null pointer. +static_assert(std::is_nothrow_constructible_v<rs_std::SliceRef<const uint8_t>>); +static_assert( + std::is_trivially_destructible_v<rs_std::SliceRef<const uint8_t>>); +static_assert(std::is_trivially_copyable_v<rs_std::SliceRef<const uint8_t>>); +static_assert( + std::is_trivially_copy_constructible_v<rs_std::SliceRef<const uint8_t>>); +static_assert( + std::is_trivially_copy_assignable_v<rs_std::SliceRef<const uint8_t>>); +static_assert( + std::is_trivially_move_constructible_v<rs_std::SliceRef<const uint8_t>>); +static_assert( + std::is_trivially_move_assignable_v<rs_std::SliceRef<const uint8_t>>); +// Note: `rs_std::SliceRef` is not trivially constructible because it's default +// constructor ensures that the data pointer is not null. + +// `SliceRef` does on purpose not have `operator==`, because <internal link> +// did not specify that `SliceRef` should be comparable. +static_assert(!std::equality_comparable<rs_std::SliceRef<int>>); + +// Verify that the layout of `rs_std::SliceRef` is as expected and described in +// `rust_builtin_type_abi_assumptions.md`. Sample a few wrapped types to make +// sure that the layout is the same for all of them. +// `sizeof(uintptr_t)` is guaranteed to be the size of `usize` in Rust. +constexpr size_t kSliceRefSize = sizeof(uintptr_t) * 2; +constexpr size_t kSliceRefAlign = alignof(uintptr_t); +static_assert(sizeof(rs_std::SliceRef<const uint8_t>) == kSliceRefSize); +static_assert(alignof(rs_std::SliceRef<const uint8_t>) == kSliceRefAlign); +static_assert(std::is_standard_layout_v<rs_std::SliceRef<const uint8_t>>); +static_assert(sizeof(rs_std::SliceRef<char>) == kSliceRefSize); +static_assert(alignof(rs_std::SliceRef<char>) == kSliceRefAlign); +static_assert(std::is_standard_layout_v<rs_std::SliceRef<char>>); +static_assert(sizeof(rs_std::SliceRef<int64_t>) == kSliceRefSize); +static_assert(alignof(rs_std::SliceRef<int64_t>) == kSliceRefAlign); +static_assert(std::is_standard_layout_v<rs_std::SliceRef<int64_t>>); + +// Slice assumes that the Rust slice layout is first the pointer and then the +// size. This does not appear to be standardised, so instead there are runtime +// checks for this in `format_ty_for_cc` in `cc_bindings_from_rs/bindings.rs`. +static_assert(sizeof(rs_std::SliceRef<const uint8_t>) == + sizeof(const uint8_t*) + sizeof(size_t)); +static_assert(alignof(rs_std::SliceRef<const uint8_t>) == + alignof(const uint8_t*)); +static_assert(std::is_standard_layout_v<rs_std::SliceRef<const uint8_t>>); + +TEST(SliceTest, Comparison) { + static constexpr std::array<uint8_t, 5> kArr = {1, 2, 3, 4, 5}; + static constexpr std::array<uint8_t, 5> kArrCopy = {1, 2, 3, 4, 5}; + + const rs_std::SliceRef<const uint8_t> s1(kArr); + const rs_std::SliceRef<const uint8_t> s1_copy = s1; + const rs_std::SliceRef<const uint8_t> s2(kArrCopy); + + EXPECT_EQ(s1.to_span(), s1.to_span()); + EXPECT_EQ(s1.to_span(), s1_copy.to_span()); + EXPECT_EQ(s1.to_span(), s2.to_span()); + const rs_std::SliceRef<const uint8_t> s1_prefix( + absl::MakeSpan(kArr.data(), kArr.size() - 1)); + const rs_std::SliceRef<const uint8_t> s1_suffix( + absl::MakeSpan(kArr.data() + 1, kArr.size() - 1)); + const rs_std::SliceRef<const uint8_t> s1_infix( + absl::MakeSpan(kArr.data() + 1, kArr.size() - 2)); + + EXPECT_NE(s1.to_span(), s1_prefix.to_span()); + EXPECT_NE(s1.to_span(), s1_suffix.to_span()); + EXPECT_NE(s1.to_span(), s1_infix.to_span()); +} + +TEST(SliceTest, FromAndTo) { + static constexpr std::array<uint8_t, 5> kArr = {1, 2, 3, 4, 5}; + const rs_std::SliceRef<const uint8_t> s(kArr); + EXPECT_EQ(absl::Span<const uint8_t>(kArr), s.to_span()); +} + +// To test the value of `data_`, the test includes the knowledge of +// `SliceRef`'s layout to extract it via a peeker struct. +struct SliceRefFields { + const void* ptr; + size_t size; +}; + +// This test checks that the assumption of Peeker having the same layout as +// `SliceRef` is correct. +TEST(SliceTest, Layout) { + static constexpr std::array<uint8_t, 5> kArr = {2, 3, 5}; + const rs_std::SliceRef<const uint8_t> s(kArr); + const auto fields = std::bit_cast<SliceRefFields>(s); + EXPECT_EQ(fields.ptr, kArr.data()); + EXPECT_EQ(fields.size, kArr.size()); +} + +TEST(SliceTest, Empty) { + static constexpr uint8_t kEmpty[] = {}; + const rs_std::SliceRef<const uint8_t> empty(absl::MakeSpan(kEmpty, 0)); + static constexpr rs_std::SliceRef<const uint8_t> default_constructed; + EXPECT_EQ(empty.to_span(), default_constructed.to_span()); + + const auto fields = std::bit_cast<SliceRefFields>(empty); + EXPECT_THAT(fields.ptr, Not(IsNull())); + EXPECT_EQ(fields.size, 0); + + // While `empty.data_` is not null, `data()` converts it to null for + // compatibility with `std::span`. + EXPECT_THAT(empty.data(), IsNull()); + EXPECT_EQ(empty.size(), 0); +} + +void Fuzzer(std::vector<uint8_t> data) { + const rs_std::SliceRef<const uint8_t> s(data); + EXPECT_EQ(absl::Span<const uint8_t>(data), s.to_span()); + const std::vector<uint8_t> data_copy(s.data(), s.data() + s.size()); + EXPECT_EQ(data, data_copy); +} + +FUZZ_TEST(SliceFuzzTest, Fuzzer); + +} // namespace