[Lldb-commits] [PATCH] D132148: [lldb][docs] Add documentation for LLDB fuzzers

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova added inline comments.


================
Comment at: lldb/docs/resources/fuzzing.rst:12
+
+Building the LLDB fuzzers requires a build configuration that has the address sanitizer and sanitizer coverage. This CMake invocation will configure a build directory that can be used to build the LLDB fuzzers:
+
----------------
mib wrote:
> missing word ?
Yep :)


================
Comment at: lldb/docs/resources/fuzzing.rst:14-26
+::
+   $ cmake <path to root of llvm source tree> \
+        -G Ninja \
+        -DCMAKE_BUILD_TYPE='Release' \
+        -DLLVM_USE_SANITIZER='Address' \
+        -DLLVM_USE_SANITIZE_COVERAGE=On \
+        -DLLVM_BUILD_RUNTIME=Off \
----------------
JDevlieghere wrote:
> I would simplify this a bit and say that in addition to your regular CMake arguments, you have to pass `        -DLLVM_USE_SANITIZER='Address'  -DLLVM_USE_SANITIZE_COVERAGE=On`. I think the libfuzzer documentation says something similar so in addition to listing that explicitly here, we should also include a link to that (in case that ever changes in the future). 
That's a good idea. I didn't mention this explicitly here but I put the entire CMake invocation because I assumed that someone who wanted to try this would be making a new, not-in-source-tree build directory.


================
Comment at: lldb/docs/resources/fuzzing.rst:37
+
+Note that building the LLDB expression evaluator fuzzer will require the CMake option ``-DCLANG_ENABLE_PROTO_FUZZER=ON``.
+
----------------
mib wrote:
> Is it an issue to have this enabled for the other fuzzers ? If not, may be you should just add it to the general cmake invocation 
Having this enabled doesn't cause problems for the other fuzzers. I had this on its own in case there were people that didn't want to use all of the fuzzers and therefore wouldn't need this option enabled all the time.

I can place this in the line that would say "In addition to your regular CMake arguments...". Also having a target that builds all fuzzers is a good idea.


================
Comment at: lldb/docs/resources/fuzzing.rst:42
+
+Currently, there are plans to integrate the LLDB fuzzers into the `OSS Fuzz <https://github.com/google/oss-fuzz>`_ project for continuous integration.
+
----------------
JDevlieghere wrote:
> I think this could be its own section that talks about where the fuzzers are (will be) running. 
I can add a "Continuous Integration" section for OSS Fuzz


================
Comment at: lldb/docs/resources/fuzzing.rst:47-49
+   $ ./<lldb fuzzer build directory>/bin/lldb-target-fuzzer
+   $ ./<lldb fuzzer build directory>/bin/lldb-commandinterpreter-fuzzer
+   $ ./<lldb fuzzer build directory>/bin/lldb-expression-fuzzer
----------------
JDevlieghere wrote:
> I would say  "from the build directory" and use relative paths here.
Sounds good, what's funny is that I originally had the relative directories and then I removed them to try and be more general :)


================
Comment at: lldb/docs/resources/fuzzing.rst:69-72
+If you want to reproduce the issue found by a fuzzer once you have gotten the input, you can pass the input to LLDB depending on which component you were fuzzing. For example, if you found an input that crashed target creation, you could run:
+
+::
+   $ lldb <input you are investigating>
----------------
JDevlieghere wrote:
> This is specific to LLDB's target fuzzer and not something I think folks should rely on. libfuzzer makes it really easy to reproduce bugs (as you explain below) so we should encourage everyone to use that.  
That makes sense, I used the target fuzzer as an example for using fuzzer inputs with LLDB itself but for reproducing it's better to use libFuzzer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132148/new/

https://reviews.llvm.org/D132148