Added corpus of arrays to use as inputs for the functions. Check that the two
functions modify the inputted arrays in the same way.
Details
Diff Detail
- Repository
- rC Clang
Event Timeline
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. |
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. |
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp | ||
---|---|---|
173 ↗ | (On Diff #158782) | Or just increase the scope of EE. |
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?
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp | ||
---|---|---|
128 ↗ | (On Diff #159083) | looks like code duplication, also strange name for a variable: 'x'. |
clang/tools/clang-fuzzer/handle-llvm/input_arrays.h | ||
36 ↗ | (On Diff #159083) | constants are not diverse enough. |
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. |
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.
clang/tools/clang-fuzzer/handle-llvm/input_arrays.h | ||
---|---|---|
36 ↗ | (On Diff #159083) | Yeah, I will completely redo the input arrays. |
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. |