[PATCH v4 1/2] covoar: Fix NOP execution marking
Alex White
alex.white at oarcorp.com
Mon Mar 15 15:51:50 UTC 2021
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 | 109 +++++++++++++++++++++++-------
7 files changed, 142 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;
+ };
+
/*!
* This method constructs an ExecutableInfo instance.
*
diff --git a/tester/covoar/ObjdumpProcessor.cc b/tester/covoar/ObjdumpProcessor.cc
index 56ee219..fa9894d 100644
--- a/tester/covoar/ObjdumpProcessor.cc
+++ b/tester/covoar/ObjdumpProcessor.cc
@@ -32,35 +32,92 @@ namespace Coverage {
ObjdumpProcessor::objdumpLines_t instructions
) {
// Find the symbol's coverage map.
- CoverageMapBase& coverageMap = executableInfo->findCoverageMap( symbolName );
-
- uint32_t lowAddress = coverageMap.getFirstLowAddress();
- uint32_t size = coverageMap.getSize();
- uint32_t highAddress = lowAddress + size - 1;
-
- // If there are NOT already saved instructions, save them.
- SymbolInformation* symbolInfo = SymbolsToAnalyze->find( symbolName );
- if (symbolInfo->instructions.empty()) {
- symbolInfo->sourceFile = executableInfo;
- symbolInfo->baseAddress = lowAddress;
- symbolInfo->instructions = instructions;
- }
+ try {
+ CoverageMapBase& coverageMap = executableInfo->findCoverageMap(symbolName);
- // Add the symbol to this executable's symbol table.
- SymbolTable* theSymbolTable = executableInfo->getSymbolTable();
- theSymbolTable->addSymbol(
- symbolName, lowAddress, highAddress - lowAddress + 1
- );
+ uint32_t firstInstructionAddress = UINT32_MAX;
- // Mark the start of each instruction in the coverage map.
- for (auto& instruction : instructions) {
- coverageMap.setIsStartOfInstruction( instruction.address );
- }
+ // Find the address of the first instruction.
+ for (auto& line : instructions) {
+ if (!line.isInstruction) {
+ continue;
+ }
+
+ firstInstructionAddress = line.address;
+ break;
+ }
+
+ if (firstInstructionAddress == UINT32_MAX) {
+ std::ostringstream what;
+ what << "Could not find first instruction address for symbol "
+ << symbolName << " in " << executableInfo->getFileName();
+ throw rld::error( what, "Coverage::finalizeSymbol" );
+ }
+
+ int rangeIndex;
+ uint32_t lowAddress = UINT32_MAX;
+ for (rangeIndex = 0; ; rangeIndex++) {
+ lowAddress = coverageMap.getLowAddressOfRange(rangeIndex);
+
+ if (firstInstructionAddress == lowAddress) {
+ break;
+ }
+ }
- // Create a unified coverage map for the symbol.
- SymbolsToAnalyze->createCoverageMap(
- executableInfo->getFileName().c_str(), symbolName, size
- );
+ uint32_t sizeWithoutNops = coverageMap.getSizeOfRange(rangeIndex);
+ uint32_t size = sizeWithoutNops;
+ uint32_t highAddress = lowAddress + size - 1;
+ uint32_t computedHighAddress = highAddress;
+
+ // Find the high address as reported by the address of the last NOP
+ // instruction. This ensures that NOPs get marked as executed later.
+ for (auto instruction = instructions.rbegin();
+ instruction != instructions.rend();
+ instruction++) {
+ if (instruction->isInstruction) {
+ if (instruction->isNop) {
+ computedHighAddress = instruction->address + instruction->nopSize;
+ }
+ break;
+ }
+ }
+
+ if (highAddress != computedHighAddress) {
+ std::cerr << "Function's high address differs between DWARF and objdump: "
+ << symbolName << " (0x" << std::hex << highAddress << " and 0x"
+ << computedHighAddress - 1 << ")" << std::endl;
+ size = computedHighAddress - lowAddress;
+ }
+
+ // If there are NOT already saved instructions, save them.
+ SymbolInformation* symbolInfo = SymbolsToAnalyze->find( symbolName );
+ if (symbolInfo->instructions.empty()) {
+ symbolInfo->sourceFile = executableInfo;
+ symbolInfo->baseAddress = lowAddress;
+ symbolInfo->instructions = instructions;
+ }
+
+ // Add the symbol to this executable's symbol table.
+ SymbolTable* theSymbolTable = executableInfo->getSymbolTable();
+ theSymbolTable->addSymbol(
+ symbolName, lowAddress, highAddress - lowAddress + 1
+ );
+
+ // Mark the start of each instruction in the coverage map.
+ for (auto& instruction : instructions) {
+ coverageMap.setIsStartOfInstruction( instruction.address );
+ }
+
+ // Create a unified coverage map for the symbol.
+ SymbolsToAnalyze->createCoverageMap(
+ executableInfo->getFileName().c_str(), symbolName, size, sizeWithoutNops
+ );
+ } catch (const ExecutableInfo::CoverageMapNotFoundError& e) {
+ // Allow execution to continue even if a coverage map could not be
+ // found.
+ std::cerr << "Coverage map not found for symbol " << e.what()
+ << std::endl;
+ }
}
ObjdumpProcessor::ObjdumpProcessor()
--
2.27.0
More information about the devel
mailing list