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

Code coverage support for gofuzz projects #2817

Closed
Dor1s opened this issue Sep 9, 2019 · 14 comments
Closed

Code coverage support for gofuzz projects #2817

Dor1s opened this issue Sep 9, 2019 · 14 comments

Comments

@Dor1s
Copy link
Contributor

Dor1s commented Sep 9, 2019

There was a good discussion in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=16549

better to continue it here //cc @guidovranken @dvyukov

@Dor1s
Copy link
Contributor Author

Dor1s commented Sep 9, 2019

This is blocking #2714

@Dor1s
Copy link
Contributor Author

Dor1s commented Dec 12, 2019

One idea how to implement this was suggested by @catenacyber in #3106.

It sounds doable, but I'm not sure if that's the best way to move forward, as well as I'm not sure anyone plans to work on it in the foreseeable future.

An extra context for this is that we might change the way how we build fuzz targets (I think there are some changes in the most recent release), that might affect our coverage plans.

@catenacyber
Copy link
Contributor

Which code should I try to modify to get my proposition running ? infra/base-images/base-runner/coverage ?

It sounds doable, but I'm not sure if that's the best way to move forward, as well as I'm not sure anyone plans to work on it in the foreseeable future.

I can try to propose something even if it is not perfect, I think it can still be useful

An extra context for this is that we might change the way how we build fuzz targets (I think there are some changes in the most recent release), that might affect our coverage plans.

Well, my methodology (which is like the one discussed in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=16549) should not depend on how the fuzz targets are built.
Coverage should be measured by another run (and not while fuzzing)

@Dor1s
Copy link
Contributor Author

Dor1s commented Dec 13, 2019

infra/base-images/base-runner/coverage is a runtime script, but we also need a build config

as of now we have --sanitizer coverage to distinguish coverage build, but there is no flag to distinguish a Go project from C/C++ project

so we might need to invent it, or consider some other name for sanitizer, or call it a new fuzzing engine instead

that's why I'd rather wait on further gofuzz developments to see a standard way of generating coverage

If you want to experiment with you solution, you can try adding extra code to your build.sh under

if [[ $SANITIZER = *coverage* ]]; then
  exit 0
fi

and updating the infra/base-images/base-runner/coverage, but again you'd need some indicator that you're running a Go project and default coverage workload should be skipped

@Dor1s
Copy link
Contributor Author

Dor1s commented Nov 10, 2020

@catenacyber after we land #3142, two more things need to be done:

  1. in infra/base-images/base-runner/coverage we need to generate SUMMARY_FILE="$REPORT_PLATFORM_DIR/summary.json" file in the following format:
{
    "data": [
        {
            "totals": {
                "functions": {
                    "count": 171,
                    "covered": 113,
                    "percent": 66.08187134502924
                },
                "lines": {
                    "count": 5610,
                    "covered": 3820,
                    "percent": 68.09269162210339
                },
                "regions": {
                    "count": 5263,
                    "covered": 3478,
                    "notcovered": 1785,
                    "percent": 66.08398251947558
                }
            }
        }
    ],
    "type": "oss-fuzz.go.coverage.json.export",
    "version": "1.0.0"
}

functions and regions values are the most important. Originally, regions term is coming from Clang Source-based Code Coverage. Logically, it's kinda similar to "edge coverage" or "basic block coverage". Can you derive a similar value from the Go coverage tooling you use?

lines is also nice to have, but it's not a must-have, as we don't parse in ClusterFuzz: https://github.com/google/clusterfuzz/blob/1d27ac053f2cefaa5f71722d83b264acbfb49f61/src/appengine/handlers/cron/fuzzer_coverage.py#L74

If that is doable, we should be able to enable Go coverage reports generation on the OSS-Fuzz build infrastructure.

  1. we need to create a compile_go_fuzzer bash script in the base-builder image. That script will be implementing the compile_fuzzer function, including the commands you've added for making Go coverage work. Once that script is implemented and placed in the base-builder image, we need to migrate go-dns, gonids, and all other Go projects to use it instead of the copy-pasted compile_fuzzer function.

    Alternatively, we can create a wrapper for go-fuzz that would check the $SANITIZER value and if it's coverage it'll execute your set of commands needed for coverage report generation. For any other sanitizer it'll just invoke the real go-fuzz as-is. This approach is probably easier to implement, but IMO it's less preferred.

//cc @inferno-chromium @oliverchang

@Dor1s
Copy link
Contributor Author

Dor1s commented Nov 10, 2020

And if/when 1) is done, we'll need to slightly update infra build scripts to enable code coverage for Go language. The change is trivial, but only someone from OSS-Fuzz team would be able to test it, as that would require access to the OSS-Fuzz build infra.

@dvyukov
Copy link
Contributor

dvyukov commented Nov 11, 2020

CC @katiehockman re what go-fuzz support is needed for OSS-Fuzz integration.

@catenacyber
Copy link
Contributor

1 seems doable with some new golang script

2 is doable as well
There is a slight difference between compile_fuzzer in go-dns and gonids : the tags passed to go-fuzz
I will take care of that

@catenacyber
Copy link
Contributor

Here is a proposal for 1
https://github.com/catenacyber/gocovsum/blob/main/gocovsum.go

Example output is

{
  "data": [
    {
      "totals": {
        "functions": {
          "count": 106,
          "covered": 105,
          "uncovered": 1,
          "percent": 99.05660377358491
        },
        "lines": {
          "count": 956,
          "covered": 836,
          "uncovered": 120,
          "percent": 87.44769874476988
        },
        "regions": {
          "count": 682,
          "covered": 569,
          "uncovered": 113,
          "percent": 83.43108504398828
        }
      }
    }
  ],
  "type": "oss-fuzz.go.coverage.json.export",
  "version": "1.0.0"
}

Does it look good to you ?

If you look into the code, you will see that functions coverage is not straightforward

@catenacyber
Copy link
Contributor

I can also add the performance profiling while running a target over a corpus.
I have found this useful.
But I am not sure if this fits into your model.

@Dor1s
Copy link
Contributor Author

Dor1s commented Nov 12, 2020

Here is a proposal for 1
https://github.com/catenacyber/gocovsum/blob/main/gocovsum.go

Looks great!

I can also add the performance profiling while running a target over a corpus.

I'd leave it out of scope for now, to avoid unnecessary complications. But it may be interesting in future, so it's good to know //cc @inferno-chromium

@catenacyber
Copy link
Contributor

Working on converting all the projects, it seems 2 projects with project.yaml having language: go do not have build.sh files :
excelize and go-ethereum
What should I do with them ?

@Dor1s
Copy link
Contributor Author

Dor1s commented Nov 20, 2020

@catenacyber the latest version failed on the builder during the fuzzer stats uploading:

upload_fuzzer_stats_url = UPLOAD_URL_FORMAT.format(project=project_name,
type='fuzzer_stats',
date=report_date)
build_steps.append(build_lib.gsutil_rm_rf_step(upload_fuzzer_stats_url))
build_steps.append({
'name':
'gcr.io/cloud-builders/gsutil',
'args': [
'-m',
'cp',
'-r',
os.path.join(out, 'fuzzer_stats'),
upload_fuzzer_stats_url,
],
})

that fuzzer stats directory (

llvm-cov export -summary-only -instr-profile=$profdata_file -object=$target \
$shared_libraries $LLVM_COV_COMMON_ARGS > $FUZZER_STATS_DIR/$target.json
) is supposed to have json files for each individual fuzz target. The jsons are identical to the summary.json file you generate for the whole project.

In other words, those are summary.json files for every single fuzz target. Would it be possible to generate those as well?

Dor1s added a commit that referenced this issue Nov 20, 2020
…4671)

* Golang coverage summary for each fuzz target

* Document usage of compile_go_fuzzer

* update the documentation change

Co-authored-by: Max Moroz <mmoroz@chromium.org>
@jonathanmetzman
Copy link
Contributor

I think @catenacyber has completed this work.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants