implemented proto to llvm

[emmettneyman]

Summary

implemented proto_to_llvm

• also removed div and mod to eliminate UB

[emmettneyman]

• removed unneeded file

[emmettneyman]

• fixed proto_to_cxx.cpp

[morehouse]

Where is the fuzz target?

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp

33

I'd suggest wrapper functions that return unused variable names, so your code below won't need to juggle these globals.

43

The way these globals are being used seems confusing and error-prone.

Maybe what you want is to replace the operator<< overloads with functions that emit the LLVM and then return the variable name that the result is stored in. Then the parent node can use the returned variable name without juggling globals.

48

getelementptr should make this much simpler.

80

When will this last return be executed?

132

Probably not a big deal, but maybe load the rvalue first? I'd expect the frontend to do that normally to simplify register allocation.

132

Also, I think it is easier to read if you put each statement on its own line.

144

Space before %b.

153

Can we put %z = ... on the next line for consistency?

156

Can we put the first newline on previous statement for consistency?

[emmettneyman]

• refactored loop_proto_to_llvm.cpp

[emmettneyman]

In D48106#1130581, @morehouse wrote:

Where is the fuzz target?

I wanted to implement the proto_to_llvm converter before the fuzz target.

[emmettneyman]

• removed typo in emitted llvm insn

[morehouse]

In D48106#1131625, @emmettneyman wrote:

I wanted to implement the proto_to_llvm converter before the fuzz target.

The fuzz target should make testing your converter way easier. I'd recommend adding it to this patch so that you're less likely to need a bug-fixing patch later.

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp

33

You can hide this as a static int inside get_var.

47

What's the point of storing then loading the value? Can you just return val?

50

Returning a pair can be confusing (which element is which?). I'd suggest passing os to these functions, writing the instructions to os, and then returning just the result variable.

121

If all these cases are the same, we can simplify the code to

case BinaryOp::EQ:
case BinaryOp::NE:
...
op = "add";
break;

130

If AssignmentStatementToString has no extra return value, I think its more concise to just keep the operator<< overload. (Same for below functions)

[emmettneyman]

Implemented HandleLLVM fuzz target

-HandleLLVM function compiles LLVM IR
-ExampleClangLLVMProtoFuzzer wraps the fuzz target
-Next step is to compile at different opt levels and run the executables

[emmettneyman]

• removed unnecessary includes and linked libs

[morehouse]

tools/clang-fuzzer/CMakeLists.txt

71

llvm

76

Maybe remove clangHandleCXX here, so you can use this variable for linking clang-llvm-proto-fuzzer.

tools/clang-fuzzer/handle-llvm/CMakeLists.txt

5 ↗

(On Diff #151801)

There's fewer libraries linked here than in handle-cxx/ (not saying this is wrong, but it could be). Do you get link errors if you build clang-llvm-proto-fuzzer with shared libraries?

tools/clang-fuzzer/handle-llvm/\

31 ↗

(On Diff #151801)

Please sort includes alphabetically, with handle_llvm.h separate at the top.

42 ↗

(On Diff #151801)

Nit: avoid empty lines at beginning or end of a {} block (here and below)

62 ↗

(On Diff #151801)

Does this initialization need to happen every time the fuzzer generates a new input, or can we call this from LLVMFuzzerInitialize() instead?

71 ↗

(On Diff #151801)

1. Avoid using new when you could create the object on the stack instead. This will prevent you from introducing memory leaks if you forget to call delete later.
2. I don't think you need to construct StringRefs or the MemoryBufferRef here. Instead you can probably do parseIR(MemoryBufferRef(S, "IR"), Err, ...) below.

79 ↗

(On Diff #151801)

I'd move the unique_ptr definition to the same line as parseIR.

81 ↗

(On Diff #151801)

What's the point of wrapping sys::getDefaultTargetTriple() if we always unwrap TheTriple below?

84 ↗

(On Diff #151801)

What does UpgradeDebugInfo=false do here? The documentation warns about setting this bool to false.

84 ↗

(On Diff #151801)

What if parseIR returns nullptr?

88 ↗

(On Diff #151801)

What if lookupTarget returns nullptr?

90 ↗

(On Diff #151801)

Please separate to 2 statements.

98 ↗

(On Diff #151801)

Fix indentation here.

101 ↗

(On Diff #151801)

Maybe add a default case here with an error message?

104 ↗

(On Diff #151801)

It might make sense to move command line arg parsing to a helper function, and then call that function closer to the top. That way if there's a bad argument we can quit before doing all the IR parsing.

113 ↗

(On Diff #151801)

Any reason not to use the newer PassManager?

127 ↗

(On Diff #151801)

What's the reason for this cast? MachineModuleInfo() accepts TargetMachine * as an argument.

130 ↗

(On Diff #151801)

Eventually you will want to save the outputs to do A/B runs. This is fine for now though.

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

14 ↗

(On Diff #151801)

Apparently you created a file called \ with the same text. Please remove that file, and apply my comments there to this file instead.

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp

58

Simpler to do os << ptr_var << " = getelementptr" ... (here and below)

72

Is it still possible for the protobuf to not have a constant, a binOp, or a varRef?

114

Nit: Maybe var_ref or lvalue would be more descriptive names.

115

Might make for slightly faster IR if you do the rvalue first. Would at least simplify register allocation.

[emmettneyman]

tools/clang-fuzzer/handle-llvm/CMakeLists.txt

5 ↗

(On Diff #151801)

It builds without any errors both with all the libraries from handle-cxx/ linked in and without any of the libraries linked in (as I have here). Which is preferred?

[morehouse]

tools/clang-fuzzer/handle-llvm/CMakeLists.txt

5 ↗

(On Diff #151801)

I prefer what you have here, if shared libraries are OK.

[emmettneyman]

tools/clang-fuzzer/handle-llvm/\

62 ↗

(On Diff #151801)

I was following the pattern from handle_cxx.cpp in which the initialization calls were made every time a new input was generated. However looking at the documentation and at llvm-isel-fuzzer.cpp, it seems like putting it in LLVMFuzzerInitialize() makes more sense. I will make this change in the next commit.

[emmettneyman]

tools/clang-fuzzer/handle-llvm/\

113 ↗

(On Diff #151801)

Clang (llc) and the llvm-isel-fuzzer both still use the legacy PassManager. The newer PassManager has a new interface that doesn't use Modules.

[emmettneyman]

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp

72

Right now, since the protobufs use oneof, it is possible for the protobuf to not have a constant binOp, or varRef. For now, I'll generate "dummy" code for this case.

Then, I'll submit a new patch that eliminates the use of oneof for both cxx_proto.proto and cxx_loop_proto so that "dummy" code isn't necessary.

[emmettneyman]

• made changes to handle_llvm.cpp in response to reviewer comments

[morehouse]

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.

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

23 ↗

(On Diff #151986)

This should come before llvm/IR/Module.h.

35 ↗

(On Diff #151986)

Can make this static to avoid future namespace collisions.

48 ↗

(On Diff #151986)

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.

58 ↗

(On Diff #151986)

It's easier to follow if you move this closer to where Context is first used.

66 ↗

(On Diff #151986)

You can actually omit the last argument here since you're using the default.

81 ↗

(On Diff #151986)

Easier to follow if you move these two lines closer to where CPUStr and FeaturesStr are used.

105 ↗

(On Diff #151986)

I believe you can avoid creating the MMI here since it looks like addPassesToEmitFile will do it for you.

[emmettneyman]

• minor changes to improve readability and style

[morehouse]

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

50 ↗

(On Diff #152003)

exit(1) will indicate an abnormal exit. (here and below)

64 ↗

(On Diff #152003)

This comment is unnecessary. Clearly a new Context is being created on the next line.

[emmettneyman]

• removed unnecessary comment and fixed exit statement

[morehouse]

Looks like exit(0) is still there.

[emmettneyman]

• actually fixed error statement

[emmettneyman]

In D48106#1137224, @morehouse wrote:

Looks like exit(0) is still there.

oops, forgot to :w

[sbc100]

I think this broke the BUILD_SHARED_LIBS=1 build: https://reviews.llvm.org/D48503

tools/clang-fuzzer/CMakeLists.txt

add_clang_library(clangCXXLoopProto
     )
 
   # Build and include libprotobuf-mutator
   include(ProtobufMutator)
   include_directories(${ProtobufMutator_INCLUDE_DIRS})
 
   # Build the protobuf->C++ translation library and driver.
   add_clang_subdirectory(proto-to-cxx)
-  
+  # Build the protobuf->LLVM IR translation library and driver.
+  add_clang_subdirectory(proto-to-llvm)
+  
   # Build the fuzzer initialization library.
   add_clang_subdirectory(fuzzer-initialize)
 
   # Build the protobuf fuzzer
   add_clang_executable(clang-proto-fuzzer
     ${DUMMY_MAIN}
     ExampleClangProtoFuzzer.cpp
     )
 
   # Build the loop protobuf fuzzer
   add_clang_executable(clang-loop-proto-fuzzer
     ${DUMMY_MAIN}
     ExampleClangLoopProtoFuzzer.cpp
     )
 
   set(COMMON_PROTO_FUZZ_LIBRARIES
     ${ProtobufMutator_LIBRARIES}
     ${PROTOBUF_LIBRARIES}
     ${LLVM_LIB_FUZZING_ENGINE}
     clangFuzzerInitialize
     clangHandleCXX
     )
 
   target_link_libraries(clang-proto-fuzzer
     PRIVATE
     ${COMMON_PROTO_FUZZ_LIBRARIES}
     clangCXXProto
     clangProtoToCXX
     )

tools/clang-fuzzer/cxx_loop_proto.proto

message VarRef {
   required Arr arr = 1;
 }
 
 message BinaryOp {
   enum Op {
     PLUS = 0;
     MINUS = 1;
     MUL = 2;
-    DIV = 3;
-    MOD = 4;
-    XOR = 5;
-    AND = 6;
-    OR = 7;
-    EQ = 8;
-    NE = 9;
-    LE = 10;
-    GE = 11;
-    LT = 12;
-    GT = 13;
+    XOR = 3;
+    AND = 4;
+    OR = 5;
+    EQ = 6;
+    NE = 7;
+    LE = 8;
+    GE = 9;
+    LT = 10;
+    GT = 11;
   };
   required Op op = 1;
   required Rvalue left = 2;
   required Rvalue right = 3;
 }
 
 message Rvalue {
   oneof rvalue_oneof {
     Const cons = 1;
     BinaryOp binop = 2;
     VarRef varref = 3;
   }
 }
 
 message AssignmentStatement {
   required VarRef varref = 1;
   required Rvalue rvalue = 2;
 }
 
-message IfElse {
-  required Rvalue cond = 1;
-  required StatementSeq if_body = 2;
-  required StatementSeq else_body = 3;
-}
-
 message Statement {
   required AssignmentStatement assignment = 1;
 }
 
 message StatementSeq {
   repeated Statement statements = 1;
 }
 
 message LoopFunction {
   required StatementSeq statements = 1;
 }
 
 package clang_fuzzer;

tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp

case BinaryOp::PLUS:
     os << "+";
     break;
   case BinaryOp::MINUS:
     os << "-";
     break;
   case BinaryOp::MUL:
     os << "*";
     break;
-  case BinaryOp::DIV:
-    os << "/";
-    break;
-  case BinaryOp::MOD:
-    os << "%";
-    break;
   case BinaryOp::XOR:
     os << "^";
     break;
   case BinaryOp::AND:
     os << "&";
     break;
   case BinaryOp::OR:
     os << "|";
case BinaryOp::GT:
     os << ">";
     break;
   }
   return os << x.right() << ")";
 }
 std::ostream &operator<<(std::ostream &os, const AssignmentStatement &x) {
   return os << x.varref() << "=" << x.rvalue() << ";\n";
 }
-std::ostream &operator<<(std::ostream &os, const IfElse &x) {
-  return os << "if (" << x.cond() << "){\n"
-            << x.if_body() << "} else { \n"
-            << x.else_body() << "}\n";
-}
 std::ostream &operator<<(std::ostream &os, const Statement &x) {
   return os << x.assignment();
 }
 std::ostream &operator<<(std::ostream &os, const StatementSeq &x) {
   for (auto &st : x.statements())
     os << st;
   return os;
 }

tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx_main.cpp

 //==-- loop_proto_to_cxx_main.cpp - Driver for protobuf-C++ conversion -----==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Implements a simple driver to print a C++ program from a protobuf with loops.
 //
 //===----------------------------------------------------------------------===//
 
-// This is a copy and will be updated later to introduce changes
 
 #include <fstream>
 #include <iostream>
 #include <streambuf>
 #include <string>
 
 #include "proto_to_cxx.h"
 

tools/clang-fuzzer/proto-to-llvm/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
+set(CMAKE_CXX_FLAGS ${CXX_FLAGS_NOFUZZ})
+
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES loop_proto_to_llvm.cpp loop_proto_to_llvm_main.cpp)
+
+add_clang_library(clangLoopProtoToLLVM loop_proto_to_llvm.cpp
+                  DEPENDS clangCXXLoopProto
+                  LINK_LIBS clangCXXLoopProto ${PROTOBUF_LIBRARIES}
+                  )
+
+add_clang_executable(clang-loop-proto-to-llvm loop_proto_to_llvm_main.cpp)
+
+target_link_libraries(clang-loop-proto-to-llvm PRIVATE clangLoopProtoToLLVM)

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.h

+//==-- loop_proto_to_llvm.h - Protobuf-C++ conversion ----------------------------==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines functions for converting between protobufs and LLVM IR.
+//
+//===----------------------------------------------------------------------===//
+
+#include <cstdint>
+#include <cstddef>
+#include <string>
+
+namespace clang_fuzzer {
+class LoopFunction;
+
+std::string LoopFunctionToLLVMString(const LoopFunction &input);
+std::string LoopProtoToLLVM(const uint8_t *data, size_t size);
+}

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp

+//==-- loop_proto_to_llvm.cpp - Protobuf-C++ conversion
+//---------------------==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Implements functions for converting between protobufs and LLVM IR.
+//
+//
+//===----------------------------------------------------------------------===//
+
+#include "loop_proto_to_llvm.h"
+#include "cxx_loop_proto.pb.h"
+
+// The following is needed to convert protos in human-readable form
+#include <google/protobuf/text_format.h>
+
+#include <ostream>
+#include <sstream>
+
+namespace clang_fuzzer {
+
+// Forward decls.
+std::ostream &operator<<(std::ostream &os, const BinaryOp &x);
+std::ostream &operator<<(std::ostream &os, const StatementSeq &x);
+
+// Counter variables to generate new LLVM IR variable names
+int ptr_ctr = 0;
+int val_ctr = 0;
+
+// Proto to LLVM.
+std::ostream &operator<<(std::ostream &os, const Const &x) {
+  return os << "%pval_" << val_ctr << " = alloca i32\n"
+            << "store i32 " << x.val() << ", i32* %pval_" << val_ctr << "\n"
+            << "%val_" << val_ctr << " = load i32, i32* %pval_" << val_ctr
+            << "\n";
+}
+std::ostream &operator<<(std::ostream &os, const VarRef &x) {
+  ptr_ctr++;
+  switch (x.arr()) {
+  case VarRef::ARR_A:
+    return os << "%p2i_" << ptr_ctr << " = ptrtoint i32* %a to i64\n"
+              << "%m_" << ptr_ctr << " = mul i64 8, %ct\n"
+              << "%i_" << ptr_ctr << " = add i64 %p2i_" << ptr_ctr << ", %m_"
+              << ptr_ctr << "\n"
+              << "%i2p_" << ptr_ctr << " = inttoptr i64 %i_" << ptr_ctr
+              << " to i32*\n";
+  case VarRef::ARR_B:
+    return os << "%p2i_" << ptr_ctr << " = ptrtoint i32* %b to i64\n"
+              << "%m_" << ptr_ctr << " = mul i64 8, %ct\n"
+              << "%i_" << ptr_ctr << " = add i64 %p2i_" << ptr_ctr << ", %m_"
+              << ptr_ctr << "\n"
+              << "%i2p_" << ptr_ctr << " = inttoptr i64 %i_" << ptr_ctr
+              << " to i32*\n";
+  case VarRef::ARR_C:
+    return os << "%p2i_" << ptr_ctr << " = ptrtoint i32* %c to i64\n"
+              << "%m_" << ptr_ctr << " = mul i64 8, %ct\n"
+              << "%i_" << ptr_ctr << " = add i64 %p2i_" << ptr_ctr << ", %m_"
+              << ptr_ctr << "\n"
+              << "%i2p_" << ptr_ctr << " = inttoptr i64 %i_" << ptr_ctr
+              << " to i32*\n";
+  }
+}
+std::ostream &operator<<(std::ostream &os, const Rvalue &x) {
+  val_ctr++;
+  if (x.has_cons())
+    return os << x.cons();
+  if (x.has_binop())
+    return os << x.binop();
+  if (x.has_varref())
+    return os << x.varref() << "%val_" << val_ctr << " = load i32, i32* %i2p_"
+              << ptr_ctr << "\n";
+  return os << "%pval_" << val_ctr << " = alloca i32\n"
+            << "store i32 1, i32* %pval_" << val_ctr << "\n"
+            << "%val_" << val_ctr << " = load i32, i32* %pval_" << val_ctr
+            << "\n";
+}
+std::ostream &operator<<(std::ostream &os, const BinaryOp &x) {
+  os << x.left();
+  int v1 = val_ctr;
+  os << x.right();
+  int v2 = val_ctr;
+  val_ctr++;
+  os << "%val_" << val_ctr << " = ";
+  switch (x.op()) {
+  case BinaryOp::PLUS:
+    os << "add";
+    break;
+  case BinaryOp::MINUS:
+    os << "sub";
+    break;
+  case BinaryOp::MUL:
+    os << "mul";
+    break;
+  case BinaryOp::XOR:
+    os << "xor";
+    break;
+  case BinaryOp::AND:
+    os << "and";
+    break;
+  case BinaryOp::OR:
+    os << "or";
+    break;
+  // Support for Boolean operators will be added later
+  case BinaryOp::EQ:
+    os << "add";
+    break;
+  case BinaryOp::NE:
+    os << "add";
+    break;
+  case BinaryOp::LE:
+    os << "add";
+    break;
+  case BinaryOp::GE:
+    os << "add";
+    break;
+  case BinaryOp::LT:
+    os << "add";
+    break;
+  case BinaryOp::GT:
+    os << "add";
+    break;
+  }
+  return os << " i32 %val_" << v1 << ", %val_" << v2 << "\n";
+}
+std::ostream &operator<<(std::ostream &os, const AssignmentStatement &x) {
+  int p = ptr_ctr + 1;
+  return os << x.varref() << x.rvalue() << "store i32 %val_" << val_ctr
+            << ", i32* %i2p_" << p << "\n";
+}
+std::ostream &operator<<(std::ostream &os, const Statement &x) {
+  return os << x.assignment();
+}
+std::ostream &operator<<(std::ostream &os, const StatementSeq &x) {
+  for (auto &st : x.statements())
+    os << st;
+  return os;
+}
+std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
+  return os << "define void @foo(i32* %a, i32*%b, i32* noalias %c, i64 %s) {\n"
+            << "%i = alloca i64\n"
+            << "store i64 0, i64* %i\n"
+            << "br label %loop\n\n"
+            << "loop:\n"
+            << "%ct = load i64, i64* %i\n"
+            << "%comp = icmp eq i64 %ct, %s\n"
+            << "br i1 %comp, label %endloop, label %body\n\n"
+            << "body:\n"
+            << x.statements() << "%z = add i64 1, %ct\n"
+            << "store i64 %z, i64* %i\n"
+            << "br label %loop\n"
+            << "\nendloop:\n"
+            << "ret void\n}\n";
+}
+
+// ---------------------------------
+
+std::string LoopFunctionToLLVMString(const LoopFunction &input) {
+  std::ostringstream os;
+  os << input;
+  return os.str();
+}
+std::string LoopProtoToLLVM(const uint8_t *data, size_t size) {
+  LoopFunction message;
+  if (!message.ParsePartialFromArray(data, size))
+    return "#error invalid proto\n";
+  return LoopFunctionToLLVMString(message);
+}
+
+} // namespace clang_fuzzer

tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm_main.cpp

-//==-- loop_proto_to_cxx_main.cpp - Driver for protobuf-C++ conversion -----==//
+//==-- loop_proto_to_llvm_main.cpp - Driver for protobuf-LLVM conversion----==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
-// Implements a simple driver to print a C++ program from a protobuf with loops.
+// Implements a simple driver to print a LLVM program from a protobuf with loops
 //
 //===----------------------------------------------------------------------===//
 
-// This is a copy and will be updated later to introduce changes
 
 #include <fstream>
 #include <iostream>
 #include <streambuf>
 #include <string>
 
-#include "proto_to_cxx.h"
+#include "loop_proto_to_llvm.h"
 
 int main(int argc, char **argv) {
   for (int i = 1; i < argc; i++) {
     std::fstream in(argv[i]);
     std::string str((std::istreambuf_iterator<char>(in)),
                     std::istreambuf_iterator<char>());
-    std::cout << "// " << argv[i] << std::endl;
-    std::cout << clang_fuzzer::LoopProtoToCxx(
+    std::cout << ";; " << argv[i] << std::endl;
+    std::cout << clang_fuzzer::LoopProtoToLLVM(
         reinterpret_cast<const uint8_t *>(str.data()), str.size());
   }
 }