Refactored clang-fuzzer and added new (copy) files

[emmettneyman]

Summary

Refactored LLVMFuzzerInitialize function into its own file.
Copied and renamed some files in preparation for new loop-proto-fuzzer.

[emmettneyman]

• Took out a debug print statement

[vitalybuka]

Good practice is to avoid merging changes into a single one.
Here one patch should be "refactoring" and the second for "loop-proto-fuzzer."

tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp

29 ↗

(On Diff #149585)

Please delete commented code.

tools/clang-fuzzer/FuzzerInitialize.h

11 ↗

(On Diff #149585)

static here means that each module which includes this header is going to have own instance of the variable.
I guess you need only one instance in FuzzerInitialize.cpp which can be achieved with

extern std::vector<const char *> CLArgs;

However I'd recommend getter:

const std::vector<const char *>& GetCLArgs();

with implementation and

static std::vector<const char *> CLArgs;

in FuzzerInitialize.cpp

12 ↗

(On Diff #149585)

you need only CLArgs here.
code which includes this header is not going to call LLVMFuzzerInitialize

tools/clang-fuzzer/experimental/ExampleClangLoopProtoFuzzer.cpp

30 ↗

(On Diff #149585)

Please remove deleted code

[vitalybuka]

In D47666#1119821, @vitalybuka wrote:

Good practice is to avoid merging changes into a single one.
Here one patch should be "refactoring" and the second for "loop-proto-fuzzer."

We are doing this for several reasons:

1. smaller patches, faster review
2. easier to investigate regressions caused by smaller patches

[emmettneyman]

Changed CLArgs into getter and deleted commented code

Made changes in response to comments
Removed commented out code
Changed CLArgs to be a getter method
Removed LLVMFuzzerInitialize decl from header file

[vitalybuka]

tools/clang-fuzzer/FuzzerInitialize.cpp

11 ↗

(On Diff #149807)

Could you please update this description?

tools/clang-fuzzer/experimental/ExampleClangLoopProtoFuzzer.cpp

22 ↗

(On Diff #149807)

Please move ExampleClangLoopProtoFuzzer into a separate patch

[emmettneyman]

• Updated and added header comments to two new files. Deleted loop fuzzer files.

[emmettneyman]

• Fixed file header comment

[emmettneyman]

• Another edit to the file header comments.

[emmettneyman]

In D47666#1121608, @emmettneyman wrote:

• Updated and added header comments to two new files. Deleted loop fuzzer files.

I will commit the loop fuzzer files in a future patch.

[morehouse]

tools/clang-fuzzer/CMakeLists.txt

48 ↗

(On Diff #149854)

Rather than compiling FuzzerInitialize.cpp into the binary, can we make it a library like handle-cxx or proto-to-cxx?

tools/clang-fuzzer/FuzzerInitialize.cpp

17 ↗

(On Diff #149854)

Do we need this include?

tools/clang-fuzzer/FuzzerInitialize.h

20 ↗

(On Diff #149854)

Why do we need these includes in the header? Doesn't look like they're used here.

[emmettneyman]

• Refactored FuzzerInitialize into library

[morehouse]

tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp

23 ↗

(On Diff #149867)

I think cstring is no longer used after this change. So we can probably remove this include.

tools/clang-fuzzer/fuzzer-initialize/CMakeLists.txt

3 ↗

(On Diff #149867)

Nit: clangFuzzerInitialize would better follow the naming convention of the other libraries.

tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h

15 ↗

(On Diff #149867)

I don't think this include is used in this file either. Can we remove it?

[emmettneyman]

• Removed unecessary includes and renamed library.

[morehouse]

LGTM.

[kcc]

Some feedback on the generated code:

while (1){

let's not have the while loops inside the for loops for now.
If the initial goal is to stress the loop optimizations (e.g. vectorizer),
loops likes this are just a distraction

for (int loop_ctr = 0

too verbose. Use 'i'm 'j', 'k'
also, alternate int, unsigned, size_t, long (later).

a[436863498 % s]=1;

this is good for keeping the code UB-free, but it will render the tests non-vectorizable.
need more tests that won't use % s

void foo(int *a, size_t s) {

ok for a starter, but will need a more rich signature in the future versions.

cfe/trunk/tools/clang-fuzzer/CMakeLists.txt

if(CLANG_ENABLE_PROTO_FUZZER)
 
   # 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 fuzzer initialization library.
+  add_clang_subdirectory(fuzzer-initialize)
+
   # Build the protobuf fuzzer
   add_clang_executable(clang-proto-fuzzer
     ${DUMMY_MAIN}
     ExampleClangProtoFuzzer.cpp
     )
 
   target_link_libraries(clang-proto-fuzzer
     PRIVATE
     ${ProtobufMutator_LIBRARIES}
     ${PROTOBUF_LIBRARIES}
     ${LLVM_LIB_FUZZING_ENGINE}
     clangCXXProto
+    clangFuzzerInitialize
     clangHandleCXX
     clangProtoToCXX
     )
 endif()
 
 add_clang_subdirectory(handle-cxx)
 
 add_clang_executable(clang-fuzzer

cfe/trunk/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp

 ///  input and uses libprotobuf-mutator to find new inputs. This function is
 ///  then linked into the Fuzzer library.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "cxx_proto.pb.h"
 #include "handle-cxx/handle_cxx.h"
 #include "proto-to-cxx/proto_to_cxx.h"
-
+#include "fuzzer-initialize/fuzzer_initialize.h"
 #include "src/libfuzzer/libfuzzer_macro.h"
 
-#include <cstring>
-
 using namespace clang_fuzzer;
 
-static std::vector<const char *> CLArgs;
-
-extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
-  CLArgs.push_back("-O2");
-  for (int I = 1; I < *argc; I++) {
-    if (strcmp((*argv)[I], "-ignore_remaining_args=1") == 0) {
-      for (I++; I < *argc; I++)
-        CLArgs.push_back((*argv)[I]);
-      break;
-    }
-  }
-  return 0;
-}
-
 DEFINE_BINARY_PROTO_FUZZER(const Function& input) {
   auto S = FunctionToString(input);
-  HandleCXX(S, CLArgs);
+  HandleCXX(S, GetCLArgs());
 }

cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} Support)
+
+add_clang_library(clangFuzzerInitialize fuzzer_initialize.cpp)

cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h

+//==-- fuzzer_initialize.h - Fuzz Clang ------------------------------------==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines a function that returns the command line arguments for a specific
+// call to the fuzz target.
+//
+//===----------------------------------------------------------------------===//
+
+#include <vector>
+
+namespace clang_fuzzer {
+const std::vector<const char *>& GetCLArgs();
+}

cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp

+//===-- fuzzer_initialize.cpp - Fuzz Clang --------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file implements two functions: one that returns the command line
+/// arguments for a given call to the fuzz target and one that initializes
+/// the fuzzer with the correct command line arguments.
+///
+//===----------------------------------------------------------------------===//
+
+#include "fuzzer_initialize.h"
+#include <cstring>
+
+using namespace clang_fuzzer;
+
+
+namespace clang_fuzzer {
+
+static std::vector<const char *> CLArgs;
+
+const std::vector<const char *>& GetCLArgs() {
+  return CLArgs;
+}
+
+}
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  CLArgs.push_back("-O2");
+  for (int I = 1; I < *argc; I++) {
+    if (strcmp((*argv)[I], "-ignore_remaining_args=1") == 0) {
+      for (I++; I < *argc; I++)
+        CLArgs.push_back((*argv)[I]);
+      break;
+    }
+  }
+  return 0;
+}