[lldb/Fuzzer] Add fuzzer for expression evaluator

[cassanova]

Summary

This commit adds a fuzzer for LLDB's expression evaluator. The fuzzer takes a different approach than the current fuzzers present, and uses an approach that is currently being used for clang fuzzers.

Instead of fuzzing the evaluator with randomly mutated characters, protobufs are used to generate a subset of C++. This is then converted to valid C++ code and sent to the expression evaluator. In addition, libprotobuf_mutator is used to mutate the fuzzer's inputs from valid C++ code to valid C++ code, rather than mutating from valid code to total nonsense.

[cassanova]

Building the expression evaluator fuzzer is now conditional on the CLANG_ENABLE_PROTO_FUZZER CMake variable being enabled.

Copying the source and header files from is no longer being done in the top-level CMake file, this is instead added to the subdirectories of the clang fuzzer.

The fuzzer uses Clang's CMake modules for libprotobuf_mutator instead of copying the module into LLDB.

[JDevlieghere]

The review shows the diff with the previous iteration of the patch. Can you generate a diff with the current top-of-tree?

[cassanova]

Shows top-of-tree changes, however CMake generation fails that the ProtobufMutator target already exists for clang-fuzzer:

CMake Error at /opt/homebrew/Cellar/cmake/3.23.1_1/share/cmake/Modules/ExternalProject.cmake:3453 (add_custom_target):
  add_custom_target cannot create target "protobuf_mutator" because another
  target with the same name already exists.  The existing target is a custom
  target created in source directory
  "/Users/chelseacassanova/code/llvm-project/clang/tools/clang-fuzzer".

[cassanova]

lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/CMakeLists.txt

22

Commenting out this line causes the project to generate, but I get file not found errors when including the protobuf errors when trying to build the fuzzer.

[cassanova]

The ProtobufMutator CMake module will build targets for clang and lldb individually depending on which project is building the mutator, instead of both fuzzers trying to build the same target.

The expression fuzzer's source file only includes handle-cxx and proto-to-cxx directly, instead of including them from their folders.

The expression fuzzer's CMake file adds the clang-fuzzer binary directory as a include directory so that the lldb fuzzer does not need to generate a second copy of cxx_proto.pb.h and cxx_proto.pb.cc. It also requires the Protobuf library, grabs its definitions and includes the protobuf include dirs to prevent a protobuf header from not being found in the expression fuzzer source file.

[mib]

clang/cmake/modules/ProtobufMutator.cmake

4–5

If feels wrong to me that the clang protobuf cmake module knows about lldb.

May be we should just have 2 separate files for clang and lldb

[cassanova]

clang/cmake/modules/ProtobufMutator.cmake

4–5

My preferred solution to this was just creating a target called ${LLVM_VARIABLE_THAT_TELLS_YOU_THE_SUBPROJECT_NAME}_protobuf_mutator to avoid using if-statements and direct strings but it looks like clang and lldb aren't defined as subprojects unless they're being built standalone.

Also in the event that some other subproject wanted to use this library then this solution just gets messier. Having 2 different CMake module files for the clang and lldb sides each or putting protobuf mutator in a more central location is another way to approach this

[JDevlieghere]

clang/cmake/modules/ProtobufMutator.cmake

4–5

I agree with Ismail. I think we can fix this issue by making it possible to set the PBM_PREFIX in the scope that includes ProtobufMutator.cmake like this:

In clang/cmake/modules/ProtobufMutator.cmake:

if (NOT PBM_PREFIX) 
  set (PBM_PREFIX protobuf_mutator)
endif()

In lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/CMakeLists.txt:

set (PBM_PREFIX lldb_protobuf_mutator)
include(ProtobufMutator)

That said, I think an even better way to do this is by using a cached variable that keep track of whether we've already included the external project. That way we don't have to create two different targets that are essentially the same. If I'm following the code correctly, all the variables that we rely on can be computed without the ExternalProject_Add. I imagine something like:

if (NOT PBM_PROJECT_ADDED) 
  ExternalProject_Add(${PBM_PREFIX}
    PREFIX ${PBM_PREFIX}
    GIT_REPOSITORY https://github.com/google/libprotobuf-mutator.git
    GIT_TAG master
    CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
    CMAKE_CACHE_ARGS -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER}
                     -DCMAKE_CXX_COMPILER:FILEPATH=${CMAKE_CXX_COMPILER}
    BUILD_BYPRODUCTS ${PBM_LIB_PATH} ${PBM_FUZZ_LIB_PATH}
    UPDATE_COMMAND ""
    INSTALL_COMMAND ""
    )
    set(PBM_PROJECT_ADDED TRUE CACHE BOOL
    "Whether the ProtoBufMutator external project was added")
endif()

I think that should preclude us from creating the target twice.

[cassanova]

Updated the ProtobufMutator CMake module and expression fuzzer CMakeLists file so that the expression fuzzer will create its own target name in its CMake file, and the ProtobufMutator will not attempt to create another target if one already exists

Also updated the expression fuzzer's CMake file to create a directory to store expression fuzzer artifacts.

[JDevlieghere]

lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/cxx_proto.proto

1 ↗

(On Diff #444361)

Do we still need a copy of this for LLDB?

[cassanova]

lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/cxx_proto.proto

1 ↗

(On Diff #444361)

This file is used to generate the cxx_proto.pb.h which is included in the source file. Since we just use the clang-fuzzer directory as an include directory then we shouldn't need this file anymore

[cassanova]

Removed the cxx_proto.proto file since we include the headers that it generates from the clang side.

[JDevlieghere]

A few nits about naming, but otherwise this LGTM.

lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp

48

52–53

60

63

[mstorsjo]

This broke building of Clang, when it's set up by symlinking llvm-project/clang into llvm-project/llvm/tools. In that case, cmake configure errors out like this:

-- Configuring done 
CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
  Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
  path:
 
    "/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/." 

  which is prefixed in the source directory.

See e.g. https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source for some discussion on the issue.

Can we revert this commit until this has been sorted out?

[mib]

In D129377#3673204, @mstorsjo wrote:

This broke building of Clang, when it's set up by symlinking llvm-project/clang into llvm-project/llvm/tools. In that case, cmake configure errors out like this:

-- Configuring done 
CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
  Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
  path:
 
    "/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/." 

  which is prefixed in the source directory.

See e.g. https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source for some discussion on the issue.

Can we revert this commit until this has been sorted out?

@mstorsjo I reverted Chelsea's patch in b797834748f1, since she's offline now. Do you have a link to a bot failure that would help us investigate the issue ?

Thanks!

[mstorsjo]

In D129377#3673237, @mib wrote:

In D129377#3673204, @mstorsjo wrote:

This broke building of Clang, when it's set up by symlinking llvm-project/clang into llvm-project/llvm/tools. In that case, cmake configure errors out like this:

-- Configuring done 
CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
  Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
  path:
 
    "/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/." 

  which is prefixed in the source directory.

See e.g. https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source for some discussion on the issue.

Can we revert this commit until this has been sorted out?

@mstorsjo I reverted Chelsea's patch in b797834748f1, since she's offline now. Do you have a link to a bot failure that would help us investigate the issue ?

Thanks!

Thanks!

It’s not on a public buildbot but only in my local continuous builds unfortunately - but I’ll have look at the issue myself today - I was running out of time yesterday.

[mstorsjo]

In D129377#3673237, @mib wrote:

In D129377#3673204, @mstorsjo wrote:

This broke building of Clang, when it's set up by symlinking llvm-project/clang into llvm-project/llvm/tools. In that case, cmake configure errors out like this:

-- Configuring done 
CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
  Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
  path:
 
    "/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/." 

  which is prefixed in the source directory.

See e.g. https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source for some discussion on the issue.

Can we revert this commit until this has been sorted out?

@mstorsjo I reverted Chelsea's patch in b797834748f1, since she's offline now. Do you have a link to a bot failure that would help us investigate the issue ?

So in short, I think this would work if you'd simply remove the newly added target_include_directories(clangHandleCXX PUBLIC .) in clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt and target_include_directories(clangProtoToCXX PUBLIC .) and target_include_directories(clangLoopProtoToCXX PUBLIC .) in clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt.

To reproduce the issue - instead of configuring llvm+clang+lldb by passing -DLLVM_ENABLE_PROJECTS="clang;lldb" make symlinks in llvm-project/llvm/tools pointing to llvm-project/clang and llvm-project/lldb. (I.e., cd llvm-project/llvm/tools; ln -s ../../clang .; ln -s ../../lldb .) This slightly affects the effective path at where CMake sees the clang files, which causes CMake to report the unexpected path overlap issue here.

I've tried to build all these fuzzers - I haven't really managed to build it all, but I think I'm fairly close. Anyway, I tried removing those target_include_directories() in that setup, and it didn't change anything, but maybe it only makes a difference if one gets further than what I got. (Changing PUBLIC into PRIVATE in those lines avoided the issue too.)

clang/cmake/modules/ProtobufMutator.cmake

 include(ExternalProject)
-set(PBM_PREFIX protobuf_mutator)
+
+if (NOT PBM_PREFIX)
+  set (PBM_PREFIX protobuf_mutator)
+endif()
+
 set(PBM_PATH ${CMAKE_CURRENT_BINARY_DIR}/${PBM_PREFIX}/src/${PBM_PREFIX})
 set(PBM_LIB_PATH ${PBM_PATH}-build/src/libprotobuf-mutator.a)
 set(PBM_FUZZ_LIB_PATH ${PBM_PATH}-build/src/libfuzzer/libprotobuf-mutator-libfuzzer.a)
 
 ExternalProject_Add(${PBM_PREFIX}
   PREFIX ${PBM_PREFIX}
   GIT_REPOSITORY https://github.com/google/libprotobuf-mutator.git
   GIT_TAG master

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

 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} Support)
 
 add_clang_library(clangHandleCXX
   handle_cxx.cpp
 
   LINK_LIBS
   clangBasic
   clangCodeGen
   clangFrontend
   clangLex
   clangSerialization
   clangTooling
   )
+
+target_include_directories(clangHandleCXX PUBLIC .)

clang/tools/clang-fuzzer/proto-to-cxx/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 proto_to_cxx.cpp proto_to_cxx_main.cpp
                           loop_proto_to_cxx.cpp loop_proto_to_cxx_main.cpp)
 
 add_clang_library(clangProtoToCXX proto_to_cxx.cpp
                   DEPENDS clangCXXProto
                   LINK_LIBS clangCXXProto ${PROTOBUF_LIBRARIES}
                   )
 
 add_clang_library(clangLoopProtoToCXX loop_proto_to_cxx.cpp
                   DEPENDS clangCXXLoopProto
                   LINK_LIBS clangCXXLoopProto ${PROTOBUF_LIBRARIES}
                   )
+target_include_directories(clangProtoToCXX PUBLIC .)
+target_include_directories(clangLoopProtoToCXX PUBLIC .)
 
 add_clang_executable(clang-proto-to-cxx proto_to_cxx_main.cpp)
 add_clang_executable(clang-loop-proto-to-cxx loop_proto_to_cxx_main.cpp)
 
 target_link_libraries(clang-proto-to-cxx PRIVATE clangProtoToCXX)
 target_link_libraries(clang-loop-proto-to-cxx PRIVATE clangLoopProtoToCXX)

lldb/tools/lldb-fuzzer/CMakeLists.txt

 add_subdirectory(lldb-commandinterpreter-fuzzer)
+add_subdirectory(lldb-expression-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)

lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/CMakeLists.txt

+if(CLANG_ENABLE_PROTO_FUZZER)
+  set(LLVM_LINK_COMPONENTS
+    Support
+    )
+
+  add_llvm_fuzzer(lldb-expression-fuzzer
+    EXCLUDE_FROM_ALL
+    lldb-expression-fuzzer.cpp
+    )
+
+  if(TARGET lldb-expression-fuzzer)
+    target_include_directories(lldb-expression-fuzzer PRIVATE ..)
+    find_package(Protobuf REQUIRED)
+    add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI)
+    include_directories(${PROTOBUF_INCLUDE_DIRS})
+    include_directories(${CMAKE_CURRENT_BINARY_DIR}/../../../../clang/tools/clang-fuzzer PRIVATE ..)
+
+    set(CLANG_CMAKE_MODULE_PATH
+      ${CMAKE_CURRENT_SOURCE_DIR}/../../../../clang/cmake/modules)
+
+    set(CMAKE_MODULE_PATH
+      ${CMAKE_MODULE_PATH}
+      ${CLANG_CMAKE_MODULE_PATH})
+
+
+    set (PBM_PREFIX lldb_protobuf_mutator)
+    include(ProtobufMutator)
+    include_directories(${ProtobufMutator_INCLUDE_DIRS})
+
+    target_link_libraries(lldb-expression-fuzzer
+      PRIVATE
+      ${ProtobufMutator_LIBRARIES}
+      ${LLVM_LIB_FUZZING_ENGINE}
+      clangHandleCXX
+      clangCXXProto
+      clangProtoToCXX
+      liblldb
+      )
+
+    add_custom_command(TARGET lldb-expression-fuzzer PRE_BUILD
+      COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_BINARY_DIR}/fuzzer-artifacts/expression-artifacts
+      # Create and compile a simple C program using the command line. This is
+      # needed because LLDB's expression evaluator needs a legitmate target
+      # instead of a dummy target
+      COMMAND echo 'int main (int argc, char** argv) { return 0\; }' | clang -o main.out -xc -
+      )
+
+    # Create a directory for storing the fuzzer's artifacts and run the fuzzer with arguments that will
+    # not attempt to reduce the size of the inputs being generated
+    add_custom_target(fuzz-lldb-expression
+      COMMENT "Running the LLDB expression evaluator fuzzer..."
+      WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/fuzzer-artifacts/expression-artifacts
+      COMMAND $<TARGET_FILE:lldb-expression-fuzzer> -artifact_prefix=expression- -reduce_inputs=0
+      USES_TERMINAL
+      )
+  endif()
+endif()

lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp

+//===-- lldb-expression-fuzzer.cpp ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+//
+// \file
+// This file is a fuzzer for LLDB's expression evaluator. It uses protobufs
+// and the libprotobuf-mutator to create valid C-like inputs for the
+// expression evaluator.
+//
+//===---------------------------------------------------------------------===//
+
+#include <string>
+
+#include "cxx_proto.pb.h"
+#include "handle_cxx.h"
+#include "lldb/API/SBBreakpoint.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBLaunchInfo.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/API/SBTarget.h"
+#include "proto_to_cxx.h"
+#include "src/libfuzzer/libfuzzer_macro.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+using namespace lldb;
+using namespace llvm;
+using namespace clang_fuzzer;
+
+char **originalargv;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+
+  // The path for a simple compiled program is needed to create a
+  // target for the debugger and that path is passed in through argv
+  originalargv = *argv;
+  return 0;
+}
+
+DEFINE_BINARY_PROTO_FUZZER(const clang_fuzzer::Function &input) {
+  auto input_string = clang_fuzzer::FunctionToString(input);
+
+  // Get the second argument from argv and strip the '--' from it.
+  // This will be used as the path for the object file to create a target from
+  std::string raw_path = originalargv[2];
+  StringRef obj_path = raw_path.erase(0, 2);
+
+  // Create a debugger and a target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.CreateTarget(obj_path.str().c_str());
+
+  // Create a breakpoint on the only line in the program
+  SBBreakpoint breakpoint = target.BreakpointCreateByLocation(obj_path.str().c_str(), 1);
+
+  // Create launch info and error for launching the process
+  SBLaunchInfo launch_info = target.GetLaunchInfo();
+  SBError error;
+
+  // Launch the process and evaluate the fuzzer's input data
+  // as an expression
+  SBProcess process = target.Launch(launch_info, error);
+  target.EvaluateExpression(input_string.c_str());
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+}