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

(Potentially) broken resiprocate build #826

Closed
Dor1s opened this issue Sep 6, 2017 · 6 comments
Closed

(Potentially) broken resiprocate build #826

Dor1s opened this issue Sep 6, 2017 · 6 comments

Comments

@Dor1s
Copy link
Contributor

Dor1s commented Sep 6, 2017

https://oss-fuzz-build-logs.storage.googleapis.com/index.html

Step #4: ++ sancov -print-coverage-pcs /workspace/out/address/aresfuzz
Step #4: ++ wc -l
Step #4: + local NUMBER_OF_EDGES=89
Step #4: + ((  89 < 90  ))
Step #4: + echo 'BAD BUILD: the code does not seem to have coverage instrumentation.'
Step #4: + CHECK_FAILED=1
Step #4: + ((  1 > 0  ))
Step #4: + exit 1
Step #4: BAD BUILD: the code does not seem to have coverage instrumentation.

89 is so close to the threshold, need to take a closer look whether the target is so small or instrumentation flags are being handled improperly

Dor1s added a commit that referenced this issue Sep 6, 2017
…t fixed.

Need to take a closer look at the following issues:
- #815
- #816
- #817
- #818
- #819
- #820
- #821
- #822
- #823
- #824
- #825
- #826
- #827
- #828
- #829
- #830
oliverchang pushed a commit that referenced this issue Sep 7, 2017
…t fixed. (#831)

Need to take a closer look at the following issues:
- #815
- #816
- #817
- #818
- #819
- #820
- #821
- #822
- #823
- #824
- #825
- #826
- #827
- #828
- #829
- #830
@Dor1s
Copy link
Contributor Author

Dor1s commented Sep 8, 2017

Well, this one seems legit. I haven't noticed any issues with instrumentation. However, there is one more target which is even smaller:

root@07916516ef2c:/out# sancov -print-coverage-pcs ./aresfuzz | wc -l
89
root@07916516ef2c:/out# sancov -print-coverage-pcs ./aresfuzzname | wc -l
21

I'm not sure that second target makes sense at all... e.g. if I instrument a dummy program like the following one:

#include <stddef.h>
#include <stdint.h>
#include <string.h>

extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
  int result = 0;
  if (size % 2) {
    result = strlen((const char*)data);
  }
  if (size % 4) {
    result *= result;
  }
  for (int i = 0; i < result; ++i) {
    result -= i;
  }
  if (result > 0) return 0;
  return 0;
}

It gives me 6 edges:

root@07916516ef2c:/out# sancov -print-coverage-pcs ./test | wc -l
6

So, the second fuzz target named aresfuzzname is just 3.5X more complex that my useless program listed above.

@gjasny, is there a way to extend those fuzz targets to cover more code paths?

@gjasny
Copy link
Contributor

gjasny commented Sep 17, 2017

Hello,

the currently activated fuzzing targets in resiprocate just cover the embedded ares library. Those tests are copied / inspired by the c-ares projects fuzz targets. The fuzz-name test could be found here. Extending the ares might be possible, but as a first step, combining the existing ares fuzz targets into one might be feasible.

For resiprocate in general I have a PR pending to add a broader coverage across the whole project: resiprocate/resiprocate#82

Thanks,
Gregor

@Dor1s
Copy link
Contributor Author

Dor1s commented Sep 18, 2017

Thanks for taking a look! Let's wait for your PR to land.

@Dor1s
Copy link
Contributor Author

Dor1s commented Mar 1, 2018

resiprocate/resiprocate#82 is merged, but I don't see those new fuzz targets on OSS-Fuzz: https://oss-fuzz.com/v2/fuzzer-stats/by-fuzzer/2018-02-22/2018-02-28/fuzzer/libFuzzer/job/libfuzzer_asan_resiprocate

Could you please send a pull request updating https://github.com/google/oss-fuzz/blob/master/projects/resiprocate/build.sh ?

The best move would be write it in a such way that it would copy all fuzz targets at once, so you won't need to update it again after adding a new target upstream.

Instrumentation numbers are still low for existing fuzz targets, but that's fine. We'll find another way to check instrumentation.

root@06363dbf5730:/out# objdump -t aresfuzz | egrep -c __sancov    
20
root@06363dbf5730:/out# objdump -t aresfuzzname | egrep -c __sancov
21

@gjasny
Copy link
Contributor

gjasny commented Mar 3, 2018

I filed #1206 to add the new targets.

@inferno-chromium
Copy link
Collaborator

Duplicate of #1331

@inferno-chromium inferno-chromium marked this as a duplicate of #1331 Apr 22, 2018
tmatth pushed a commit to tmatth/oss-fuzz that referenced this issue Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants