On Thu, Mar 25, 2021 at 9:25 PM Chris Johns <chr...@rtems.org> wrote:
>
> On 26/3/21 10:28 am, Alex White wrote:
> > On Thu, Mar 25, 2021 at 6:10 PM Chris Johns <chr...@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?
I believe that would be helpful, yes. I'll add that.
Alex
>
> Chris
> _______________________________________________
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
