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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to