covoar: Fix NOP execution marking

Some NOP instructions were not being marked as executed because they
are located at the end of uncovered ranges. This has been fixed.

7 files changed, 139 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..d5ccb19 100644
--- a/tester/covoar/ExecutableInfo.h
+++ b/tester/covoar/ExecutableInfo.h
@@ -29,6 +29,11 @@ namespace Coverage {
 
   public:
 
+    class CoverageMapNotFoundError : public std::runtime_error {
+      /* Use the base class constructors. */
+      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..05a50f1 100644
--- a/tester/covoar/ObjdumpProcessor.cc
+++ b/tester/covoar/ObjdumpProcessor.cc
@@ -32,35 +32,88 @@ 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) {
+          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;
+           firstInstructionAddress != lowAddress;
+           rangeIndex++) {
+        lowAddress = coverageMap.getLowAddressOfRange(rangeIndex);
+      }
+
+      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::dec << std::endl;
+        size = computedHighAddress - lowAddress;
+      }
 
-    // Create a unified coverage map for the symbol.
-    SymbolsToAnalyze->createCoverageMap(
-      executableInfo->getFileName().c_str(), symbolName, size
-    );
+      // 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()