[DO NOT MERGE] Revert "feat: display success/revert hit count in coverage report (#3…

fuzzing/coverage/coverage_maps.go

@@ -1,13 +1,13 @@
 package coverage
 
 import (
-	"golang.org/x/exp/slices"
+	"bytes"
+	"sync"
 
 	compilationTypes "github.com/crytic/medusa/compilation/types"
 	"github.com/crytic/medusa/utils"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
-	"sync"
 )
 
 // CoverageMaps represents a data structure used to identify instruction execution coverage of various smart contracts
@@ -169,8 +169,8 @@ func (cm *CoverageMaps) Update(coverageMaps *CoverageMaps) (bool, bool, error) {
 	return successCoverageChanged, revertedCoverageChanged, nil
 }
 
-// UpdateAt updates the hit count of a given program counter location within code coverage data.
-func (cm *CoverageMaps) UpdateAt(codeAddress common.Address, codeLookupHash common.Hash, codeSize int, pc uint64) (bool, error) {
+// SetAt sets the coverage state of a given program counter location within code coverage data.
+func (cm *CoverageMaps) SetAt(codeAddress common.Address, codeLookupHash common.Hash, codeSize int, pc uint64) (bool, error) {
 	// If the code size is zero, do nothing
 	if codeSize == 0 {
 		return false, nil
@@ -211,7 +211,7 @@ func (cm *CoverageMaps) UpdateAt(codeAddress common.Address, codeLookupHash comm
 	}
 
 	// Set our coverage in the map and return our change state
-	changedInMap, err = coverageMap.updateCoveredAt(codeSize, pc)
+	changedInMap, err = coverageMap.setCoveredAt(codeSize, pc)
 
 	return addedNewMap || changedInMap, err
 }
@@ -318,18 +318,18 @@ func (cm *ContractCoverageMap) update(coverageMap *ContractCoverageMap) (bool, b
 	return successfulCoverageChanged, revertedCoverageChanged, nil
 }
 
-// updateCoveredAt updates the hit counter at a given program counter location within a ContractCoverageMap used for
+// setCoveredAt sets the coverage state at a given program counter location within a ContractCoverageMap used for
 // "successful" coverage (non-reverted).
 // Returns a boolean indicating whether new coverage was achieved, or an error if one occurred.
-func (cm *ContractCoverageMap) updateCoveredAt(codeSize int, pc uint64) (bool, error) {
+func (cm *ContractCoverageMap) setCoveredAt(codeSize int, pc uint64) (bool, error) {
 	// Set our coverage data for the successful path.
-	return cm.successfulCoverage.updateCoveredAt(codeSize, pc)
+	return cm.successfulCoverage.setCoveredAt(codeSize, pc)
 }
 
 // CoverageMapBytecodeData represents a data structure used to identify instruction execution coverage of some init
 // or runtime bytecode.
 type CoverageMapBytecodeData struct {
-	executedFlags []uint
+	executedFlags []byte
 }
 
 // Reset resets the bytecode coverage map data to be empty.
@@ -343,29 +343,27 @@ func (cm *CoverageMapBytecodeData) Equal(b *CoverageMapBytecodeData) bool {
 	// Return an equality comparison on the data, ignoring size checks by stopping at the end of the shortest slice.
 	// We do this to avoid comparing arbitrary length constructor arguments appended to init bytecode.
 	smallestSize := utils.Min(len(cm.executedFlags), len(b.executedFlags))
-	// TODO: Currently we are checking equality by making sure the two maps have the same hit counts
-	//  it may make sense to just check that both of them are greater than zero
-	return slices.Equal(cm.executedFlags[:smallestSize], b.executedFlags[:smallestSize])
+	return bytes.Equal(cm.executedFlags[:smallestSize], b.executedFlags[:smallestSize])
 }
 
-// HitCount returns the number of times that the provided program counter (PC) has been hit. If zero is returned, then
-// the PC has not been hit, the map is empty, or the PC is out-of-bounds
-func (cm *CoverageMapBytecodeData) HitCount(pc int) uint {
+// IsCovered checks if a given program counter location is covered by the map.
+// Returns a boolean indicating if the program counter was executed on this map.
+func (cm *CoverageMapBytecodeData) IsCovered(pc int) bool {
 	// If the coverage map bytecode data is nil, this is not covered.
 	if cm == nil {
-		return 0
+		return false
 	}
 
 	// If this map has no execution data or is out of bounds, it is not covered.
 	if cm.executedFlags == nil || len(cm.executedFlags) <= pc {
-		return 0
+		return false
 	}
 
-	// Otherwise, return the hit count
-	return cm.executedFlags[pc]
+	// Otherwise, return the execution flag
+	return cm.executedFlags[pc] != 0
 }
 
-// update updates the hit count of the current CoverageMapBytecodeData with the provided one.
+// update creates updates the current CoverageMapBytecodeData with the provided one.
 // Returns a boolean indicating whether new coverage was achieved, or an error if one was encountered.
 func (cm *CoverageMapBytecodeData) update(coverageMap *CoverageMapBytecodeData) (bool, error) {
 	// If the coverage map execution data provided is nil, exit early
@@ -382,33 +380,28 @@ func (cm *CoverageMapBytecodeData) update(coverageMap *CoverageMapBytecodeData)
 	// Update each byte which represents a position in the bytecode which was covered.
 	changed := false
 	for i := 0; i < len(cm.executedFlags) && i < len(coverageMap.executedFlags); i++ {
-		// Only update the map if we haven't seen this coverage before
 		if cm.executedFlags[i] == 0 && coverageMap.executedFlags[i] != 0 {
-			cm.executedFlags[i] += coverageMap.executedFlags[i]
+			cm.executedFlags[i] = 1
 			changed = true
 		}
 	}
 	return changed, nil
 }
 
-// updateCoveredAt updates the hit count at a given program counter location within a CoverageMapBytecodeData.
+// setCoveredAt sets the coverage state at a given program counter location within a CoverageMapBytecodeData.
 // Returns a boolean indicating whether new coverage was achieved, or an error if one occurred.
-func (cm *CoverageMapBytecodeData) updateCoveredAt(codeSize int, pc uint64) (bool, error) {
+func (cm *CoverageMapBytecodeData) setCoveredAt(codeSize int, pc uint64) (bool, error) {
 	// If the execution flags don't exist, create them for this code size.
 	if cm.executedFlags == nil {
-		cm.executedFlags = make([]uint, codeSize)
+		cm.executedFlags = make([]byte, codeSize)
 	}
 
-	// If our program counter is in range, determine if we achieved new coverage for the first time or increment the hit counter.
+	// If our program counter is in range, determine if we achieved new coverage for the first time, and update it.
 	if pc < uint64(len(cm.executedFlags)) {
-		// Increment the hit counter
-		cm.executedFlags[pc] += 1
-
-		// This is the first time we have hit this PC, so return true
-		if cm.executedFlags[pc] == 1 {
+		if cm.executedFlags[pc] == 0 {
+			cm.executedFlags[pc] = 1
 			return true, nil
 		}
-		// We have seen this PC before, return false
 		return false, nil
 	}
 

fuzzing/coverage/coverage_tracer.go

@@ -168,7 +168,7 @@ func (t *CoverageTracer) OnOpcode(pc uint64, op byte, gas, cost uint64, scope tr
 		}
 
 		// Record coverage for this location in our map.
-		_, coverageUpdateErr := callFrameState.pendingCoverageMap.UpdateAt(address, *callFrameState.lookupHash, codeSize, pc)
+		_, coverageUpdateErr := callFrameState.pendingCoverageMap.SetAt(address, *callFrameState.lookupHash, codeSize, pc)
 		if coverageUpdateErr != nil {
 			logging.GlobalLogger.Panic("Coverage tracer failed to update coverage map while tracing state", coverageUpdateErr)
 		}

fuzzing/coverage/report_template.gohtml

@@ -190,12 +190,12 @@
                         {{/* Output two cells for the reverted/non-reverted execution status */}}
                         <td class="row-reverted-status unselectable">
                             {{if $line.IsCovered}}
-                                <div title="The source line executed without reverting.">√ {{$line.SuccessHitCount}}</div>
+                                <div title="The source line executed without reverting.">√</div>
                             {{end}}
                         </td>
                         <td class="row-reverted-status unselectable">
                             {{if $line.IsCoveredReverted}}
-                                <div title="The source line executed, but was reverted.">⟳ {{$line.RevertHitCount}}</div>
+                                <div title="The source line executed, but was reverted.">⟳</div>
                             {{end}}
                         </td>
 

fuzzing/coverage/source_analysis.go

@@ -103,12 +103,6 @@ type SourceLineAnalysis struct {
 	// IsCovered indicates whether the source line has been executed without reverting.
 	IsCovered bool
 
-	// SuccessHitCount describes how many times this line was executed successfully
-	SuccessHitCount uint
-
-	// RevertHitCount describes how many times this line reverted during execution
-	RevertHitCount uint
-
 	// IsCoveredReverted indicates whether the source line has been executed before reverting.
 	IsCoveredReverted bool
 }
@@ -220,12 +214,12 @@ func analyzeContractSourceCoverage(compilation types.Compilation, sourceAnalysis
 			continue
 		}
 
-		// Capture the hit count of the source map element.
-		succHitCount := uint(0)
-		revertHitCount := uint(0)
+		// Check if the source map element was executed.
+		sourceMapElementCovered := false
+		sourceMapElementCoveredReverted := false
 		if contractCoverageData != nil {
-			succHitCount = contractCoverageData.successfulCoverage.HitCount(instructionOffsetLookup[sourceMapElement.Index])
-			revertHitCount = contractCoverageData.revertedCoverage.HitCount(instructionOffsetLookup[sourceMapElement.Index])
+			sourceMapElementCovered = contractCoverageData.successfulCoverage.IsCovered(instructionOffsetLookup[sourceMapElement.Index])
+			sourceMapElementCoveredReverted = contractCoverageData.revertedCoverage.IsCovered(instructionOffsetLookup[sourceMapElement.Index])
 		}
 
 		// Obtain the source file this element maps to.
@@ -238,11 +232,9 @@ func analyzeContractSourceCoverage(compilation types.Compilation, sourceAnalysis
 					// Mark the line active/executable.
 					sourceLine.IsActive = true
 
-					// Set its coverage state and increment hit counts
-					sourceLine.SuccessHitCount += succHitCount
-					sourceLine.RevertHitCount += revertHitCount
-					sourceLine.IsCovered = sourceLine.IsCovered || sourceLine.SuccessHitCount > 0
-					sourceLine.IsCoveredReverted = sourceLine.IsCoveredReverted || sourceLine.RevertHitCount > 0
+					// Set its coverage state
+					sourceLine.IsCovered = sourceLine.IsCovered || sourceMapElementCovered
+					sourceLine.IsCoveredReverted = sourceLine.IsCoveredReverted || sourceMapElementCoveredReverted
 
 					// Indicate we matched a source line, so when we stop matching sequentially, we know we can exit
 					// early.

fuzzing/fuzzer_test.go

@@ -840,16 +840,8 @@ func TestCorpusReplayability(t *testing.T) {
 			assertCorpusCallSequencesCollected(f, true)
 			newCoverage := f.fuzzer.corpus.CoverageMaps()
 
-			// Check to see if original and new coverage are the same (disregarding hit count)
-			successCovIncreased, revertCovIncreased, err := originalCoverage.Update(newCoverage)
-			assert.False(t, successCovIncreased)
-			assert.False(t, revertCovIncreased)
-			assert.NoError(t, err)
-
-			successCovIncreased, revertCovIncreased, err = newCoverage.Update(originalCoverage)
-			assert.False(t, successCovIncreased)
-			assert.False(t, revertCovIncreased)
-			assert.NoError(t, err)
+			// Check to see if original and new coverage are the same.
+			assert.True(t, originalCoverage.Equal(newCoverage))
 
 			// Verify that the fuzzer finished after fewer sequences than there are in the corpus
 			assert.LessOrEqual(t, f.fuzzer.metrics.SequencesTested().Uint64(), uint64(originalCorpusSequenceCount))