[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

If you haven't already, please apply for commit access:  https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

That way you can land this after it's accepted.



================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:23
+#include "llvm/IR/Module.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Verifier.h"
----------------
This should come before llvm/IR/Module.h.


================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:35
+
+void getOptLevel(const std::vector<const char *> &ExtraArgs,
+                              CodeGenOpt::Level &OLvl) {
----------------
Can make this static to avoid future namespace collisions.


================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:48
+        default:
+          errs() << "error: opt level must be between 0 and 3.\n";
+          return;
----------------
Maybe exit() as well on error?  (here and below)

Exiting will cause the fuzz target to fail, so you can debug more easily.  Otherwise, these error messages could go unnoticed.


================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:58
+  // Create a new Context
+  LLVMContext Context;
+  // Parse ExtraArgs to set the optimization level
----------------
It's easier to follow if you move this closer to where Context is first used.


================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:66
+  std::unique_ptr<Module> M = parseIR(MemoryBufferRef(S, "IR"), Err,
+                                      Context, true);
+  if (!M) {
----------------
You can actually omit the last argument here since you're using the default.


================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:81
+  std::string CPUStr = getCPUStr();
+  std::string FeaturesStr = getFeaturesStr();
+
----------------
Easier to follow if you move these two lines closer to where `CPUStr` and `FeaturesStr` are used.


================
Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:105
+  raw_null_ostream OS;
+  MachineModuleInfo *MMI = new MachineModuleInfo(Target.get());
+  Target->addPassesToEmitFile(PM, OS, nullptr, TargetMachine::CGFT_ObjectFile,
----------------
I believe you can avoid creating the MMI here since it looks like addPassesToEmitFile will do it for you.


Repository:
  rC Clang

https://reviews.llvm.org/D48106