Add regression test for invalid inline string code generation.

A bug was found during code review that didn't trigger any test failure
The fast TDP entry did not have the index in the aux_idx field.

PiperOrigin-RevId: 511570831
pull/11780/head
Protobuf Team Bot 2 years ago committed by Copybara-Service
parent e6d01b2edc
commit 8f5e432615
  1. 2
      src/google/protobuf/generated_message_tctable_impl.h
  2. 57
      src/google/protobuf/message_unittest.inc
  3. 22
      src/google/protobuf/unittest.proto

@ -271,7 +271,7 @@ inline void AlignFail(std::integral_constant<size_t, 1>,
class PROTOBUF_EXPORT TcParser final {
public:
template <typename T>
static constexpr const TcParseTableBase* GetTable() {
static constexpr auto GetTable() -> decltype(&T::_table_.header) {
return &T::_table_.header;
}

@ -62,11 +62,12 @@
#include "google/protobuf/arena.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/generated_message_reflection.h"
#include "google/protobuf/message.h"
#include "google/protobuf/generated_message_tctable_impl.h"
#include "google/protobuf/io/coded_stream.h"
#include "google/protobuf/io/io_win32.h"
#include "google/protobuf/io/zero_copy_stream.h"
#include "google/protobuf/io/zero_copy_stream_impl.h"
#include "google/protobuf/message.h"
#include "google/protobuf/test_util2.h"
@ -1684,6 +1685,60 @@ class TestInputStream final : public io::ZeroCopyInputStream {
size_t break_pos_;
};
template <typename T>
static const internal::TcParseTableBase* GetTableIfAvailable(...) {
return nullptr;
}
template <typename T>
static const internal::TcParseTableBase* GetTableIfAvailable(
decltype(internal::TcParser::GetTable<T>())) {
return internal::TcParser::GetTable<T>();
}
TEST(MESSAGE_TEST_NAME, TestRegressionInlinedStringAuxIdxMismatchOnFastParser) {
using Proto = UNITTEST::InlinedStringIdxRegressionProto;
auto* table = GetTableIfAvailable<Proto>(nullptr);
// Only test when TDP is on, and we have these fields inlined.
if (table != nullptr &&
table->fast_entry(1)->target() == internal::TcParser::FastSiS1) {
// optional string str1 = 1;
EXPECT_EQ(table->fast_entry(1)->bits.aux_idx(), 1);
// optional InlinedStringIdxRegressionProto sub = 2;
EXPECT_EQ(table->fast_entry(2)->bits.aux_idx(), 2);
// optional string str2 = 3;
// The aux_idx points to the inlined_string_idx and not the actual aux_idx.
EXPECT_EQ(table->fast_entry(3)->bits.aux_idx(), 2);
// optional string str3 = 4;
// The aux_idx points to the inlined_string_idx and not the actual aux_idx.
EXPECT_EQ(table->fast_entry(0)->bits.aux_idx(), 3);
}
std::string encoded;
{
Proto proto;
// We use strings longer than SSO.
proto.set_str1(std::string(100, 'a'));
proto.set_str2(std::string(100, 'a'));
proto.set_str3(std::string(100, 'a'));
encoded = proto.SerializeAsString();
}
Arena arena;
auto* proto = Arena::CreateMessage<Proto>(&arena);
// We don't alter donation here, so it works even if the idx are bad.
ASSERT_TRUE(proto->ParseFromString(encoded));
// Now we alter donation bits. str2's bit (#2) will be off, but its aux_idx
// (#3) will point to a donated string.
proto = Arena::CreateMessage<Proto>(&arena);
proto->mutable_str1();
proto->mutable_str2();
proto->mutable_str3();
// With the bug, this breaks the cleanup list, causing UB on arena
// destruction.
ASSERT_TRUE(proto->ParseFromString(encoded));
}
TEST(MESSAGE_TEST_NAME, TestRepeatedStringParsers) {
google::protobuf::Arena arena;

@ -1549,7 +1549,7 @@ message EnumParseTester {
// An arbitrary field we can append to to break the runs of repeated fields.
optional int32 other_field = 99;
};
}
// This message contains different kind of bool fields to exercise the different
// parsers in table-drived.
@ -1566,7 +1566,7 @@ message BoolParseTester {
// An arbitrary field we can append to to break the runs of repeated fields.
optional int32 other_field = 99;
};
}
message Int32ParseTester {
optional int32 optional_int32_lowfield = 1;
@ -1581,7 +1581,7 @@ message Int32ParseTester {
// An arbitrary field we can append to to break the runs of repeated fields.
optional int32 other_field = 99;
};
}
message Int64ParseTester {
optional int64 optional_int64_lowfield = 1;
@ -1596,7 +1596,19 @@ message Int64ParseTester {
// An arbitrary field we can append to to break the runs of repeated fields.
optional int32 other_field = 99;
};
}
message InlinedStringIdxRegressionProto {
// We mix data to make sure aux ids and inlined string idx do not match.
// aux_idx == inlined_string_idx == 1
optional string str1 = 1;
// aux_idx == 2
optional InlinedStringIdxRegressionProto sub = 2;
// aux_idx == 3, inlined_string_idx == 2
optional string str2 = 3;
// aux_idx == 4, inlined_string_idx == 3
optional bytes str3 = 4;
}
message StringParseTester {
optional string optional_string_lowfield = 1;
@ -1605,7 +1617,7 @@ message StringParseTester {
repeated string repeated_string_lowfield = 2;
repeated string repeated_string_midfield = 1002;
repeated string repeated_string_hifield = 1000002;
};
}
message BadFieldNames{
optional int32 OptionalInt32 = 1;

Loading…
Cancel
Save