Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed data loss bug in the core #497

Merged
merged 2 commits into from
Jan 28, 2022
Merged

Fixed data loss bug in the core #497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
032400a
Fixed data corruption when total hasbits are a power of two.
haberman Jan 27, 2022
31f7e2a
Added C++ test for the bug.
haberman Jan 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 35 additions & 0 deletions tests/bindings/lua/test.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,38 @@ message TestLargeFieldNumber {
message TestTimestamp {
optional google.protobuf.Timestamp ts = 1;
}

message HelloRequest {
optional uint32 id = 1;
optional uint32 random_name_a0 = 2;
optional uint32 random_name_a1 = 3;
optional uint32 random_name_a2 = 4;
optional uint32 random_name_a3 = 5;
optional uint32 random_name_a4 = 6;
optional uint32 random_name_a5 = 7;
optional uint32 random_name_a6 = 8;
optional uint32 random_name_a7 = 9;
optional uint32 random_name_a8 = 10;
optional uint32 random_name_a9 = 11;
optional uint32 random_name_b0 = 12;
optional uint32 random_name_b1 = 13;
optional uint32 random_name_b2 = 14;
optional uint32 random_name_b3 = 15;
optional uint32 random_name_b4 = 16;
optional uint32 random_name_b5 = 17;
optional uint32 random_name_b6 = 18;
optional uint32 random_name_b7 = 19;
optional uint32 random_name_b8 = 20;
optional uint32 random_name_b9 = 21;
optional uint32 random_name_c0 = 22;
optional uint32 random_name_c1 = 23;
optional uint32 random_name_c2 = 24;
optional uint32 random_name_c3 = 25;
optional uint32 random_name_c4 = 26;
optional uint32 random_name_c5 = 27;
optional uint32 random_name_c6 = 28;
optional uint32 random_name_c7 = 29;
optional uint32 random_name_c8 = 30;
optional uint32 random_name_c9 = 31;
optional string version = 32;
}
8 changes: 8 additions & 0 deletions tests/bindings/lua/test_upb.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,14 @@ function test_gc()
end
end

function test_b9440()
local m = upb_test.HelloRequest()
m.id = 8
assert_equal(8, m.id)
m.version = "1"
assert_equal(8, m.id)
end

local stats = lunit.main()

if stats.failed > 0 or stats.errors > 0 then
Expand Down
35 changes: 35 additions & 0 deletions tests/test.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,38 @@ message MessageName {
optional int32 field1 = 1;
optional int32 field2 = 2;
}

message HelloRequest {
optional uint32 id = 1;
optional uint32 random_name_a0 = 2;
optional uint32 random_name_a1 = 3;
optional uint32 random_name_a2 = 4;
optional uint32 random_name_a3 = 5;
optional uint32 random_name_a4 = 6;
optional uint32 random_name_a5 = 7;
optional uint32 random_name_a6 = 8;
optional uint32 random_name_a7 = 9;
optional uint32 random_name_a8 = 10;
optional uint32 random_name_a9 = 11;
optional uint32 random_name_b0 = 12;
optional uint32 random_name_b1 = 13;
optional uint32 random_name_b2 = 14;
optional uint32 random_name_b3 = 15;
optional uint32 random_name_b4 = 16;
optional uint32 random_name_b5 = 17;
optional uint32 random_name_b6 = 18;
optional uint32 random_name_b7 = 19;
optional uint32 random_name_b8 = 20;
optional uint32 random_name_b9 = 21;
optional uint32 random_name_c0 = 22;
optional uint32 random_name_c1 = 23;
optional uint32 random_name_c2 = 24;
optional uint32 random_name_c3 = 25;
optional uint32 random_name_c4 = 26;
optional uint32 random_name_c5 = 27;
optional uint32 random_name_c6 = 28;
optional uint32 random_name_c7 = 29;
optional uint32 random_name_c8 = 30;
optional uint32 random_name_c9 = 31;
optional string version = 32;
}
13 changes: 12 additions & 1 deletion tests/test_generated_code.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
* upb/def.c and tests/conformance_upb.c, respectively).
*/

#include "gtest/gtest.h"
#include "src/google/protobuf/test_messages_proto3.upb.h"
#include "tests/test.upb.h"
#include "gtest/gtest.h"
#include "upb/upb.hpp"

#define MIN(x, y) ((x) < (y) ? (x) : (y))

Expand Down Expand Up @@ -411,6 +412,16 @@ TEST(GeneratedCode, TestRepeated) {
upb_Arena_Free(arena);
}

TEST(GeneratedCode, Issue9440) {
upb::Arena arena;
upb_test_HelloRequest* msg = upb_test_HelloRequest_new(arena.ptr());
upb_test_HelloRequest_set_id(msg, 8);
EXPECT_EQ(8, upb_test_HelloRequest_id(msg));
char str[] = "1";
upb_test_HelloRequest_set_version(msg, upb_StringView{str, strlen(str)});
EXPECT_EQ(8, upb_test_HelloRequest_id(msg));
}

TEST(GeneratedCode, NullDecodeBuffer) {
upb_Arena* arena = upb_Arena_New();
protobuf_test_messages_proto3_TestAllTypesProto3* msg =
Expand Down
2 changes: 1 addition & 1 deletion upb/def.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1574,7 +1574,7 @@ static void make_layout(symtab_addctx* ctx, const upb_MessageDef* m) {
}

/* Account for space used by hasbits. */
l->size = div_round_up(hasbit, 8);
l->size = hasbit ? div_round_up(hasbit + 1, 8) : 0;

/* Allocate non-oneof fields. */
for (int i = 0; i < m->field_count; i++) {
Expand Down
2 changes: 1 addition & 1 deletion upbc/message_layout.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ void MessageLayout::PlaceNonOneofFields(
}

// Place hasbits at the beginning.
hasbit_bytes_ = DivRoundUp(hasbit_count_, 8);
hasbit_bytes_ = hasbit_count_ ? DivRoundUp(hasbit_count_ + 1, 8) : 0;
Place(SizeAndAlign{{hasbit_bytes_, hasbit_bytes_}, {1, 1}});

// Place non-oneof fields.
Expand Down