From 5e1cc249bf88b7f553297c81ee1e6ecaf4054a44 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Fri, 15 Nov 2024 15:24:40 -0800 Subject: [PATCH] Use noncontiguous unknown fields API in upb message compare PiperOrigin-RevId: 697017028 --- .../Reflection/FeatureSetDescriptor.g.cs | 17 --- upb/message/BUILD | 13 +- upb/message/compare.c | 10 +- upb/message/internal/compare_unknown.c | 116 +++++++++++++----- upb/message/internal/compare_unknown.h | 4 +- upb/message/internal/compare_unknown_test.cc | 16 ++- 6 files changed, 115 insertions(+), 61 deletions(-) delete mode 100644 csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs diff --git a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs b/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs deleted file mode 100644 index 208ce1fcb6..0000000000 --- a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs +++ /dev/null @@ -1,17 +0,0 @@ -#region Copyright notice and license -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file or at -// https://developers.google.com/open-source/licenses/bsd -#endregion - -namespace Google.Protobuf.Reflection; - -internal sealed partial class FeatureSetDescriptor -{ - // Canonical serialized form of the edition defaults, generated by embed_edition_defaults. - private const string DefaultsBase64 = - "ChMYhAciACoMCAEQAhgCIAMoATACChMY5wciACoMCAIQARgBIAIoATABChMY6AciDAgBEAEYASACKAEwASoAIOYHKOgH"; -} diff --git a/upb/message/BUILD b/upb/message/BUILD index 43d391d512..d7b83a10c6 100644 --- a/upb/message/BUILD +++ b/upb/message/BUILD @@ -57,7 +57,6 @@ cc_library( cc_library( name = "internal", srcs = [ - "internal/compare_unknown.c", "internal/extension.c", "internal/message.c", "value.h", @@ -65,7 +64,6 @@ cc_library( hdrs = [ "internal/accessors.h", "internal/array.h", - "internal/compare_unknown.h", "internal/extension.h", "internal/map.h", "internal/map_sorter.h", @@ -110,9 +108,11 @@ cc_library( name = "compare", srcs = [ "compare.c", + "internal/compare_unknown.c", ], hdrs = [ "compare.h", + "internal/compare_unknown.h", ], copts = UPB_DEFAULT_COPTS, visibility = ["//visibility:public"], @@ -121,8 +121,11 @@ cc_library( ":iterator", ":message", "//upb:base", + "//upb:eps_copy_input_stream", + "//upb:mem", "//upb:mini_table", "//upb:port", + "//upb:wire_reader", "//upb/mini_table:internal", ], ) @@ -288,10 +291,16 @@ cc_test( name = "compare_unknown_test", srcs = ["internal/compare_unknown_test.cc"], deps = [ + ":compare", ":internal", + ":message", + "//upb:base", + "//upb:mem", "//upb:port", + "//upb:wire", "//upb:wire_reader", "//upb/base:internal", + "//upb/test:test_messages_proto2_upb_proto", "@com_google_absl//absl/types:variant", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", diff --git a/upb/message/compare.c b/upb/message/compare.c index 70ad026bc4..49ee33eabb 100644 --- a/upb/message/compare.c +++ b/upb/message/compare.c @@ -8,6 +8,7 @@ #include "upb/message/compare.h" #include +#include #include "upb/base/descriptor_constants.h" #include "upb/message/accessors.h" @@ -181,12 +182,7 @@ bool upb_Message_IsEqual(const upb_Message* msg1, const upb_Message* msg2, if (!(options & kUpb_CompareOption_IncludeUnknownFields)) return true; - // Check the unknown fields. - size_t usize1, usize2; - const char* uf1 = upb_Message_GetUnknown(msg1, &usize1); - const char* uf2 = upb_Message_GetUnknown(msg2, &usize2); - // The wire encoder enforces a maximum depth of 100 so we match that here. - return UPB_PRIVATE(_upb_Message_UnknownFieldsAreEqual)( - uf1, usize1, uf2, usize2, 100) == kUpb_UnknownCompareResult_Equal; + return UPB_PRIVATE(_upb_Message_UnknownFieldsAreEqual)(msg1, msg2, 100) == + kUpb_UnknownCompareResult_Equal; } diff --git a/upb/message/internal/compare_unknown.c b/upb/message/internal/compare_unknown.c index a7a60f2ef2..b2d51fa767 100644 --- a/upb/message/internal/compare_unknown.c +++ b/upb/message/internal/compare_unknown.c @@ -7,10 +7,12 @@ #include "upb/message/internal/compare_unknown.h" +#include #include #include "upb/base/string_view.h" #include "upb/mem/alloc.h" +#include "upb/message/message.h" #include "upb/wire/eps_copy_input_stream.h" #include "upb/wire/reader.h" #include "upb/wire/types.h" @@ -47,6 +49,14 @@ typedef struct { jmp_buf err; } upb_UnknownField_Context; +typedef struct { + upb_UnknownField* arr_base; + upb_UnknownField* arr_ptr; + upb_UnknownField* arr_end; + uint32_t last_tag; + bool sorted; +} upb_UnknownFields_Builder; + UPB_NORETURN static void upb_UnknownFields_OutOfMemory( upb_UnknownField_Context* ctx) { ctx->status = kUpb_UnknownCompareResult_OutOfMemory; @@ -118,14 +128,19 @@ static void upb_UnknownFields_Sort(upb_UnknownField_Context* ctx, upb_UnknownFields_SortRecursive(fields->fields, 0, fields->size, ctx->tmp); } -static upb_UnknownFields* upb_UnknownFields_DoBuild( - upb_UnknownField_Context* ctx, const char** buf) { - upb_UnknownField* arr_base = NULL; - upb_UnknownField* arr_ptr = NULL; - upb_UnknownField* arr_end = NULL; +static upb_UnknownFields* upb_UnknownFields_BuildFromBuffer( + upb_UnknownField_Context* ctx, const char** buf); + +// Combines two unknown fields into one. +static void upb_CombineUnknownFields(upb_UnknownField_Context* ctx, + upb_UnknownFields_Builder* builder, + const char** buf) { + upb_UnknownField* arr_base = builder->arr_base; + upb_UnknownField* arr_ptr = builder->arr_ptr; + upb_UnknownField* arr_end = builder->arr_end; const char* ptr = *buf; - uint32_t last_tag = 0; - bool sorted = true; + uint32_t last_tag = builder->last_tag; + bool sorted = builder->sorted; while (!upb_EpsCopyInputStream_IsDone(&ctx->stream, &ptr)) { uint32_t tag; ptr = upb_WireReader_ReadTag(ptr, &tag); @@ -167,34 +182,71 @@ static upb_UnknownFields* upb_UnknownFields_DoBuild( ctx->status = kUpb_UnknownCompareResult_MaxDepthExceeded; UPB_LONGJMP(ctx->err, 1); } - field->data.group = upb_UnknownFields_DoBuild(ctx, &ptr); + field->data.group = upb_UnknownFields_BuildFromBuffer(ctx, &ptr); ctx->depth++; break; default: UPB_UNREACHABLE(); } } - *buf = ptr; + builder->arr_base = arr_base; + builder->arr_ptr = arr_ptr; + builder->arr_end = arr_end; + builder->sorted = sorted; + builder->last_tag = last_tag; +} + +static upb_UnknownFields* upb_UnknownFields_DoBuild( + upb_UnknownField_Context* ctx, upb_UnknownFields_Builder* builder) { upb_UnknownFields* ret = upb_Arena_Malloc(ctx->arena, sizeof(*ret)); if (!ret) upb_UnknownFields_OutOfMemory(ctx); - ret->fields = arr_base; - ret->size = arr_ptr - arr_base; - ret->capacity = arr_end - arr_base; - if (!sorted) { + ret->fields = builder->arr_base; + ret->size = builder->arr_ptr - builder->arr_base; + ret->capacity = builder->arr_end - builder->arr_base; + if (!builder->sorted) { upb_UnknownFields_Sort(ctx, ret); } return ret; } // Builds a upb_UnknownFields data structure from the binary data in buf. +static upb_UnknownFields* upb_UnknownFields_BuildFromBuffer( + upb_UnknownField_Context* ctx, const char** buf) { + upb_UnknownFields_Builder builder = { + .arr_base = NULL, + .arr_ptr = NULL, + .arr_end = NULL, + .sorted = true, + .last_tag = 0, + }; + const char* ptr = *buf; + upb_CombineUnknownFields(ctx, &builder, &ptr); + upb_UnknownFields* fields = upb_UnknownFields_DoBuild(ctx, &builder); + *buf = ptr; + return fields; +} + +// Builds a upb_UnknownFields data structure from the unknown fields of a +// upb_Message. static upb_UnknownFields* upb_UnknownFields_Build(upb_UnknownField_Context* ctx, - const char* ptr, - size_t size) { - upb_EpsCopyInputStream_Init(&ctx->stream, &ptr, size, true); - upb_UnknownFields* fields = upb_UnknownFields_DoBuild(ctx, &ptr); - UPB_ASSERT(upb_EpsCopyInputStream_IsDone(&ctx->stream, &ptr) && - !upb_EpsCopyInputStream_IsError(&ctx->stream)); + const upb_Message* msg) { + upb_UnknownFields_Builder builder = { + .arr_base = NULL, + .arr_ptr = NULL, + .arr_end = NULL, + .sorted = true, + .last_tag = 0, + }; + uintptr_t iter = kUpb_Message_UnknownBegin; + upb_StringView view; + while (upb_Message_NextUnknown(msg, &view, &iter)) { + upb_EpsCopyInputStream_Init(&ctx->stream, &view.data, view.size, true); + upb_CombineUnknownFields(ctx, &builder, &view.data); + UPB_ASSERT(upb_EpsCopyInputStream_IsDone(&ctx->stream, &view.data) && + !upb_EpsCopyInputStream_IsError(&ctx->stream)); + } + upb_UnknownFields* fields = upb_UnknownFields_DoBuild(ctx, &builder); return fields; } @@ -235,13 +287,13 @@ static bool upb_UnknownFields_IsEqual(const upb_UnknownFields* uf1, } static upb_UnknownCompareResult upb_UnknownField_DoCompare( - upb_UnknownField_Context* ctx, const char* buf1, size_t size1, - const char* buf2, size_t size2) { + upb_UnknownField_Context* ctx, const upb_Message* msg1, + const upb_Message* msg2) { upb_UnknownCompareResult ret; // First build both unknown fields into a sorted data structure (similar // to the UnknownFieldSet in C++). - upb_UnknownFields* uf1 = upb_UnknownFields_Build(ctx, buf1, size1); - upb_UnknownFields* uf2 = upb_UnknownFields_Build(ctx, buf2, size2); + upb_UnknownFields* uf1 = upb_UnknownFields_Build(ctx, msg1); + upb_UnknownFields* uf2 = upb_UnknownFields_Build(ctx, msg2); // Now perform the equality check on the sorted structures. if (upb_UnknownFields_IsEqual(uf1, uf2)) { @@ -253,11 +305,11 @@ static upb_UnknownCompareResult upb_UnknownField_DoCompare( } static upb_UnknownCompareResult upb_UnknownField_Compare( - upb_UnknownField_Context* const ctx, const char* const buf1, - const size_t size1, const char* const buf2, const size_t size2) { + upb_UnknownField_Context* const ctx, const upb_Message* msg1, + const upb_Message* msg2) { upb_UnknownCompareResult ret; if (UPB_SETJMP(ctx->err) == 0) { - ret = upb_UnknownField_DoCompare(ctx, buf1, size1, buf2, size2); + ret = upb_UnknownField_DoCompare(ctx, msg1, msg2); } else { ret = ctx->status; UPB_ASSERT(ret != kUpb_UnknownCompareResult_Equal); @@ -269,11 +321,11 @@ static upb_UnknownCompareResult upb_UnknownField_Compare( } upb_UnknownCompareResult UPB_PRIVATE(_upb_Message_UnknownFieldsAreEqual)( - const char* buf1, size_t size1, const char* buf2, size_t size2, - int max_depth) { - if (size1 == 0 && size2 == 0) return kUpb_UnknownCompareResult_Equal; - if (size1 == 0 || size2 == 0) return kUpb_UnknownCompareResult_NotEqual; - if (memcmp(buf1, buf2, size1) == 0) return kUpb_UnknownCompareResult_Equal; + const upb_Message* msg1, const upb_Message* msg2, int max_depth) { + bool msg1_empty = !upb_Message_HasUnknown(msg1); + bool msg2_empty = !upb_Message_HasUnknown(msg2); + if (msg1_empty && msg2_empty) return kUpb_UnknownCompareResult_Equal; + if (msg1_empty || msg2_empty) return kUpb_UnknownCompareResult_NotEqual; upb_UnknownField_Context ctx = { .arena = upb_Arena_New(), @@ -285,5 +337,5 @@ upb_UnknownCompareResult UPB_PRIVATE(_upb_Message_UnknownFieldsAreEqual)( if (!ctx.arena) return kUpb_UnknownCompareResult_OutOfMemory; - return upb_UnknownField_Compare(&ctx, buf1, size1, buf2, size2); + return upb_UnknownField_Compare(&ctx, msg1, msg2); } diff --git a/upb/message/internal/compare_unknown.h b/upb/message/internal/compare_unknown.h index ba67554064..ad655f7537 100644 --- a/upb/message/internal/compare_unknown.h +++ b/upb/message/internal/compare_unknown.h @@ -11,6 +11,7 @@ #include // Must be last. +#include "upb/message/message.h" #include "upb/port/def.inc" #ifdef __cplusplus @@ -37,8 +38,7 @@ typedef enum { } upb_UnknownCompareResult; upb_UnknownCompareResult UPB_PRIVATE(_upb_Message_UnknownFieldsAreEqual)( - const char* buf1, size_t size1, const char* buf2, size_t size2, - int max_depth); + const upb_Message* msg1, const upb_Message* msg2, int max_depth); #ifdef __cplusplus } /* extern "C" */ diff --git a/upb/message/internal/compare_unknown_test.cc b/upb/message/internal/compare_unknown_test.cc index 03cd2d006f..d655f66ee9 100644 --- a/upb/message/internal/compare_unknown_test.cc +++ b/upb/message/internal/compare_unknown_test.cc @@ -15,7 +15,10 @@ #include #include "absl/types/variant.h" +#include "google/protobuf/test_messages_proto2.upb.h" #include "upb/base/internal/endian.h" +#include "upb/base/upcast.h" +#include "upb/mem/arena.hpp" #include "upb/wire/types.h" // Must be last. @@ -103,10 +106,21 @@ std::string ToBinaryPayload(const UnknownFields& fields) { upb_UnknownCompareResult CompareUnknownWithMaxDepth(UnknownFields uf1, UnknownFields uf2, int max_depth) { + upb::Arena arena1; + upb::Arena arena2; + protobuf_test_messages_proto2_TestAllTypesProto2* msg1 = + protobuf_test_messages_proto2_TestAllTypesProto2_new(arena1.ptr()); + protobuf_test_messages_proto2_TestAllTypesProto2* msg2 = + protobuf_test_messages_proto2_TestAllTypesProto2_new(arena2.ptr()); + // Add the unknown fields to the messages. std::string buf1 = ToBinaryPayload(uf1); std::string buf2 = ToBinaryPayload(uf2); + UPB_PRIVATE(_upb_Message_AddUnknown)(UPB_UPCAST(msg1), buf1.data(), + buf1.size(), arena1.ptr()); + UPB_PRIVATE(_upb_Message_AddUnknown)(UPB_UPCAST(msg2), buf2.data(), + buf2.size(), arena2.ptr()); return UPB_PRIVATE(_upb_Message_UnknownFieldsAreEqual)( - buf1.data(), buf1.size(), buf2.data(), buf2.size(), max_depth); + UPB_UPCAST(msg1), UPB_UPCAST(msg2), max_depth); } upb_UnknownCompareResult CompareUnknown(UnknownFields uf1, UnknownFields uf2) {