Fix vector of table with naked ptr (#8830)

* create test that fails to compile

* fix the issue

* add test body

* force commit gneerated header

* build failures

* fix bazel some more

CMakeLists.txt

@@ -234,6 +234,8 @@ set(FlatBuffers_Tests_SRCS
   tests/test_builder.h
   tests/test_builder.cpp
   tests/util_test.cpp
+  tests/vector_table_naked_ptr_test.h
+  tests/vector_table_naked_ptr_test.cpp
   tests/native_type_test_impl.h
   tests/native_type_test_impl.cpp
   tests/alignment_test.h

scripts/generate_code.py

@@ -239,6 +239,12 @@ flatc(
     ],
 )
 
+flatc(
+    BASE_OPTS + CPP_OPTS + ["--cpp-ptr-type", "naked"],
+    prefix="vector_table_naked_ptr",
+    schema="vector_table_naked_ptr.fbs",
+)
+
 flatc(
     BASE_OPTS + CPP_OPTS + CS_OPTS + JAVA_OPTS + KOTLIN_OPTS + PHP_OPTS,
     prefix="union_vector",

src/idl_gen_cpp.cpp

@@ -3488,8 +3488,8 @@ class CppGenerator : public BaseGenerator {
             const bool is_pointer = IsVectorOfPointers(field);
             if (is_pointer) {
               code += "if(_o->" + name + "[_i]" + ") { ";
-              code += indexing + "->UnPackTo(_o->" + name +
-                      "[_i].get(), _resolver);";
+              code += indexing + "->UnPackTo(_o->" + name + "[_i]" +
+                      GenPtrGet(field) + ", _resolver);";
               code += " } else { ";
             }
             code += "_o->" + name + "[_i]" + access + " = ";
@@ -3551,7 +3551,8 @@ class CppGenerator : public BaseGenerator {
 
           if (is_pointer) {
             code += "{ if(" + out_field + ") { ";
-            code += "_e->UnPackTo(" + out_field + ".get(), _resolver);";
+            code += "_e->UnPackTo(" + out_field + GenPtrGet(field) +
+                    ", _resolver);";
             code += " } else { ";
           }
           code += out_field + " = ";

tests/BUILD.bazel

@@ -73,6 +73,9 @@ cc_test(
         "union_vector/union_vector_generated.h",
         "util_test.cpp",
         "util_test.h",
+        "vector_table_naked_ptr/vector_table_naked_ptr_generated.h",
+        "vector_table_naked_ptr_test.h",
+        "vector_table_naked_ptr_test.cpp",
     ],
     copts = [
         "-DFLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE",

tests/test.cpp

@@ -58,6 +58,7 @@
 #include "native_type_test_generated.h"
 #include "test_assert.h"
 #include "util_test.h"
+#include "vector_table_naked_ptr_test.h"
 
 void FlatBufferBuilderTest();
 
@@ -1723,6 +1724,7 @@ int FlatBufferTests(const std::string& tests_data_path) {
   FixedLengthArraySpanTest(tests_data_path);
   DoNotRequireEofTest(tests_data_path);
   JsonUnionStructTest();
+  VectorTableNakedPtrTest();
 #else
   // Guard against -Wunused-parameter.
   (void)tests_data_path;

tests/vector_table_naked_ptr.fbs

@@ -0,0 +1,9 @@
+table B {
+  id: int32;
+}
+
+table A {
+  b: [B];
+}
+
+root_type A;

tests/vector_table_naked_ptr/vector_table_naked_ptr_generated.h

@@ -0,0 +1,331 @@
+// automatically generated by the FlatBuffers compiler, do not modify
+
+
+#ifndef FLATBUFFERS_GENERATED_VECTORTABLENAKEDPTR_H_
+#define FLATBUFFERS_GENERATED_VECTORTABLENAKEDPTR_H_
+
+#include "flatbuffers/flatbuffers.h"
+
+// Ensure the included flatbuffers.h is the same version as when this file was
+// generated, otherwise it may not be compatible.
+static_assert(FLATBUFFERS_VERSION_MAJOR == 25 &&
+              FLATBUFFERS_VERSION_MINOR == 9 &&
+              FLATBUFFERS_VERSION_REVISION == 23,
+             "Non-compatible flatbuffers version included");
+
+struct B;
+struct BBuilder;
+struct BT;
+
+struct A;
+struct ABuilder;
+struct AT;
+
+bool operator==(const BT &lhs, const BT &rhs);
+bool operator!=(const BT &lhs, const BT &rhs);
+bool operator==(const AT &lhs, const AT &rhs);
+bool operator!=(const AT &lhs, const AT &rhs);
+
+inline const ::flatbuffers::TypeTable *BTypeTable();
+
+inline const ::flatbuffers::TypeTable *ATypeTable();
+
+struct BT : public ::flatbuffers::NativeTable {
+  typedef B TableType;
+  int32_t id = 0;
+};
+
+struct B FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
+  typedef BT NativeTableType;
+  typedef BBuilder Builder;
+  static const ::flatbuffers::TypeTable *MiniReflectTypeTable() {
+    return BTypeTable();
+  }
+  enum FlatBuffersVTableOffset FLATBUFFERS_VTABLE_UNDERLYING_TYPE {
+    VT_ID = 4
+  };
+  int32_t id() const {
+    return GetField<int32_t>(VT_ID, 0);
+  }
+  bool mutate_id(int32_t _id = 0) {
+    return SetField<int32_t>(VT_ID, _id, 0);
+  }
+  template <bool B = false>
+  bool Verify(::flatbuffers::VerifierTemplate<B> &verifier) const {
+    return VerifyTableStart(verifier) &&
+           VerifyField<int32_t>(verifier, VT_ID, 4) &&
+           verifier.EndTable();
+  }
+  BT *UnPack(const ::flatbuffers::resolver_function_t *_resolver = nullptr) const;
+  void UnPackTo(BT *_o, const ::flatbuffers::resolver_function_t *_resolver = nullptr) const;
+  static ::flatbuffers::Offset<B> Pack(::flatbuffers::FlatBufferBuilder &_fbb, const BT* _o, const ::flatbuffers::rehasher_function_t *_rehasher = nullptr);
+};
+
+struct BBuilder {
+  typedef B Table;
+  ::flatbuffers::FlatBufferBuilder &fbb_;
+  ::flatbuffers::uoffset_t start_;
+  void add_id(int32_t id) {
+    fbb_.AddElement<int32_t>(B::VT_ID, id, 0);
+  }
+  explicit BBuilder(::flatbuffers::FlatBufferBuilder &_fbb)
+        : fbb_(_fbb) {
+    start_ = fbb_.StartTable();
+  }
+  ::flatbuffers::Offset<B> Finish() {
+    const auto end = fbb_.EndTable(start_);
+    auto o = ::flatbuffers::Offset<B>(end);
+    return o;
+  }
+};
+
+inline ::flatbuffers::Offset<B> CreateB(
+    ::flatbuffers::FlatBufferBuilder &_fbb,
+    int32_t id = 0) {
+  BBuilder builder_(_fbb);
+  builder_.add_id(id);
+  return builder_.Finish();
+}
+
+::flatbuffers::Offset<B> CreateB(::flatbuffers::FlatBufferBuilder &_fbb, const BT *_o, const ::flatbuffers::rehasher_function_t *_rehasher = nullptr);
+
+struct AT : public ::flatbuffers::NativeTable {
+  typedef A TableType;
+  std::vector<BT *> b{};
+  AT() = default;
+  AT(const AT &o);
+  AT(AT&&) FLATBUFFERS_NOEXCEPT = default;
+  AT &operator=(AT o) FLATBUFFERS_NOEXCEPT;
+};
+
+struct A FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
+  typedef AT NativeTableType;
+  typedef ABuilder Builder;
+  static const ::flatbuffers::TypeTable *MiniReflectTypeTable() {
+    return ATypeTable();
+  }
+  enum FlatBuffersVTableOffset FLATBUFFERS_VTABLE_UNDERLYING_TYPE {
+    VT_B = 4
+  };
+  const ::flatbuffers::Vector<::flatbuffers::Offset<B>> *b() const {
+    return GetPointer<const ::flatbuffers::Vector<::flatbuffers::Offset<B>> *>(VT_B);
+  }
+  ::flatbuffers::Vector<::flatbuffers::Offset<B>> *mutable_b() {
+    return GetPointer<::flatbuffers::Vector<::flatbuffers::Offset<B>> *>(VT_B);
+  }
+  template <bool B = false>
+  bool Verify(::flatbuffers::VerifierTemplate<B> &verifier) const {
+    return VerifyTableStart(verifier) &&
+           VerifyOffset(verifier, VT_B) &&
+           verifier.VerifyVector(b()) &&
+           verifier.VerifyVectorOfTables(b()) &&
+           verifier.EndTable();
+  }
+  AT *UnPack(const ::flatbuffers::resolver_function_t *_resolver = nullptr) const;
+  void UnPackTo(AT *_o, const ::flatbuffers::resolver_function_t *_resolver = nullptr) const;
+  static ::flatbuffers::Offset<A> Pack(::flatbuffers::FlatBufferBuilder &_fbb, const AT* _o, const ::flatbuffers::rehasher_function_t *_rehasher = nullptr);
+};
+
+struct ABuilder {
+  typedef A Table;
+  ::flatbuffers::FlatBufferBuilder &fbb_;
+  ::flatbuffers::uoffset_t start_;
+  void add_b(::flatbuffers::Offset<::flatbuffers::Vector<::flatbuffers::Offset<B>>> b) {
+    fbb_.AddOffset(A::VT_B, b);
+  }
+  explicit ABuilder(::flatbuffers::FlatBufferBuilder &_fbb)
+        : fbb_(_fbb) {
+    start_ = fbb_.StartTable();
+  }
+  ::flatbuffers::Offset<A> Finish() {
+    const auto end = fbb_.EndTable(start_);
+    auto o = ::flatbuffers::Offset<A>(end);
+    return o;
+  }
+};
+
+inline ::flatbuffers::Offset<A> CreateA(
+    ::flatbuffers::FlatBufferBuilder &_fbb,
+    ::flatbuffers::Offset<::flatbuffers::Vector<::flatbuffers::Offset<B>>> b = 0) {
+  ABuilder builder_(_fbb);
+  builder_.add_b(b);
+  return builder_.Finish();
+}
+
+inline ::flatbuffers::Offset<A> CreateADirect(
+    ::flatbuffers::FlatBufferBuilder &_fbb,
+    const std::vector<::flatbuffers::Offset<B>> *b = nullptr) {
+  auto b__ = b ? _fbb.CreateVector<::flatbuffers::Offset<B>>(*b) : 0;
+  return CreateA(
+      _fbb,
+      b__);
+}
+
+::flatbuffers::Offset<A> CreateA(::flatbuffers::FlatBufferBuilder &_fbb, const AT *_o, const ::flatbuffers::rehasher_function_t *_rehasher = nullptr);
+
+
+inline bool operator==(const BT &lhs, const BT &rhs) {
+  return
+      (lhs.id == rhs.id);
+}
+
+inline bool operator!=(const BT &lhs, const BT &rhs) {
+    return !(lhs == rhs);
+}
+
+
+inline BT *B::UnPack(const ::flatbuffers::resolver_function_t *_resolver) const {
+  auto _o = std::unique_ptr<BT>(new BT());
+  UnPackTo(_o.get(), _resolver);
+  return _o.release();
+}
+
+inline void B::UnPackTo(BT *_o, const ::flatbuffers::resolver_function_t *_resolver) const {
+  (void)_o;
+  (void)_resolver;
+  { auto _e = id(); _o->id = _e; }
+}
+
+inline ::flatbuffers::Offset<B> CreateB(::flatbuffers::FlatBufferBuilder &_fbb, const BT *_o, const ::flatbuffers::rehasher_function_t *_rehasher) {
+  return B::Pack(_fbb, _o, _rehasher);
+}
+
+inline ::flatbuffers::Offset<B> B::Pack(::flatbuffers::FlatBufferBuilder &_fbb, const BT* _o, const ::flatbuffers::rehasher_function_t *_rehasher) {
+  (void)_rehasher;
+  (void)_o;
+  struct _VectorArgs { ::flatbuffers::FlatBufferBuilder *__fbb; const BT* __o; const ::flatbuffers::rehasher_function_t *__rehasher; } _va = { &_fbb, _o, _rehasher}; (void)_va;
+  auto _id = _o->id;
+  return CreateB(
+      _fbb,
+      _id);
+}
+
+
+inline bool operator==(const AT &lhs, const AT &rhs) {
+  return
+      (lhs.b.size() == rhs.b.size() && std::equal(lhs.b.cbegin(), lhs.b.cend(), rhs.b.cbegin(), [](BT * const &a, BT * const &b) { return (a == b) || (a && b && *a == *b); }));
+}
+
+inline bool operator!=(const AT &lhs, const AT &rhs) {
+    return !(lhs == rhs);
+}
+
+
+inline AT::AT(const AT &o) {
+  b.reserve(o.b.size());
+  for (const auto &b_ : o.b) { b.emplace_back((b_) ? new BT(*b_) : nullptr); }
+}
+
+inline AT &AT::operator=(AT o) FLATBUFFERS_NOEXCEPT {
+  std::swap(b, o.b);
+  return *this;
+}
+
+inline AT *A::UnPack(const ::flatbuffers::resolver_function_t *_resolver) const {
+  auto _o = std::unique_ptr<AT>(new AT());
+  UnPackTo(_o.get(), _resolver);
+  return _o.release();
+}
+
+inline void A::UnPackTo(AT *_o, const ::flatbuffers::resolver_function_t *_resolver) const {
+  (void)_o;
+  (void)_resolver;
+  { auto _e = b(); if (_e) { _o->b.resize(_e->size()); for (::flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { if(_o->b[_i]) { _e->Get(_i)->UnPackTo(_o->b[_i], _resolver); } else { _o->b[_i] = (_e->Get(_i)->UnPack(_resolver)); } } } else { _o->b.resize(0); } }
+}
+
+inline ::flatbuffers::Offset<A> CreateA(::flatbuffers::FlatBufferBuilder &_fbb, const AT *_o, const ::flatbuffers::rehasher_function_t *_rehasher) {
+  return A::Pack(_fbb, _o, _rehasher);
+}
+
+inline ::flatbuffers::Offset<A> A::Pack(::flatbuffers::FlatBufferBuilder &_fbb, const AT* _o, const ::flatbuffers::rehasher_function_t *_rehasher) {
+  (void)_rehasher;
+  (void)_o;
+  struct _VectorArgs { ::flatbuffers::FlatBufferBuilder *__fbb; const AT* __o; const ::flatbuffers::rehasher_function_t *__rehasher; } _va = { &_fbb, _o, _rehasher}; (void)_va;
+  auto _b = _o->b.size() ? _fbb.CreateVector<::flatbuffers::Offset<B>> (_o->b.size(), [](size_t i, _VectorArgs *__va) { return CreateB(*__va->__fbb, __va->__o->b[i], __va->__rehasher); }, &_va ) : 0;
+  return CreateA(
+      _fbb,
+      _b);
+}
+
+inline const ::flatbuffers::TypeTable *BTypeTable() {
+  static const ::flatbuffers::TypeCode type_codes[] = {
+    { ::flatbuffers::ET_INT, 0, -1 }
+  };
+  static const char * const names[] = {
+    "id"
+  };
+  static const ::flatbuffers::TypeTable tt = {
+    ::flatbuffers::ST_TABLE, 1, type_codes, nullptr, nullptr, nullptr, names
+  };
+  return &tt;
+}
+
+inline const ::flatbuffers::TypeTable *ATypeTable() {
+  static const ::flatbuffers::TypeCode type_codes[] = {
+    { ::flatbuffers::ET_SEQUENCE, 1, 0 }
+  };
+  static const ::flatbuffers::TypeFunction type_refs[] = {
+    BTypeTable
+  };
+  static const char * const names[] = {
+    "b"
+  };
+  static const ::flatbuffers::TypeTable tt = {
+    ::flatbuffers::ST_TABLE, 1, type_codes, type_refs, nullptr, nullptr, names
+  };
+  return &tt;
+}
+
+inline const A *GetA(const void *buf) {
+  return ::flatbuffers::GetRoot<A>(buf);
+}
+
+inline const A *GetSizePrefixedA(const void *buf) {
+  return ::flatbuffers::GetSizePrefixedRoot<A>(buf);
+}
+
+inline A *GetMutableA(void *buf) {
+  return ::flatbuffers::GetMutableRoot<A>(buf);
+}
+
+inline A *GetMutableSizePrefixedA(void *buf) {
+  return ::flatbuffers::GetMutableSizePrefixedRoot<A>(buf);
+}
+
+template <bool B = false>
+inline bool VerifyABuffer(
+    ::flatbuffers::VerifierTemplate<B> &verifier) {
+  return verifier.template VerifyBuffer<A>(nullptr);
+}
+
+template <bool B = false>
+inline bool VerifySizePrefixedABuffer(
+    ::flatbuffers::VerifierTemplate<B> &verifier) {
+  return verifier.template VerifySizePrefixedBuffer<A>(nullptr);
+}
+
+inline void FinishABuffer(
+    ::flatbuffers::FlatBufferBuilder &fbb,
+    ::flatbuffers::Offset<A> root) {
+  fbb.Finish(root);
+}
+
+inline void FinishSizePrefixedABuffer(
+    ::flatbuffers::FlatBufferBuilder &fbb,
+    ::flatbuffers::Offset<A> root) {
+  fbb.FinishSizePrefixed(root);
+}
+
+inline AT * UnPackA(
+    const void *buf,
+    const ::flatbuffers::resolver_function_t *res = nullptr) {
+  return (GetA(buf)->UnPack(res));
+}
+
+inline AT * UnPackSizePrefixedA(
+    const void *buf,
+    const ::flatbuffers::resolver_function_t *res = nullptr) {
+  return (GetSizePrefixedA(buf)->UnPack(res));
+}
+
+#endif  // FLATBUFFERS_GENERATED_VECTORTABLENAKEDPTR_H_

tests/vector_table_naked_ptr_test.cpp

@@ -0,0 +1,75 @@
+#include "vector_table_naked_ptr_test.h"
+
+#include "test_assert.h"
+#include "vector_table_naked_ptr/vector_table_naked_ptr_generated.h"
+
+namespace flatbuffers {
+namespace tests {
+
+void VectorTableNakedPtrTest() {
+  // ---------------------------------------
+  // 1) Build original native objects
+  // ---------------------------------------
+  auto* b1 = new BT();
+  auto* b2 = new BT();
+  b1->id = 777;
+  b2->id = 888;
+
+  AT a_src;
+  a_src.b.push_back(b1);
+  a_src.b.push_back(b2);
+
+  // ---------------------------------------
+  // 2) Pack into FlatBuffer
+  // ---------------------------------------
+  flatbuffers::FlatBufferBuilder fbb;
+  auto a_offset = A::Pack(fbb, &a_src);
+  FinishABuffer(fbb, a_offset);
+
+  const void* buf = fbb.GetBufferPointer();
+  const A* a_fb = GetA(buf);
+
+  // ---------------------------------------
+  // 3) Pre-allocate destination with garbage
+  // ---------------------------------------
+  AT a_dst;
+
+  // Pre-fill with wrong pointers to ensure UnPackTo overwrites correctly
+  a_dst.b.resize(2);
+  a_dst.b[0] = new BT();
+  a_dst.b[1] = new BT();
+  a_dst.b[0]->id = -1;
+  a_dst.b[1]->id = -1;
+
+  // ---------------------------------------
+  // 4) Call the function under test: UnPackTo
+  // ---------------------------------------
+  a_fb->UnPackTo(&a_dst, nullptr);
+
+  // ---------------------------------------
+  // 5) Validate overwrite + deep copy
+  // ---------------------------------------
+  TEST_ASSERT(a_dst.b.size() == 2);
+
+  TEST_ASSERT(a_dst.b[0] != nullptr);
+  TEST_ASSERT(a_dst.b[1] != nullptr);
+
+  TEST_ASSERT(a_dst.b[0]->id == 777);
+  TEST_ASSERT(a_dst.b[1]->id == 888);
+
+  // Ensure original pointers were not reused
+  TEST_ASSERT(a_dst.b[0] != b1);
+  TEST_ASSERT(a_dst.b[1] != b2);
+
+  // ---------------------------------------
+  // 6) Clean up ownership
+  // ---------------------------------------
+  delete a_dst.b[0];
+  delete a_dst.b[1];
+
+  delete b1;
+  delete b2;
+}
+
+}  // namespace tests
+}  // namespace flatbuffers

tests/vector_table_naked_ptr_test.h

@@ -0,0 +1,14 @@
+#ifndef VECTOR_TABLE_NAKED_PTR_TEST_H
+#define VECTOR_TABLE_NAKED_PTR_TEST_H
+
+#include <string>
+
+namespace flatbuffers {
+namespace tests {
+
+void VectorTableNakedPtrTest();
+
+}  // namespace tests
+}  // namespace flatbuffers
+
+#endif