Skip to content

Commit

Permalink
Update lite_unittest to pad varints before calling `internal::VarintP…
Browse files Browse the repository at this point in the history
…arse()`

The `VarintParse()` function is ordinarily called only by the proto parser,
which always provides 16 bytes of "slop" space at the end of the buffer. The
ARM-specific optimized path takes advantage of this by always reading at least
8 bytes. However, this caused some test failures in msan due to unit tests not
providing a sufficient amount of initialized padding. This CL fixes the problem
by making sure the unit tests initialize the full 10-byte buffer and adding a
comment stating that this is a precondition of the function.

PiperOrigin-RevId: 592366291
  • Loading branch information
acozzette authored and copybara-github committed Dec 19, 2023
1 parent 1250d5f commit bc66a18
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/google/protobuf/lite_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// Author: [email protected] (Kenton Varda)

#include <climits>
#include <cstdint>
#include <iostream>
#include <limits>
#include <string>
Expand Down Expand Up @@ -104,7 +105,7 @@ void SetSomeTypesInEmptyMessageUnknownFields(

TEST(ParseVarintTest, Varint32) {
auto test_value = [](uint32_t value, int varint_length) {
uint8_t buffer[10];
uint8_t buffer[10] = {0};
uint8_t* p = io::CodedOutputStream::WriteVarint32ToArray(value, buffer);
ASSERT_EQ(p - buffer, varint_length) << "Value = " << value;

Expand All @@ -131,7 +132,7 @@ TEST(ParseVarintTest, Varint32) {

TEST(ParseVarintTest, Varint64) {
auto test_value = [](uint64_t value, int varint_length) {
uint8_t buffer[10];
uint8_t buffer[10] = {0};
uint8_t* p = io::CodedOutputStream::WriteVarint64ToArray(value, buffer);
ASSERT_EQ(p - buffer, varint_length) << "Value = " << value;

Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/parse_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,7 @@ static const char* VarintParseSlowArm(const char* p, uint64_t* out,
}
#endif

// The caller must ensure that p points to at least 10 valid bytes.
template <typename T>
PROTOBUF_NODISCARD const char* VarintParse(const char* p, T* out) {
#if defined(__aarch64__) && defined(ABSL_IS_LITTLE_ENDIAN)
Expand Down

0 comments on commit bc66a18

Please sign in to comment.