Commit 949f7353 authored by Kenton Varda's avatar Kenton Varda

Fix bug where calling a list setter using a list obtained from a similarly-typed…

Fix bug where calling a list setter using a list obtained from a similarly-typed getter, but where the underlying pointer was null, would write an incorrectly-typed pointer in the destination (specifically, an empty List(Void)). Now it sets an empty list of the correct type.
parent 2ed4b4e7
......@@ -388,7 +388,7 @@ struct List<AnyPointer, Kind::OTHER> {
public:
typedef List<AnyPointer> Reads;
Reader() = default;
inline Reader(): reader(ElementSize::POINTER) {}
inline explicit Reader(_::ListReader reader): reader(reader) {}
inline uint size() const { return reader.size() / ELEMENTS; }
......@@ -417,7 +417,7 @@ struct List<AnyPointer, Kind::OTHER> {
typedef List<AnyPointer> Builds;
Builder() = delete;
inline Builder(decltype(nullptr)) {}
inline Builder(decltype(nullptr)): builder(ElementSize::POINTER) {}
inline explicit Builder(_::ListBuilder builder): builder(builder) {}
inline operator Reader() { return Reader(builder.asReader()); }
......@@ -546,7 +546,7 @@ class List<AnyStruct, Kind::OTHER>::Reader {
public:
typedef List<AnyStruct> Reads;
Reader() = default;
inline Reader(): reader(ElementSize::INLINE_COMPOSITE) {}
inline explicit Reader(_::ListReader reader): reader(reader) {}
inline uint size() const { return reader.size() / ELEMENTS; }
......@@ -575,7 +575,7 @@ public:
typedef List<AnyStruct> Builds;
Builder() = delete;
inline Builder(decltype(nullptr)) {}
inline Builder(decltype(nullptr)): builder(ElementSize::INLINE_COMPOSITE) {}
inline explicit Builder(_::ListBuilder builder): builder(builder) {}
inline operator Reader() { return Reader(builder.asReader()); }
......@@ -602,7 +602,7 @@ private:
class AnyList::Reader {
public:
Reader() = default;
inline Reader(): _reader(ElementSize::VOID) {}
inline Reader(_::ListReader reader): _reader(reader) {}
#if !_MSC_VER // TODO(msvc): MSVC ICEs on this. Try restoring when compiler improves.
......@@ -635,7 +635,7 @@ private:
class AnyList::Builder {
public:
inline Builder(decltype(nullptr)) {}
inline Builder(decltype(nullptr)): _builder(ElementSize::VOID) {}
inline Builder(_::ListBuilder builder): _builder(builder) {}
#if !_MSC_VER // TODO(msvc): MSVC ICEs on this. Try restoring when compiler improves.
......
......@@ -351,7 +351,7 @@ class DynamicList::Reader {
public:
typedef DynamicList Reads;
Reader() = default;
inline Reader(): reader(ElementSize::VOID) {}
template <typename T, typename = kj::EnableIf<kind<FromReader<T>>() == Kind::LIST>>
inline Reader(T&& value): Reader(toDynamic(value)) {}
......@@ -392,8 +392,8 @@ class DynamicList::Builder {
public:
typedef DynamicList Builds;
Builder() = default;
inline Builder(decltype(nullptr)) {}
inline Builder(): builder(ElementSize::VOID) {}
inline Builder(decltype(nullptr)): builder(ElementSize::VOID) {}
template <typename T, typename = kj::EnableIf<kind<FromBuilder<T>>() == Kind::LIST>>
inline Builder(T&& value): Builder(toDynamic(value)) {}
......
......@@ -516,6 +516,80 @@ TEST(Encoding, SmallStructLists) {
}
}
TEST(Encoding, SetListToEmpty) {
// Test initializing list fields from various ways of constructing zero-sized lists.
// At one point this would often fail because the lists would have ElementSize::VOID which is
// incompatible with other list sizes.
#define ALL_LIST_TYPES(MACRO) \
MACRO(Void, Void) \
MACRO(Bool, bool) \
MACRO(UInt8, uint8_t) \
MACRO(UInt16, uint16_t) \
MACRO(UInt32, uint32_t) \
MACRO(UInt64, uint64_t) \
MACRO(Int8, int8_t) \
MACRO(Int16, int16_t) \
MACRO(Int32, int32_t) \
MACRO(Int64, int64_t) \
MACRO(Float32, float) \
MACRO(Float64, double) \
MACRO(Text, Text) \
MACRO(Data, Data) \
MACRO(Struct, TestAllTypes)
#define SET_FROM_READER_ACCESSOR(name, type) \
root.set##name##List(reader.get##name##List());
#define SET_FROM_BUILDER_ACCESSOR(name, type) \
root.set##name##List(root.get##name##List());
#define SET_FROM_READER_CONSTRUCTOR(name, type) \
root.set##name##List(List<type>::Reader());
#define SET_FROM_BUILDER_CONSTRUCTOR(name, type) \
root.set##name##List(List<type>::Builder());
#define CHECK_EMPTY_NONNULL(name, type) \
EXPECT_TRUE(root.has##name##List()); \
EXPECT_EQ(0, root.get##name##List().size());
{
MallocMessageBuilder builder;
auto root = builder.initRoot<test::TestAllTypes>();
auto reader = root.asReader();
ALL_LIST_TYPES(SET_FROM_READER_ACCESSOR)
ALL_LIST_TYPES(CHECK_EMPTY_NONNULL)
}
{
MallocMessageBuilder builder;
auto root = builder.initRoot<test::TestAllTypes>();
ALL_LIST_TYPES(SET_FROM_BUILDER_ACCESSOR)
ALL_LIST_TYPES(CHECK_EMPTY_NONNULL)
}
{
MallocMessageBuilder builder;
auto root = builder.initRoot<test::TestAllTypes>();
ALL_LIST_TYPES(SET_FROM_READER_CONSTRUCTOR)
ALL_LIST_TYPES(CHECK_EMPTY_NONNULL)
}
{
MallocMessageBuilder builder;
auto root = builder.initRoot<test::TestAllTypes>();
ALL_LIST_TYPES(SET_FROM_BUILDER_CONSTRUCTOR)
ALL_LIST_TYPES(CHECK_EMPTY_NONNULL)
}
#undef SET_FROM_READER_ACCESSOR
#undef SET_FROM_BUILDER_ACCESSOR
#undef SET_FROM_READER_CONSTRUCTOR
#undef SET_FROM_BUILDER_CONSTRUCTOR
#undef CHECK_EMPTY_NONNULL
}
// =======================================================================================
TEST(Encoding, ListUpgrade) {
......
......@@ -1073,7 +1073,7 @@ struct WireHelpers {
useDefault:
if (defaultValue == nullptr ||
reinterpret_cast<const WirePointer*>(defaultValue)->isNull()) {
return ListBuilder();
return ListBuilder(elementSize);
}
origRefTarget = copyMessage(
origSegment, origRef, reinterpret_cast<const WirePointer*>(defaultValue));
......@@ -1195,7 +1195,7 @@ struct WireHelpers {
useDefault:
if (defaultValue == nullptr ||
reinterpret_cast<const WirePointer*>(defaultValue)->isNull()) {
return ListBuilder();
return ListBuilder(ElementSize::VOID);
}
origRefTarget = copyMessage(
origSegment, origRef, reinterpret_cast<const WirePointer*>(defaultValue));
......@@ -1248,7 +1248,7 @@ struct WireHelpers {
useDefault:
if (defaultValue == nullptr ||
reinterpret_cast<const WirePointer*>(defaultValue)->isNull()) {
return ListBuilder();
return ListBuilder(ElementSize::INLINE_COMPOSITE);
}
origRefTarget = copyMessage(
origSegment, origRef, reinterpret_cast<const WirePointer*>(defaultValue));
......@@ -1905,7 +1905,7 @@ struct WireHelpers {
useDefault:
if (defaultValue == nullptr ||
reinterpret_cast<const WirePointer*>(defaultValue)->isNull()) {
return ListReader();
return ListReader(expectedElementSize);
}
segment = nullptr;
ref = reinterpret_cast<const WirePointer*>(defaultValue);
......
......@@ -562,16 +562,16 @@ private:
class ListBuilder: public kj::DisallowConstCopy {
public:
inline ListBuilder()
inline explicit ListBuilder(ElementSize elementSize)
: segment(nullptr), ptr(nullptr), elementCount(0 * ELEMENTS),
step(0 * BITS / ELEMENTS), elementSize(ElementSize::VOID) {}
step(0 * BITS / ELEMENTS), elementSize(elementSize) {}
MSVC_DEFAULT_ASSIGNMENT_WORKAROUND(, ListBuilder)
inline word* getLocation() {
// Get the object's location.
if (elementSize == ElementSize::INLINE_COMPOSITE) {
if (elementSize == ElementSize::INLINE_COMPOSITE && ptr != nullptr) {
return reinterpret_cast<word*>(ptr) - POINTER_SIZE_IN_WORDS;
} else {
return reinterpret_cast<word*>(ptr);
......@@ -640,9 +640,10 @@ private:
class ListReader {
public:
inline ListReader()
inline explicit ListReader(ElementSize elementSize)
: segment(nullptr), ptr(nullptr), elementCount(0), step(0 * BITS / ELEMENTS),
structDataSize(0), structPointerCount(0), nestingLimit(0x7fffffff) {}
structDataSize(0), structPointerCount(0), elementSize(elementSize),
nestingLimit(0x7fffffff) {}
MSVC_DEFAULT_ASSIGNMENT_WORKAROUND(const, ListReader)
......
......@@ -111,7 +111,7 @@ struct List<T, Kind::PRIMITIVE> {
public:
typedef List<T> Reads;
Reader() = default;
inline Reader(): reader(_::elementSizeForType<T>()) {}
inline explicit Reader(_::ListReader reader): reader(reader) {}
inline uint size() const { return reader.size() / ELEMENTS; }
......@@ -139,7 +139,7 @@ struct List<T, Kind::PRIMITIVE> {
public:
typedef List<T> Builds;
Builder() = delete;
inline Builder(): builder(_::elementSizeForType<T>()) {}
inline Builder(decltype(nullptr)) {}
inline explicit Builder(_::ListBuilder builder): builder(builder) {}
......@@ -207,7 +207,7 @@ struct List<T, Kind::STRUCT> {
public:
typedef List<T> Reads;
Reader() = default;
inline Reader(): reader(ElementSize::INLINE_COMPOSITE) {}
inline explicit Reader(_::ListReader reader): reader(reader) {}
inline uint size() const { return reader.size() / ELEMENTS; }
......@@ -235,7 +235,7 @@ struct List<T, Kind::STRUCT> {
public:
typedef List<T> Builds;
Builder() = delete;
inline Builder(): builder(ElementSize::INLINE_COMPOSITE) {}
inline Builder(decltype(nullptr)) {}
inline explicit Builder(_::ListBuilder builder): builder(builder) {}
......@@ -325,7 +325,7 @@ struct List<List<T>, Kind::LIST> {
public:
typedef List<List<T>> Reads;
Reader() = default;
inline Reader(): reader(ElementSize::POINTER) {}
inline explicit Reader(_::ListReader reader): reader(reader) {}
inline uint size() const { return reader.size() / ELEMENTS; }
......@@ -354,7 +354,7 @@ struct List<List<T>, Kind::LIST> {
public:
typedef List<List<T>> Builds;
Builder() = delete;
inline Builder(): builder(ElementSize::POINTER) {}
inline Builder(decltype(nullptr)) {}
inline explicit Builder(_::ListBuilder builder): builder(builder) {}
......@@ -434,7 +434,7 @@ struct List<T, Kind::BLOB> {
public:
typedef List<T> Reads;
Reader() = default;
inline Reader(): reader(ElementSize::POINTER) {}
inline explicit Reader(_::ListReader reader): reader(reader) {}
inline uint size() const { return reader.size() / ELEMENTS; }
......@@ -462,7 +462,7 @@ struct List<T, Kind::BLOB> {
public:
typedef List<T> Builds;
Builder() = delete;
inline Builder(): builder(ElementSize::POINTER) {}
inline Builder(decltype(nullptr)) {}
inline explicit Builder(_::ListBuilder builder): builder(builder) {}
......
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