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

infra: make it possible to do a full introspector run #9243

Merged
merged 11 commits into from Dec 21, 2022

Conversation

DavidKorczynski
Copy link
Collaborator

@DavidKorczynski DavidKorczynski commented Dec 19, 2022

Make it possible to do a full run of introspector locally. This will make it a lot easier for users to integrate it into the fuzzer building workflow.

To trigger, just run: python3 infra/helper.py introspector PROJ_NAME

Other example commands:
python3 infra/helper.py introspector --public-corpora PROJ_NAME : will download the latest public corpus for project PROJ_NAME and use that when collecting coverage
python3 infra/helper.py introspector --seconds=X PROJ_NAME: will run the fuzzers for X seconds for corpus collection
python3 infra/helper.py introspector PROJ_NAME LOCAL_PATH will do the introspector run using the LOCAL_PATH as source code folder (for testing modifications)

CC @evverx -- please let me know if there are other features you might like.

Ref: ossf/fuzz-introspector#587

Signed-off-by: David Korczynski david@adalogics.com

Signed-off-by: David Korczynski <david@adalogics.com>
@evverx
Copy link
Contributor

evverx commented Dec 19, 2022

I think it would be great if it was possible to point helper.py to local directories using something like:

./infra/helper.py introspector dbus-broker ~/dbus-broker/
usage: helper.py [-h] {generate,build_image,build_fuzzers,check_build,run_fuzzer,coverage,introspector,download_corpora,reproduce,shell,run_clusterfuzzlite,pull_images} ...
helper.py: error: unrecognized arguments: /home/vagrant/dbus-broker/

by analogy with ossf/fuzz-introspector#637. I don't think it can work out of the box because helper.py doesn't clean up after itself but it should be possible to get it around by editing build scripts or pointing helper.py to podman with a patch replacing '%s:%s' with '%s:%s:O' (which leads to modifications to build directories being destroyed when the container stops).

@evverx
Copy link
Contributor

evverx commented Dec 19, 2022

please let me know if there are other features you might like

In general I think ossf/fuzz-introspector#6 would be a feature I'd pick once it's easy to build FI reports because usually depending on the project new fuzz targets should cover places that aren't even shown by FI because OSS-Fuzz builds are reduced as much as possible. FI helps to improve existing fuzz targets though and new ones once blind spots found with "full" coverage reports are more or less covered. It has nothing to do with this PR though.

@DavidKorczynski
Copy link
Collaborator Author

DavidKorczynski commented Dec 20, 2022

I think it would be great if it was possible to point helper.py to local directories using something like:

./infra/helper.py introspector dbus-broker ~/dbus-broker/
usage: helper.py [-h] {generate,build_image,build_fuzzers,check_build,run_fuzzer,coverage,introspector,download_corpora,reproduce,shell,run_clusterfuzzlite,pull_images} ...
helper.py: error: unrecognized arguments: /home/vagrant/dbus-broker/

by analogy with ossf/fuzz-introspector#637. I don't think it can work out of the box because helper.py doesn't clean up after itself but it should be possible to get it around by editing build scripts or pointing helper.py to podman with a patch replacing '%s:%s' with '%s:%s:O' (which leads to modifications to build directories being destroyed when the container stops).

Does this argument to build_fuzzers do enough in this context?

oss-fuzz/infra/helper.py

Lines 278 to 280 in ba8bea4

build_fuzzers_parser.add_argument('source_path',
help='path of local source',
nargs='?')

If so, I'll reuse that logic -- but it has had some issues in the past where naming of folders conflicts a bit (#7634 ), so wanted to check up

@DavidKorczynski
Copy link
Collaborator Author

please let me know if there are other features you might like

In general I think ossf/fuzz-introspector#6 would be a feature I'd pick once it's easy to build FI reports because usually depending on the project new fuzz targets should cover places that aren't even shown by FI because OSS-Fuzz builds are reduced as much as possible. FI helps to improve existing fuzz targets though and new ones once blind spots found with "full" coverage reports are more or less covered. It has nothing to do with this PR though.

Good idea, this is something I'd really like too.

Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
@evverx
Copy link
Contributor

evverx commented Dec 20, 2022

Does this argument to build_fuzzers do enough in this context?

I think it should work as long as it's passed on to build_fuzzers --sanitizer=address, --sanitizer=coverage and --sanitizer=introspector.

it has had some issues in the past where naming of folders conflicts a bit (#7634 )

I haven't seen that before. Let me take a look.

@evverx
Copy link
Contributor

evverx commented Dec 20, 2022

I took a look at #7634 and it appears it shouldn't affect introspector (or any other command) as long as the layout of local directories match the layout created (and expected) by Dockerfiles and build scripts.

@evverx
Copy link
Contributor

evverx commented Dec 20, 2022

I opened #9249. I'm not sure it's going to be fixed but it should at least somewhat help to document the status quo and given that it's easy to get it around by just cleaning build directories in build scripts I don't think it should prevent the introspector command from accepting paths to local dirs.

Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
@DavidKorczynski DavidKorczynski marked this pull request as ready for review December 20, 2022 19:35
Signed-off-by: David Korczynski <david@adalogics.com>
@DavidKorczynski
Copy link
Collaborator Author

@evverx could you take a look at this now? I think I got all of the features that we discussed (with exception of the cleaning up in build). From my personal perspective it has the parts that I use when working with introspector locally

@DavidKorczynski
Copy link
Collaborator Author

@Navidem -- WDYT? This will make introspector a lot more accessible for developers by making it straightforward to test and observe results of modifications.

infra/helper.py Show resolved Hide resolved
infra/helper.py Outdated Show resolved Hide resolved
infra/helper.py Show resolved Hide resolved
@Navidem
Copy link
Contributor

Navidem commented Dec 20, 2022

@Navidem -- WDYT? This will make introspector a lot more accessible for developers by making it straightforward to test and observe results of modifications.

Thanks for the PR!
+1 it is very useful and brings introspector closer to the development pipline.

Copy link
Contributor

@Navidem Navidem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
@DavidKorczynski DavidKorczynski merged commit 7d27c4c into master Dec 21, 2022
@DavidKorczynski DavidKorczynski deleted the integrate-introspector-command branch December 21, 2022 12:48
@evverx
Copy link
Contributor

evverx commented Dec 21, 2022

@DavidKorczynski thanks! I haven't tested it yet but I think it should work. If there are any issues I'll try to fix/report them.

Now that it's merged I think it would be great if it was possible to update the FI documentation. I think it still says that the OSS-Fuzz images should be replaced with the FI images (or the custom clang should be built) and that is kind of a steep learning curve I believe (though building the custom clang was fun in resource-constrained VMs :-)).

evverx added a commit to evverx/oss-fuzz that referenced this pull request Dec 22, 2022
Those directories aren't empty usually so `rmdir` fails with
```
INFO:fuzz_introspector.json_report:Finish handling sections that need json output
INFO:__main__:Ending fuzz introspector report generation
INFO:__main__:Ending fuzz introspector post-processing
Traceback (most recent call last):
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 1513, in <module>
    sys.exit(main())
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 192, in main
    result = introspector(args)
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 1243, in introspector
    os.rmdir(introspector_dst)
OSError: [Errno 39] Directory not empty: '/home/vagrant/oss-fuzz-2/build/out/dbus-broker/introspector-report'
```

It should make it possible to run `introspector` a few times in a row
when for example fuzz targets are changed locally between subsequent
runs.

It's a follow-up to google#9243.
Navidem pushed a commit that referenced this pull request Dec 22, 2022
Those directories aren't empty usually so `rmdir` fails with
```
INFO:fuzz_introspector.json_report:Finish handling sections that need json output
INFO:__main__:Ending fuzz introspector report generation
INFO:__main__:Ending fuzz introspector post-processing
Traceback (most recent call last):
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 1513, in <module>
    sys.exit(main())
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 192, in main
    result = introspector(args)
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 1243, in introspector
    os.rmdir(introspector_dst)
OSError: [Errno 39] Directory not empty: '/home/vagrant/oss-fuzz-2/build/out/dbus-broker/introspector-report'
```

It should make it possible to run `introspector` a few times in a row
when for example fuzz targets are changed locally between subsequent
runs.

It's a follow-up to #9243.
@DavidKorczynski
Copy link
Collaborator Author

@DavidKorczynski thanks! I haven't tested it yet but I think it should work. If there are any issues I'll try to fix/report them.

Now that it's merged I think it would be great if it was possible to update the FI documentation. I think it still says that the OSS-Fuzz images should be replaced with the FI images (or the custom clang should be built) and that is kind of a steep learning curve I believe (though building the custom clang was fun in resource-constrained VMs :-)).

Thanks for the reminder @evverx -- fixed here ossf/fuzz-introspector#731

eamonnmcmanus pushed a commit to eamonnmcmanus/oss-fuzz that referenced this pull request Mar 15, 2023
Make it possible to do a full run of introspector locally. This will
make it a lot easier for users to integrate it into the fuzzer building
workflow.

To trigger, just run: `python3 infra/helper.py introspector PROJ_NAME`

Other example commands:
`python3 infra/helper.py introspector --public-corpora PROJ_NAME` : will
download the latest public corpus for project PROJ_NAME and use that
when collecting coverage
`python3 infra/helper.py introspector --seconds=X PROJ_NAME`: will run
the fuzzers for X seconds for corpus collection
`python3 infra/helper.py introspector PROJ_NAME LOCAL_PATH` will do the
introspector run using the LOCAL_PATH as source code folder (for testing
modifications)

Ref: ossf/fuzz-introspector#587

Signed-off-by: David Korczynski <david@adalogics.com>
eamonnmcmanus pushed a commit to eamonnmcmanus/oss-fuzz that referenced this pull request Mar 15, 2023
Those directories aren't empty usually so `rmdir` fails with
```
INFO:fuzz_introspector.json_report:Finish handling sections that need json output
INFO:__main__:Ending fuzz introspector report generation
INFO:__main__:Ending fuzz introspector post-processing
Traceback (most recent call last):
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 1513, in <module>
    sys.exit(main())
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 192, in main
    result = introspector(args)
  File "/home/vagrant/oss-fuzz-2/./infra/helper.py", line 1243, in introspector
    os.rmdir(introspector_dst)
OSError: [Errno 39] Directory not empty: '/home/vagrant/oss-fuzz-2/build/out/dbus-broker/introspector-report'
```

It should make it possible to run `introspector` a few times in a row
when for example fuzz targets are changed locally between subsequent
runs.

It's a follow-up to google#9243.
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

Successfully merging this pull request may close these issues.

None yet

3 participants