Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Medusa is counting coverage multiple times during the contract construction #484

Open
ggrieco-tob opened this issue Sep 19, 2024 · 3 comments
Labels
bug Something isn't working high-priority

Comments

@ggrieco-tob
Copy link
Member

Given the following contract:

contract C {
}

contract Test {
  C x;
  constructor() {
    new C();
  }

  function h() public {
    x = new C(); 
  }
}

You can run medusa like this:

$ medusa fuzz --compilation-target test.sol --target-contracts Test
...
⇾ fuzz: elapsed: 3s, calls: 97787 (32594/sec), seq/s: 324, coverage: 1854, corpus: 100, failures: 0/974, gas/s: 2321324861

While medusa correctly executes only the f function (you can see it on the coverage report), the coverage count is incremented up to 1.8k. However, echidna PC counting is around 10 times smaller:

$ echidna test.sol --contract Test --test-mode assertion --test-limit 10000000
...
Unique instructions: 157
@0xalpharush
Copy link
Contributor

0xalpharush commented Sep 19, 2024

We aren't deduplicating by hash. It's possible the two code hashes have disjoint coverage so we can't just ignore a map if it's hash has already been seen. I do wonder if it makes sense to consider two different contracts with the same code as uniquely covered e.g. if the contract is only called under some circumstance, reaching that coverage is unique and not fungible

for _, contractCoverageMap := range mapsByAddress {
// TODO: Note we are not checking for nil dereference here because we are guaranteed that the successful
// coverage and reverted coverage arrays have been instantiated if we are iterating over it
// Iterate across each PC in the successful coverage array
// We do not separately iterate over the reverted coverage array because if there is no data about a
// successful PC execution, then it is not possible for that PC to have ever reverted either
for i, hits := range contractCoverageMap.successfulCoverage.executedFlags {
// If we hit the PC at least once, we have a unique PC hit
if hits != 0 {
uniquePCs++

@ggrieco-tob
Copy link
Member Author

From the user perspective, the issue here is that with the current approach medusa thinks that it is exploring more, but in reality, it is not. But it keeps adding sequences into the corpus that are useless.

@0xalpharush
Copy link
Contributor

It's not being added to the corpus AFAICT bc that uses the codehash.

@anishnaik anishnaik added bug Something isn't working high-priority labels Oct 15, 2024
@anishnaik anishnaik added this to the Release 0.1.9 milestone Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high-priority
Projects
None yet
Development

No branches or pull requests

3 participants