[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:17
 
-# Depend on LLVM IR intrinsic generation.
+# Depend on LLVM IR instrinsic generation.
 set(handle_llvm_deps intrinsics_gen)
----------------
Typo introduced here.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:25
   handle_llvm.cpp
-
+  
   DEPENDS
----------------
Please don't add whitespace to a previously empty line.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:10
 //
-// Implements HandleLLVM for use by the Clang fuzzers. Mimics the llc tool to
-// compile an LLVM IR file to X86_64 assembly.
+// Implements HandleLLVM for use by the Clang fuzzers. First runs an loop
+// vectorizer optimization pass over the given IR code. Then mimics lli on both
----------------
an -> a


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:112
+// Mimics the opt tool to run an optimization pass over the provided IR
+std::string OptLLVM(const std::string IR, CodeGenOpt::Level &OLvl) {
+  InitEverything();
----------------
Maybe this should be `const std::string &IR` or `StringRef`.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:113
+std::string OptLLVM(const std::string IR, CodeGenOpt::Level &OLvl) {
+  InitEverything();
+ 
----------------
Does this need to be called every time we get a new input?  Can we call this from `LLVMFuzzerInitialize()`?


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:123
+  args[1] = arg1.c_str();
+  cl::ParseCommandLineOptions(2, args);
 
----------------
This shouldn't be required.  You should be able to directly add the passes you want.  See `llvm/lib/Passes/PassBuilder.cpp`.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:132
+
+  Triple ModuleTriple(M->getTargetTriple());
+  TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
----------------
Move this closer to where it's used.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:133
+  Triple ModuleTriple(M->getTargetTriple());
+  TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
+  std::string CPUStr;
----------------
`Options` is never used.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:136
+  std::string FeaturesStr;
+  std::unique_ptr<TargetMachine> TM(nullptr);
+  setFunctionAttributes(CPUStr, FeaturesStr, *M);
----------------
`TM` is never used.


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:137
+  std::unique_ptr<TargetMachine> TM(nullptr);
+  setFunctionAttributes(CPUStr, FeaturesStr, *M);
+  
----------------
Does this do anything since `CPUStr` and `FeaturesStr` are empty?


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:145
+  std::unique_ptr<legacy::FunctionPassManager> FPasses;
+  FPasses.reset(new legacy::FunctionPassManager(M.get()));
+  FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
----------------
Can we just make `FPasses` on stack instead?


================
Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:190
+  builder.setMCJITMemoryManager(
+      std::unique_ptr<RTDyldMemoryManager>(RTDyldMM));
+  builder.setOptLevel(OLvl);
----------------
These 3 lines can be combined to `builder.setMCJITMemoryManager(new SectionMemoryManager())`


Repository:
  rC Clang

https://reviews.llvm.org/D49526