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

27–28

Please delete commented code.

tools/clang-fuzzer/FuzzerInitialize.h

12

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

13

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

10–11

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

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

Do we need this include?

tools/clang-fuzzer/FuzzerInitialize.h

20

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

24

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.

tools/clang-fuzzer/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
 set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
 set(DUMMY_MAIN DummyClangFuzzer.cpp)
 if(LLVM_LIB_FUZZING_ENGINE)
   unset(DUMMY_MAIN)
 elseif(LLVM_USE_SANITIZE_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
   set(CXX_FLAGS_NOFUZZ "${CXX_FLAGS_NOFUZZ} -fsanitize=fuzzer-no-link")
   unset(DUMMY_MAIN)
 endif()
 
 # Needed by LLVM's CMake checks because this file defines multiple targets.
 set(LLVM_OPTIONAL_SOURCES
   ClangFuzzer.cpp
   DummyClangFuzzer.cpp
   ExampleClangProtoFuzzer.cpp
+  FuzzerInitialize.cpp
   )
 
 if(CLANG_ENABLE_PROTO_FUZZER)
   # Create protobuf .h and .cc files, and put them in a library for use by
   # clang-proto-fuzzer components.
   find_package(Protobuf REQUIRED)
   add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI)
   include_directories(${PROTOBUF_INCLUDE_DIRS})
if(CLANG_ENABLE_PROTO_FUZZER)
 
   # Build the protobuf->C++ translation library and driver.
   add_clang_subdirectory(proto-to-cxx)
 
   # Build the protobuf fuzzer
   add_clang_executable(clang-proto-fuzzer
     ${DUMMY_MAIN}
     ExampleClangProtoFuzzer.cpp
+    FuzzerInitialize.cpp
     )
 
   target_link_libraries(clang-proto-fuzzer
     PRIVATE
     ${ProtobufMutator_LIBRARIES}
     ${PROTOBUF_LIBRARIES}
     ${LLVM_LIB_FUZZING_ENGINE}
     clangCXXProto

tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp

 ///  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 "FuzzerInitialize.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());
 }

tools/clang-fuzzer/FuzzerInitialize.h

+//==-- FuzzerInitialize.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 "handle-cxx/handle_cxx.h"
+#include "proto-to-cxx/proto_to_cxx.h"
+
+#include "src/libfuzzer/libfuzzer_macro.h"
+
+#include <cstring>
+
+namespace clang_fuzzer {
+const std::vector<const char *>& GetCLArgs();
+}
+

tools/clang-fuzzer/FuzzerInitialize.cpp

-//===-- ExampleClangProtoFuzzer.cpp - Fuzz Clang --------------------------===//
+//===-- FuzzerInitialize.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 a function that runs Clang on a single
-///  input and uses libprotobuf-mutator to find new inputs. This function is
-///  then linked into the Fuzzer library.
+/// 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 "cxx_proto.pb.h"
-#include "handle-cxx/handle_cxx.h"
-#include "proto-to-cxx/proto_to_cxx.h"
 
-#include "src/libfuzzer/libfuzzer_macro.h"
-
-#include <cstring>
+#include "FuzzerInitialize.h"
 
 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;
 }
 
-DEFINE_BINARY_PROTO_FUZZER(const Function& input) {
-  auto S = FunctionToString(input);
-  HandleCXX(S, CLArgs);
-}