Make string/vector field serialization order not depend on optimizer.

Multiple calls of e.g. CreateString inside a call to a CreateTable
could cause those strings to end up in different locations in the
wire format, since order or argument evaluation is undefined.

This is allowed by the FlatBuffer format, but it is not helpful,
especially when debugging the contents of binaries, or comparing
against a "golden" binary for tests etc.

Now making sure that all the CreateTableDirect calls first serialize
sub strings/vectors before calling CreateTable.

Also made similar changes to the serialization of "binary schemas".

Change-Id: I5747c4038b37a0d400aca2bc592bec751cf5c172
This commit is contained in:
Wouter van Oortmerssen
2018-11-16 16:22:18 -08:00
parent 5f32f94810
commit f575b02fda
7 changed files with 192 additions and 133 deletions

View File

@@ -2652,14 +2652,15 @@ void Parser::Serialize() {
service_offsets.push_back(offset);
(*it)->serialized_location = offset.o;
}
auto schema_offset = reflection::CreateSchema(
builder_,
builder_.CreateVectorOfSortedTables(&object_offsets),
builder_.CreateVectorOfSortedTables(&enum_offsets),
builder_.CreateString(file_identifier_),
builder_.CreateString(file_extension_),
(root_struct_def_ ? root_struct_def_->serialized_location : 0),
builder_.CreateVectorOfSortedTables(&service_offsets));
auto objs__ = builder_.CreateVectorOfSortedTables(&object_offsets);
auto enum__ = builder_.CreateVectorOfSortedTables(&enum_offsets);
auto fiid__ = builder_.CreateString(file_identifier_);
auto fext__ = builder_.CreateString(file_extension_);
auto serv__ = builder_.CreateVectorOfSortedTables(&service_offsets);
auto schema_offset =
reflection::CreateSchema(builder_, objs__, enum__, fiid__, fext__,
(root_struct_def_ ? root_struct_def_->serialized_location : 0),
serv__);
if (opts.size_prefixed) {
builder_.FinishSizePrefixed(schema_offset, reflection::SchemaIdentifier());
} else {
@@ -2675,49 +2676,49 @@ Offset<reflection::Object> StructDef::Serialize(FlatBufferBuilder *builder,
builder, static_cast<uint16_t>(it - fields.vec.begin()), parser));
}
auto qualified_name = defined_namespace->GetFullyQualifiedName(name);
return reflection::CreateObject(
*builder,
builder->CreateString(qualified_name),
builder->CreateVectorOfSortedTables(&field_offsets),
fixed,
static_cast<int>(minalign),
static_cast<int>(bytesize),
SerializeAttributes(builder, parser),
parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0);
auto name__ = builder->CreateString(qualified_name);
auto flds__ = builder->CreateVectorOfSortedTables(&field_offsets);
auto attr__ = SerializeAttributes(builder, parser);
auto docs__ = parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0;
return reflection::CreateObject(*builder, name__, flds__, fixed,
static_cast<int>(minalign),
static_cast<int>(bytesize),
attr__, docs__);
}
Offset<reflection::Field> FieldDef::Serialize(FlatBufferBuilder *builder,
uint16_t id,
const Parser &parser) const {
return reflection::CreateField(
*builder, builder->CreateString(name), value.type.Serialize(builder), id,
value.offset,
auto name__ = builder->CreateString(name);
auto type__ = value.type.Serialize(builder);
auto attr__ = SerializeAttributes(builder, parser);
auto docs__ = parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0;
return reflection::CreateField(*builder, name__, type__, id, value.offset,
// Is uint64>max(int64) tested?
IsInteger(value.type.base_type) ? StringToInt(value.constant.c_str()) : 0,
// result may be platform-dependent if underlying is float (not double)
IsFloat(value.type.base_type) ? strtod(value.constant.c_str(), nullptr)
: 0.0,
deprecated, required, key, SerializeAttributes(builder, parser),
parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0);
deprecated, required, key, attr__, docs__);
// TODO: value.constant is almost always "0", we could save quite a bit of
// space by sharing it. Same for common values of value.type.
}
Offset<reflection::RPCCall> RPCCall::Serialize(FlatBufferBuilder *builder,
const Parser &parser) const {
return reflection::CreateRPCCall(
*builder,
builder->CreateString(name),
request->serialized_location,
response->serialized_location,
SerializeAttributes(builder, parser),
parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0);
auto name__ = builder->CreateString(name);
auto attr__ = SerializeAttributes(builder, parser);
auto docs__ = parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0;
return reflection::CreateRPCCall(*builder, name__,
request->serialized_location,
response->serialized_location,
attr__, docs__);
}
Offset<reflection::Service> ServiceDef::Serialize(FlatBufferBuilder *builder,
@@ -2727,14 +2728,13 @@ Offset<reflection::Service> ServiceDef::Serialize(FlatBufferBuilder *builder,
servicecall_offsets.push_back((*it)->Serialize(builder, parser));
}
auto qualified_name = defined_namespace->GetFullyQualifiedName(name);
return reflection::CreateService(
*builder,
builder->CreateString(qualified_name),
builder->CreateVector(servicecall_offsets),
SerializeAttributes(builder, parser),
parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0);
auto name__ = builder->CreateString(qualified_name);
auto call__ = builder->CreateVector(servicecall_offsets);
auto attr__ = SerializeAttributes(builder, parser);
auto docs__ = parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0;
return reflection::CreateService(*builder, name__, call__, attr__, docs__);
}
Offset<reflection::Enum> EnumDef::Serialize(FlatBufferBuilder *builder,
@@ -2744,29 +2744,27 @@ Offset<reflection::Enum> EnumDef::Serialize(FlatBufferBuilder *builder,
enumval_offsets.push_back((*it)->Serialize(builder, parser));
}
auto qualified_name = defined_namespace->GetFullyQualifiedName(name);
return reflection::CreateEnum(
*builder,
builder->CreateString(qualified_name),
builder->CreateVector(enumval_offsets),
is_union,
underlying_type.Serialize(builder),
SerializeAttributes(builder, parser),
parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0);
auto name__ = builder->CreateString(qualified_name);
auto vals__ = builder->CreateVector(enumval_offsets);
auto type__ = underlying_type.Serialize(builder);
auto attr__ = SerializeAttributes(builder, parser);
auto docs__ = parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0;
return reflection::CreateEnum(*builder, name__, vals__, is_union, type__,
attr__, docs__);
}
Offset<reflection::EnumVal> EnumVal::Serialize(FlatBufferBuilder *builder,
const Parser &parser) const {
return reflection::CreateEnumVal(
*builder,
builder->CreateString(name),
value,
auto name__ = builder->CreateString(name);
auto type__ = union_type.Serialize(builder);
auto docs__ = parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0;
return reflection::CreateEnumVal(*builder, name__, value,
union_type.struct_def ? union_type.struct_def->serialized_location : 0,
union_type.Serialize(builder),
parser.opts.binary_schema_comments
? builder->CreateVectorOfStrings(doc_comment)
: 0);
type__, docs__);
}
Offset<reflection::Type> Type::Serialize(FlatBufferBuilder *builder) const {
@@ -2786,9 +2784,9 @@ Definition::SerializeAttributes(FlatBufferBuilder *builder,
auto it = parser.known_attributes_.find(kv->first);
FLATBUFFERS_ASSERT(it != parser.known_attributes_.end());
if (parser.opts.binary_schema_builtins || !it->second) {
attrs.push_back(reflection::CreateKeyValue(
*builder, builder->CreateString(kv->first),
builder->CreateString(kv->second->constant)));
auto key = builder->CreateString(kv->first);
auto val = builder->CreateString(kv->second->constant);
attrs.push_back(reflection::CreateKeyValue(*builder, key, val));
}
}
if (attrs.size()) {