Skip to content

C
capnproto
Commit db7ca960 authored by Kenton Varda's avatar Kenton Varda
Browse files
Options

Fixes #219, with POSSIBLE (but obscure) WIRE FORMAT BREAKAGE. 

Unfortunately, the layout algorithm had a bug which caused incorrect layout when declaring a union whose lowest-ordinal field was of type Void and nested in an inner union. That is:

    union {
      a :union {
        b @0 :Void
        ...
      }
      ...
    }

In this case, all the fields in the struct after the Void field -- including both unions' discriminants -- would end up misplaced. Although they did not end up overlapping (and therefore the incorrect layout "worked"), the result broke schema evolution rules around "retroactive unionization".

Unfortunately, we must break compatibility with any protocol that happened to contain the above pattern. Luckily, it's a fairly obscure case. Unluckily, Cap'n Proto's own schema format contains such a pattern. Luckily, the use of this pattern was introduced in v0.6.x and therefore has not been in any release build so far.
parent 1d0b60c8
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 60 additions and 17 deletions
c++/src/capnp/compiler/node-translator.c++
View file @ db7ca960
......@@ -427,15 +427,25 @@ public:
inline Group(Union& parent): parent(parent) {}
KJ_DISALLOW_COPY(Group);
void addVoid() override {
void addMember() {
if (!hasMembers) {
hasMembers = true;
parent.newGroupAddingFirstMember();
}
}
void addVoid() override {
addMember();
// Make sure that if this is a member of a union which is in turn a member of another union,
// that we let the outer union know that a field is being added, even though it is a
// zero-size field. This is important because the union needs to allocate its discriminant
// just before its second member is added.
parent.parent.addVoid();
}
uint addData(uint lgSize) override {
addVoid();
addMember();
uint bestSize = kj::maxValue;
kj::Maybe<uint> bestLocation = nullptr;
......@@ -476,7 +486,7 @@ public:
}
uint addPointer() override {
addVoid();
addMember();
if (parentPointerLocationUsage < parent.pointerLocations.size()) {
return parent.pointerLocations[parentPointerLocationUsage++];
......
c++/src/capnp/encoding-test.c++
View file @ db7ca960
......@@ -1228,6 +1228,22 @@ TEST(Encoding, UpgradeListInBuilder) {
EXPECT_NONFATAL_FAILURE(root.getAnyPointerField().getAs<List<Text>>());
}
TEST(Encoding, UpgradeUnion) {
// This tests for a specific case that was broken originally.
MallocMessageBuilder builder;
{
auto root = builder.getRoot<test::TestOldUnionVersion>();
root.setB(123);
}
{
auto root = builder.getRoot<test::TestNewUnionVersion>();
ASSERT_TRUE(root.isB())
EXPECT_EQ(123, root.getB());
}
}
// =======================================================================================
// Tests of generated code, not really of the encoding.
// TODO(cleanup): Move to a different test?
......
c++/src/capnp/schema.capnp.c++
View file @ db7ca960
......@@ -1550,7 +1550,7 @@ const ::capnp::_::RawSchema s_9500cce23b334d80 = {
static const ::capnp::_::AlignedData<255> b_d07378ede1f9cc60 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
96, 204, 249, 225, 237, 120, 115, 208,
19, 0, 0, 0, 1, 0, 2, 0,
19, 0, 0, 0, 1, 0, 3, 0,
217, 114, 76, 98, 9, 197, 63, 169,
1, 0, 7, 0, 0, 0, 19, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -1823,7 +1823,7 @@ const ::capnp::_::RawSchema s_d07378ede1f9cc60 = {
static const ::capnp::_::AlignedData<32> b_87e739250a60ea97 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
151, 234, 96, 10, 37, 57, 231, 135,
24, 0, 0, 0, 1, 0, 2, 0,
24, 0, 0, 0, 1, 0, 3, 0,
96, 204, 249, 225, 237, 120, 115, 208,
1, 0, 7, 0, 1, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -1869,7 +1869,7 @@ const ::capnp::_::RawSchema s_87e739250a60ea97 = {
static const ::capnp::_::AlignedData<45> b_9e0e78711a7f87a9 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
169, 135, 127, 26, 113, 120, 14, 158,
24, 0, 0, 0, 1, 0, 2, 0,
24, 0, 0, 0, 1, 0, 3, 0,
96, 204, 249, 225, 237, 120, 115, 208,
1, 0, 7, 0, 1, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -1929,7 +1929,7 @@ const ::capnp::_::RawSchema s_9e0e78711a7f87a9 = {
static const ::capnp::_::AlignedData<45> b_ac3a6f60ef4cc6d3 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
211, 198, 76, 239, 96, 111, 58, 172,
24, 0, 0, 0, 1, 0, 2, 0,
24, 0, 0, 0, 1, 0, 3, 0,
96, 204, 249, 225, 237, 120, 115, 208,
1, 0, 7, 0, 1, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -1989,7 +1989,7 @@ const ::capnp::_::RawSchema s_ac3a6f60ef4cc6d3 = {
static const ::capnp::_::AlignedData<46> b_ed8bca69f7fb0cbf = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
191, 12, 251, 247, 105, 202, 139, 237,
24, 0, 0, 0, 1, 0, 2, 0,
24, 0, 0, 0, 1, 0, 3, 0,
96, 204, 249, 225, 237, 120, 115, 208,
1, 0, 7, 0, 1, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -2050,10 +2050,10 @@ const ::capnp::_::RawSchema s_ed8bca69f7fb0cbf = {
static const ::capnp::_::AlignedData<46> b_c2573fe8a23e49f1 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
241, 73, 62, 162, 232, 63, 87, 194,
24, 0, 0, 0, 1, 0, 2, 0,
24, 0, 0, 0, 1, 0, 3, 0,
96, 204, 249, 225, 237, 120, 115, 208,
1, 0, 7, 0, 1, 0, 3, 0,
2, 0, 0, 0, 0, 0, 0, 0,
4, 0, 0, 0, 0, 0, 0, 0,
21, 0, 0, 0, 26, 1, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -2113,10 +2113,10 @@ const ::capnp::_::RawSchema s_c2573fe8a23e49f1 = {
static const ::capnp::_::AlignedData<77> b_8e3b5f79fe593656 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
86, 54, 89, 254, 121, 95, 59, 142,
35, 0, 0, 0, 1, 0, 2, 0,
35, 0, 0, 0, 1, 0, 3, 0,
241, 73, 62, 162, 232, 63, 87, 194,
1, 0, 7, 0, 1, 0, 4, 0,
1, 0, 0, 0, 0, 0, 0, 0,
5, 0, 0, 0, 0, 0, 0, 0,
21, 0, 0, 0, 138, 1, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -2204,7 +2204,7 @@ const ::capnp::_::RawSchema s_8e3b5f79fe593656 = {
static const ::capnp::_::AlignedData<48> b_9dd1f724f4614a85 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
133, 74, 97, 244, 36, 247, 209, 157,
35, 0, 0, 0, 1, 0, 2, 0,
35, 0, 0, 0, 1, 0, 3, 0,
241, 73, 62, 162, 232, 63, 87, 194,
1, 0, 7, 0, 1, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -2221,14 +2221,14 @@ static const ::capnp::_::AlignedData<48> b_9dd1f724f4614a85 = {
101, 114, 46, 112, 97, 114, 97, 109,
101, 116, 101, 114, 0, 0, 0, 0,
8, 0, 0, 0, 3, 0, 4, 0,
0, 0, 0, 0, 1, 0, 0, 0,
0, 0, 0, 0, 2, 0, 0, 0,
0, 0, 1, 0, 19, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
41, 0, 0, 0, 66, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
36, 0, 0, 0, 2, 0, 1, 0,
44, 0, 0, 0, 2, 0, 1, 0,
1, 0, 0, 0, 1, 0, 0, 0,
1, 0, 0, 0, 5, 0, 0, 0,
0, 0, 1, 0, 20, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
41, 0, 0, 0, 122, 0, 0, 0,
......@@ -2266,7 +2266,7 @@ const ::capnp::_::RawSchema s_9dd1f724f4614a85 = {
static const ::capnp::_::AlignedData<36> b_baefc9120c56e274 = {
{ 0, 0, 0, 0, 5, 0, 6, 0,
116, 226, 86, 12, 18, 201, 239, 186,
35, 0, 0, 0, 1, 0, 2, 0,
35, 0, 0, 0, 1, 0, 3, 0,
241, 73, 62, 162, 232, 63, 87, 194,
1, 0, 7, 0, 1, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
......@@ -2285,7 +2285,7 @@ static const ::capnp::_::AlignedData<36> b_baefc9120c56e274 = {
100, 80, 97, 114, 97, 109, 101, 116,
101, 114, 0, 0, 0, 0, 0, 0,
4, 0, 0, 0, 3, 0, 4, 0,
0, 0, 0, 0, 1, 0, 0, 0,
0, 0, 0, 0, 5, 0, 0, 0,
0, 0, 1, 0, 24, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
13, 0, 0, 0, 122, 0, 0, 0,
......
c++/src/capnp/schema.capnp.h
View file @ db7ca960
This diff is collapsed.
c++/src/capnp/test.capnp
View file @ db7ca960
......@@ -478,6 +478,23 @@ struct TestNewVersion {
new2 @4 :Text = "baz";
}
struct TestOldUnionVersion {
union {
a @0 :Void;
b @1 :UInt64;
}
}
struct TestNewUnionVersion {
union {
a :union {
a0 @0 :Void;
a1 @2 :UInt64;
}
b @1 :UInt64;
}
}
struct TestStructUnion {
un @0! :union {
struct @1 :SomeStruct;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment