Skip to content

Commit 507f86b

Browse files
protobuf-github-botcopybara-github
protobuf-github-bot
authored and
copybara-github
committed
Remove first implementation of protobuf out of bounds enforcement.
PiperOrigin-RevId: 861408394
1 parent 1486093 commit 507f86b

9 files changed

Lines changed: 50 additions & 182 deletions

File tree

‎src/google/protobuf/compiler/cpp/field_generators/message_field.cc‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -783,20 +783,19 @@ void RepeatedMessage::GenerateAccessorDeclarations(io::Printer* p) const {
783783
void RepeatedMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
784784
// TODO: move insertion points
785785

786-
p->Emit({GetEmitRepeatedFieldMutableSub(*opts_, p)},
787-
R"cc(
788-
//~ Note: no need to set hasbit in mutable_$name$(int index).
789-
//~ Hasbits only need to be updated if a new element is
790-
//~ (potentially) added, not if an existing element is mutated.
791-
inline $Submsg$* $nonnull$ $Msg$::mutable_$name$(int index)
792-
ABSL_ATTRIBUTE_LIFETIME_BOUND {
793-
$WeakDescriptorSelfPin$;
794-
$annotate_mutable$;
795-
// @@protoc_insertion_point(field_mutable:$pkg.Msg.field$)
796-
$StrongRef$;
797-
return $mutable$;
798-
}
799-
)cc");
786+
p->Emit(R"cc(
787+
//~ Note: no need to set hasbit in mutable_$name$(int index).
788+
//~ Hasbits only need to be updated if a new element is
789+
//~ (potentially) added, not if an existing element is mutated.
790+
inline $Submsg$* $nonnull$ $Msg$::mutable_$name$(int index)
791+
ABSL_ATTRIBUTE_LIFETIME_BOUND {
792+
$WeakDescriptorSelfPin$;
793+
$annotate_mutable$;
794+
// @@protoc_insertion_point(field_mutable:$pkg.Msg.field$)
795+
$StrongRef$;
796+
return _internal_mutable_$name_internal$()->Mutable(index);
797+
}
798+
)cc");
800799

801800
p->Emit(R"cc(
802801
inline $pb$::RepeatedPtrField<$Submsg$>* $nonnull$ $Msg$::mutable_$name$()
@@ -810,17 +809,16 @@ void RepeatedMessage::GenerateInlineAccessorDefinitions(io::Printer* p) const {
810809
return _internal_mutable_$name_internal$();
811810
}
812811
)cc");
813-
p->Emit({GetEmitRepeatedFieldGetterSub(*opts_, p)},
814-
R"cc(
815-
inline const $Submsg$& $Msg$::$name$(int index) const
816-
ABSL_ATTRIBUTE_LIFETIME_BOUND {
817-
$WeakDescriptorSelfPin$;
818-
$annotate_get$;
819-
// @@protoc_insertion_point(field_get:$pkg.Msg.field$)
820-
$StrongRef$;
821-
return $getter$;
822-
}
823-
)cc");
812+
p->Emit(R"cc(
813+
inline const $Submsg$& $Msg$::$name$(int index) const
814+
ABSL_ATTRIBUTE_LIFETIME_BOUND {
815+
$WeakDescriptorSelfPin$;
816+
$annotate_get$;
817+
// @@protoc_insertion_point(field_get:$pkg.Msg.field$)
818+
$StrongRef$;
819+
return _internal_$name_internal$().Get(index);
820+
}
821+
)cc");
824822
p->Emit(R"cc(
825823
inline $Submsg$* $nonnull$ $Msg$::add_$name$()
826824
ABSL_ATTRIBUTE_LIFETIME_BOUND {

‎src/google/protobuf/compiler/cpp/field_generators/string_field.cc‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -793,14 +793,12 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
793793
bool bytes = field_->type() == FieldDescriptor::TYPE_BYTES;
794794
p->Emit(
795795
{
796-
GetEmitRepeatedFieldGetterSub(*opts_, p),
797796
{"bytes_tag",
798797
[&] {
799798
if (bytes) {
800799
p->Emit(", $pbi$::BytesTag{}");
801800
}
802801
}},
803-
GetEmitRepeatedFieldMutableSub(*opts_, p),
804802
},
805803
R"cc(
806804
inline ::std::string* $nonnull$ $Msg$::add_$name$()
@@ -820,7 +818,7 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
820818
$WeakDescriptorSelfPin$;
821819
$annotate_get$;
822820
// @@protoc_insertion_point(field_get:$pkg.Msg.field$)
823-
return $getter$;
821+
return _internal_$name_internal$().Get(index);
824822
}
825823
//~ Note: no need to set hasbit in mutable_$name$(int index). Hasbits
826824
//~ only need to be updated if a new element is (potentially) added, not
@@ -830,16 +828,17 @@ void RepeatedString::GenerateInlineAccessorDefinitions(io::Printer* p) const {
830828
$WeakDescriptorSelfPin$;
831829
$annotate_mutable$;
832830
// @@protoc_insertion_point(field_mutable:$pkg.Msg.field$)
833-
return $mutable$;
831+
return _internal_mutable_$name_internal$()->Mutable(index);
834832
}
835833
//~ Note: no need to set hasbit in set_$name$(int index). Hasbits
836834
//~ only need to be updated if a new element is (potentially) added, not
837835
//~ if an existing element is mutated.
838836
template <typename Arg_, typename... Args_>
839837
inline void $Msg$::set_$name$(int index, Arg_&& value, Args_... args) {
840838
$WeakDescriptorSelfPin$;
841-
$pbi$::AssignToString(*$mutable$, ::std::forward<Arg_>(value),
842-
args... $bytes_tag$);
839+
$pbi$::AssignToString(
840+
*_internal_mutable_$name_internal$()->Mutable(index),
841+
::std::forward<Arg_>(value), args... $bytes_tag$);
843842
$annotate_set$;
844843
// @@protoc_insertion_point(field_set:$pkg.Msg.field$)
845844
}

‎src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -668,30 +668,30 @@ void RepeatedStringView::GenerateInlineAccessorDefinitions(
668668
io::Printer* p) const {
669669
p->Emit(
670670
{
671-
{GetEmitRepeatedFieldGetterSub(*opts_, p)},
672671
{"bytes_tag",
673672
[&] {
674673
if (field_->type() == FieldDescriptor::TYPE_BYTES) {
675674
p->Emit(", $pbi$::BytesTag{}");
676675
}
677676
}},
678-
{GetEmitRepeatedFieldMutableSub(*opts_, p)},
679677
},
680678
R"cc(
681679
inline ::absl::string_view $Msg$::$name$(int index) const
682680
ABSL_ATTRIBUTE_LIFETIME_BOUND {
683681
$WeakDescriptorSelfPin$;
684682
$annotate_get$;
685683
// @@protoc_insertion_point(field_get:$pkg.Msg.field$)
686-
return $getter$;
684+
return _internal_$name_internal$().Get(index);
687685
}
688686
//~ Note: no need to set hasbit in set_$name$(int index). Hasbits only
689687
//~ need to be updated if a new element is (potentially) added, not if
690688
//~ an existing element is mutated.
691689
template <typename Arg_>
692690
inline void $Msg$::set_$name$(int index, Arg_&& value) {
693691
$WeakDescriptorSelfPin$;
694-
$pbi$::AssignToString(*$mutable$, ::std::forward<Arg_>(value) $bytes_tag$);
692+
$pbi$::AssignToString(
693+
*_internal_mutable_$name_internal$()->Mutable(index),
694+
::std::forward<Arg_>(value) $bytes_tag$);
695695
$annotate_set$;
696696
// @@protoc_insertion_point(field_set:$pkg.Msg.field$)
697697
}

‎src/google/protobuf/compiler/cpp/helpers.h‎

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,69 +1238,6 @@ bool HasOnDeserializeTracker(const Descriptor* descriptor,
12381238
// signature.
12391239
bool NeedsPostLoopHandler(const Descriptor* descriptor, const Options& options);
12401240

1241-
// Emit the repeated field getter for the custom options.
1242-
// Depending on the bounds check mode specified, this will emit the
1243-
// corresponding getter.
1244-
inline auto GetEmitRepeatedFieldGetterSub(const Options& options,
1245-
io::Printer* p) {
1246-
return io::Printer::Sub{
1247-
"getter",
1248-
[&options, p] {
1249-
switch (options.bounds_check_mode) {
1250-
case BoundsCheckMode::kNoEnforcement:
1251-
p->Emit(R"cc(_internal_$name_internal$().Get(index))cc");
1252-
break;
1253-
case BoundsCheckMode::kReturnDefaultValue:
1254-
p->Emit(R"cc(
1255-
$pbi$::CheckedGetOrDefault(_internal_$name_internal$(), index)
1256-
)cc");
1257-
break;
1258-
case BoundsCheckMode::kAbort:
1259-
p->Emit(R"cc(
1260-
$pbi$::CheckedGetOrAbort(_internal_$name_internal$(), index)
1261-
)cc");
1262-
break;
1263-
}
1264-
}}
1265-
.WithSuffix("");
1266-
}
1267-
1268-
// Emit the code for getting a mutable element from a repeated field. This will
1269-
// generate different code depending on the `bounds_check_mode` specified in the
1270-
// options.
1271-
// TODO: b/347304492 Harden this function by taking in the field and checking
1272-
// if splitting is supported.
1273-
inline auto GetEmitRepeatedFieldMutableSub(const Options& options,
1274-
io::Printer* p,
1275-
bool use_stringpiecefield = false) {
1276-
return io::Printer::Sub{
1277-
"mutable",
1278-
[&options, p, use_stringpiecefield] {
1279-
switch (options.bounds_check_mode) {
1280-
case BoundsCheckMode::kNoEnforcement:
1281-
case BoundsCheckMode::kReturnDefaultValue:
1282-
if (use_stringpiecefield) {
1283-
p->Emit("$field$.Mutable(index)");
1284-
} else {
1285-
p->Emit(
1286-
R"cc(_internal_mutable_$name_internal$()->Mutable(index))cc");
1287-
}
1288-
break;
1289-
case BoundsCheckMode::kAbort:
1290-
if (use_stringpiecefield) {
1291-
p->Emit("$pbi$::CheckedMutableOrAbort(&$field$, index)");
1292-
} else {
1293-
p->Emit(R"cc(
1294-
$pbi$::CheckedMutableOrAbort(
1295-
_internal_mutable_$name_internal$(), index)
1296-
)cc");
1297-
}
1298-
break;
1299-
}
1300-
}}
1301-
.WithSuffix("");
1302-
}
1303-
13041241
// Priority used for static initializers.
13051242
enum InitPriority {
13061243
kInitPriority101,

‎src/google/protobuf/compiler/cpp/options.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,6 @@ enum class EnforceOptimizeMode {
3333
kLiteRuntime,
3434
};
3535

36-
enum class BoundsCheckMode {
37-
kNoEnforcement, // No enforcement.
38-
kReturnDefaultValue, // Return default value if out of bounds.
39-
kAbort, // TrapOrAbort if out of bounds.
40-
};
41-
4236
struct FieldListenerOptions {
4337
bool inject_field_listener_events = false;
4438
absl::flat_hash_set<std::string> forbidden_field_listener_events;
@@ -56,7 +50,6 @@ struct Options {
5650
FieldListenerOptions field_listener_options;
5751
EnforceOptimizeMode enforce_mode = EnforceOptimizeMode::kNoEnforcement;
5852
int num_cc_files = 0;
59-
BoundsCheckMode bounds_check_mode = BoundsCheckMode::kNoEnforcement;
6053
bool proto_h = false;
6154
bool transitive_pb_h = true;
6255
bool annotate_headers = false;

‎src/google/protobuf/compiler/plugin.pb.h‎

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/google/protobuf/descriptor.pb.h‎

Lines changed: 15 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/google/protobuf/repeated_ptr_field.h‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2500,36 +2500,6 @@ class UnsafeArenaAllocatedRepeatedPtrFieldBackInsertIterator {
25002500
private:
25012501
RepeatedPtrField<T>* field_;
25022502
};
2503-
2504-
template <typename T>
2505-
const T& CheckedGetOrDefault(const RepeatedPtrField<T>& field, int index) {
2506-
if (ABSL_PREDICT_FALSE(index < 0 || index >= field.size())) {
2507-
LogIndexOutOfBounds(index, field.size());
2508-
return GenericTypeHandler<T>::default_instance();
2509-
}
2510-
return field.Get(index);
2511-
}
2512-
2513-
template <typename T>
2514-
inline void CheckIndexInBoundsOrAbort(const RepeatedPtrField<T>& field,
2515-
int index) {
2516-
if (ABSL_PREDICT_FALSE(index < 0 || index >= field.size())) {
2517-
LogIndexOutOfBoundsAndAbort(index, field.size());
2518-
}
2519-
}
2520-
2521-
template <typename T>
2522-
const T& CheckedGetOrAbort(const RepeatedPtrField<T>& field, int index) {
2523-
CheckIndexInBoundsOrAbort(field, index);
2524-
return field.Get(index);
2525-
}
2526-
2527-
template <typename T>
2528-
inline T* CheckedMutableOrAbort(RepeatedPtrField<T>* field, int index) {
2529-
CheckIndexInBoundsOrAbort(*field, index);
2530-
return field->Mutable(index);
2531-
}
2532-
25332503
} // namespace internal
25342504

25352505
// Provides a back insert iterator for RepeatedPtrField instances,

‎src/google/protobuf/repeated_ptr_field_unittest.cc‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,41 +1185,6 @@ TEST(RepeatedPtrFieldTest, Cleanups) {
11851185
}
11861186

11871187

1188-
TEST(RepeatedPtrFieldDeathTest, CheckedGetOrAbortTest) {
1189-
RepeatedPtrField<std::string> field;
1190-
1191-
// Empty container tests.
1192-
EXPECT_DEATH(internal::CheckedGetOrAbort(field, -1),
1193-
"Index \\(-1\\) out of bounds of container with size \\(0\\)");
1194-
EXPECT_DEATH(internal::CheckedGetOrAbort(field, field.size()),
1195-
"Index \\(0\\) out of bounds of container with size \\(0\\)");
1196-
1197-
// Non-empty container tests
1198-
field.Add()->assign("foo");
1199-
field.Add()->assign("bar");
1200-
EXPECT_DEATH(internal::CheckedGetOrAbort(field, 2),
1201-
"Index \\(2\\) out of bounds of container with size \\(2\\)");
1202-
EXPECT_DEATH(internal::CheckedGetOrAbort(field, -1),
1203-
"Index \\(-1\\) out of bounds of container with size \\(2\\)");
1204-
}
1205-
1206-
TEST(RepeatedPtrFieldDeathTest, CheckedMutableOrAbortTest) {
1207-
RepeatedPtrField<std::string> field;
1208-
1209-
// Empty container tests.
1210-
EXPECT_DEATH(internal::CheckedMutableOrAbort(&field, -1),
1211-
"Index \\(-1\\) out of bounds of container with size \\(0\\)");
1212-
EXPECT_DEATH(internal::CheckedMutableOrAbort(&field, field.size()),
1213-
"Index \\(0\\) out of bounds of container with size \\(0\\)");
1214-
1215-
// Non-empty container tests
1216-
field.Add()->assign("foo");
1217-
field.Add()->assign("bar");
1218-
EXPECT_DEATH(internal::CheckedMutableOrAbort(&field, 2),
1219-
"Index \\(2\\) out of bounds of container with size \\(2\\)");
1220-
EXPECT_DEATH(internal::CheckedMutableOrAbort(&field, -1),
1221-
"Index \\(-1\\) out of bounds of container with size \\(2\\)");
1222-
}
12231188
// ===================================================================
12241189

12251190
class RepeatedPtrFieldIteratorTest : public testing::Test {

0 commit comments

Comments
 (0)