Page MenuHomePhabricator

LLVM Proto Fuzzer - Run Functions on Suite of Inputs
ClosedPublic

Authored by emmettneyman on Aug 2 2018, 9:44 AM.

Details

Summary

Added corpus of arrays to use as inputs for the functions. Check that the two
functions modify the inputted arrays in the same way.

Diff Detail

Repository
rC Clang

Event Timeline

emmettneyman created this revision.Aug 2 2018, 9:44 AM
morehouse added inline comments.Aug 2 2018, 10:35 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
173 ↗(On Diff #158782)

Why do we need to copy the function somewhere else? Looks very error-prone and unnecessary. Also makes this patch larger than it needs to be.

clang/tools/clang-fuzzer/handle-llvm/input_arrays.cpp
30 ↗(On Diff #158782)

Do the generated functions ever modify arrays a and b, or just c? If just c, we can avoid lots of memcpys here.

clang/tools/clang-fuzzer/handle-llvm/input_arrays.h
34 ↗(On Diff #158782)

Use the constants you just defined.

emmettneyman added inline comments.Aug 2 2018, 10:58 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
173 ↗(On Diff #158782)

I'm copying the functions because otherwise, the generated machine code gets lost as soon as we exit that function's scope. So I'd have to run the functions inside CreateJITFunction if I don't copy it.

I thought about doing it this way: moving the code from RunFuncsOnInputs to the bottom of CreateJITFunction and then comparing the arrays after both calls to CreateJITFunction inside HandleLLVM. Do you think that would be cleaner?

clang/tools/clang-fuzzer/handle-llvm/input_arrays.cpp
30 ↗(On Diff #158782)

Right now the generated functions can modify any of the arrays.

Replaced hardcoded numbers with variables

morehouse added inline comments.Aug 2 2018, 11:18 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
173 ↗(On Diff #158782)

Or just increase the scope of EE.

emmettneyman added inline comments.Aug 3 2018, 10:57 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
173 ↗(On Diff #158782)

When I tried to increase the scope of EE (and also M since EntryFunc lives inside the module), the program segfaulted immediately after exiting CreateJITFunc, I think while trying to deconstruct an object from the function. I'm not sure if there's a way around this since there are so many objects being created inside CreateJITFunc. But it's definitely possible I'm missing something with how unique_ptr work.

An unrelated question:
Right now I have a mix of static and non-static functions in handle_llvm.cpp. Should they all be static?

Refactored code to avoid memcpy-ing function

kcc added inline comments.Aug 3 2018, 2:25 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
128 ↗(On Diff #159083)

looks like code duplication, also strange name for a variable: 'x'.
Can't we just have one loop here and pass OptArrays/UnoptArrays as the parameter?

clang/tools/clang-fuzzer/handle-llvm/input_arrays.h
36 ↗(On Diff #159083)

constants are not diverse enough.

morehouse added inline comments.Aug 3 2018, 2:41 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
75 ↗(On Diff #159083)

Unnecessary comment. The naming and implementation of this function are intuitive.

170 ↗(On Diff #159083)

Does reinterpret_cast work here?

184 ↗(On Diff #159083)

The size here is reused a few times. Let's create a variable for it.

An unrelated question:
Right now I have a mix of static and non-static functions in handle_llvm.cpp. Should they all be static?

Any functions that are only used in the same file can and should be static or in an unnamed namespace. Functions that implement something from a header file must not be static.

emmettneyman added inline comments.Aug 3 2018, 2:48 PM
clang/tools/clang-fuzzer/handle-llvm/input_arrays.h
36 ↗(On Diff #159083)

Yeah, I will completely redo the input arrays.

Added static to some functions, made small fixes

morehouse accepted this revision.Aug 3 2018, 5:15 PM
morehouse added inline comments.
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
132 ↗(On Diff #159100)

CreateAndRunJITFunc describes this better.

clang/tools/clang-fuzzer/handle-llvm/input_arrays.h
15 ↗(On Diff #159100)

Are these includes needed?

19 ↗(On Diff #159100)

Maybe prefix these names with k since they are constant.

36 ↗(On Diff #159083)

Please land today once you do.

This revision is now accepted and ready to land.Aug 3 2018, 5:15 PM

New input arrays and minor fixes

ready to land

This revision was automatically updated to reflect the committed changes.