From 2d6ee175e23b2b349f21c4820809be13bcebf743 Mon Sep 17 00:00:00 2001 From: Chris Kennelly Date: Fri, 6 Jan 2023 15:35:01 -0800 Subject: [PATCH] Remove runtime code paths for field stripping. PiperOrigin-RevId: 500274174 --- .../protobuf/generated_message_reflection.cc | 54 ++----------------- .../protobuf/generated_message_reflection.h | 10 ---- src/google/protobuf/message.h | 16 ------ src/google/protobuf/reflection_ops.cc | 8 +-- src/google/protobuf/text_format.cc | 5 +- 5 files changed, 8 insertions(+), 85 deletions(-) diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 7c9ff378d9..babcc194b4 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -240,12 +240,6 @@ static void ReportReflectionUsageEnumTypeError( << value->full_name(); } -inline void CheckInvalidAccess(const internal::ReflectionSchema& schema, - const FieldDescriptor* field) { - GOOGLE_ABSL_CHECK(!schema.IsFieldStripped(field)) - << "invalid access to a stripped field " << field->full_name(); -} - #define USAGE_CHECK(CONDITION, METHOD, ERROR_DESCRIPTION) \ if (!(CONDITION)) \ ReportReflectionUsageError(descriptor_, field, #METHOD, ERROR_DESCRIPTION) @@ -1081,7 +1075,6 @@ void Reflection::SwapFieldsImpl( const Message* prototype = message_factory_->GetPrototype(message1->GetDescriptor()); for (const auto* field : fields) { - CheckInvalidAccess(schema_, field); if (field->is_extension()) { if (unsafe_shallow_swap) { MutableExtensionSet(message1)->UnsafeShallowSwapExtension( @@ -1152,7 +1145,6 @@ bool Reflection::HasField(const Message& message, const FieldDescriptor* field) const { USAGE_CHECK_MESSAGE_TYPE(HasField); USAGE_CHECK_SINGULAR(HasField); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { return GetExtensionSet(message).Has(field->number()); @@ -1178,7 +1170,6 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const { for (int i = 0; i <= last_non_weak_field_index_; i++) { const FieldDescriptor* field = descriptor_->field(i); if (schema_.InRealOneof(field)) continue; - if (schema_.IsFieldStripped(field)) continue; if (schema_.IsSplit(field)) { continue; } @@ -1252,7 +1243,6 @@ int Reflection::FieldSize(const Message& message, const FieldDescriptor* field) const { USAGE_CHECK_MESSAGE_TYPE(FieldSize); USAGE_CHECK_REPEATED(FieldSize); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { return GetExtensionSet(message).ExtensionSize(field->number()); @@ -1297,7 +1287,6 @@ int Reflection::FieldSize(const Message& message, void Reflection::ClearField(Message* message, const FieldDescriptor* field) const { USAGE_CHECK_MESSAGE_TYPE(ClearField); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { MutableExtensionSet(message)->ClearExtension(field->number()); @@ -1408,7 +1397,6 @@ void Reflection::RemoveLast(Message* message, const FieldDescriptor* field) const { USAGE_CHECK_MESSAGE_TYPE(RemoveLast); USAGE_CHECK_REPEATED(RemoveLast); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { MutableExtensionSet(message)->RemoveLast(field->number()); @@ -1456,7 +1444,6 @@ void Reflection::RemoveLast(Message* message, Message* Reflection::ReleaseLast(Message* message, const FieldDescriptor* field) const { USAGE_CHECK_ALL(ReleaseLast, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); Message* released; if (field->is_extension()) { @@ -1482,7 +1469,6 @@ Message* Reflection::ReleaseLast(Message* message, Message* Reflection::UnsafeArenaReleaseLast( Message* message, const FieldDescriptor* field) const { USAGE_CHECK_ALL(UnsafeArenaReleaseLast, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { return static_cast( @@ -1503,7 +1489,6 @@ void Reflection::SwapElements(Message* message, const FieldDescriptor* field, int index1, int index2) const { USAGE_CHECK_MESSAGE_TYPE(Swap); USAGE_CHECK_REPEATED(Swap); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { MutableExtensionSet(message)->SwapElements(field->number(), index1, index2); @@ -1575,9 +1560,8 @@ bool CreateUnknownEnumValues(const FieldDescriptor* field) { } // namespace internal using internal::CreateUnknownEnumValues; -void Reflection::ListFieldsMayFailOnStripped( - const Message& message, bool should_fail, - std::vector* output) const { +void Reflection::ListFields(const Message& message, + std::vector* output) const { output->clear(); // Optimization: The default instance never has any fields set. @@ -1601,9 +1585,6 @@ void Reflection::ListFieldsMayFailOnStripped( }; for (int i = 0; i <= last_non_weak_field_index; i++) { const FieldDescriptor* field = descriptor_->field(i); - if (!should_fail && schema_.IsFieldStripped(field)) { - continue; - } if (field->is_repeated()) { if (FieldSize(message, field) > 0) { append_to_output(field); @@ -1620,7 +1601,6 @@ void Reflection::ListFieldsMayFailOnStripped( append_to_output(field); } } else if (has_bits && has_bits_indices[i] != static_cast(-1)) { - CheckInvalidAccess(schema_, field); // Equivalent to: HasBit(message, field) if (IsIndexInHasBitSet(has_bits, has_bits_indices[i])) { append_to_output(field); @@ -1657,16 +1637,6 @@ void Reflection::ListFieldsMayFailOnStripped( } } -void Reflection::ListFields(const Message& message, - std::vector* output) const { - ListFieldsMayFailOnStripped(message, true, output); -} - -void Reflection::ListFieldsOmitStripped( - const Message& message, std::vector* output) const { - ListFieldsMayFailOnStripped(message, false, output); -} - // ------------------------------------------------------------------- #undef DEFINE_PRIMITIVE_ACCESSORS @@ -2089,7 +2059,6 @@ const Message& Reflection::GetMessage(const Message& message, const FieldDescriptor* field, MessageFactory* factory) const { USAGE_CHECK_ALL(GetMessage, SINGULAR, MESSAGE); - CheckInvalidAccess(schema_, field); if (factory == nullptr) factory = message_factory_; @@ -2112,7 +2081,6 @@ Message* Reflection::MutableMessage(Message* message, const FieldDescriptor* field, MessageFactory* factory) const { USAGE_CHECK_ALL(MutableMessage, SINGULAR, MESSAGE); - CheckInvalidAccess(schema_, field); if (factory == nullptr) factory = message_factory_; @@ -2148,7 +2116,6 @@ void Reflection::UnsafeArenaSetAllocatedMessage( Message* message, Message* sub_message, const FieldDescriptor* field) const { USAGE_CHECK_ALL(SetAllocatedMessage, SINGULAR, MESSAGE); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { @@ -2184,7 +2151,6 @@ void Reflection::SetAllocatedMessage(Message* message, Message* sub_message, GOOGLE_ABSL_DCHECK( sub_message == nullptr || sub_message->GetOwningArena() == nullptr || sub_message->GetOwningArena() == message->GetArenaForAllocation()); - CheckInvalidAccess(schema_, field); // If message and sub-message are in different memory ownership domains // (different arenas, or one is on heap and one is not), then we may need to @@ -2215,7 +2181,6 @@ Message* Reflection::UnsafeArenaReleaseMessage(Message* message, const FieldDescriptor* field, MessageFactory* factory) const { USAGE_CHECK_ALL(ReleaseMessage, SINGULAR, MESSAGE); - CheckInvalidAccess(schema_, field); if (factory == nullptr) factory = message_factory_; @@ -2244,8 +2209,6 @@ Message* Reflection::UnsafeArenaReleaseMessage(Message* message, Message* Reflection::ReleaseMessage(Message* message, const FieldDescriptor* field, MessageFactory* factory) const { - CheckInvalidAccess(schema_, field); - Message* released = UnsafeArenaReleaseMessage(message, field, factory); #ifdef PROTOBUF_FORCE_COPY_IN_RELEASE released = MaybeForceCopy(message->GetArenaForAllocation(), released); @@ -2262,7 +2225,6 @@ const Message& Reflection::GetRepeatedMessage(const Message& message, const FieldDescriptor* field, int index) const { USAGE_CHECK_ALL(GetRepeatedMessage, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { return static_cast( @@ -2283,7 +2245,6 @@ Message* Reflection::MutableRepeatedMessage(Message* message, const FieldDescriptor* field, int index) const { USAGE_CHECK_ALL(MutableRepeatedMessage, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { return static_cast( @@ -2304,7 +2265,6 @@ Message* Reflection::MutableRepeatedMessage(Message* message, Message* Reflection::AddMessage(Message* message, const FieldDescriptor* field, MessageFactory* factory) const { USAGE_CHECK_ALL(AddMessage, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); if (factory == nullptr) factory = message_factory_; @@ -2347,7 +2307,6 @@ void Reflection::AddAllocatedMessage(Message* message, const FieldDescriptor* field, Message* new_entry) const { USAGE_CHECK_ALL(AddAllocatedMessage, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { MutableExtensionSet(message)->AddAllocatedMessage(field, new_entry); @@ -2367,7 +2326,6 @@ void Reflection::UnsafeArenaAddAllocatedMessage(Message* message, const FieldDescriptor* field, Message* new_entry) const { USAGE_CHECK_ALL(UnsafeArenaAddAllocatedMessage, REPEATED, MESSAGE); - CheckInvalidAccess(schema_, field); if (field->is_extension()) { MutableExtensionSet(message)->UnsafeArenaAddAllocatedMessage(field, @@ -2391,7 +2349,6 @@ void* Reflection::MutableRawRepeatedField(Message* message, const Descriptor* desc) const { (void)ctype; // Parameter is used by Google-internal code. USAGE_CHECK_REPEATED("MutableRawRepeatedField"); - CheckInvalidAccess(schema_, field); if (field->cpp_type() != cpptype && (field->cpp_type() != FieldDescriptor::CPPTYPE_ENUM || @@ -2690,9 +2647,6 @@ bool Reflection::HasBit(const Message& message, return IsIndexInHasBitSet(GetHasBits(message), schema_.HasBitIndex(field)); } - // Intentionally check here because HasBitIndex(field) != -1 means valid. - CheckInvalidAccess(schema_, field); - // proto3: no has-bits. All fields present except messages, which are // present only if their message-field pointer is non-null. if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { @@ -3277,8 +3231,6 @@ const internal::TcParseTableBase* Reflection::CreateTcParseTable() const { std::vector inlined_string_indices = has_bit_indices; for (int i = 0; i < descriptor_->field_count(); ++i) { auto* field = descriptor_->field(i); - if (schema_.IsFieldStripped(field)) continue; - fields.push_back(field); has_bit_indices[static_cast(field->index())] = static_cast(schema_.HasBitIndex(field)); @@ -3624,7 +3576,7 @@ void UnknownFieldSetSerializer(const uint8_t* base, uint32_t offset, bool IsDescendant(Message& root, const Message& message) { const Reflection* reflection = root.GetReflection(); std::vector fields; - reflection->ListFieldsOmitStripped(root, &fields); + reflection->ListFields(root, &fields); for (const auto* field : fields) { // Skip non-message fields. diff --git a/src/google/protobuf/generated_message_reflection.h b/src/google/protobuf/generated_message_reflection.h index 1b5485b093..4561a9439b 100644 --- a/src/google/protobuf/generated_message_reflection.h +++ b/src/google/protobuf/generated_message_reflection.h @@ -236,16 +236,6 @@ struct ReflectionSchema { return false; } - bool IsFieldStripped(const FieldDescriptor* field) const { - (void)field; - return false; - } - - bool IsMessageStripped(const Descriptor* descriptor) const { - (void)descriptor; - return false; - } - bool IsSplit() const { return split_offset_ != -1; } bool IsSplit(const FieldDescriptor* field) const { diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h index 8317c92fa7..eb6123732c 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h @@ -1024,22 +1024,6 @@ class PROTOBUF_EXPORT Reflection final { const internal::RepeatedFieldAccessor* RepeatedFieldAccessor( const FieldDescriptor* field) const; - // Lists all fields of the message which are currently set, except for unknown - // fields and stripped fields. See ListFields for details. - void ListFieldsOmitStripped( - const Message& message, - std::vector* output) const; - - bool IsMessageStripped(const Descriptor* descriptor) const { - return schema_.IsMessageStripped(descriptor); - } - - friend class TextFormat; - - void ListFieldsMayFailOnStripped( - const Message& message, bool should_fail, - std::vector* output) const; - // Returns true if the message field is backed by a LazyField. // // A message field may be backed by a LazyField without the user annotation diff --git a/src/google/protobuf/reflection_ops.cc b/src/google/protobuf/reflection_ops.cc index 67b12f6ba5..59c476b499 100644 --- a/src/google/protobuf/reflection_ops.cc +++ b/src/google/protobuf/reflection_ops.cc @@ -87,7 +87,7 @@ void ReflectionOps::Merge(const Message& from, Message* to) { google::protobuf::MessageFactory::generated_factory()); std::vector fields; - from_reflection->ListFieldsOmitStripped(from, &fields); + from_reflection->ListFields(from, &fields); for (const FieldDescriptor* field : fields) { if (field->is_repeated()) { // Use map reflection if both are in map status and have the @@ -182,7 +182,7 @@ void ReflectionOps::Clear(Message* message) { const Reflection* reflection = GetReflectionOrDie(*message); std::vector fields; - reflection->ListFieldsOmitStripped(*message, &fields); + reflection->ListFields(*message, &fields); for (const FieldDescriptor* field : fields) { reflection->ClearField(message, field); } @@ -274,7 +274,7 @@ bool ReflectionOps::IsInitialized(const Message& message) { std::vector fields; // Should be safe to skip stripped fields because required fields are not // stripped. - reflection->ListFieldsOmitStripped(message, &fields); + reflection->ListFields(message, &fields); for (const FieldDescriptor* field : fields) { if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { @@ -403,7 +403,7 @@ void ReflectionOps::FindInitializationErrors(const Message& message, // Check sub-messages. std::vector fields; - reflection->ListFieldsOmitStripped(message, &fields); + reflection->ListFields(message, &fields); for (const FieldDescriptor* field : fields) { if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc index 3623bcb9fa..abf0bfdf51 100644 --- a/src/google/protobuf/text_format.cc +++ b/src/google/protobuf/text_format.cc @@ -2252,10 +2252,7 @@ void TextFormat::Printer::PrintMessage(const Message& message, fields.push_back(descriptor->field(0)); fields.push_back(descriptor->field(1)); } else { - reflection->ListFieldsOmitStripped(message, &fields); - if (reflection->IsMessageStripped(message.GetDescriptor())) { - generator->Print(kDoNotParse, std::strlen(kDoNotParse)); - } + reflection->ListFields(message, &fields); } if (print_message_fields_in_index_order_) {