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