PDL: Test fixed-size struct fields

Add an end_index for bounds checking fields, and use that instead
of subviews, to support extraction in struct Parse() functions.

Bug: 138260498
Test: bluetooth_packet_parser_test --gtest_filter=*FixedTypesStruct*
Change-Id: I1e6819aafdd31d53de046299e7d17fe7dd2de197

diff --git a/gd/packet/parser/fields/array_field.cc b/gd/packet/parser/fields/array_field.cc
index 041e483..2dca9c1 100644 (file)
--- a/gd/packet/parser/fields/array_field.cc
+++ b/gd/packet/parser/fields/array_field.cc
@@ -69,22 +69,21 @@ std::string ArrayField::GetDataType() const {
 void ArrayField::GenExtractor(std::ostream& s, Size start_offset, Size end_offset) const {
   GenBounds(s, start_offset, end_offset, GetSize());
 
-  s << " auto subview = GetLittleEndianSubview(field_begin, field_end); ";
-  s << "auto it = subview.begin();";
+  s << "auto it = begin_it + field_begin;";
 
   // Add the element size so that we will extract as many elements as we can.
-  s << GetDataType() << " ret;";
+  s << GetDataType() << " value;";
   if (element_size_ != -1) {
     std::string type = (type_def_ != nullptr) ? type_def_->name_ : util::GetTypeForSize(element_size_);
-    s << GetDataType() << "::iterator ret_it = ret.begin();";
-    s << "while (it + sizeof(" << type << ") <= subview.end()) {";
+    s << GetDataType() << "::iterator ret_it = value.begin();";
+    s << "while (it + sizeof(" << type << ") <= begin_it + field_end) {";
     s << "*ret_it = it.extract<" << type << ">();";
     s << "ret_it++;";
     s << "}";
   } else {
     s << "std::size_t ret_idx = 0;";
-    s << "while (it < subview.end()) {";
-    s << "it = " << type_def_->name_ << "::ParseArray(ret, &ret_idx, it);";
+    s << "while (it < begin_it + field_end) {";
+    s << "it = " << type_def_->name_ << "::ParseArray(value, &ret_idx, it);";
     s << "ret_idx++;";
     s << "}";
   }
@@ -94,10 +93,12 @@ void ArrayField::GenGetter(std::ostream& s, Size start_offset, Size end_offset)
   s << GetDataType();
   s << " Get" << util::UnderscoreToCamelCase(GetName()) << "() {";
   s << "ASSERT(was_validated_);";
+  s << "size_t end_index = size();";
+  s << "auto begin_it = begin();";
 
   GenExtractor(s, start_offset, end_offset);
 
-  s << "return ret;";
+  s << "return value;";
   s << "}\n";
 }
 

diff --git a/gd/packet/parser/fields/checksum_field.cc b/gd/packet/parser/fields/checksum_field.cc
index fcb5862..a5cea67 100644 (file)
--- a/gd/packet/parser/fields/checksum_field.cc
+++ b/gd/packet/parser/fields/checksum_field.cc
@@ -30,6 +30,8 @@ std::string ChecksumField::GetDataType() const {
   return type_name_;
 }
 
+void ChecksumField::GenExtractor(std::ostream&, Size, Size) const {}
+
 void ChecksumField::GenGetter(std::ostream&, Size, Size) const {}
 
 bool ChecksumField::GenBuilderParameter(std::ostream&) const {

diff --git a/gd/packet/parser/fields/checksum_field.h b/gd/packet/parser/fields/checksum_field.h
index fdaf92b..d4c0ad0 100644 (file)
--- a/gd/packet/parser/fields/checksum_field.h
+++ b/gd/packet/parser/fields/checksum_field.h
@@ -30,6 +30,8 @@ class ChecksumField : public ScalarField {
 
   virtual std::string GetDataType() const override;
 
+  virtual void GenExtractor(std::ostream& s, Size start_offset, Size end_offset) const override;
+
   virtual void GenGetter(std::ostream& s, Size start_offset, Size end_offset) const override;
 
   virtual bool GenBuilderParameter(std::ostream& s) const override;

diff --git a/gd/packet/parser/fields/custom_field.cc b/gd/packet/parser/fields/custom_field.cc
index 101164d..55ab97d 100644 (file)
--- a/gd/packet/parser/fields/custom_field.cc
+++ b/gd/packet/parser/fields/custom_field.cc
@@ -50,6 +50,7 @@ void CustomField::GenExtractor(std::ostream& s, Size start_offset, Size end_offs
 void CustomField::GenGetter(std::ostream& s, Size start_offset, Size end_offset) const {
   s << "std::vector<" << GetDataType() << "> Get" << util::UnderscoreToCamelCase(GetName()) << "() const {";
   s << "ASSERT(was_validated_);";
+  s << "size_t end_index = size();";
 
   GenExtractor(s, start_offset, end_offset);
   s << "return vec;";

diff --git a/gd/packet/parser/fields/packet_field.cc b/gd/packet/parser/fields/packet_field.cc
index 33838b2..6b2b2de 100644 (file)
--- a/gd/packet/parser/fields/packet_field.cc
+++ b/gd/packet/parser/fields/packet_field.cc
@@ -48,11 +48,11 @@ void PacketField::GenBounds(std::ostream& s, Size start_offset, Size end_offset,
   if (!start_offset.empty()) {
     s << "size_t field_begin = (" << start_offset << ") / 8;";
   } else {
-    s << "size_t field_begin = size() - (" << end_offset << " + " << field_size << ") / 8;";
+    s << "size_t field_begin = end_index - (" << end_offset << " + " << field_size << ") / 8;";
   }
 
   if (!end_offset.empty()) {
-    s << "size_t field_end = size() - (" << end_offset << ") / 8;";
+    s << "size_t field_end = end_index - (" << end_offset << ") / 8;";
     // If the field has a known size, use the minimum for the end
     if (!field_size.empty()) {
       s << "size_t field_sized_end = field_begin + (" << field_size << ") / 8;";

diff --git a/gd/packet/parser/fields/payload_field.cc b/gd/packet/parser/fields/payload_field.cc
index 1eea773..ad79032 100644 (file)
--- a/gd/packet/parser/fields/payload_field.cc
+++ b/gd/packet/parser/fields/payload_field.cc
@@ -61,12 +61,14 @@ void PayloadField::GenExtractor(std::ostream&, Size, Size) const {
 void PayloadField::GenGetter(std::ostream& s, Size start_offset, Size end_offset) const {
   s << "PacketView<kLittleEndian> GetPayload() const {";
   s << "ASSERT(was_validated_);";
+  s << "size_t end_index = size();";
   GenBounds(s, start_offset, end_offset, GetSize());
   s << "return GetLittleEndianSubview(field_begin, field_end);";
   s << "}\n\n";
 
   s << "PacketView<!kLittleEndian> GetPayloadBigEndian() const {";
   s << "ASSERT(was_validated_);";
+  s << "size_t end_index = size();";
   GenBounds(s, start_offset, end_offset, GetSize());
   s << "return GetBigEndianSubview(field_begin, field_end);";
   s << "}\n";

diff --git a/gd/packet/parser/fields/struct_field.cc b/gd/packet/parser/fields/struct_field.cc
index 8139f4d..d0cb840 100644 (file)
--- a/gd/packet/parser/fields/struct_field.cc
+++ b/gd/packet/parser/fields/struct_field.cc
@@ -62,6 +62,7 @@ void StructField::GenGetter(std::ostream& s, Size start_offset, Size end_offset)
     s << "std::vector<" << GetDataType() << "> Get" << util::UnderscoreToCamelCase(GetName()) << "() const {";
   }
   s << "ASSERT(was_validated_);";
+  s << "size_t end_index = size();";
   GenExtractor(s, start_offset, end_offset);
 
   if (size_ != -1) {

diff --git a/gd/packet/parser/fields/vector_field.cc b/gd/packet/parser/fields/vector_field.cc
index f201825..b43e36a 100644 (file)
--- a/gd/packet/parser/fields/vector_field.cc
+++ b/gd/packet/parser/fields/vector_field.cc
@@ -115,6 +115,7 @@ void VectorField::GenGetter(std::ostream& s, Size start_offset, Size end_offset)
   s << GetDataType();
   s << " Get" << util::UnderscoreToCamelCase(GetName()) << "() {";
   s << "ASSERT(was_validated_);";
+  s << "size_t end_index = size();";
 
   GenExtractor(s, start_offset, end_offset);
 

diff --git a/gd/packet/parser/struct_def.cc b/gd/packet/parser/struct_def.cc
index 7befbf7..1496f51 100644 (file)
--- a/gd/packet/parser/struct_def.cc
+++ b/gd/packet/parser/struct_def.cc
@@ -40,11 +40,14 @@ TypeDef::Type StructDef::GetDefinitionType() const {
 
 void StructDef::GenParse(std::ostream& s) const {
   if (is_little_endian_) {
-    s << "static Iterator<kLittleEndian> Parse(std::vector<" << name_ << ">& vec, Iterator<kLittleEndian> it) {";
+    s << "static Iterator<kLittleEndian> Parse(std::vector<" << name_ << ">& vec, Iterator<kLittleEndian> struct_it) {";
   } else {
-    s << "static Iterator<!kLittleEndian> Parse(std::vector<" << name_ << ">& vec, Iterator<!kLittleEndian> it) {";
+    s << "static Iterator<!kLittleEndian> Parse(std::vector<" << name_
+      << ">& vec, Iterator<!kLittleEndian> struct_it) {";
   }
-  s << "auto begin_it = it;";
+  s << "auto begin_it = struct_it;";
+  s << "size_t end_index = struct_it.NumBytesRemaining();";
+  s << "if (end_index < " << GetSize().bytes() << ") { return struct_it + struct_it.NumBytesRemaining();}";
   s << name_ << " one;";
   if (parent_ != nullptr) {
     s << "begin_it += one." << parent_->name_ << "::BitsOfHeader() / 8;";
@@ -54,6 +57,7 @@ void StructDef::GenParse(std::ostream& s) const {
     Size next_field_offset = field->GetSize() + field_offset.bits();
     if (field->GetFieldType() != ReservedField::kFieldType && field->GetFieldType() != BodyField::kFieldType &&
         field->GetFieldType() != FixedScalarField::kFieldType && field->GetFieldType() != SizeField::kFieldType &&
+        field->GetFieldType() != ChecksumStartField::kFieldType && field->GetFieldType() != ChecksumField::kFieldType &&
         field->GetFieldType() != CountField::kFieldType) {
       s << "{";
       field->GenExtractor(s, field_offset, next_field_offset);
@@ -63,7 +67,7 @@ void StructDef::GenParse(std::ostream& s) const {
     field_offset = next_field_offset;
   }
   s << "vec.push_back(one);";
-  s << "return it + " << field_offset.bytes() << ";";
+  s << "return struct_it + " << field_offset.bytes() << ";";
   s << "}";
 }
 

diff --git a/gd/packet/parser/test/generated_packet_test.cc b/gd/packet/parser/test/generated_packet_test.cc
index c424ecd..b510522 100644 (file)
--- a/gd/packet/parser/test/generated_packet_test.cc
+++ b/gd/packet/parser/test/generated_packet_test.cc
@@ -823,6 +823,44 @@ TEST(GeneratedPacketTest, testArrayOfStruct) {
   }
 }
 
+vector<uint8_t> one_fixed_types_struct{
+    0x05,                                // four_bits = FIVE, reserved
+    0xf3,                                // _fixed_
+    0x0d,                                // id = 0x0d
+    0x01, 0x02, 0x03,                    // array = { 1, 2, 3}
+    0x06, 0x01,                          // example_checksum
+    0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6,  // six_bytes
+};
+
+TEST(GeneratedPacketTest, testOneFixedTypesStruct) {
+  StructWithFixedTypes swf;
+  swf.four_bits_ = FourBits::FIVE;
+  swf.id_ = 0x0d;
+  swf.array_ = {{0x01, 0x02, 0x03}};
+  swf.six_bytes_ = SixBytes{{0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6}};
+
+  auto packet = OneFixedTypesStructBuilder::Create(swf);
+  ASSERT_EQ(one_fixed_types_struct.size(), packet->size());
+
+  std::shared_ptr<std::vector<uint8_t>> packet_bytes = std::make_shared<std::vector<uint8_t>>();
+  BitInserter it(*packet_bytes);
+  packet->Serialize(it);
+
+  ASSERT_EQ(one_fixed_types_struct.size(), packet_bytes->size());
+  for (size_t i = 0; i < one_fixed_types_struct.size(); i++) {
+    ASSERT_EQ(one_fixed_types_struct[i], packet_bytes->at(i));
+  }
+
+  PacketView<kLittleEndian> packet_bytes_view(packet_bytes);
+  auto view = OneFixedTypesStructView::Create(packet_bytes_view);
+  ASSERT_TRUE(view.IsValid());
+  auto one = view.GetOne();
+  ASSERT_EQ(one.four_bits_, swf.four_bits_);
+  ASSERT_EQ(one.id_, swf.id_);
+  ASSERT_EQ(one.array_, swf.array_);
+  ASSERT_EQ(one.six_bytes_, swf.six_bytes_);
+}
+
 vector<uint8_t> array_of_struct_and_another{
     0x03,              // _count_
     0x01, 0x01, 0x02,  // id, id * 0x0201

diff --git a/gd/packet/parser/test/test_packets.pdl b/gd/packet/parser/test/test_packets.pdl
index 899b9b7..4ed0f0c 100644 (file)
--- a/gd/packet/parser/test/test_packets.pdl
+++ b/gd/packet/parser/test/test_packets.pdl
@@ -185,6 +185,21 @@ packet ArrayOfStruct {
   array : TwoRelatedNumbers[],
 }
 
+struct StructWithFixedTypes {
+  four_bits : FourBits,
+  _reserved_ : 4,
+  _checksum_start_(example_checksum),
+  _fixed_ = 0xf3 : 8,
+  id : 8,
+  array : 8[3],
+  example_checksum : SimpleSum,
+  six_bytes : SixBytes,
+}
+
+packet OneFixedTypesStruct {
+  one : StructWithFixedTypes,
+}
+
 packet ArrayOfStructAndAnother {
   _count_(array) : 8,
   array : TwoRelatedNumbers[],