Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9856c2c1 authored by Myles Watson's avatar Myles Watson
Browse files

Only pad the previous field 

Bug: 182216573
Test: atest bluetooth_test_gd
      libbt_packets_test
      bluetooth_packet_parser_test
Tag: #gd-refactor
Change-Id: I8e43aee420be58af81cd9f47fcf53a566af1b3fa
parent 210d9ebf

system/gd/hci/hci_packets.pdl

+13 −7
Original line number Diff line number Diff line
@@ -2210,7 +2210,11 @@ packet ReadExtendedInquiryResponseComplete : CommandComplete (command_op_code = 
packet WriteExtendedInquiryResponse : Command (op_code = WRITE_EXTENDED_INQUIRY_RESPONSE) {

  fec_required : FecRequired,

  extended_inquiry_response : GapData[],

  _padding_[244], // Zero padding to be 240 octets (GapData[]) + 2 (opcode) + 1 (size) + 1 (FecRequired)

  _padding_[240], // Zero padding GapData[] to be 240 octets

}



test WriteExtendedInquiryResponse {

  "\x52\x0c\xf1\x01\x0b\x09\x50\x69\x78\x65\x6c\x20\x33\x20\x58\x4c\x19\x03\x05\x11\x0a\x11\x0c\x11\x0e\x11\x12\x11\x15\x11\x16\x11\x1f\x11\x2d\x11\x2f\x11\x00\x12\x32\x11\x01\x05\x81\x07\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",

}



packet WriteExtendedInquiryResponseComplete : CommandComplete (command_op_code = WRITE_EXTENDED_INQUIRY_RESPONSE) {
@@ -2779,7 +2783,7 @@ packet LeReadAdvertisingPhysicalChannelTxPowerComplete : CommandComplete (comman 
packet LeSetAdvertisingData : LeAdvertisingCommand (op_code = LE_SET_ADVERTISING_DATA) {

  _size_(advertising_data) : 8,

  advertising_data : GapData[],

  _padding_[35], // Zero padding to 31 bytes of advertising_data + 1 size + 2 opcode + 1 total_size

  _padding_[31], // Zero padding to 31 bytes of advertising_data

}



packet LeSetAdvertisingDataComplete : CommandComplete (command_op_code = LE_SET_ADVERTISING_DATA) {
@@ -2789,7 +2793,7 @@ packet LeSetAdvertisingDataComplete : CommandComplete (command_op_code = LE_SET_ 
packet LeSetScanResponseData : LeAdvertisingCommand (op_code = LE_SET_SCAN_RESPONSE_DATA) {

  _size_(advertising_data) : 8,

  advertising_data : GapData[],

  _padding_[35], // Zero padding to 31 bytes of advertising_data + 1 size + 2 opcode + 1 total_size

  _padding_[31], // Zero padding to 31 bytes of advertising_data

}



packet LeSetScanResponseDataComplete : CommandComplete (command_op_code = LE_SET_SCAN_RESPONSE_DATA) {
@@ -4151,6 +4155,7 @@ packet LeGetVendorCapabilitiesComplete : CommandComplete (command_op_code = LE_G 
  base_vendor_capabilities : BaseVendorCapabilities,

  _payload_,

}



packet LeGetVendorCapabilitiesComplete095 : LeGetVendorCapabilitiesComplete {

  version_supported: 16,

  total_num_of_advt_tracked: 16,
@@ -4158,6 +4163,7 @@ packet LeGetVendorCapabilitiesComplete095 : LeGetVendorCapabilitiesComplete { 
  debug_logging_supported: 8,

  _payload_,

}



packet LeGetVendorCapabilitiesComplete096 : LeGetVendorCapabilitiesComplete095 {

  le_address_generation_offloading_support: 8,

  _payload_,
@@ -4206,7 +4212,7 @@ packet LeMultiAdvtParamComplete : LeMultiAdvtComplete (sub_cmd = SET_PARAM) { 
packet LeMultiAdvtSetData : LeMultiAdvt (sub_cmd = SET_DATA) {

  _size_(advertising_data) : 8,

  advertising_data : GapData[],

  _padding_[36], // Zero padding to 31 bytes of advertising_data + 1 size + 2 opcode + 1 total_size + 1 set

  _padding_[31], // Zero padding to 31 bytes of advertising_data

  advertising_instance : 8,

}
@@ -4216,7 +4222,7 @@ packet LeMultiAdvtSetDataComplete : LeMultiAdvtComplete (sub_cmd = SET_DATA) { 
packet LeMultiAdvtSetScanResp : LeMultiAdvt (sub_cmd = SET_SCAN_RESP) {

  _size_(advertising_data) : 8,

  advertising_data : GapData[],

  _padding_[36], // Zero padding to 31 bytes of advertising_data + 1 size + 2 opcode + 1 total_size + 1 set

  _padding_[31], // Zero padding to 31 bytes of advertising_data

  advertising_instance : 8,

}
@@ -4887,9 +4893,9 @@ packet ExtendedInquiryResult : Event (event_code = EXTENDED_INQUIRY_RESULT) { 
  rssi : 8,

  extended_inquiry_response : GapData[],

  // Extended inquiry Result is always 255 bytes long

  // padded with zeroes as necessary

  // padded GapData with zeroes as necessary

  // Refer to BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C Section 8 on page 1340

  _padding_[255],

  _padding_[240],

}



packet EncryptionKeyRefreshComplete : Event (event_code = ENCRYPTION_KEY_REFRESH_COMPLETE) {

system/gd/hci/hci_packets_test.cc

+9 −8
Original line number Diff line number Diff line
@@ -632,18 +632,19 @@ TEST(HciPacketsTest, testLeMultiAdvSetAdvertisingDataBuilderLength) { 
  gap_data.data_ = std::vector<uint8_t>({'A', ' ', 'g', 'o', 'o', 'd', ' ', 'n', 'a', 'm', 'e'});

  uint8_t set = 3;

  auto builder = LeMultiAdvtSetDataBuilder::Create({gap_data}, set);

  ASSERT_EQ(2 /*opcode*/ + 1 /* parameter size */ + 1 /* data_length */ + 31 /* data */ + 1 /* set */, builder->size());



  auto packet_bytes = std::make_shared<std::vector<uint8_t>>();

  packet_bytes->reserve(builder->size());

  BitInserter bit_inserter(*packet_bytes);

  builder->Serialize(bit_inserter);

  auto command_view = LeMultiAdvtView::Create(

      LeAdvertisingCommandView::Create(CommandView::Create(PacketView<kLittleEndian>(packet_bytes))));

  auto command_view = LeMultiAdvtSetDataView::Create(LeMultiAdvtView::Create(

      LeAdvertisingCommandView::Create(CommandView::Create(PacketView<kLittleEndian>(packet_bytes)))));

  ASSERT_TRUE(command_view.IsValid());

  EXPECT_EQ(1 /* data_length */ + 31 /* data */ + 1 /* set */, command_view.GetPayload().size());

  auto view = LeMultiAdvtSetDataView::Create(command_view);

  ASSERT_TRUE(view.IsValid());

  ASSERT_TRUE(view.GetAdvertisingData().size() > 0);

  ASSERT_EQ(view.GetAdvertisingData()[0].data_, gap_data.data_);

  ASSERT_EQ(view.GetAdvertisingInstance(), 3);

}



TEST(HciPacketsTest, testLeMultiAdvSetScanResponseDataBuilderLength) {
@@ -652,18 +653,18 @@ TEST(HciPacketsTest, testLeMultiAdvSetScanResponseDataBuilderLength) { 
  gap_data.data_ = std::vector<uint8_t>({'A', ' ', 'g', 'o', 'o', 'd', ' ', 'n', 'a', 'm', 'e'});

  uint8_t set = 3;

  auto builder = LeMultiAdvtSetScanRespBuilder::Create({gap_data}, set);

  EXPECT_EQ(2 /*opcode*/ + 1 /* parameter size */ + 1 /*data_length */ + 31 /* data */ + 1 /* set */, builder->size());



  auto packet_bytes = std::make_shared<std::vector<uint8_t>>();

  packet_bytes->reserve(builder->size());

  BitInserter bit_inserter(*packet_bytes);

  builder->Serialize(bit_inserter);

  auto command_view = LeMultiAdvtView::Create(

      LeAdvertisingCommandView::Create(CommandView::Create(PacketView<kLittleEndian>(packet_bytes))));

  auto command_view = LeMultiAdvtSetScanRespView::Create(LeMultiAdvtView::Create(

      LeAdvertisingCommandView::Create(CommandView::Create(PacketView<kLittleEndian>(packet_bytes)))));

  ASSERT_TRUE(command_view.IsValid());

  ASSERT_EQ(1 /* data_length */ + 31 /* data */ + 1 /* set */, command_view.GetPayload().size());

  auto view = LeMultiAdvtSetScanRespView::Create(command_view);

  ASSERT_TRUE(view.IsValid());

  ASSERT_EQ(view.GetAdvertisingData()[0].data_, gap_data.data_);

  ASSERT_EQ(view.GetAdvertisingInstance(), 3);

}



std::vector<uint8_t> controller_bqr = {0x5e, 0xfd, 0x07, 0x00, 0x1f, 0x00, 0x07, 0x00, 0x88, 0x13};

system/gd/packet/parser/parent_def.cc

+80 −33
Original line number Diff line number Diff line
@@ -172,6 +172,17 @@ Size ParentDef::GetOffsetForField(std::string field_name, bool from_end) const { 
    ERROR() << "Can't find a field offset for nonexistent field named: " << field_name << " in " << name_;

  }



  PacketField* padded_field = nullptr;

  {

    PacketField* last_field = nullptr;

    for (const auto field : fields_) {

      if (field->GetFieldType() == PaddingField::kFieldType) {

        padded_field = last_field;

      }

      last_field = field;

    }

  }



  // We have to use a generic lambda to conditionally change iteration direction

  // due to iterator and reverse_iterator being different types.

  auto size_lambda = [&](auto from, auto to) -> Size {
@@ -181,13 +192,15 @@ Size ParentDef::GetOffsetForField(std::string field_name, bool from_end) const { 
      if ((*it)->GetName() == field_name) break;

      const auto& field = *it;

      // If there is a field with an unknown size before the field, return an empty Size.

      if (field->GetSize().empty()) {

      if (field->GetSize().empty() && padded_field != field) {

        return Size();

      }

      if (field->GetFieldType() != PaddingField::kFieldType || !from_end) {

      if (field != padded_field) {

        if (!from_end || field->GetFieldType() != PaddingField::kFieldType) {

          size += field->GetSize();

        }

      }

    }

    return size;

  };
@@ -260,6 +273,20 @@ void ParentDef::GenSize(std::ostream& s) const { 
  auto header_fields = fields_.GetFieldsBeforePayloadOrBody();

  auto footer_fields = fields_.GetFieldsAfterPayloadOrBody();



  Size padded_size;

  const PacketField* padded_field = nullptr;

  const PacketField* last_field = nullptr;

  for (const auto& field : fields_) {

    if (field->GetFieldType() == PaddingField::kFieldType) {

      if (!padded_size.empty()) {

        ERROR() << "Only one padding field is allowed.  Second field: " << field->GetName();

      }

      padded_field = last_field;

      padded_size = field->GetSize();

    }

    last_field = field;

  }



  s << "protected:";

  s << "size_t BitsOfHeader() const {";

  s << "return 0";
@@ -273,8 +300,12 @@ void ParentDef::GenSize(std::ostream& s) const { 
  }



  for (const auto& field : header_fields) {

    if (field == padded_field) {

      s << " + " << padded_size;

    } else {

      s << " + " << field->GetBuilderSize();

    }

  }

  s << ";";



  s << "}\n\n";
@@ -282,8 +313,12 @@ void ParentDef::GenSize(std::ostream& s) const { 
  s << "size_t BitsOfFooter() const {";

  s << "return 0";

  for (const auto& field : footer_fields) {

    if (field == padded_field) {

      s << " + " << padded_size;

    } else {

      s << " + " << field->GetBuilderSize();

    }

  }



  if (parent_ != nullptr) {

    if (parent_->GetDefinitionType() == Type::PACKET) {
@@ -302,22 +337,8 @@ void ParentDef::GenSize(std::ostream& s) const { 
    s << ";}\n\n";

  }



  Size padded_size;

  for (const auto& field : header_fields) {

    if (field->GetFieldType() == PaddingField::kFieldType) {

      if (!padded_size.empty()) {

        ERROR() << "Only one padding field is allowed.  Second field: " << field->GetName();

      }

      padded_size = field->GetSize();

    }

  }



  s << "public:";

  s << "virtual size_t size() const override {";

  if (!padded_size.empty()) {

    s << "return " << padded_size.bytes() << ";}";

    s << "size_t unpadded_size() const {";

  }

  s << "return (BitsOfHeader() / 8)";

  if (fields_.HasPayload()) {

    s << "+ payload_->size()";
@@ -355,6 +376,17 @@ void ParentDef::GenSerialize(std::ostream& s) const { 
    }

  }



  const PacketField* padded_field = nullptr;

  {

    PacketField* last_field = nullptr;

    for (const auto field : header_fields) {

      if (field->GetFieldType() == PaddingField::kFieldType) {

        padded_field = last_field;

      }

      last_field = field;

    }

  }



  for (const auto& field : header_fields) {

    if (field->GetFieldType() == SizeField::kFieldType) {

      const auto& field_name = ((SizeField*)field)->GetSizedFieldName();
@@ -415,14 +447,17 @@ void ParentDef::GenSerialize(std::ostream& s) const { 
      s << "[shared_checksum_ptr](uint8_t byte){ shared_checksum_ptr->AddByte(byte);},";

      s << "[shared_checksum_ptr](){ return static_cast<uint64_t>(shared_checksum_ptr->GetChecksum());}));";

    } else if (field->GetFieldType() == PaddingField::kFieldType) {

      s << "ASSERT(unpadded_size() <= " << field->GetSize().bytes() << ");";

      s << "ASSERT(unpadded_size <= " << field->GetSize().bytes() << ");";

      s << "size_t padding_bytes = ";

      s << field->GetSize().bytes() << " - unpadded_size();";

      s << field->GetSize().bytes() << " - unpadded_size;";

      s << "for (size_t padding = 0; padding < padding_bytes; padding++) {i.insert_byte(0);}";

    } else if (field->GetFieldType() == CountField::kFieldType) {

      const auto& vector_name = ((SizeField*)field)->GetSizedFieldName() + "_";

      s << "insert(" << vector_name << ".size(), i, " << field->GetSize().bits() << ");";

    } else {

      if (field == padded_field) {

        s << "size_t unpadded_size = (" << field->GetBuilderSize() << ") / 8;";

      }

      field->GenInserter(s);

    }

  }
@@ -653,11 +688,25 @@ void ParentDef::GenSizeRetVal(std::ostream& s) const { 
  auto fields = fields_.GetFieldsWithoutTypes({

      BodyField::kFieldType,

  });

  const PacketField* padded_field = nullptr;

  auto padding_fields = fields_.GetFieldsWithTypes({

      PaddingField::kFieldType,

  });

  if (padding_fields.size()) {

    PacketField* last_field = nullptr;

    for (const auto field : fields) {

      if (field->GetFieldType() == PaddingField::kFieldType) {

        padded_field = last_field;

      }

      last_field = field;

    }

  }



  s << "let ret = 0;";

  for (const auto field : fields) {

    bool is_padding = field->GetFieldType() == PaddingField::kFieldType;

    bool is_vector = field->GetFieldType() == VectorField::kFieldType;

    if (is_padding || is_vector) {

    if (field != padded_field) {  // Skip the size of padded fields

      if (is_vector) {

        if (size > 0) {

          if (size % 8 != 0) {

            ERROR() << "size is not a multiple of 8!\n";
@@ -666,7 +715,6 @@ void ParentDef::GenSizeRetVal(std::ostream& s) const { 
          size = 0;

        }



      if (is_vector) {

        const VectorField* vector = (VectorField*)field;

        if (vector->element_size_.empty() || vector->element_size_.has_dynamic()) {

          s << "let ret = ret + self." << vector->GetName() << ".iter().fold(0, |acc, x| acc + x.get_total_size());";
@@ -674,11 +722,10 @@ void ParentDef::GenSizeRetVal(std::ostream& s) const { 
          s << "let ret = ret + (self." << vector->GetName() << ".len() * ((" << vector->element_size_ << ") / 8));";

        }

      } else {

        auto padded = field->GetSize().bytes();

        s << "let ret = if ret < " << padded << " { " << padded << " } else { ret };";

        size += field->GetSize().bits();

      }

    } else {

      size += field->GetSize().bits();

      s << "/* Skipping " << field->GetName() << " since it is padded */";

    }

  }

  if (size > 0) {

system/gd/packet/parser/test/generated_packet_test.cc

+25 −23
Original line number Diff line number Diff line
@@ -1764,29 +1764,29 @@ TEST(GeneratedPacketTest, testOneLengthTypeValueStruct) { 
  }

}



vector<uint8_t> one_length_type_value_struct_padded_20{

    0x27,  // _size_(payload),

    // _size_(value):16 type value

vector<uint8_t> one_length_type_value_struct_padded_10{

    0x20,                                                        // _size_(payload),

    0x14,                                                        // valid bytes

    0x04, 0x00, 0x01, 'o',  'n',  'e',                           // ONE

    0x04, 0x00, 0x02, 't',  'w',  'o',                           // TWO

    0x06, 0x00, 0x03, 't',  'h',  'r',  'e',  'e',               // THREE

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,        // padding to 30

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  // padding to 40

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  // padding to 30

    0x20,                                                        // after padding

};



vector<uint8_t> one_length_type_value_struct_padded_28{

    0x27,  // _size_(payload),

    // _size_(value):16 type value

vector<uint8_t> one_length_type_value_struct_padded_18{

    0x20,                                                        // _size_(payload),

    0x0C,                                                        // valid bytes

    0x04, 0x00, 0x01, 'o',  'n',  'e',                           // ONE

    0x04, 0x00, 0x02, 't',  'w',  'o',                           // TWO

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,                    // padding to 20

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,              // padding to 20

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  // padding to 30

    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,  // padding to 40

    0x20,                                                        // after padding

};



// TODO: Revisit LTV parsing.  Right now, the padding bytes are parsed

// DEFINE_AND_INSTANTIATE_OneLengthTypeValueStructPaddedReflectionTest(one_length_type_value_struct_padded_20,

// one_length_type_value_struct_padded_28);

// DEFINE_AND_INSTANTIATE_OneLengthTypeValueStructPaddedReflectionTest(one_length_type_value_struct_padded_10,

//                                                                     one_length_type_value_struct_padded_18);



TEST(GeneratedPacketTest, testOneLengthTypeValueStructPaddedGeneration) {

  std::vector<LengthTypeValueStruct> ltv_vector;
@@ -1805,17 +1805,18 @@ TEST(GeneratedPacketTest, testOneLengthTypeValueStructPaddedGeneration) { 
      'o',

  };

  ltv_vector.push_back(ltv);

  uint8_t after_padding = 0x20;



  auto packet = OneLengthTypeValueStructPaddedBuilder::Create(ltv_vector);

  ASSERT_EQ(one_length_type_value_struct_padded_28.size(), packet->size());

  auto packet = OneLengthTypeValueStructPaddedBuilder::Create(12, ltv_vector, after_padding);

  ASSERT_EQ(one_length_type_value_struct_padded_18.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_length_type_value_struct_padded_28.size(), packet_bytes->size());

  for (size_t i = 0; i < one_length_type_value_struct_padded_28.size(); i++) {

    ASSERT_EQ(one_length_type_value_struct_padded_28[i], packet_bytes->at(i));

  ASSERT_EQ(one_length_type_value_struct_padded_18.size(), packet_bytes->size());

  for (size_t i = 0; i < one_length_type_value_struct_padded_18.size(); i++) {

    ASSERT_EQ(one_length_type_value_struct_padded_18[i], packet_bytes->at(i));

  }



  PacketView<kLittleEndian> packet_bytes_view(packet_bytes);
@@ -1823,11 +1824,12 @@ TEST(GeneratedPacketTest, testOneLengthTypeValueStructPaddedGeneration) { 
  ASSERT_TRUE(view.IsValid());

  auto an_array = view.GetOneArray();

  // TODO: Revisit LTV parsing.  Right now, the padding bytes are parsed

  // ASSERT_EQ(ltv_vector.size(), an_array.size());

  ASSERT_LE(ltv_vector.size(), an_array.size());

  for (size_t i = 0; i < ltv_vector.size(); i++) {

    ASSERT_EQ(ltv_vector[i].type_, an_array[i].type_);

    ASSERT_EQ(ltv_vector[i].value_, an_array[i].value_);

  }

  ASSERT_EQ(after_padding, view.GetAfterPadding());

}



vector<uint8_t> byte_sized{

system/gd/packet/parser/test/test_packets.pdl

+3 −1
Original line number Diff line number Diff line
@@ -391,8 +391,10 @@ packet SizedParent { 
}



packet OneLengthTypeValueStructPadded : SizedParent {

  valid_bytes : 8,

  one_array : LengthTypeValueStruct[],

  _padding_[40],

  _padding_[30],

  after_padding : 8,

}



packet ByteSizedFields {