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 <matteo.martincigh@arm.com>
Change-Id: Ifb23c341c442ff3d33b234d4213b739a77ceb658
diff --git a/src/profiling/SendCounterPacket.cpp b/src/profiling/SendCounterPacket.cpp
index f3190ee..b41f53c 100644
--- a/src/profiling/SendCounterPacket.cpp
+++ b/src/profiling/SendCounterPacket.cpp
@@ -181,6 +181,8 @@
CategoryRecord& categoryRecord,
std::string& errorMessage)
{
+ using namespace boost::numeric;
+
BOOST_ASSERT(category);
const std::string& categoryName = category->m_Name;
@@ -225,9 +227,8 @@
std::vector<EventRecord> eventRecords(counterCount);
std::vector<uint32_t> 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<uint32_t>((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 @@
// 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<uint32_t>(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<uint32_t>(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 @@
// 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 @@
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 @@
EventRecord& eventRecord,
std::string& errorMessage)
{
+ using namespace boost::numeric;
+
BOOST_ASSERT(counter);
uint16_t counterUid = counter->m_Uid;
@@ -470,10 +465,8 @@
// 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<uint32_t>(counterNameBuffer.size() * uint32_t_size);
// Convert the counter description into a SWTrace string
std::vector<uint32_t> counterDescriptionBuffer;
@@ -490,10 +483,11 @@
// 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<uint32_t>(counterDescriptionBuffer.size() * uint32_t_size) :
+ 0;
// Convert the counter units into a SWTrace namestring (optional)
std::vector<uint32_t> counterUnitsBuffer;
@@ -522,6 +516,7 @@
// 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 @@
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 @@
// 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<uint32_t>(deviceRecord.size() * uint32_t_size);
deviceIndex++;
deviceRecordOffsetIndex++;
@@ -625,9 +617,7 @@
// 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<uint32_t>(counterSetRecord.size() * uint32_t_size);
counterSetIndex++;
counterSetRecordOffsetIndex++;
@@ -660,17 +650,16 @@
// 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<uint32_t>(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 @@
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<uint32_t> counterDirectoryPacket(counterDirectoryPacketSize, 0);
@@ -697,9 +691,7 @@
// 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<uint32_t>(counterDirectoryPacketDataLength * uint32_t_size);
// Create the packet header
uint32_t packetHeader[2]
@@ -729,9 +721,10 @@
// 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<uint32_t>(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 @@
// 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<uint32_t>(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 @@
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<uint32_t>(counterDirectoryPacketSize * uint32_t_size);
// Reserve space in the buffer for the packet
uint32_t reserved = 0;
@@ -838,9 +815,7 @@
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>(uint32_t_size);
}
m_Buffer.Commit(totalSize);
diff --git a/src/profiling/test/ProfilingTests.cpp b/src/profiling/test/ProfilingTests.cpp
index 2a20aac..32a41f3 100644
--- a/src/profiling/test/ProfilingTests.cpp
+++ b/src/profiling/test/ProfilingTests.cpp
@@ -2022,7 +2022,7 @@
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<uint16_t>(bodyHeaderWord0 >> 16);
@@ -2061,7 +2061,7 @@
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 f32c368..90bc922 100644
--- a/src/profiling/test/SendCounterPacketTests.cpp
+++ b/src/profiling/test/SendCounterPacketTests.cpp
@@ -1143,7 +1143,7 @@
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);