Skip to content

Commit 72c6729

Browse files
Use .data.rel.ro section for message globals.
Putting message globals on .data.rel.ro section has two benefits: (1) makes it truly read only and forces seg faults rather than subtle bug when users unintentionally mutate the default instance. (2) collocates it on the same section where the type's other const data (vtbl). PiperOrigin-RevId: 892545502
1 parent 6b5e811 commit 72c6729

5 files changed

Lines changed: 87 additions & 31 deletions

File tree

src/google/protobuf/compiler/cpp/message.cc

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5652,11 +5652,22 @@ void MessageGenerator::GenerateSourceDefaultInstance(io::Printer* p) {
56525652
}},
56535653
{"section_decl",
56545654
[&] {
5655-
if (!use_implicit_weak_descriptor) return;
5656-
p->Emit({{"section",
5657-
WeakDefaultInstanceSection(
5658-
descriptor_, index_in_file_messages_, options_)}},
5659-
R"cc(__attribute__((section("$section$"))))cc");
5655+
if (use_implicit_weak_descriptor) {
5656+
p->Emit({{"section",
5657+
WeakDefaultInstanceSection(
5658+
descriptor_, index_in_file_messages_, options_)}},
5659+
R"cc(__attribute__((section("$section$"))))cc");
5660+
return;
5661+
}
5662+
// File descriptor proto is mutable.
5663+
if (is_file_descriptor_proto) return;
5664+
5665+
p->Emit(
5666+
R"cc(
5667+
#ifdef PROTOBUF_MESSAGE_GLOBALS
5668+
__attribute__((section(".data.rel.ro")))
5669+
#endif // PROTOBUF_MESSAGE_GLOBALS
5670+
)cc");
56605671
}},
56615672
{"const",
56625673
[&] {

src/google/protobuf/compiler/java/java_features.pb.cc

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

src/google/protobuf/compiler/plugin.pb.cc

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

src/google/protobuf/cpp_features.pb.cc

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/google/protobuf/message_unittest.inc

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,17 @@ UNITTEST::NestedTestAllTypes InitNestedProto(int depth) {
9999
}
100100
} // namespace
101101

102+
TEST(MESSAGE_TEST_NAME, DieWhenDefaultInstanceIsMutated) {
103+
#ifndef PROTOBUF_MESSAGE_GLOBALS
104+
GTEST_SKIP() << "Skipping test as the failure is expected only with "
105+
"PROTOBUF_MESSAGE_GLOBALS";
106+
#endif // !PROTOBUF_MESSAGE_GLOBALS
107+
108+
auto* message = const_cast<UNITTEST::TestAllTypes*>(
109+
&UNITTEST::TestAllTypes::default_instance());
110+
EXPECT_DEATH(message->set_optional_int32(2), "SIGSEGV|Check failed");
111+
}
112+
102113
TEST(MESSAGE_TEST_NAME, SerializeHelpers) {
103114
// TODO: Test more helpers? They're all two-liners so it seems
104115
// like a waste of time.
@@ -284,9 +295,11 @@ TEST(MESSAGE_TEST_NAME, ParseFailsIfNotInitialized) {
284295

285296
{
286297
absl::ScopedMockLog log(absl::MockLogDefault::kDisallowUnexpected);
287-
EXPECT_CALL(log, Log(absl::LogSeverity::kError, testing::_, absl::StrCat(
288-
"Can't parse message of type \"", UNITTEST_PACKAGE_NAME,
289-
".TestRequired\" because it is missing required fields: a, b, c")));
298+
EXPECT_CALL(log, Log(absl::LogSeverity::kError, testing::_,
299+
absl::StrCat("Can't parse message of type \"",
300+
UNITTEST_PACKAGE_NAME,
301+
".TestRequired\" because it is missing "
302+
"required fields: a, b, c")));
290303
log.StartCapturingLogs();
291304
EXPECT_FALSE(message.ParseFromString(""));
292305
}
@@ -302,10 +315,13 @@ TEST(MESSAGE_TEST_NAME, ParseFailsIfSubmessageNotInitialized) {
302315

303316
{
304317
absl::ScopedMockLog log(absl::MockLogDefault::kDisallowUnexpected);
305-
EXPECT_CALL(log, Log(absl::LogSeverity::kError, testing::_, absl::StrCat(
306-
"Can't parse message of type \"", UNITTEST_PACKAGE_NAME,
307-
".TestRequiredForeign\" because it is missing required fields: "
308-
"optional_message.a, optional_message.b, optional_message.c")));
318+
EXPECT_CALL(
319+
log,
320+
Log(absl::LogSeverity::kError, testing::_,
321+
absl::StrCat(
322+
"Can't parse message of type \"", UNITTEST_PACKAGE_NAME,
323+
".TestRequiredForeign\" because it is missing required fields: "
324+
"optional_message.a, optional_message.b, optional_message.c")));
309325
log.StartCapturingLogs();
310326
EXPECT_FALSE(message.ParseFromString(source.SerializePartialAsString()));
311327
}
@@ -321,15 +337,17 @@ TEST(MESSAGE_TEST_NAME, ParseFailsIfExtensionNotInitialized) {
321337
EXPECT_TRUE(message.ParsePartialFromString(serialized));
322338
EXPECT_FALSE(message.IsInitialized());
323339

324-
{
340+
{
325341
absl::ScopedMockLog log(absl::MockLogDefault::kDisallowUnexpected);
326-
EXPECT_CALL(log, Log(absl::LogSeverity::kError, testing::_, absl::Substitute(
327-
"Can't parse message of type \"$0.TestChildExtension\" "
328-
"because it is missing required fields: "
329-
"optional_extension.($0.TestRequired.single).a, "
330-
"optional_extension.($0.TestRequired.single).b, "
331-
"optional_extension.($0.TestRequired.single).c",
332-
UNITTEST_PACKAGE_NAME)));
342+
EXPECT_CALL(log,
343+
Log(absl::LogSeverity::kError, testing::_,
344+
absl::Substitute(
345+
"Can't parse message of type \"$0.TestChildExtension\" "
346+
"because it is missing required fields: "
347+
"optional_extension.($0.TestRequired.single).a, "
348+
"optional_extension.($0.TestRequired.single).b, "
349+
"optional_extension.($0.TestRequired.single).c",
350+
UNITTEST_PACKAGE_NAME)));
333351
log.StartCapturingLogs();
334352
EXPECT_FALSE(message.ParseFromString(source.SerializePartialAsString()));
335353
}
@@ -1430,9 +1448,8 @@ TEST(MESSAGE_TEST_NAME, MessageTraitsWork) {
14301448
EXPECT_EQ(
14311449
&UNITTEST::TestAllTypes::default_instance(),
14321450
internal::MessageTraits<UNITTEST::TestAllTypes>::default_instance());
1433-
EXPECT_EQ(
1434-
internal::GetClassData(UNITTEST::TestAllTypes::default_instance()),
1435-
internal::MessageTraits<UNITTEST::TestAllTypes>::class_data());
1451+
EXPECT_EQ(internal::GetClassData(UNITTEST::TestAllTypes::default_instance()),
1452+
internal::MessageTraits<UNITTEST::TestAllTypes>::class_data());
14361453
}
14371454

14381455

@@ -1885,7 +1902,7 @@ TEST(MESSAGE_TEST_NAME, TestInt32Parsers) {
18851902
if (field->name() == "other_field") continue;
18861903
SCOPED_TRACE(field->full_name());
18871904
for (int32_t value : {1, 0, -1, (std::numeric_limits<int32_t>::min)(),
1888-
(std::numeric_limits<int32_t>::max)()}) {
1905+
(std::numeric_limits<int32_t>::max)()}) {
18891906
SCOPED_TRACE(value);
18901907
auto encoded =
18911908
EncodeInt32Value(field->number(), value, non_canonical_bytes);

0 commit comments

Comments
 (0)