[PATCH v6 1/2] covoar: Fix NOP execution marking
Chris Johns
chrisj at rtems.org
Fri Mar 26 02:24:57 UTC 2021
On 26/3/21 10:28 am, Alex White wrote:
> On Thu, Mar 25, 2021 at 6:10 PM Chris Johns <chrisj at rtems.org> wrote:
>>
>> On 26/3/21 8:49 am, Alex White wrote:
>>> Some NOP instructions were not being marked as executed because they
>>> are located at the end of uncovered ranges. This has been fixed.
>>> ---
>>> tester/covoar/CoverageMapBase.cc | 10 +++
>>> tester/covoar/CoverageMapBase.h | 4 ++
>>> tester/covoar/DesiredSymbols.cc | 38 +++++++++--
>>> tester/covoar/DesiredSymbols.h | 11 +++-
>>> tester/covoar/ExecutableInfo.cc | 2 +-
>>> tester/covoar/ExecutableInfo.h | 4 ++
>>> tester/covoar/ObjdumpProcessor.cc | 105 ++++++++++++++++++++++--------
>>> 7 files changed, 138 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/tester/covoar/CoverageMapBase.cc b/tester/covoar/CoverageMapBase.cc
>>> index ad0080d..6ca5cf7 100644
>>> --- a/tester/covoar/CoverageMapBase.cc
>>> +++ b/tester/covoar/CoverageMapBase.cc
>>> @@ -142,6 +142,11 @@ namespace Coverage {
>>> return size;
>>> }
>>>
>>> + uint32_t CoverageMapBase::getSizeOfRange( size_t index ) const
>>> + {
>>> + return Ranges.at(index).size();
>>> + }
>>> +
>>> bool CoverageMapBase::getBeginningOfInstruction(
>>> uint32_t address,
>>> uint32_t* beginning
>>> @@ -178,6 +183,11 @@ namespace Coverage {
>>> return Ranges.front().lowAddress;
>>> }
>>>
>>> + uint32_t CoverageMapBase::getLowAddressOfRange( size_t index ) const
>>> + {
>>> + return Ranges.at(index).lowAddress;
>>> + }
>>> +
>>> bool CoverageMapBase::getRange( uint32_t address, AddressRange& range ) const
>>> {
>>> for ( auto r : Ranges ) {
>>> diff --git a/tester/covoar/CoverageMapBase.h b/tester/covoar/CoverageMapBase.h
>>> index 6ad76d3..a58c696 100644
>>> --- a/tester/covoar/CoverageMapBase.h
>>> +++ b/tester/covoar/CoverageMapBase.h
>>> @@ -156,6 +156,8 @@ namespace Coverage {
>>> */
>>> int32_t getFirstLowAddress() const;
>>>
>>> + uint32_t getLowAddressOfRange( size_t index ) const;
>>> +
>>> /*!
>>> * This method returns true and sets the address range if
>>> * the address falls with the bounds of an address range
>>> @@ -177,6 +179,8 @@ namespace Coverage {
>>> */
>>> uint32_t getSize() const;
>>>
>>> + uint32_t getSizeOfRange( size_t index ) const;
>>> +
>>> /*!
>>> * This method returns the address of the beginning of the
>>> * instruction that contains the specified address.
>>> diff --git a/tester/covoar/DesiredSymbols.cc b/tester/covoar/DesiredSymbols.cc
>>> index b9a5bb7..c97b25c 100644
>>> --- a/tester/covoar/DesiredSymbols.cc
>>> +++ b/tester/covoar/DesiredSymbols.cc
>>> @@ -142,7 +142,7 @@ namespace Coverage {
>>> CoverageMapBase* theCoverageMap = s.second.unifiedCoverageMap;
>>> if (theCoverageMap)
>>> {
>>> - // Increment the total sizeInBytes byt the bytes in the symbol
>>> + // Increment the total sizeInBytes by the bytes in the symbol
>>> stats.sizeInBytes += s.second.stats.sizeInBytes;
>>>
>>> // Now scan through the coverage map of this symbol.
>>> @@ -202,6 +202,26 @@ namespace Coverage {
>>> uint32_t count;
>>>
>>> // Mark NOPs as executed
>>> + a = s.second.stats.sizeInBytes - 1;
>>> + count = 0;
>>> + while (a > 0) {
>>> + if (theCoverageMap->isStartOfInstruction( a )) {
>>> + break;
>>> + }
>>> +
>>> + count++;
>>> +
>>> + if (theCoverageMap->isNop( a )) {
>>> + for (la = a; la < (a + count); la++) {
>>> + theCoverageMap->setWasExecuted( la );
>>> + }
>>> +
>>> + count = 0;
>>> + }
>>> +
>>> + a--;
>>> + }
>>> +
>>> endAddress = s.second.stats.sizeInBytes - 1;
>>> a = 0;
>>> while (a < endAddress) {
>>> @@ -223,12 +243,13 @@ namespace Coverage {
>>> ha++;
>>> if ( ha >= endAddress )
>>> break;
>>> - } while ( !theCoverageMap->isStartOfInstruction( ha ) );
>>> + } while ( !theCoverageMap->isStartOfInstruction( ha ) ||
>>> + theCoverageMap->isNop( ha ) );
>>> a = ha;
>>> }
>>>
>>> // Now scan through the coverage map of this symbol.
>>> - endAddress = s.second.stats.sizeInBytes - 1;
>>> + endAddress = s.second.stats.sizeInBytesWithoutNops - 1;
>>> a = 0;
>>> while (a <= endAddress) {
>>> // If an address was NOT executed, find consecutive unexecuted
>>> @@ -316,7 +337,8 @@ namespace Coverage {
>>> void DesiredSymbols::createCoverageMap(
>>> const std::string& exefileName,
>>> const std::string& symbolName,
>>> - uint32_t size
>>> + uint32_t size,
>>> + uint32_t sizeWithoutNops
>>> )
>>> {
>>> CoverageMapBase* aCoverageMap;
>>> @@ -354,9 +376,10 @@ namespace Coverage {
>>> << '/' << size << ')'
>>> << std::endl;
>>>
>>> - if ( itr->second.stats.sizeInBytes < size )
>>> + if ( itr->second.stats.sizeInBytes < size ) {
>>> itr->second.stats.sizeInBytes = size;
>>> - else
>>> + itr->second.stats.sizeInBytesWithoutNops = sizeWithoutNops;
>>> + } else
>>> size = itr->second.stats.sizeInBytes;
>>> }
>>> }
>>> @@ -376,6 +399,7 @@ namespace Coverage {
>>> );
>>> itr->second.unifiedCoverageMap = aCoverageMap;
>>> itr->second.stats.sizeInBytes = size;
>>> + itr->second.stats.sizeInBytesWithoutNops = sizeWithoutNops;
>>> }
>>> }
>>>
>>> @@ -479,7 +503,7 @@ namespace Coverage {
>>> // are the same size.
>>> // Changed from ERROR msg to INFO, because size mismatch is not
>>> // treated as error anymore. 2015-07-20
>>> - uint32_t dMapSize = sinfo.stats.sizeInBytes;
>>> + uint32_t dMapSize = sinfo.stats.sizeInBytesWithoutNops;
>>> uint32_t sBaseAddress = sourceCoverageMap->getFirstLowAddress();
>>> uint32_t sMapSize = sourceCoverageMap->getSize();
>>> if (dMapSize != 0 && dMapSize != sMapSize) {
>>> diff --git a/tester/covoar/DesiredSymbols.h b/tester/covoar/DesiredSymbols.h
>>> index 5c45af8..367ca95 100644
>>> --- a/tester/covoar/DesiredSymbols.h
>>> +++ b/tester/covoar/DesiredSymbols.h
>>> @@ -57,7 +57,12 @@ namespace Coverage {
>>> uint32_t sizeInBytes;
>>>
>>> /*!
>>> - * This member contains the size in Bytes.
>>> + * This member contains the size in Bytes not accounting for NOPs.
>>> + */
>>> + uint32_t sizeInBytesWithoutNops;
>>> +
>>> + /*!
>>> + * This member contains the size in instructions.
>>> */
>>> uint32_t sizeInInstructions;
>>>
>>> @@ -100,6 +105,7 @@ namespace Coverage {
>>> branchesNeverTaken(0),
>>> branchesNotExecuted(0),
>>> sizeInBytes(0),
>>> + sizeInBytesWithoutNops(0),
>>> sizeInInstructions(0),
>>> uncoveredBytes(0),
>>> uncoveredInstructions(0),
>>> @@ -227,7 +233,8 @@ namespace Coverage {
>>> void createCoverageMap(
>>> const std::string& exefileName,
>>> const std::string& symbolName,
>>> - uint32_t size
>>> + uint32_t size,
>>> + uint32_t sizeWithoutNops
>>> );
>>>
>>> /*!
>>> diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc
>>> index c593e1d..ddd2987 100644
>>> --- a/tester/covoar/ExecutableInfo.cc
>>> +++ b/tester/covoar/ExecutableInfo.cc
>>> @@ -127,7 +127,7 @@ namespace Coverage {
>>> {
>>> CoverageMaps::iterator cmi = coverageMaps.find( symbolName );
>>> if ( cmi == coverageMaps.end() )
>>> - throw rld::error (symbolName, "ExecutableInfo::findCoverageMap");
>>> + throw CoverageMapNotFoundError(symbolName);
>>> return *(cmi->second);
>>> }
>>>
>>> diff --git a/tester/covoar/ExecutableInfo.h b/tester/covoar/ExecutableInfo.h
>>> index dcb4dcb..ab74b7f 100644
>>> --- a/tester/covoar/ExecutableInfo.h
>>> +++ b/tester/covoar/ExecutableInfo.h
>>> @@ -29,6 +29,10 @@ namespace Coverage {
>>>
>>> public:
>>>
>>> + class CoverageMapNotFoundError : public std::runtime_error {
>>> + using std::runtime_error::runtime_error;
>>
>> Ok sorry for raising C++ things (again) ....
>>
>> Why is `using` being used here?
>>
>> I do not use `using` anywhere execpt come special `chrono` related codes. I do
>> not know `std::runtime_error::runtime_error` enough to know where it is being
>> used in the code below. If those places had the full namespace I would.
>
> This usage of `using` was introduced in C++11. It makes the constructors in std::runtime_error::runtime_error visible to overload resolution when initializing CoverageMapNotFoundError.
>
> It's just a shortcut so that the constructors don't have to be redefined. It refers to the constructor `std::runtime_error::runtime_error`.
Thanks for the explaination. Can I suggest a comment to this effect. ie Use the
defined constructors?
Chris
More information about the devel
mailing list