[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a reviewer: bogner.
kcc added a comment.

+bogner@ FYI



================
Comment at: clang/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp:25
+
+static void MaybePrint(const std::string &S) {
+  static const char *env = getenv("CXXFUZZ_PRINT");
----------------
this is debug code, not worth having here, plz remove


================
Comment at: clang/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp:34
+  MaybePrint(S);
+  HandleCXX(S, {"-O2", "-mllvm", "-scalar-evolution-max-arith-depth=4"});
+  if (getenv("CXX_FUZZ_MORE")) {
----------------
Remove "-mllvm", "-scalar-evolution-max-arith-depth=4".
It's there as a workaround for a performance bug (https://bugs.llvm.org/show_bug.cgi?id=33494) but it shouldn't be here. 


================
Comment at: clang/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp:35
+  HandleCXX(S, {"-O2", "-mllvm", "-scalar-evolution-max-arith-depth=4"});
+  if (getenv("CXX_FUZZ_MORE")) {
+    HandleCXX(S, {"-O1", "-triple", "arm-apple-darwin10", "-mllvm",
----------------
Remove this section. 
In a later change, please allow to change the tripple (and any other flags) similar to https://reviews.llvm.org/D36275


================
Comment at: clang/tools/clang-fuzzer/cxx_proto.proto:16
+
+syntax = "proto2";
+//option cc_api_version = 2;
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > I'd suggest proto3
> proto3 has no required, to avoid backward compatibility issues.
> Same is useful for us, we don't wont to discard corpus if we drop some field in the future.
I'm afraid it's much more convenient to have 'required' here. 
How else could you express a binary op node? 


================
Comment at: clang/tools/clang-fuzzer/cxx_proto.proto:93
+}
+
+package clang_fuzzer;
----------------
vitalybuka wrote:
> morehouse wrote:
> > vitalybuka wrote:
> > > message CxxInput {
> > >   required Function f = 1;
> > >   required int/enum opt_level = 2;
> > >   required enum tripple = 3;
> > >   required scalar-evolution-max-arith-depth ...
> > > }
> > Interesting idea.  This would allow for protobuf-mutator to choose different option combinations, if I understand correctly.
> > 
> > Is that worth adding to this initial patch, though?
> yes, instead of CXX_FUZZ_MORE
For now, keep it as is, please (see my other comment about flags) 


https://reviews.llvm.org/D36324