From f74ff2ff7f67ff0141c058a242758de97e10dd99 Mon Sep 17 00:00:00 2001 From: Matteo Martincigh Date: Tue, 24 Sep 2019 11:38:32 +0100 Subject: IVGCVSW-3691 Fix the Counter Directory Packet data length * The data_length field in the header represents only the size of the data included in the packet after the header, so the header size is not included * Removed a number of conversion macros in SendCounterPacket by using numeric casts where possible * Updated the unit tests accordingly Signed-off-by: Matteo Martincigh Change-Id: Ifb23c341c442ff3d33b234d4213b739a77ceb658 --- src/profiling/SendCounterPacket.cpp | 113 ++++++++++---------------- src/profiling/test/ProfilingTests.cpp | 4 +- src/profiling/test/SendCounterPacketTests.cpp | 2 +- 3 files changed, 47 insertions(+), 72 deletions(-) (limited to 'src') diff --git a/src/profiling/SendCounterPacket.cpp b/src/profiling/SendCounterPacket.cpp index f3190eecde..b41f53ca24 100644 --- a/src/profiling/SendCounterPacket.cpp +++ b/src/profiling/SendCounterPacket.cpp @@ -181,6 +181,8 @@ bool SendCounterPacket::CreateCategoryRecord(const CategoryPtr& category, CategoryRecord& categoryRecord, std::string& errorMessage) { + using namespace boost::numeric; + BOOST_ASSERT(category); const std::string& categoryName = category->m_Name; @@ -225,9 +227,8 @@ bool SendCounterPacket::CreateCategoryRecord(const CategoryPtr& category, std::vector eventRecords(counterCount); std::vector eventRecordOffsets(counterCount, 0); size_t eventRecordsSize = 0; - ARMNN_NO_CONVERSION_WARN_BEGIN - uint32_t eventRecordsOffset = (eventRecords.size() + categoryNameBuffer.size()) * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + uint32_t eventRecordsOffset = + numeric_cast((eventRecords.size() + categoryNameBuffer.size()) * uint32_t_size); for (size_t counterIndex = 0, eventRecordIndex = 0, eventRecordOffsetIndex = 0; counterIndex < counterCount; counterIndex++, eventRecordIndex++, eventRecordOffsetIndex++) @@ -248,16 +249,12 @@ bool SendCounterPacket::CreateCategoryRecord(const CategoryPtr& category, // Add the event record offset to the event pointer table offset field eventRecordOffsets[eventRecordOffsetIndex] = eventRecordsOffset; - ARMNN_NO_CONVERSION_WARN_BEGIN - eventRecordsOffset += eventRecord.size() * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + eventRecordsOffset += numeric_cast(eventRecord.size() * uint32_t_size); } // Category record word 3: // 0:31 [32] name_offset (offset from the beginning of the category data pool to the name field) - ARMNN_NO_CONVERSION_WARN_BEGIN - uint32_t categoryRecordWord3 = eventRecordOffsets.size() * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + uint32_t categoryRecordWord3 = numeric_cast(eventRecordOffsets.size() * uint32_t_size); // Calculate the size in words of the category record size_t categoryRecordSize = 4u + // The size of the fixed part (device + counter_set + event_count + reserved + @@ -269,6 +266,7 @@ bool SendCounterPacket::CreateCategoryRecord(const CategoryPtr& category, // Allocate the necessary space for the category record categoryRecord.resize(categoryRecordSize); + ARMNN_NO_CONVERSION_WARN_BEGIN // Create the category record categoryRecord[0] = categoryRecordWord0; // device + counter_set categoryRecord[1] = categoryRecordWord1; // event_count + reserved @@ -276,20 +274,15 @@ bool SendCounterPacket::CreateCategoryRecord(const CategoryPtr& category, categoryRecord[3] = categoryRecordWord3; // name_offset auto offset = categoryRecord.begin() + 4u; std::copy(eventRecordOffsets.begin(), eventRecordOffsets.end(), offset); // event_pointer_table - ARMNN_NO_CONVERSION_WARN_BEGIN offset += eventRecordOffsets.size(); - ARMNN_NO_CONVERSION_WARN_END std::copy(categoryNameBuffer.begin(), categoryNameBuffer.end(), offset); // name - ARMNN_NO_CONVERSION_WARN_BEGIN offset += categoryNameBuffer.size(); - ARMNN_NO_CONVERSION_WARN_END for (const EventRecord& eventRecord : eventRecords) { std::copy(eventRecord.begin(), eventRecord.end(), offset); // event_record - ARMNN_NO_CONVERSION_WARN_BEGIN offset += eventRecord.size(); - ARMNN_NO_CONVERSION_WARN_END } + ARMNN_NO_CONVERSION_WARN_END return true; } @@ -399,6 +392,8 @@ bool SendCounterPacket::CreateEventRecord(const CounterPtr& counter, EventRecord& eventRecord, std::string& errorMessage) { + using namespace boost::numeric; + BOOST_ASSERT(counter); uint16_t counterUid = counter->m_Uid; @@ -470,10 +465,8 @@ bool SendCounterPacket::CreateEventRecord(const CounterPtr& counter, // Event record word 6: // 0:31 [32] description_offset: offset from the beginning of the event record pool to the description field - ARMNN_NO_CONVERSION_WARN_BEGIN // The size of the name buffer in bytes - uint32_t eventRecordWord6 = counterNameBuffer.size() * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + uint32_t eventRecordWord6 = numeric_cast(counterNameBuffer.size() * uint32_t_size); // Convert the counter description into a SWTrace string std::vector counterDescriptionBuffer; @@ -490,10 +483,11 @@ bool SendCounterPacket::CreateEventRecord(const CounterPtr& counter, // 0:31 [32] units_offset: (optional) offset from the beginning of the event record pool to the units field. // An offset value of zero indicates this field is not provided bool includeUnits = !counterUnits.empty(); - ARMNN_NO_CONVERSION_WARN_BEGIN // The size of the description buffer in bytes - uint32_t eventRecordWord7 = includeUnits ? eventRecordWord6 + counterDescriptionBuffer.size() * uint32_t_size : 0; - ARMNN_NO_CONVERSION_WARN_END + uint32_t eventRecordWord7 = includeUnits ? + eventRecordWord6 + + numeric_cast(counterDescriptionBuffer.size() * uint32_t_size) : + 0; // Convert the counter units into a SWTrace namestring (optional) std::vector counterUnitsBuffer; @@ -522,6 +516,7 @@ bool SendCounterPacket::CreateEventRecord(const CounterPtr& counter, // Allocate the space for the event record eventRecord.resize(eventRecordSize); + ARMNN_NO_CONVERSION_WARN_BEGIN // Create the event record eventRecord[0] = eventRecordWord0; // max_counter_uid + counter_uid eventRecord[1] = eventRecordWord1; // device + counter_set @@ -533,23 +528,22 @@ bool SendCounterPacket::CreateEventRecord(const CounterPtr& counter, eventRecord[7] = eventRecordWord7; // units_offset auto offset = eventRecord.begin() + 8u; std::copy(counterNameBuffer.begin(), counterNameBuffer.end(), offset); // name - ARMNN_NO_CONVERSION_WARN_BEGIN offset += counterNameBuffer.size(); - ARMNN_NO_CONVERSION_WARN_END std::copy(counterDescriptionBuffer.begin(), counterDescriptionBuffer.end(), offset); // description if (includeUnits) { - ARMNN_NO_CONVERSION_WARN_BEGIN offset += counterDescriptionBuffer.size(); - ARMNN_NO_CONVERSION_WARN_END std::copy(counterUnitsBuffer.begin(), counterUnitsBuffer.end(), offset); // units } + ARMNN_NO_CONVERSION_WARN_END return true; } void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& counterDirectory) { + using namespace boost::numeric; + // Get the amount of data that needs to be put into the packet uint16_t categoryCount = counterDirectory.GetCategoryCount(); uint16_t deviceCount = counterDirectory.GetDeviceCount(); @@ -590,9 +584,7 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun // Add the device record offset to the device records pointer table offset field deviceRecordOffsets[deviceRecordOffsetIndex] = pointerTableOffset; - ARMNN_NO_CONVERSION_WARN_BEGIN - pointerTableOffset += deviceRecord.size() * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + pointerTableOffset += numeric_cast(deviceRecord.size() * uint32_t_size); deviceIndex++; deviceRecordOffsetIndex++; @@ -625,9 +617,7 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun // Add the counter set record offset to the counter set records pointer table offset field counterSetRecordOffsets[counterSetRecordOffsetIndex] = pointerTableOffset; - ARMNN_NO_CONVERSION_WARN_BEGIN - pointerTableOffset += counterSetRecord.size() * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + pointerTableOffset += numeric_cast(counterSetRecord.size() * uint32_t_size); counterSetIndex++; counterSetRecordOffsetIndex++; @@ -660,17 +650,16 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun // Add the category record offset to the category records pointer table offset field categoryRecordOffsets[categoryRecordOffsetIndex] = pointerTableOffset; - ARMNN_NO_CONVERSION_WARN_BEGIN - pointerTableOffset += categoryRecord.size() * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + pointerTableOffset += numeric_cast(categoryRecord.size() * uint32_t_size); categoryIndex++; categoryRecordOffsetIndex++; } - // Calculate the size in words of the counter directory packet - size_t counterDirectoryPacketSize = - packetHeaderSize + // The size of the packet header + + + // Calculate the length in words of the counter directory packet's data (excludes the packet header size) + size_t counterDirectoryPacketDataLength = bodyHeaderSize + // The size of the body header deviceRecordOffsets.size() + // The size of the device records pointer table counterSetRecordOffsets.size() + // The size of counter set pointer table @@ -679,6 +668,11 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun counterSetRecordsSize + // The total size of the counter set records categoryRecordsSize; // The total size of the category records + // Calculate the size in words of the counter directory packet (the data length plus the packet header size) + size_t counterDirectoryPacketSize = packetHeaderSize + // The size of the packet header + counterDirectoryPacketDataLength; // The data length + + // Allocate the necessary space for the counter directory packet std::vector counterDirectoryPacket(counterDirectoryPacketSize, 0); @@ -697,9 +691,7 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun // Packet header word 1: // 0:31 [32] data_length: length of data, in bytes - ARMNN_NO_CONVERSION_WARN_BEGIN - uint32_t packetHeaderWord1 = counterDirectoryPacketSize * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + uint32_t packetHeaderWord1 = numeric_cast(counterDirectoryPacketDataLength * uint32_t_size); // Create the packet header uint32_t packetHeader[2] @@ -729,9 +721,10 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun // Body header word 3: // 0:31 [32] counter_set_pointer_table_offset: offset to the counter_set_pointer_table - ARMNN_NO_CONVERSION_WARN_BEGIN - uint32_t bodyHeaderWord3 = deviceRecordOffsets.size() * uint32_t_size; // The size of the device records pointer - ARMNN_NO_CONVERSION_WARN_END // table + uint32_t bodyHeaderWord3 = + numeric_cast(deviceRecordOffsets.size() * uint32_t_size); // The size of the device records + // pointer table + // Body header word 4: // 16:31 [16] categories_count: number of entries in the categories_pointer_table @@ -740,10 +733,10 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun // Body header word 3: // 0:31 [32] categories_pointer_table_offset: offset to the categories_pointer_table - ARMNN_NO_CONVERSION_WARN_BEGIN - uint32_t bodyHeaderWord5 = deviceRecordOffsets.size() * uint32_t_size + // The size of the device records pointer - counterSetRecordOffsets.size() * uint32_t_size; // table, plus the size of the counter - ARMNN_NO_CONVERSION_WARN_END // set pointer table + uint32_t bodyHeaderWord5 = + numeric_cast(deviceRecordOffsets.size() * uint32_t_size + // The size of the device records + counterSetRecordOffsets.size() * uint32_t_size); // pointer table, plus the size of + // the counter set pointer table // Create the body header uint32_t bodyHeader[6] @@ -756,62 +749,46 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun bodyHeaderWord5 // categories_pointer_table_offset }; + ARMNN_NO_CONVERSION_WARN_BEGIN // Create the counter directory packet auto counterDirectoryPacketOffset = counterDirectoryPacket.begin(); // packet_header std::copy(packetHeader, packetHeader + packetHeaderSize, counterDirectoryPacketOffset); - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += packetHeaderSize; - ARMNN_NO_CONVERSION_WARN_END // body_header std::copy(bodyHeader, bodyHeader + bodyHeaderSize, counterDirectoryPacketOffset); - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += bodyHeaderSize; - ARMNN_NO_CONVERSION_WARN_END // device_records_pointer_table std::copy(deviceRecordOffsets.begin(), deviceRecordOffsets.end(), counterDirectoryPacketOffset); - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += deviceRecordOffsets.size(); - ARMNN_NO_CONVERSION_WARN_END // counter_set_pointer_table std::copy(counterSetRecordOffsets.begin(), counterSetRecordOffsets.end(), counterDirectoryPacketOffset); - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += counterSetRecordOffsets.size(); - ARMNN_NO_CONVERSION_WARN_END // category_pointer_table std::copy(categoryRecordOffsets.begin(), categoryRecordOffsets.end(), counterDirectoryPacketOffset); - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += categoryRecordOffsets.size(); - ARMNN_NO_CONVERSION_WARN_END // device_records for (const DeviceRecord& deviceRecord : deviceRecords) { std::copy(deviceRecord.begin(), deviceRecord.end(), counterDirectoryPacketOffset); // device_record - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += deviceRecord.size(); - ARMNN_NO_CONVERSION_WARN_END } // counter_set_records for (const CounterSetRecord& counterSetRecord : counterSetRecords) { std::copy(counterSetRecord.begin(), counterSetRecord.end(), counterDirectoryPacketOffset); // counter_set_record - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += counterSetRecord.size(); - ARMNN_NO_CONVERSION_WARN_END } // category_records for (const CategoryRecord& categoryRecord : categoryRecords) { std::copy(categoryRecord.begin(), categoryRecord.end(), counterDirectoryPacketOffset); // category_record - ARMNN_NO_CONVERSION_WARN_BEGIN counterDirectoryPacketOffset += categoryRecord.size(); - ARMNN_NO_CONVERSION_WARN_END } + ARMNN_NO_CONVERSION_WARN_END // Calculate the total size in bytes of the counter directory packet - ARMNN_NO_CONVERSION_WARN_BEGIN - uint32_t totalSize = counterDirectoryPacketSize * uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + uint32_t totalSize = numeric_cast(counterDirectoryPacketSize * uint32_t_size); // Reserve space in the buffer for the packet uint32_t reserved = 0; @@ -838,9 +815,7 @@ void SendCounterPacket::SendCounterDirectoryPacket(const ICounterDirectory& coun for (uint32_t counterDirectoryPacketWord : counterDirectoryPacket) { WriteUint32(writeBuffer, offset, counterDirectoryPacketWord); - ARMNN_NO_CONVERSION_WARN_BEGIN - offset += uint32_t_size; - ARMNN_NO_CONVERSION_WARN_END + offset += numeric_cast(uint32_t_size); } m_Buffer.Commit(totalSize); diff --git a/src/profiling/test/ProfilingTests.cpp b/src/profiling/test/ProfilingTests.cpp index 2a20aac8a4..32a41f37c2 100644 --- a/src/profiling/test/ProfilingTests.cpp +++ b/src/profiling/test/ProfilingTests.cpp @@ -2022,7 +2022,7 @@ BOOST_AUTO_TEST_CASE(RequestCounterDirectoryCommandHandlerTest0) BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0); // packet family BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 2); // packet id - BOOST_TEST(headerWord1 == 32); // data lenght; + BOOST_TEST(headerWord1 == 24); // data length uint32_t bodyHeaderWord0 = ReadUint32(readBuffer, 8); uint16_t deviceRecordCount = numeric_cast(bodyHeaderWord0 >> 16); @@ -2061,7 +2061,7 @@ BOOST_AUTO_TEST_CASE(RequestCounterDirectoryCommandHandlerTest1) BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0); // packet family BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 2); // packet id - BOOST_TEST(headerWord1 == 248); // data lenght; + BOOST_TEST(headerWord1 == 240); // data length uint32_t bodyHeaderWord0 = ReadUint32(readBuffer, 8); uint32_t bodyHeaderWord1 = ReadUint32(readBuffer, 12); diff --git a/src/profiling/test/SendCounterPacketTests.cpp b/src/profiling/test/SendCounterPacketTests.cpp index f32c3684c1..90bc9225a0 100644 --- a/src/profiling/test/SendCounterPacketTests.cpp +++ b/src/profiling/test/SendCounterPacketTests.cpp @@ -1143,7 +1143,7 @@ BOOST_AUTO_TEST_CASE(SendCounterDirectoryPacketTest2) uint32_t packetHeaderWord1 = ReadUint32(readBuffer, 4); BOOST_TEST(((packetHeaderWord0 >> 26) & 0x3F) == 0); // packet_family BOOST_TEST(((packetHeaderWord0 >> 16) & 0x3FF) == 2); // packet_id - BOOST_TEST(packetHeaderWord1 == 944); // data_length + BOOST_TEST(packetHeaderWord1 == 936); // data_length // Check the body header uint32_t bodyHeaderWord0 = ReadUint32(readBuffer, 8); -- cgit v1.2.1