This is an archive of the discontinued LLVM Phabricator instance.

Updated llvm-proto-fuzzer to execute the compiled code
ClosedPublic

Authored by emmettneyman on Jul 18 2018, 5:43 PM.

Details

Summary

Made changes to the llvm-proto-fuzzer

  • Added loop vectorizer optimization pass in order to have two IR versions
  • Updated old fuzz target to handle two different IR versions
  • Wrote code to execute both versions in memory

Diff Detail

Event Timeline

emmettneyman created this revision.Jul 18 2018, 5:43 PM

The files

Object.h
Object.cpp
llvm-objcopy.h

are from llvm/tools/llvm-obj-copy with only slight modifications, mostly deleting irrelevant parts.

pcc added a subscriber: pcc.Jul 18 2018, 5:52 PM
pcc added inline comments.
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
200

Did you look at using LLVM's JIT infrastructure to do this?

You can probably get rid of the llvm-objcopy code and make this a lot simpler with something like:

  1. Call getSection() on the Binary object to get the text section.
  2. Read the sh_offset and sh_size of that section.
  3. Copy sh_size bytes from the start of the binary buffer + sh_offset into your executable memory.
  4. Run it.
  • Switched to JIT for compilation and execution
  • Cleaned up leftover code from mmap memcpy
  • Fixed typo that broke build
morehouse added inline comments.Jul 19 2018, 3:20 PM
clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
21

How are you doing your diff? Some of these changes are already upstream. Please rebase

clang/tools/clang-fuzzer/handle-llvm/fuzzer_initialize.cpp
52 ↗(On Diff #156370)

Can we use fuzzer-initialize/ now?

clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
54

Unless there's a good reason for global, let's make this a local.

pcc added inline comments.Jul 19 2018, 3:22 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
100–113

Can you move this code (and maybe more of the duplicated code below) into a function and call it twice?

Made fixes to patch, rebased CMake file

Cleaned up code

Tried to get rid of ParseCommandLineOptions() call but could not figure out
how to initialize a PassInfo object without it.

morehouse added inline comments.Jul 24 2018, 4:04 PM
clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
17

Typo introduced here.

25

Please don't add whitespace to a previously empty line.

clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
10–13

an -> a

82

Move this closer to where it's used.

83

Options is never used.

86

TM is never used.

87

Does this do anything since CPUStr and FeaturesStr are empty?

95

Can we just make FPasses on stack instead?

108

These 3 lines can be combined to builder.setMCJITMemoryManager(new SectionMemoryManager())

110

Maybe this should be const std::string &IR or StringRef.

111

Does this need to be called every time we get a new input? Can we call this from LLVMFuzzerInitialize()?

121

This shouldn't be required. You should be able to directly add the passes you want. See llvm/lib/Passes/PassBuilder.cpp.

emmettneyman added inline comments.Jul 24 2018, 5:36 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
108

I use RTDyldMM on line 208. Should I just keep these lines as they are?

morehouse added inline comments.Jul 24 2018, 5:43 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
108

Ah, missed that. In that case, you can probably simplify this line to
builder.setMCJITMemoryManager(RTDyldMM).

emmettneyman added inline comments.Jul 25 2018, 10:53 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
108

It looks like set MCJITMemoryManager() needs to take a unique_ptr. I'm not sure how to clean it up any more than it already is.

morehouse added inline comments.Jul 25 2018, 11:03 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
108

Does it not implicitly cast?

emmettneyman added inline comments.Jul 25 2018, 11:11 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
108

No, I think it only works in the other direction.

morehouse added inline comments.Jul 25 2018, 11:24 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
108

Ok, I see. The constructor we need is marked explicit...

113

This cast shouldn't be necessary.

emmettneyman added inline comments.Jul 25 2018, 12:02 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
113

Turns out this line is redundant anyways. EE->finalizeObject()calls invalidateInstructionCache().

  • cleaned up code and moved initialization code
  • removed fake command line parsing
morehouse added inline comments.Jul 25 2018, 1:07 PM
clang/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp
44

Unnecessary llvm::

clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
80

If we do this, do we need to explicitly add the vectorizer pass below?

84

Let's simplify to setFunctionAttributes(getCPUStr(), getFeaturesStr(), *M).

86

OLvl is never used.

89

Can simplify to Passes.add(new TargetLibraryInfoWrapperPass(M->getTargetTriple)).

94

Why do we add a TargetTransformInfoWrapperPass to both Passes and FPasses?

94

Do we need both Passes and FPasses?

96

Why not just rename Owner to M and remove this line?

100

Let's simplify to Passes.add(createLoopVectorizePass()).

106

Why do we keep adding passes in different places?

108

Please simplify to builder.setMCJITMemoryManager(std::make_unique<RTDyldMemoryManager>()).

108

Move this closer to where it's used.

108

Do we need to call this again to run destructors after we execute f() ?

109

If the JIT does optimization already, do we need to call OptLLVM at all? Will the vectorizer kick in without OptLLVM?

109–112

I think f(a, b, c, 1) will also work.

emmettneyman added inline comments.Jul 25 2018, 4:09 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
80

No we don't. I've deleted the line below.

89

Contrary to the function's name, getTargetTriple() actually returns a string, not a Triple. But I changed it to new TargetLibraryInfoWrapperPass(ModuleTriple) and deleted line 109.

94

I think because we're just adding a loop vectorize pass, we don't need FPasses. The loop vectorize pass lives within the regular ModulePassManager, not the FunctionPassManager. I decided to put it in the code so, in the future, it would be easier to add different kinds of passes to the fuzz target. Should I remove it?

109

I'll look into this more. I couldn't find an answer quickly.

Fixed some things, made code cleaner

emmettneyman added inline comments.Jul 25 2018, 4:17 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
109

No, the JIT doesn't run any optimization passes. The optimization level given just controls CodeGen.

Source: http://lists.llvm.org/pipermail/llvm-dev/2016-May/099631.html

morehouse added inline comments.Jul 26 2018, 9:17 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
94

Let's remove it since it's currently unnecessary.

95

The indentation looks off. Please fix.

106

This uses llvm:make_unique, which was written when LLVM used C++11. But since LLVM now uses C++14, I think we should use std::make_unique instead.

116

We're allowing the JIT opt-level to be specified on the command line, but we're hard-coding OptLevel=3 for the optimization passes.

Do we need to allow the opt-level to be specified on the command line?

121

Remove whitespace on empty lines.

Actually, you could probably remove this line altogether since the call to f() can be grouped with the parameter setup above.

pcc added inline comments.Jul 26 2018, 11:44 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
106

LLVM doesn't use C++14 yet.

emmettneyman added inline comments.Jul 26 2018, 11:48 AM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
96

Reverted it back since the change caused the fuzzer to crash.

106

We talked offline, but just to put it in writing: My current LLVM build uses C++11 so I'm keeping it llvm::make_unique.

116

Good point. Will change to make OptLLVM() use the same optimization level as the JIT Engine.

  • Code style fixes
  • Removed FPasses
  • Allowed CL Args to specify opt level for OptLLVM()

Small change to fix line length

Do we need to parse the arguments for opt-level, or can we just hardcode -O2 and remove the argument parsing code?

clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
90

Shouldn't OLvl be a CodeGenOpt::Level?

96

You will need to move any uses of M above the call to std::move, since that leaves M in an invalid state.

Do we need to parse the arguments for opt-level, or can we just hardcode -O2 and remove the argument parsing code?

I have the argument parsing code since the original clang-proto-fuzzer code had it and @kcc mentioned we might want to change the command line arguments in the future.

Changed int to CodeGenOpt::Level and fixed unique_ptr issue

morehouse added inline comments.Jul 26 2018, 2:16 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
124

We should be able to get rid of this line now, and rename Owner again.

morehouse added inline comments.Jul 26 2018, 2:18 PM
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
152

Can we use reinterpret_cast here?

Made some minor fixes

morehouse accepted this revision.Jul 26 2018, 2:35 PM
This revision is now accepted and ready to land.Jul 26 2018, 2:35 PM
This revision was automatically updated to reflect the committed changes.