From 424a473e1fcfb35f59c18e0facdf09788af69b86 Mon Sep 17 00:00:00 2001 From: Vladimir Glavnyy <31897320+vglavnyy@users.noreply.github.com> Date: Fri, 15 May 2020 01:39:58 +0700 Subject: [PATCH] Schema parser: prohibit declaration of an array of pointers inside structs (#5907) * Fix issue #5906, Prohibit declaration of an array of pointers inside structs - idl_parser.cpp: Prohibit declaration of an array of pointers inside structs - idl_gen_cpp.cpp: Extract GenStructConstructor() method from GenStruct() to simplify future modification - idl_gen_cpp.cpp: Add assert for checking of Array fields in structs on code-generation stage * Fix the error 'unused local variable' in release build * Fix: format the PR code according to coding rules * Add test-case and fix review notes --- src/idl_gen_cpp.cpp | 131 ++++++++++++++++++++++++-------------------- src/idl_parser.cpp | 12 +++- tests/test.cpp | 3 + 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index 7521bf890..3ce96980e 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -2854,6 +2854,76 @@ class CppGenerator : public BaseGenerator { *code_ptr += " (void)padding" + NumToString((*id)++) + "__;"; } + void GenStructConstructor(const StructDef& struct_def) { + std::string arg_list; + std::string init_list; + int padding_id = 0; + auto first = struct_def.fields.vec.begin(); + for (auto it = struct_def.fields.vec.begin(); + it != struct_def.fields.vec.end(); ++it) { + const auto &field = **it; + const auto &field_type = field.value.type; + if (IsArray(field_type)) { + first++; + continue; + } + const auto member_name = Name(field) + "_"; + const auto arg_name = "_" + Name(field); + const auto arg_type = + GenTypeGet(field_type, " ", "const ", " &", true); + + if (it != first) { arg_list += ", "; } + arg_list += arg_type; + arg_list += arg_name; + if (!IsArray(field_type)) { + if (it != first && init_list != "") { init_list += ",\n "; } + init_list += member_name; + if (IsScalar(field_type.base_type)) { + auto type = GenUnderlyingCast(field, false, arg_name); + init_list += "(flatbuffers::EndianScalar(" + type + "))"; + } else { + init_list += "(" + arg_name + ")"; + } + } + if (field.padding) { + GenPadding(field, &init_list, &padding_id, PaddingInitializer); + } + } + + if (!arg_list.empty()) { + code_.SetValue("ARG_LIST", arg_list); + code_.SetValue("INIT_LIST", init_list); + if (!init_list.empty()) { + code_ += " {{STRUCT_NAME}}({{ARG_LIST}})"; + code_ += " : {{INIT_LIST}} {"; + } else { + code_ += " {{STRUCT_NAME}}({{ARG_LIST}}) {"; + } + padding_id = 0; + for (auto it = struct_def.fields.vec.begin(); + it != struct_def.fields.vec.end(); ++it) { + const auto &field = **it; + const auto &field_type = field.value.type; + if (IsArray(field_type)) { + const auto &elem_type = field_type.VectorType(); + (void)elem_type; + FLATBUFFERS_ASSERT( + (IsScalar(elem_type.base_type) || IsStruct(elem_type)) && + "invalid declaration"); + const auto &member = Name(field) + "_"; + code_ += + " std::memset(" + member + ", 0, sizeof(" + member + "));"; + } + if (field.padding) { + std::string padding; + GenPadding(field, &padding, &padding_id, PaddingNoop); + code_ += padding; + } + } + code_ += " }"; + } + } + // Generate an accessor struct with constructor for a flatbuffers struct. void GenStruct(const StructDef &struct_def) { // Generate an accessor struct, with private variables of the form: @@ -2912,66 +2982,7 @@ class CppGenerator : public BaseGenerator { // Generate a constructor that takes all fields as arguments, // excluding arrays - std::string arg_list; - std::string init_list; - padding_id = 0; - auto first = struct_def.fields.vec.begin(); - for (auto it = struct_def.fields.vec.begin(); - it != struct_def.fields.vec.end(); ++it) { - const auto &field = **it; - if (IsArray(field.value.type)) { - first++; - continue; - } - const auto member_name = Name(field) + "_"; - const auto arg_name = "_" + Name(field); - const auto arg_type = - GenTypeGet(field.value.type, " ", "const ", " &", true); - - if (it != first) { arg_list += ", "; } - arg_list += arg_type; - arg_list += arg_name; - if (!IsArray(field.value.type)) { - if (it != first && init_list != "") { init_list += ",\n "; } - init_list += member_name; - if (IsScalar(field.value.type.base_type)) { - auto type = GenUnderlyingCast(field, false, arg_name); - init_list += "(flatbuffers::EndianScalar(" + type + "))"; - } else { - init_list += "(" + arg_name + ")"; - } - } - if (field.padding) { - GenPadding(field, &init_list, &padding_id, PaddingInitializer); - } - } - - if (!arg_list.empty()) { - code_.SetValue("ARG_LIST", arg_list); - code_.SetValue("INIT_LIST", init_list); - if (!init_list.empty()) { - code_ += " {{STRUCT_NAME}}({{ARG_LIST}})"; - code_ += " : {{INIT_LIST}} {"; - } else { - code_ += " {{STRUCT_NAME}}({{ARG_LIST}}) {"; - } - padding_id = 0; - for (auto it = struct_def.fields.vec.begin(); - it != struct_def.fields.vec.end(); ++it) { - const auto &field = **it; - if (IsArray(field.value.type)) { - const auto &member = Name(field) + "_"; - code_ += - " std::memset(" + member + ", 0, sizeof(" + member + "));"; - } - if (field.padding) { - std::string padding; - GenPadding(field, &padding, &padding_id, PaddingNoop); - code_ += padding; - } - } - code_ += " }"; - } + GenStructConstructor(struct_def); // Generate accessor methods of the form: // type name() const { return flatbuffers::EndianScalar(name_); } diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 10ff4a68b..446532e78 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -677,9 +677,15 @@ CheckedError Parser::ParseField(StructDef &struct_def) { Type type; ECHECK(ParseType(type)); - if (struct_def.fixed && !IsScalar(type.base_type) && !IsStruct(type) && - !IsArray(type)) - return Error("structs_ may contain only scalar or struct fields"); + if (struct_def.fixed) { + auto valid = IsScalar(type.base_type) || IsStruct(type); + if (!valid && IsArray(type)) { + const auto& elem_type = type.VectorType(); + valid |= IsScalar(elem_type.base_type) || IsStruct(elem_type); + } + if (!valid) + return Error("structs may contain only scalar or struct fields"); + } if (!struct_def.fixed && IsArray(type)) return Error("fixed-length array in table must be wrapped in struct"); diff --git a/tests/test.cpp b/tests/test.cpp index 87a11691d..1b00b4d15 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -1623,6 +1623,9 @@ void ErrorTest() { TestError("table X { Y:int; } root_type X; { Y:1.0 }", "float"); TestError("table X { Y:bool; } root_type X; { Y:1.0 }", "float"); TestError("enum X:bool { Y = true }", "must be integral"); + // Array of non-scalar + TestError("table X { x:int; } struct Y { y:[X:2]; }", + "may contain only scalar or struct fields"); } template