<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Aug 1, 2021, 10:11 PM Chris Johns <<a href="mailto:chrisj@rtems.org" target="_blank" rel="noreferrer">chrisj@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2/8/21 9:23 am, Ryan Long wrote:<br>
> - Removed SymbolsToAnalyze from app_common and replaced it with the<br>
> symbolsToAnalyze_m member variable in DesiredSymbols, GcovData, and<br>
> ObjdumpProcessor<br>
> - Added a parameter to constructors to initialize symbolsToAnalyze_m<br>
> - Moved the definition of objdumpLine_t out of ObjdumpProcessor to make<br>
> it work with DesiredSymbols<br>
> ---<br>
> tester/covoar/DesiredSymbols.cc | 12 +++--<br>
> tester/covoar/DesiredSymbols.h | 17 +++++--<br>
> tester/covoar/ExecutableInfo.cc | 14 +++---<br>
> tester/covoar/ExecutableInfo.h | 14 +++++-<br>
> tester/covoar/GcovData.cc | 7 +--<br>
> tester/covoar/GcovData.h | 12 ++++-<br>
> tester/covoar/GcovFunctionData.cc | 14 ++++--<br>
> tester/covoar/GcovFunctionData.h | 6 ++-<br>
> tester/covoar/ObjdumpProcessor.cc | 22 +++++----<br>
> tester/covoar/ObjdumpProcessor.h | 90 +++++++++++++++++++----------------<br>
> tester/covoar/ReportsBase.cc | 98 +++++++++++++++++++++------------------<br>
> tester/covoar/ReportsBase.h | 33 +++++++++----<br>
> tester/covoar/ReportsHtml.cc | 16 ++++---<br>
> tester/covoar/ReportsHtml.h | 11 +++--<br>
> tester/covoar/ReportsText.cc | 8 ++--<br>
> tester/covoar/ReportsText.h | 3 +-<br>
> tester/covoar/TraceConverter.cc | 17 +++++--<br>
> tester/covoar/app_common.cc | 1 -<br>
> tester/covoar/app_common.h | 1 -<br>
> tester/covoar/covoar.cc | 40 +++++++++-------<br>
> 20 files changed, 271 insertions(+), 165 deletions(-)<br>
> <br>
> diff --git a/tester/covoar/DesiredSymbols.cc b/tester/covoar/DesiredSymbols.cc<br>
> index 5d5e637..d76c5af 100644<br>
> --- a/tester/covoar/DesiredSymbols.cc<br>
> +++ b/tester/covoar/DesiredSymbols.cc<br>
> @@ -25,7 +25,6 @@<br>
> #include "DesiredSymbols.h"<br>
> #include "app_common.h"<br>
> #include "CoverageMap.h"<br>
> -#include "ObjdumpProcessor.h"<br>
> <br>
> namespace Coverage {<br>
> <br>
> @@ -116,10 +115,10 @@ namespace Coverage {<br>
> }<br>
> }<br>
> <br>
> - void DesiredSymbols::preprocess( void )<br>
> + void DesiredSymbols::preprocess( const DesiredSymbols& symbolsToAnalyze )<br>
> {<br>
> // Look at each symbol.<br>
> - for (auto& s : SymbolsToAnalyze->set) {<br>
> + for (auto& s : symbolsToAnalyze.set) {<br>
> // If the unified coverage map does not exist, the symbol was<br>
> // never referenced by any executable. Just skip it.<br>
> CoverageMapBase* theCoverageMap = s.second.unifiedCoverageMap;<br>
> @@ -441,10 +440,13 @@ namespace Coverage {<br>
> return &set[ symbolName ];<br>
> }<br>
> <br>
> - void DesiredSymbols::findSourceForUncovered( bool verbose )<br>
> + void DesiredSymbols::findSourceForUncovered(<br>
> + bool verbose,<br>
> + const DesiredSymbols& symbolsToAnalyze<br>
> + )<br>
> {<br>
> // Process uncovered ranges and/or branches for each symbol.<br>
> - for (auto& d : SymbolsToAnalyze->set) {<br>
> + for (auto& d : symbolsToAnalyze.set) {<br>
> // First the unexecuted ranges, ...<br>
> CoverageRanges* theRanges = d.second.uncoveredRanges;<br>
> if (theRanges != nullptr) {<br>
> diff --git a/tester/covoar/DesiredSymbols.h b/tester/covoar/DesiredSymbols.h<br>
> index 791b4ea..70bc39b 100644<br>
> --- a/tester/covoar/DesiredSymbols.h<br>
> +++ b/tester/covoar/DesiredSymbols.h<br>
> @@ -19,6 +19,11 @@<br>
> <br>
> namespace Coverage {<br>
> <br>
> + class ObjdumpProcessor;<br>
> + struct objdumpLine;<br>
> + using objdumpLine_t = objdumpLine;<br>
> + class ExecutableInfo;<br>
> +<br>
> <br>
> /*!<br>
> *<br>
> @@ -139,7 +144,7 @@ namespace Coverage {<br>
> /*!<br>
> * This member contains the disassembly associated with a symbol.<br>
> */<br>
> - std::list<ObjdumpProcessor::objdumpLine_t> instructions;<br>
> + std::list<objdumpLine_t> instructions;<br>
> <br>
> /*!<br>
> * This member contains the executable that was used to<br>
> @@ -263,8 +268,12 @@ namespace Coverage {<br>
> * uncovered ranges or branches.<br>
> *<br>
> * @param[in] verbose specifies whether to be verbose with output<br>
> + * @param[in] symbolsToAnalyze the symbols to be analyzed<br>
> */<br>
> - void findSourceForUncovered( bool verbose );<br>
> + void findSourceForUncovered(<br>
> + bool verbose,<br>
> + const DesiredSymbols& symbolsToAnalyze<br>
> + );<br>
> <br>
> /*!<br>
> * This method returns the total number of branches always taken<br>
> @@ -398,8 +407,10 @@ namespace Coverage {<br>
> /*!<br>
> * This method preprocesses each symbol's coverage map to mark nop<br>
> * and branch information.<br>
> + *<br>
> + * @param[in] symbolsToAnalyze the symbols to be analyzed<br>
> */<br>
> - void preprocess( void );<br>
> + void preprocess( const DesiredSymbols& symbolsToAnalyze );<br>
> <br>
> private:<br>
> <br>
> diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc<br>
> index 9c3031e..328f970 100644<br>
> --- a/tester/covoar/ExecutableInfo.cc<br>
> +++ b/tester/covoar/ExecutableInfo.cc<br>
> @@ -10,9 +10,9 @@<br>
> #include <rld.h><br>
> <br>
> #include "ExecutableInfo.h"<br>
> +#include "ObjdumpProcessor.h"<br>
> #include "app_common.h"<br>
> #include "CoverageMap.h"<br>
> -#include "DesiredSymbols.h"<br>
> #include "SymbolTable.h"<br>
> <br>
> namespace Coverage {<br>
> @@ -20,9 +20,11 @@ namespace Coverage {<br>
> ExecutableInfo::ExecutableInfo(<br>
> const char* const theExecutableName,<br>
> const std::string& theLibraryName,<br>
> - bool verbose<br>
> + bool verbose,<br>
> + DesiredSymbols& symbolsToAnalyze<br>
> ) : fileName(theExecutableName),<br>
> - loadAddress(0)<br>
> + loadAddress(0),<br>
> + symbolsToAnalyze_m(symbolsToAnalyze)<br>
> {<br>
> if ( !theLibraryName.empty() )<br>
> libraryName = theLibraryName;<br>
> @@ -59,7 +61,7 @@ namespace Coverage {<br>
> continue;<br>
> }<br>
> <br>
> - if (!SymbolsToAnalyze->isDesired(<a href="http://func.name" rel="noreferrer noreferrer noreferrer" target="_blank">func.name</a>())) {<br>
> + if (!symbolsToAnalyze_m.isDesired(<a href="http://func.name" rel="noreferrer noreferrer noreferrer" target="_blank">func.name</a>())) {<br>
> continue;<br>
> }<br>
> <br>
> @@ -209,8 +211,8 @@ namespace Coverage {<br>
> <br>
> void ExecutableInfo::mergeCoverage( void ) {<br>
> for (auto& cm : coverageMaps) {<br>
> - if (SymbolsToAnalyze->isDesired( cm.first ))<br>
> - SymbolsToAnalyze->mergeCoverageMap( cm.first, cm.second );<br>
> + if (symbolsToAnalyze_m.isDesired( cm.first ))<br>
> + symbolsToAnalyze_m.mergeCoverageMap( cm.first, cm.second );<br>
> }<br>
> }<br>
> <br>
> diff --git a/tester/covoar/ExecutableInfo.h b/tester/covoar/ExecutableInfo.h<br>
> index 851a59d..0adebcb 100644<br>
> --- a/tester/covoar/ExecutableInfo.h<br>
> +++ b/tester/covoar/ExecutableInfo.h<br>
> @@ -18,9 +18,12 @@<br>
> #include "AddressToLineMapper.h"<br>
> #include "CoverageMapBase.h"<br>
> #include "SymbolTable.h"<br>
> +#include "DesiredSymbols.h"<br>
> <br>
> namespace Coverage {<br>
> <br>
> +class DesiredSymbols;<br>
> +<br>
> /*! @class ExecutableInfo<br>
> *<br>
> * This class holds a collection of information for an executable<br>
> @@ -41,11 +44,13 @@ namespace Coverage {<br>
> * @param[in] theExecutableName specifies the name of the executable<br>
> * @param[in] theLibraryName specifies the name of the executable<br>
> * @param[in] verbose specifies whether to be verbose with output<br>
> + * @param[in] symbolsToAnalyze the symbols to be analyzed<br>
> */<br>
> ExecutableInfo(<br>
> const char* const theExecutableName,<br>
> - const std::string& theLibraryName = "",<br>
> - bool verbose = false<br>
> + const std::string& theLibraryName,<br>
> + bool verbose,<br>
> + DesiredSymbols& symbolsToAnalyze<br>
> );<br>
> <br>
> /*!<br>
> @@ -198,6 +203,11 @@ namespace Coverage {<br>
> */<br>
> SymbolTable theSymbolTable;<br>
> <br>
> + /*!<br>
> + * This member variable contains the symbols to be analyzed.<br>
> + */<br>
> + DesiredSymbols& symbolsToAnalyze_m;<br>
> +<br>
> };<br>
> }<br>
> #endif<br>
> diff --git a/tester/covoar/GcovData.cc b/tester/covoar/GcovData.cc<br>
> index e8b8573..11c26ce 100644<br>
> --- a/tester/covoar/GcovData.cc<br>
> +++ b/tester/covoar/GcovData.cc<br>
> @@ -25,9 +25,10 @@<br>
> <br>
> namespace Gcov {<br>
> <br>
> - GcovData::GcovData()<br>
> + GcovData::GcovData( Coverage::DesiredSymbols& symbolsToAnalyze ):<br>
> + numberOfFunctions( 0 ),<br>
> + symbolsToAnalyze_m( symbolsToAnalyze )<br>
> {<br>
> - numberOfFunctions = 0;<br>
> }<br>
> <br>
> GcovData::~GcovData()<br>
> @@ -416,7 +417,7 @@ namespace Gcov {<br>
> function->setChecksum( intBuffer[1] );<br>
> <br>
> header.length -= readString( buffer, gcovFile );<br>
> - function->setFunctionName( buffer );<br>
> + function->setFunctionName( buffer, symbolsToAnalyze_m );<br>
> header.length -= readString( buffer, gcovFile );<br>
> function->setFileName( buffer );<br>
> status = fread( &intBuffer, 4, header.length, gcovFile );<br>
> diff --git a/tester/covoar/GcovData.h b/tester/covoar/GcovData.h<br>
> index 0e74b02..b00b27a 100644<br>
> --- a/tester/covoar/GcovData.h<br>
> +++ b/tester/covoar/GcovData.h<br>
> @@ -11,6 +11,7 @@<br>
> #include <list><br>
> #include <iostream><br>
> #include "GcovFunctionData.h"<br>
> +#include "DesiredSymbols.h"<br>
> <br>
> namespace Gcov {<br>
> <br>
> @@ -56,6 +57,8 @@ struct gcov_statistics<br>
> uint64_t sumMax; // sum of individual runs max values<br>
> };<br>
> <br>
> +class DesiredSymbols;<br>
> +<br>
> /*! @class GcovData<br>
> *<br>
> * This is the specification of the GcovData class.<br>
> @@ -66,8 +69,10 @@ struct gcov_statistics<br>
> <br>
> /*!<br>
> * This method constructs a GcnoReader instance.<br>
> + *<br>
> + * @param[in] symbolsToAnalyze the symbols to be analyzed<br>
> */<br>
> - GcovData();<br>
> + GcovData( Coverage::DesiredSymbols& symbolsToAnalyze );<br>
> <br>
> /*!<br>
> * This method destructs a GcnoReader instance.<br>
> @@ -194,6 +199,11 @@ struct gcov_statistics<br>
> * to a specified report file<br>
> */<br>
> void printGcnoFileInfo( FILE * textFile );<br>
> +<br>
> + /*!<br>
> + * This member variable contains the symbols to be analyzed<br>
> + */<br>
> + Coverage::DesiredSymbols& symbolsToAnalyze_m;<br>
> };<br>
> }<br>
> #endif<br>
> diff --git a/tester/covoar/GcovFunctionData.cc b/tester/covoar/GcovFunctionData.cc<br>
> index 90b1be0..1767f18 100644<br>
> --- a/tester/covoar/GcovFunctionData.cc<br>
> +++ b/tester/covoar/GcovFunctionData.cc<br>
> @@ -13,6 +13,7 @@<br>
> #include "GcovFunctionData.h"<br>
> #include "ObjdumpProcessor.h"<br>
> #include "CoverageMapBase.h"<br>
> +#include "DesiredSymbols.h"<br>
> <br>
> <br>
> namespace Gcov {<br>
> @@ -44,7 +45,10 @@ namespace Gcov {<br>
> firstLineNumber = lineNo;<br>
> }<br>
> <br>
> - bool GcovFunctionData::setFunctionName( const char* fcnName )<br>
> + bool GcovFunctionData::setFunctionName(<br>
> + const char* fcnName,<br>
> + Coverage::DesiredSymbols& symbolsToAnalyze<br>
> + )<br>
> {<br>
> std::string symbolName;<br>
> <br>
> @@ -62,7 +66,7 @@ namespace Gcov {<br>
> strcpy (functionName, fcnName);<br>
> <br>
> // Tie function to its coverage map<br>
> - symbolInfo = SymbolsToAnalyze->find( symbolName );<br>
> + symbolInfo = symbolsToAnalyze.find( symbolName );<br>
> if ( symbolInfo != NULL )<br>
> coverageMap = symbolInfo->unifiedCoverageMap;<br>
> <br>
> @@ -237,7 +241,7 @@ namespace Gcov {<br>
> uint32_t baseAddress = 0;<br>
> uint32_t baseSize;<br>
> uint32_t currentAddress;<br>
> - std::list<Coverage::ObjdumpProcessor::objdumpLine_t>::iterator instruction;<br>
> + std::list<Coverage::objdumpLine_t>::iterator instruction;<br>
> <br>
> if ( coverageMap != NULL ) {<br>
> <br>
> @@ -399,7 +403,7 @@ namespace Gcov {<br>
> <br>
> uint32_t baseAddress = 0;<br>
> uint32_t currentAddress = 0;<br>
> - std::list<Coverage::ObjdumpProcessor::objdumpLine_t>::iterator instruction;<br>
> + std::list<Coverage::objdumpLine_t>::iterator instruction;<br>
> blocks_iterator_t blockIterator;<br>
> blocks_iterator_t blockIterator2;<br>
> arcs_iterator_t arcIterator;<br>
> @@ -567,7 +571,7 @@ namespace Gcov {<br>
> {<br>
> uint32_t baseAddress = 0;<br>
> uint32_t currentAddress;<br>
> - std::list<Coverage::ObjdumpProcessor::objdumpLine_t>::iterator instruction;<br>
> + std::list<Coverage::objdumpLine_t>::iterator instruction;<br>
> <br>
> if ( coverageMap == NULL )<br>
> return false;<br>
> diff --git a/tester/covoar/GcovFunctionData.h b/tester/covoar/GcovFunctionData.h<br>
> index 812b45c..7d87b55 100644<br>
> --- a/tester/covoar/GcovFunctionData.h<br>
> +++ b/tester/covoar/GcovFunctionData.h<br>
> @@ -44,6 +44,8 @@ typedef std::list<gcov_arc_info>::iterator arcs_iterator_t;<br>
> typedef std::list<gcov_block_info> blocks_t;<br>
> typedef std::list<gcov_block_info>::iterator blocks_iterator_t;<br>
> <br>
> +class DesiredSymbols;<br>
> +<br>
> /*! @class GcovFunctionData<br>
> *<br>
> * This is the specification of the GcovFunctionData class.<br>
> @@ -94,11 +96,13 @@ typedef std::list<gcov_block_info>::iterator blocks_iterator_t;<br>
> * unified coverage map.<br>
> *<br>
> * @param[in] functionName passes name of the the function<br>
> + * @param[in] symbolsToAnalyze the symbols to be analyzed<br>
> *<br>
> * @return Returns TRUE if the method succeeded and FALSE if it failed.<br>
> */<br>
> bool setFunctionName(<br>
> - const char* fcnName<br>
> + const char* fcnName,<br>
> + Coverage::DesiredSymbols& symbolsToAnalyze<br>
> );<br>
> <br>
> /*!<br>
> diff --git a/tester/covoar/ObjdumpProcessor.cc b/tester/covoar/ObjdumpProcessor.cc<br>
> index d324440..f590ece 100644<br>
> --- a/tester/covoar/ObjdumpProcessor.cc<br>
> +++ b/tester/covoar/ObjdumpProcessor.cc<br>
> @@ -32,7 +32,8 @@ namespace Coverage {<br>
> ExecutableInfo* const executableInfo,<br>
> std::string& symbolName,<br>
> ObjdumpProcessor::objdumpLines_t instructions,<br>
> - bool verbose<br>
> + bool verbose,<br>
> + DesiredSymbols& symbolsToAnalyze<br>
> ) {<br>
> // Find the symbol's coverage map.<br>
> try {<br>
> @@ -88,7 +89,7 @@ namespace Coverage {<br>
> }<br>
> <br>
> // If there are NOT already saved instructions, save them.<br>
> - SymbolInformation* symbolInfo = SymbolsToAnalyze->find( symbolName );<br>
> + SymbolInformation* symbolInfo = symbolsToAnalyze.find( symbolName );<br>
> if (symbolInfo->instructions.empty()) {<br>
> symbolInfo->sourceFile = executableInfo;<br>
> symbolInfo->baseAddress = lowAddress;<br>
> @@ -107,7 +108,7 @@ namespace Coverage {<br>
> }<br>
> <br>
> // Create a unified coverage map for the symbol.<br>
> - SymbolsToAnalyze->createCoverageMap(<br>
> + symbolsToAnalyze.createCoverageMap(<br>
> executableInfo->getFileName().c_str(),<br>
> symbolName,<br>
> size,<br>
> @@ -122,7 +123,9 @@ namespace Coverage {<br>
> }<br>
> }<br>
> <br>
> - ObjdumpProcessor::ObjdumpProcessor()<br>
> + ObjdumpProcessor::ObjdumpProcessor(<br>
> + DesiredSymbols& symbolsToAnalyze<br>
> + ): symbolsToAnalyze_m( symbolsToAnalyze )<br>
> {<br>
> }<br>
> <br>
> @@ -363,7 +366,8 @@ namespace Coverage {<br>
> executableInformation,<br>
> currentSymbol,<br>
> theInstructions,<br>
> - verbose<br>
> + verbose,<br>
> + symbolsToAnalyze_m<br>
> );<br>
> fprintf(<br>
> stderr,<br>
> @@ -419,7 +423,8 @@ namespace Coverage {<br>
> executableInformation,<br>
> currentSymbol,<br>
> theInstructions,<br>
> - verbose<br>
> + verbose,<br>
> + symbolsToAnalyze_m<br>
> );<br>
> }<br>
> <br>
> @@ -444,7 +449,7 @@ namespace Coverage {<br>
> }<br>
> <br>
> // See if the new symbol is one that we care about.<br>
> - if (SymbolsToAnalyze->isDesired( symbol )) {<br>
> + if (symbolsToAnalyze_m.isDesired( symbol )) {<br>
> currentSymbol = symbol;<br>
> processSymbol = true;<br>
> theInstructions.push_back( lineInfo );<br>
> @@ -462,7 +467,8 @@ namespace Coverage {<br>
> executableInformation,<br>
> currentSymbol,<br>
> theInstructions,<br>
> - verbose<br>
> + verbose,<br>
> + symbolsToAnalyze_m<br>
> );<br>
> }<br>
> processSymbol = false;<br>
> diff --git a/tester/covoar/ObjdumpProcessor.h b/tester/covoar/ObjdumpProcessor.h<br>
> index d60a768..c7fc8bb 100644<br>
> --- a/tester/covoar/ObjdumpProcessor.h<br>
> +++ b/tester/covoar/ObjdumpProcessor.h<br>
> @@ -12,11 +12,54 @@<br>
> <br>
> #include "ExecutableInfo.h"<br>
> #include "TargetBase.h"<br>
> +#include "DesiredSymbols.h"<br>
> <br>
> #include "rld-process.h"<br>
> <br>
> namespace Coverage {<br>
> <br>
> + class DesiredSymbols;<br>
> + class ExecutableInfo;<br>
> +<br>
> + /*!<br>
> + * This type defines the elements of an objdump line.<br>
> + */<br>
> + typedef struct objdumpLine {<br>
<br>
Typedef here? I thought it was automatic with C++?<br>
<br>
> + /*!<br>
> + * This member variable contains the actual line from the object dump.<br>
> + */<br>
> + std::string line;<br>
> +<br>
> + /*!<br>
> + * This member variable contains the address from the object dump line.<br>
> + */<br>
> + uint32_t address;<br>
> +<br>
> + /*!<br>
> + * This member variable contains an indication of whether the line<br>
> + * is an instruction.<br>
> + */<br>
> + bool isInstruction;<br>
> +<br>
> + /*!<br>
> + * This member variable contains an indication of whether the line<br>
> + * is a nop instruction.<br>
> + */<br>
> + bool isNop;<br>
> +<br>
> + /*!<br>
> + * This member variable contains the size of the nop instruction.<br>
> + */<br>
> + int nopSize;<br>
> +<br>
> + /*!<br>
> + * This member variable contains an indication of whether the line<br>
> + * is a branch instruction.<br>
> + */<br>
> + bool isBranch;<br>
> +<br>
> + } objdumpLine_t;<br>
<br>
Please do not append `_t`. I understand it is reserved for standards or<br>
something like that.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">POSIX reserves _t. C and C++ have more complicated rules that don't go this far. One of which is something like double leading underscores are reserved for the implementation. </div><div dir="auto"><br></div><div dir="auto">I'm guilty of violating the _t rule and use __ for header guards.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Chris<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" rel="noreferrer noreferrer" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div></div>