[PATCH] D50342: Changed how LLVM IR was generated to increase vectorization

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:125
 std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
-  return os << "define void @foo(i32* %a, i32* %b, i32* noalias %c, i64 %s) 
{\n"
-<< "%i = alloca i64\n"
-<< "store i64 0, i64* %i\n"
-<< "br label %loop\n\n"
-<< "loop:\n"
-<< "%ct = load i64, i64* %i\n"
-<< "%comp = icmp eq i64 %ct, %s\n"
-<< "br i1 %comp, label %endloop, label %body\n\n"
-<< "body:\n"
+  return os << "target triple = \"x86_64-pc-linux-gnu\"\n"
+<< "define void @foo(i32*, i32*, i32*, i64) {\n"

What does `pc` mean in this triple?  I'm used to seeing 
`x86_64-unknown-linux-gnu`.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:126
+  return os << "target triple = \"x86_64-pc-linux-gnu\"\n"
+<< "define void @foo(i32*, i32*, i32*, i64) {\n"
+<< "%5 = icmp sgt i64 %3, 0\n"

Does removing the variable names really make this easier to vectorize?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:129
+<< "br i1 %5, label %6, label %8\n"
+<< "; :6:\n"
+<< "br label %9\n"

Does removing branch names really make this easier to vectorize?


Repository:
  rC Clang

https://reviews.llvm.org/D50342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50342: Changed how LLVM IR was generated to increase vectorization

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:129
+<< "br i1 %5, label %6, label %8\n"
+<< "; :6:\n"
+<< "br label %9\n"

emmettneyman wrote:
> morehouse wrote:
> > Does removing branch names really make this easier to vectorize?
> Same answer as above. Should I change these back?
I think the previous labels were more human-readable.


Repository:
  rC Clang

https://reviews.llvm.org/D50342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50342: Changed how LLVM IR was generated to increase vectorization

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:127
+<< "define void @foo(i32* %a, i32* %b, i32* %c, i64 %s) {\n"
+<< "%1 = icmp sgt i64 %s, 0\n"
+<< "br i1 %1, label %start, label %end\n"

Should `%s` be signed?  Do we want unsigned compare here?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:134
+<< "end:\n"
+<< "ret void\n"
 << "loop:\n"

Seems like the `endloop` label is unnecessary.  Does this help vectorize?  If 
not, lets get rid of unconditional jumps to the next line.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:138
 << x.statements()
-<< "%z = add i64 1, %ct\n"
-<< "store i64 %z, i64* %i\n"
-<< "br label %loop\n\n"
-<< "endloop:\n"
-<< "ret void\n}\n";
+<< "%ctnew = add nuw nsw i64 %ct, 1\n"
+<< "%j = icmp eq i64 %ctnew, %s\n"

This will make overflow undefined... Isn't that the opposite of what we want?  
That will permit LLVM to assume overflow never happens and modify the code in 
new ways based on that assumption.


Repository:
  rC Clang

https://reviews.llvm.org/D50342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50482: Added another optimization pass to make vectorizing possible

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:90
+getCodeModel(), OLvl);
+}
+

If you have to pass that many parameters to a 3 line function, just inline 
instead.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:126
+
+  auto 

[PATCH] D50482: Added another optimization pass to make vectorizing possible

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:103
 
+  Triple ModuleTriple(M->getTargetTriple());
+  const TargetOptions Options = InitTargetOptionsFromCodeGenFlags();

I think you can avoid creating a Triple and instead just call 
`M->getTargetTriple()` in `lookupTarget` below.


Repository:
  rC Clang

https://reviews.llvm.org/D50482



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50530: Added LLVM metadata to generated IR to increase vectorization width

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:141
+<< "!1 = !{!\"llvm.loop.vectorize.enable\", i1 true}\n"
+<< "!2 = !{!\"llvm.loop.vectorize.width\", i32 " << kArraySize
+<< "}\n";

I'm not sure `kArraySize` is what you want here.  Does "width" refer to array 
size or the SIMD width?

Any problem with letting the vectorizer determine this automatically?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:142
+<< "!2 = !{!\"llvm.loop.vectorize.width\", i32 " << kArraySize
+<< "}\n";
 }

Does this metadata change coverage in the vectorizer?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.h:18
 
+#include "../handle-llvm/input_arrays.h"
+

This include should be in the cpp file.


Repository:
  rC Clang

https://reviews.llvm.org/D50530



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50530: Added LLVM metadata to generated IR to increase vectorization width

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

Code LGTM, but let's make sure this actually helps before landing.


Repository:
  rC Clang

https://reviews.llvm.org/D50530



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of multiple loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Does having multiple loops one after another change any coverage in the 
vectorizer?


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of multiple loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D50670#1199556, @emmettneyman wrote:

> Should I switch my focus to nested loops instead? I think nested loops will 
> increase coverage.


Yes, I'd recommend doing that.


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of multiple loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Another option would be to allow simple control flow within the loop itself.


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of nested loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Does this hit new coverage in the vectorizer?




Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:46
 std::string VarRefToString(std::ostream &os, const VarRef &x) {
+  std::string var = inner_loop ? "inner" : "outer";
   std::string arr;

Please choose a better name than var.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:127
   }
+  inner_loop = true;
   return os;

This looks like a bug.  `inner_loop` never gets set to false again.  Might be a 
good reason to make it parameter instead.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:131
 std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
-  return os << "target triple = \"x86_64-unknown-linux-gnu\"\n"
-<< "define void @foo(i32* %a, i32* %b, i32* %c, i64 %s) {\n"
-<< "%1 = icmp sgt i64 %s, 0\n"
-<< "br i1 %1, label %start, label %end\n"
-<< "start:\n"
-<< "br label %loop\n"
-<< "end:\n"
-<< "ret void\n"
-<< "loop:\n"
-<< " %ct   = phi i64 [ %ctnew, %loop ], [ 0, %start ]\n"
-<< x.statements()
-<< "%ctnew = add i64 %ct, 1\n"
-<< "%j = icmp eq i64 %ctnew, %s\n"
-<< "br i1 %j, label %end, label %loop, !llvm.loop !0\n}\n"
-<< "!0 = distinct !{!0, !1, !2}\n"
-<< "!1 = !{!\"llvm.loop.vectorize.enable\", i1 true}\n"
-<< "!2 = !{!\"llvm.loop.vectorize.width\", i32 " << kArraySize
-<< "}\n";
+  os << "target triple = \"x86_64-pc-linux-gnu\"\n"
+ << "define void @foo(i32* %a, i32* %b, i32* noalias %c, i64 %s) {\n"

Why do you change this to `pc` again?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:132
+  os << "target triple = \"x86_64-pc-linux-gnu\"\n"
+ << "define void @foo(i32* %a, i32* %b, i32* noalias %c, i64 %s) {\n"
+ << "%cmp = icmp sgt i64 %s, 0\n"

I'm curious how this change affects coverage independent of the rest of this 
change.  Also what would happen if we set `%a` and `%b` to noalias as well?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:136
+ << "outer_loop_start:\n"
+ << "br label %inner_loop_start\n"
+ << "inner_loop_start:\n"

Looks like a pointless branch.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:144
+ << x.outer_statements()
+ << "%o_ct_new = add nuw nsw i64 %outer_ct, 1\n"
+ << "%jmp_outer = icmp eq i64 %o_ct_new, %s\n"

Why `nuw`, `nsw` here?



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:154
+ << "}\n"
+ << "!0 = distinct !{!0, !1, !2}\n"
+ << "!1 = !{!\"llvm.loop.vectorize.enable\", i1 true}\n"

Can we simplify the order of blocks here?  It is confusing to follow all these 
jumps forward and backward.


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of nested loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:127
   }
+  inner_loop = true;
   return os;

Maybe this fixes the bug, but modifying `inner_loop` from different functions 
is still error-prone.

Please either make this a scoped variable (with a wrapper class that sets it to 
true in the constructor and sets it to false in the destructor), or make it a 
parameter.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:140
+ << "br label %inner_loop\n"
+ << "end:\n"
+ << "ret void\n"

I don't see any jumps to `end`.  I think this will be an infinite loop.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:143
+ << "outer_loop:\n"
+ << x.outer_statements()
+ << "%o_ct_new = add i64 %outer_ct, 1\n"

IIUC this creates loop structure always like this:

```
for (int i = 0; i < s; i++) {
  for (int j = 0; j < s; j++) {
// statements
  }
  // statements
}
```

Maybe not necessary for this patch, but I'm curious if adding statements before 
the inner loop would exercise different coverage in the vectorizer.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:143
+ << "outer_loop:\n"
+ << x.outer_statements()
+ << "%o_ct_new = add i64 %outer_ct, 1\n"

morehouse wrote:
> IIUC this creates loop structure always like this:
> 
> ```
> for (int i = 0; i < s; i++) {
>   for (int j = 0; j < s; j++) {
> // statements
>   }
>   // statements
> }
> ```
> 
> Maybe not necessary for this patch, but I'm curious if adding statements 
> before the inner loop would exercise different coverage in the vectorizer.
Will all loops be double-nested now?


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of nested loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:127
   }
+  inner_loop = true;
   return os;

emmettneyman wrote:
> morehouse wrote:
> > Maybe this fixes the bug, but modifying `inner_loop` from different 
> > functions is still error-prone.
> > 
> > Please either make this a scoped variable (with a wrapper class that sets 
> > it to true in the constructor and sets it to false in the destructor), or 
> > make it a parameter.
> Would it be better if `inner_loop` was only modified in the `LoopFunction` 
> operator override? For example:
> 
> ```
> std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
>   inner_loop = false;
>   os << "target triple = \"x86_64-unknown-linux-gnu\"\n"
>  
>  << << "outer_loop:\n"
>  << x.outer_statements();
>   inner_loop = true;
>   os << "%o_ct_new = add i64 %outer_ct, 1\n"
>  
>  << "!2 = !{!\"llvm.loop.vectorize.width\", i32 " << kArraySize << "}\n";
>   return os;
> 
> ```
> And removed `inner_loop = true;` from the above function?
That would be better.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:140
+ << "br label %inner_loop\n"
+ << "end:\n"
+ << "ret void\n"

emmettneyman wrote:
> morehouse wrote:
> > I don't see any jumps to `end`.  I think this will be an infinite loop.
> There are two jumps to `end`, one before entering the `outer_loop` and one at 
> the end of `outer_loop`.
Right, sorry not sure what I missed there.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:143
+ << "outer_loop:\n"
+ << x.outer_statements()
+ << "%o_ct_new = add i64 %outer_ct, 1\n"

emmettneyman wrote:
> emmettneyman wrote:
> > morehouse wrote:
> > > morehouse wrote:
> > > > IIUC this creates loop structure always like this:
> > > > 
> > > > ```
> > > > for (int i = 0; i < s; i++) {
> > > >   for (int j = 0; j < s; j++) {
> > > > // statements
> > > >   }
> > > >   // statements
> > > > }
> > > > ```
> > > > 
> > > > Maybe not necessary for this patch, but I'm curious if adding 
> > > > statements before the inner loop would exercise different coverage in 
> > > > the vectorizer.
> > > Will all loops be double-nested now?
> > Yes, that's correct. It's also worth noting that all the statements in the 
> > inner loop will only use `j` to index into the arrays, never `i`. It 
> > shouldn't be hard to add outer loop statements before the inner loop. A 
> > third `StatementSeq` could be added to the `LoopFunction` proto: one for 
> > outer loop statements before the inner loop, one for inner loop statements, 
> > and one for outer loop statements after the inner loop.
> Yes, all loops will be double-nested. It shouldn't be difficult to make the 
> inner loop optional though. In `cxx_loop_proto`, the first `Statement_Seq`, 
> `inner_statements`, can be made `optional` and then a different IR structures 
> can be generated depending on whether or not `inner_statements` exists.
Can we do that in this patch?  I think it makes sense to be able to generate 
either single loops or nested loops.


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50670: Implementation of nested loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp:124
+void SingleLoopToString(std::ostream &os, const LoopFunction &x) {
+  inner_loop = false;
+  os << "void foo(int *a, int *b, int *__restrict__ c, size_t s) {\n"

Why do we need to set `inner_loop` from different functions?

Again, using the global like this is just too easy to screw up.  I'd like to 
see it default to false and then have a scoped wrapper that sets it to true 
only for the duration necessary.



Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:137
+ << x.outer_statements();
+  inner_loop = true;
+  os << "%o_ct_new = add i64 %outer_ct, 1\n"

Same comment as above.  I'm getting confused trying to understand what 
`inner_loop` is set to at any given point.


Repository:
  rC Clang

https://reviews.llvm.org/D50670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50829: Update README and Dockerfile to include llvm-proto-fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/README.txt:104
+To translate a cxx_loop_proto file into LLVM IR do:
+  bin/clang-loop-proto-to-llvm
+To translate a cxx_loop_proto file into C++ do:

`bin/clang-loop-proto-to-llvm CORPUS_OUTPUT_FILE`



Comment at: clang/tools/clang-fuzzer/README.txt:106
+To translate a cxx_loop_proto file into C++ do:
+  bin/clang-loop-proto-to-cxx
+

`bin/clang-loop-proto-to-cxx CORPUS_OUTPUT_FILE`



Comment at: clang/tools/clang-fuzzer/README.txt:111
+the fuzzer is not only compiling code, but also running it, as the inputs get
+large, the time necessary to fuzz one input can get very high.

Maybe add a sample cmake invocation?


Repository:
  rC Clang

https://reviews.llvm.org/D50829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51008: Enable -fsanitize=fuzzer and -fsanitize=fuzzer-no-link on Windows.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

LGTM




Comment at: lib/Driver/ToolChains/MSVC.cpp:373
+ TC.getCompilerRTArgString(Args, "fuzzer", 
false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation

Why is `-debug` needed?


Repository:
  rC Clang

https://reviews.llvm.org/D51008



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: mgorny.

Build with DummyClangFuzzer.cpp as entry point when coverage
instrumentation isn't present.


https://reviews.llvm.org/D38642

Files:
  clang/tools/clang-fuzzer/CMakeLists.txt
  clang/tools/clang-fuzzer/ClangFuzzer.cpp
  clang/tools/clang-fuzzer/DummyClangFuzzer.cpp

Index: clang/tools/clang-fuzzer/DummyClangFuzzer.cpp
===
--- /dev/null
+++ clang/tools/clang-fuzzer/DummyClangFuzzer.cpp
@@ -0,0 +1,21 @@
+//===-- DummyClangFuzzer.cpp - Entry point to sanity check fuzzers ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Provides a main() to build without linking libFuzzer.
+//
+//===--===//
+#include "llvm/FuzzMutate/FuzzerCLI.h"
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv);
+
+int main(int argc, char *argv[]) {
+  return llvm::runFuzzerOnInputs(argc, argv, LLVMFuzzerTestOneInput,
+ LLVMFuzzerInitialize);
+}
Index: clang/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- clang/tools/clang-fuzzer/ClangFuzzer.cpp
+++ clang/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -17,6 +17,8 @@
 
 using namespace clang_fuzzer;
 
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
+
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, {"-O2"});
Index: clang/tools/clang-fuzzer/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/CMakeLists.txt
+++ clang/tools/clang-fuzzer/CMakeLists.txt
@@ -1,60 +1,65 @@
-if( LLVM_USE_SANITIZE_COVERAGE )
-  set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
-  set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
+set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(DUMMY_MAIN DummyClangFuzzer.cpp)
+if(LLVM_USE_SANITIZE_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+  unset(DUMMY_MAIN)
+endif()
+
+# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
+# executables from this directory.
+set(LLVM_OPTIONAL_SOURCES
+  ClangFuzzer.cpp
+  DummyClangFuzzer.cpp
+  ExampleClangProtoFuzzer.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})
+  include_directories(${CMAKE_CURRENT_BINARY_DIR})
+  protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
+  set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES} ${PROTO_SRCS})
+  add_clang_library(clangCXXProto
+${PROTO_SRCS}
+${PROTO_HDRS}
+
+LINK_LIBS
+${PROTOBUF_LIBRARIES}
+)
 
-  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})
-include_directories(${CMAKE_CURRENT_BINARY_DIR})
-protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES
-  ClangFuzzer.cpp
-  ExampleClangProtoFuzzer.cpp
-  ${PROTO_SRCS}
-  )
-add_clang_library(clangCXXProto
-  ${PROTO_SRCS}
-  ${PROTO_HDRS}
-
-  LINK_LIBS
-  ${PROTOBUF_LIBRARIES}
-  )
-
-# Build and include libprotobuf-mutator
-include(ProtobufMutator)
-include_directories(${ProtobufMutator_INCLUDE_DIRS})
-
-# Build the protobuf->C++ translation library and driver.
-add_clang_subdirectory(proto-to-cxx)
-
-# Build the protobuf fuzzer
-add_clang_executable(clang-proto-fuzzer ExampleClangProtoFuzzer.cpp)
-target_link_libraries(clang-proto-fuzzer
-  ${ProtobufMutator_LIBRARIES}
-  clangCXXProto
-  clangHandleCXX
-  clangProtoToCXX
-  )
-  else()
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES ClangFuzzer.cpp ExampleClangProtoFuzzer.cpp)
-  endif()
-
-  add_clang_subdirectory(handle-cxx)
-
-  add_clang_executable(clang-fuzzer
-EXCLUDE_FROM_ALL
-ClangFuzzer.cpp
+  # Build and include libprotobuf-mutator
+  include(ProtobufMutator)
+  includ

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D38642#890963, @kcc wrote:

> It's not about coverage instrumentation (not) being present, but about 
> libFuzzer's main() being present, right?


Yes.

> Will we be able to reuse some of Justin's code instead of creating one more 
> main() function?

This reuses the code that Justin moved to FuzzMutate/FuzzerCLI.  That's why the 
main is so short.  But perhaps we could move the main itself into FuzzerCLI?

> Or, why not link with libFuzzer (-fsanitize=fuzzer at link time) even if we 
> don't us einstrumentation at compile time?

When I tried this, I got undefined references to all kinds of 
`__sanitizer_cov_*` symbols.


https://reviews.llvm.org/D38642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D38642#890969, @kcc wrote:

> I'd like to know more. 
>  At least simple cases work fine:


You're right.  I was trying to add `-fsanitize=fuzzer` to `CMAKE_CXX_FLAGS` 
right before the link command, which was causing a later compilation to give 
the error.  Setting `CMAKE_EXE_LINKER_FLAGS` seems to work though.

This seems simpler and cleaner than the approach @bogner took.


https://reviews.llvm.org/D38642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 118087.
morehouse added a comment.

- Remove dummy main and link with -fsantize=fuzzer.


https://reviews.llvm.org/D38642

Files:
  clang/tools/clang-fuzzer/CMakeLists.txt
  clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt

Index: clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt
+++ clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
 set(CMAKE_CXX_FLAGS ${CXX_FLAGS_NOFUZZ})
+set(CMAKE_EXE_LINKER_FLAGS ${EXE_LINKER_FLAGS_NOFUZZ})
 
 # Hack to bypass LLVM's CMake source checks so we can have both a library and
 # an executable built from this directory.
Index: clang/tools/clang-fuzzer/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/CMakeLists.txt
+++ clang/tools/clang-fuzzer/CMakeLists.txt
@@ -1,60 +1,56 @@
-if( LLVM_USE_SANITIZE_COVERAGE )
-  set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
-  set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
+set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(EXE_LINKER_FLAGS_NOFUZZ ${CMAKE_EXE_LINKER_FLAGS})
+if(LLVM_USE_SANITIZE_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
-
-  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})
-include_directories(${CMAKE_CURRENT_BINARY_DIR})
-protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES
-  ClangFuzzer.cpp
-  ExampleClangProtoFuzzer.cpp
-  ${PROTO_SRCS}
-  )
-add_clang_library(clangCXXProto
-  ${PROTO_SRCS}
-  ${PROTO_HDRS}
-
-  LINK_LIBS
-  ${PROTOBUF_LIBRARIES}
-  )
-
-# Build and include libprotobuf-mutator
-include(ProtobufMutator)
-include_directories(${ProtobufMutator_INCLUDE_DIRS})
-
-# Build the protobuf->C++ translation library and driver.
-add_clang_subdirectory(proto-to-cxx)
-
-# Build the protobuf fuzzer
-add_clang_executable(clang-proto-fuzzer ExampleClangProtoFuzzer.cpp)
-target_link_libraries(clang-proto-fuzzer
-  ${ProtobufMutator_LIBRARIES}
-  clangCXXProto
-  clangHandleCXX
-  clangProtoToCXX
-  )
-  else()
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES ClangFuzzer.cpp ExampleClangProtoFuzzer.cpp)
-  endif()
-
-  add_clang_subdirectory(handle-cxx)
-
-  add_clang_executable(clang-fuzzer
-EXCLUDE_FROM_ALL
-ClangFuzzer.cpp
+endif()
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=fuzzer")
+
+# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
+# executables from this directory.
+set(LLVM_OPTIONAL_SOURCES ClangFuzzer.cpp ExampleClangProtoFuzzer.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})
+  include_directories(${CMAKE_CURRENT_BINARY_DIR})
+  protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
+  set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES} ${PROTO_SRCS})
+  add_clang_library(clangCXXProto
+${PROTO_SRCS}
+${PROTO_HDRS}
+
+LINK_LIBS
+${PROTOBUF_LIBRARIES}
 )
 
-  target_link_libraries(clang-fuzzer
+  # Build and include libprotobuf-mutator
+  include(ProtobufMutator)
+  include_directories(${ProtobufMutator_INCLUDE_DIRS})
+
+  # Build the protobuf->C++ translation library and driver.
+  add_clang_subdirectory(proto-to-cxx)
+
+  # Build the protobuf fuzzer
+  add_clang_executable(clang-proto-fuzzer ExampleClangProtoFuzzer.cpp)
+  target_link_libraries(clang-proto-fuzzer
+${ProtobufMutator_LIBRARIES}
+clangCXXProto
 clangHandleCXX
+clangProtoToCXX
 )
 endif()
+
+add_clang_subdirectory(handle-cxx)
+
+add_clang_executable(clang-fuzzer
+  EXCLUDE_FROM_ALL
+  ClangFuzzer.cpp
+  )
+
+target_link_libraries(clang-fuzzer
+  clangHandleCXX
+  )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 118097.
morehouse added a comment.

- Revert "Remove dummy main and link with -fsantize=fuzzer."


https://reviews.llvm.org/D38642

Files:
  clang/tools/clang-fuzzer/CMakeLists.txt
  clang/tools/clang-fuzzer/ClangFuzzer.cpp
  clang/tools/clang-fuzzer/DummyClangFuzzer.cpp

Index: clang/tools/clang-fuzzer/DummyClangFuzzer.cpp
===
--- /dev/null
+++ clang/tools/clang-fuzzer/DummyClangFuzzer.cpp
@@ -0,0 +1,21 @@
+//===-- DummyClangFuzzer.cpp - Entry point to sanity check fuzzers ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Provides a main() to build without linking libFuzzer.
+//
+//===--===//
+#include "llvm/FuzzMutate/FuzzerCLI.h"
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv);
+
+int main(int argc, char *argv[]) {
+  return llvm::runFuzzerOnInputs(argc, argv, LLVMFuzzerTestOneInput,
+ LLVMFuzzerInitialize);
+}
Index: clang/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- clang/tools/clang-fuzzer/ClangFuzzer.cpp
+++ clang/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -17,6 +17,8 @@
 
 using namespace clang_fuzzer;
 
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
+
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, {"-O2"});
Index: clang/tools/clang-fuzzer/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/CMakeLists.txt
+++ clang/tools/clang-fuzzer/CMakeLists.txt
@@ -1,60 +1,65 @@
-if( LLVM_USE_SANITIZE_COVERAGE )
-  set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
-  set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
+set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(DUMMY_MAIN DummyClangFuzzer.cpp)
+if(LLVM_USE_SANITIZE_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+  unset(DUMMY_MAIN)
+endif()
+
+# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
+# executables from this directory.
+set(LLVM_OPTIONAL_SOURCES
+  ClangFuzzer.cpp
+  DummyClangFuzzer.cpp
+  ExampleClangProtoFuzzer.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})
+  include_directories(${CMAKE_CURRENT_BINARY_DIR})
+  protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
+  set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES} ${PROTO_SRCS})
+  add_clang_library(clangCXXProto
+${PROTO_SRCS}
+${PROTO_HDRS}
+
+LINK_LIBS
+${PROTOBUF_LIBRARIES}
+)
 
-  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})
-include_directories(${CMAKE_CURRENT_BINARY_DIR})
-protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES
-  ClangFuzzer.cpp
-  ExampleClangProtoFuzzer.cpp
-  ${PROTO_SRCS}
-  )
-add_clang_library(clangCXXProto
-  ${PROTO_SRCS}
-  ${PROTO_HDRS}
-
-  LINK_LIBS
-  ${PROTOBUF_LIBRARIES}
-  )
-
-# Build and include libprotobuf-mutator
-include(ProtobufMutator)
-include_directories(${ProtobufMutator_INCLUDE_DIRS})
-
-# Build the protobuf->C++ translation library and driver.
-add_clang_subdirectory(proto-to-cxx)
-
-# Build the protobuf fuzzer
-add_clang_executable(clang-proto-fuzzer ExampleClangProtoFuzzer.cpp)
-target_link_libraries(clang-proto-fuzzer
-  ${ProtobufMutator_LIBRARIES}
-  clangCXXProto
-  clangHandleCXX
-  clangProtoToCXX
-  )
-  else()
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES ClangFuzzer.cpp ExampleClangProtoFuzzer.cpp)
-  endif()
-
-  add_clang_subdirectory(handle-cxx)
-
-  add_clang_executable(clang-fuzzer
-EXCLUDE_FROM_ALL
-ClangFuzzer.cpp
+  # Build and include libprotobuf-mutator
+  include(ProtobufMutator)
+  include_directories(${Protobuf

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D38642#891074, @kcc wrote:

> If you can *easily* share main() with the one in LLVM -- do it, otherwise 
> don't bother.


Does the fuzzer main come from LLVM or compiler-rt now?  There's still 
FuzzerMain.cpp, but I'm not sure if we should be using that or not.


https://reviews.llvm.org/D38642



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315336: [clang-fuzzer] Allow building without coverage 
instrumentation. (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D38642?vs=118097&id=118420#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38642

Files:
  cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
  cfe/trunk/tools/clang-fuzzer/ClangFuzzer.cpp
  cfe/trunk/tools/clang-fuzzer/DummyClangFuzzer.cpp

Index: cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
===
--- cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
+++ cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
@@ -1,60 +1,65 @@
-if( LLVM_USE_SANITIZE_COVERAGE )
-  set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD})
-  set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
+set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
+set(DUMMY_MAIN DummyClangFuzzer.cpp)
+if(LLVM_USE_SANITIZE_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+  unset(DUMMY_MAIN)
+endif()
+
+# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
+# executables from this directory.
+set(LLVM_OPTIONAL_SOURCES
+  ClangFuzzer.cpp
+  DummyClangFuzzer.cpp
+  ExampleClangProtoFuzzer.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})
+  include_directories(${CMAKE_CURRENT_BINARY_DIR})
+  protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
+  set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES} ${PROTO_SRCS})
+  add_clang_library(clangCXXProto
+${PROTO_SRCS}
+${PROTO_HDRS}
+
+LINK_LIBS
+${PROTOBUF_LIBRARIES}
+)
 
-  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})
-include_directories(${CMAKE_CURRENT_BINARY_DIR})
-protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES
-  ClangFuzzer.cpp
-  ExampleClangProtoFuzzer.cpp
-  ${PROTO_SRCS}
-  )
-add_clang_library(clangCXXProto
-  ${PROTO_SRCS}
-  ${PROTO_HDRS}
-
-  LINK_LIBS
-  ${PROTOBUF_LIBRARIES}
-  )
-
-# Build and include libprotobuf-mutator
-include(ProtobufMutator)
-include_directories(${ProtobufMutator_INCLUDE_DIRS})
-
-# Build the protobuf->C++ translation library and driver.
-add_clang_subdirectory(proto-to-cxx)
-
-# Build the protobuf fuzzer
-add_clang_executable(clang-proto-fuzzer ExampleClangProtoFuzzer.cpp)
-target_link_libraries(clang-proto-fuzzer
-  ${ProtobufMutator_LIBRARIES}
-  clangCXXProto
-  clangHandleCXX
-  clangProtoToCXX
-  )
-  else()
-# Hack to bypass LLVM's cmake sources check and allow multiple libraries and
-# executables from this directory.
-set(LLVM_OPTIONAL_SOURCES ClangFuzzer.cpp ExampleClangProtoFuzzer.cpp)
-  endif()
-
-  add_clang_subdirectory(handle-cxx)
-
-  add_clang_executable(clang-fuzzer
-EXCLUDE_FROM_ALL
-ClangFuzzer.cpp
+  # Build and include libprotobuf-mutator
+  include(ProtobufMutator)
+  include_directories(${ProtobufMutator_INCLUDE_DIRS})
+
+  # 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
 )
 
-  target_link_libraries(clang-fuzzer
+  target_link_libraries(clang-proto-fuzzer
+${ProtobufMutator_LIBRARIES}
+clangCXXProto
 clangHandleCXX
+clangProtoToCXX
 )
 endif()
+
+add_clang_subdirectory(handle-cxx)
+
+add_clang_executable(clang-fuzzer
+  EXCLUDE_FROM_ALL
+  ${DUMMY_MAIN}
+  ClangFuzzer.cpp
+  )
+
+target_link_libraries(clang-fuzzer
+  clangHandleCXX
+  )
Index: cfe/trunk/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- cfe/trunk/tools/clang-fuzzer/ClangFuzzer.cpp
+++ cfe/trunk/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -17,6 +17,8 @@
 
 using namespace clang_fuzzer;
 
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
+
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, {"-O2"});
Index: cfe/trunk/tools/clang-fuzzer/DummyClangFuzzer.cpp
===
--- cfe/trunk/tools/clang-fuzzer/DummyClangFuzzer.cpp
+++ cfe/trunk/tools

[PATCH] D38812: [clang-fuzzer] Allow linking with any fuzzing engine.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: mgorny.

Makes clang-[proto-]fuzzer compatible with flags specified by OSS-Fuzz.

https://llvm.org/pr34314


https://reviews.llvm.org/D38812

Files:
  clang/tools/clang-fuzzer/CMakeLists.txt


Index: clang/tools/clang-fuzzer/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/CMakeLists.txt
+++ clang/tools/clang-fuzzer/CMakeLists.txt
@@ -1,7 +1,9 @@
 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
 set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
 set(DUMMY_MAIN DummyClangFuzzer.cpp)
-if(LLVM_USE_SANITIZE_COVERAGE)
+if(DEFINED 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)
@@ -48,6 +50,7 @@
   target_link_libraries(clang-proto-fuzzer
 ${ProtobufMutator_LIBRARIES}
 ${PROTOBUF_LIBRARIES}
+${LIB_FUZZING_ENGINE}
 clangCXXProto
 clangHandleCXX
 clangProtoToCXX
@@ -63,5 +66,6 @@
   )
 
 target_link_libraries(clang-fuzzer
+  ${LIB_FUZZING_ENGINE}
   clangHandleCXX
   )


Index: clang/tools/clang-fuzzer/CMakeLists.txt
===
--- clang/tools/clang-fuzzer/CMakeLists.txt
+++ clang/tools/clang-fuzzer/CMakeLists.txt
@@ -1,7 +1,9 @@
 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
 set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
 set(DUMMY_MAIN DummyClangFuzzer.cpp)
-if(LLVM_USE_SANITIZE_COVERAGE)
+if(DEFINED 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)
@@ -48,6 +50,7 @@
   target_link_libraries(clang-proto-fuzzer
 ${ProtobufMutator_LIBRARIES}
 ${PROTOBUF_LIBRARIES}
+${LIB_FUZZING_ENGINE}
 clangCXXProto
 clangHandleCXX
 clangProtoToCXX
@@ -63,5 +66,6 @@
   )
 
 target_link_libraries(clang-fuzzer
+  ${LIB_FUZZING_ENGINE}
   clangHandleCXX
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38812: [clang-fuzzer] Allow linking with any fuzzing engine.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315486: [clang-fuzzer] Allow linking with any fuzzing 
engine. (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D38812?vs=118651&id=118655#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38812

Files:
  cfe/trunk/tools/clang-fuzzer/CMakeLists.txt


Index: cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
===
--- cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
+++ cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
@@ -1,7 +1,9 @@
 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
 set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
 set(DUMMY_MAIN DummyClangFuzzer.cpp)
-if(LLVM_USE_SANITIZE_COVERAGE)
+if(DEFINED 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)
@@ -48,6 +50,7 @@
   target_link_libraries(clang-proto-fuzzer
 ${ProtobufMutator_LIBRARIES}
 ${PROTOBUF_LIBRARIES}
+${LIB_FUZZING_ENGINE}
 clangCXXProto
 clangHandleCXX
 clangProtoToCXX
@@ -63,5 +66,6 @@
   )
 
 target_link_libraries(clang-fuzzer
+  ${LIB_FUZZING_ENGINE}
   clangHandleCXX
   )


Index: cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
===
--- cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
+++ cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
@@ -1,7 +1,9 @@
 set(LLVM_LINK_COMPONENTS ${LLVM_TARGETS_TO_BUILD} FuzzMutate)
 set(CXX_FLAGS_NOFUZZ ${CMAKE_CXX_FLAGS})
 set(DUMMY_MAIN DummyClangFuzzer.cpp)
-if(LLVM_USE_SANITIZE_COVERAGE)
+if(DEFINED 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)
@@ -48,6 +50,7 @@
   target_link_libraries(clang-proto-fuzzer
 ${ProtobufMutator_LIBRARIES}
 ${PROTOBUF_LIBRARIES}
+${LIB_FUZZING_ENGINE}
 clangCXXProto
 clangHandleCXX
 clangProtoToCXX
@@ -63,5 +66,6 @@
   )
 
 target_link_libraries(clang-fuzzer
+  ${LIB_FUZZING_ENGINE}
   clangHandleCXX
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38853: [clang-format] Allow building fuzzer with OSS-Fuzz flags.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: mgorny.

https://reviews.llvm.org/D38853

Files:
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-format/fuzzer/CMakeLists.txt


Index: clang/tools/clang-format/fuzzer/CMakeLists.txt
===
--- clang/tools/clang-format/fuzzer/CMakeLists.txt
+++ clang/tools/clang-format/fuzzer/CMakeLists.txt
@@ -1,11 +1,15 @@
 set(LLVM_LINK_COMPONENTS support)
 
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+if(LLVM_USE_SANITIZE_COVERAGE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
 
 add_clang_executable(clang-format-fuzzer
   EXCLUDE_FROM_ALL
   ClangFormatFuzzer.cpp
   )
 
 target_link_libraries(clang-format-fuzzer
-  ${CLANG_FORMAT_LIB_DEPS})
+  ${CLANG_FORMAT_LIB_DEPS}
+  ${LIB_FUZZING_ENGINE}
+  )
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -15,7 +15,7 @@
   ${CLANG_FORMAT_LIB_DEPS}
   )
 
-if( LLVM_USE_SANITIZE_COVERAGE )
+if( LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
   add_subdirectory(fuzzer)
 endif()
 


Index: clang/tools/clang-format/fuzzer/CMakeLists.txt
===
--- clang/tools/clang-format/fuzzer/CMakeLists.txt
+++ clang/tools/clang-format/fuzzer/CMakeLists.txt
@@ -1,11 +1,15 @@
 set(LLVM_LINK_COMPONENTS support)
 
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+if(LLVM_USE_SANITIZE_COVERAGE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
 
 add_clang_executable(clang-format-fuzzer
   EXCLUDE_FROM_ALL
   ClangFormatFuzzer.cpp
   )
 
 target_link_libraries(clang-format-fuzzer
-  ${CLANG_FORMAT_LIB_DEPS})
+  ${CLANG_FORMAT_LIB_DEPS}
+  ${LIB_FUZZING_ENGINE}
+  )
Index: clang/tools/clang-format/CMakeLists.txt
===
--- clang/tools/clang-format/CMakeLists.txt
+++ clang/tools/clang-format/CMakeLists.txt
@@ -15,7 +15,7 @@
   ${CLANG_FORMAT_LIB_DEPS}
   )
 
-if( LLVM_USE_SANITIZE_COVERAGE )
+if( LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
   add_subdirectory(fuzzer)
 endif()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38853: [clang-format] Allow building fuzzer with OSS-Fuzz flags.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL315603: [clang-format] Allow building fuzzer with OSS-Fuzz 
flags. (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D38853?vs=118805&id=118819#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38853

Files:
  cfe/trunk/tools/clang-format/CMakeLists.txt
  cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt


Index: cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
===
--- cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
+++ cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
@@ -1,11 +1,15 @@
 set(LLVM_LINK_COMPONENTS support)
 
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+if(LLVM_USE_SANITIZE_COVERAGE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
 
 add_clang_executable(clang-format-fuzzer
   EXCLUDE_FROM_ALL
   ClangFormatFuzzer.cpp
   )
 
 target_link_libraries(clang-format-fuzzer
-  ${CLANG_FORMAT_LIB_DEPS})
+  ${CLANG_FORMAT_LIB_DEPS}
+  ${LIB_FUZZING_ENGINE}
+  )
Index: cfe/trunk/tools/clang-format/CMakeLists.txt
===
--- cfe/trunk/tools/clang-format/CMakeLists.txt
+++ cfe/trunk/tools/clang-format/CMakeLists.txt
@@ -15,7 +15,7 @@
   ${CLANG_FORMAT_LIB_DEPS}
   )
 
-if( LLVM_USE_SANITIZE_COVERAGE )
+if( LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
   add_subdirectory(fuzzer)
 endif()
 


Index: cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
===
--- cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
+++ cfe/trunk/tools/clang-format/fuzzer/CMakeLists.txt
@@ -1,11 +1,15 @@
 set(LLVM_LINK_COMPONENTS support)
 
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+if(LLVM_USE_SANITIZE_COVERAGE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=fuzzer")
+endif()
 
 add_clang_executable(clang-format-fuzzer
   EXCLUDE_FROM_ALL
   ClangFormatFuzzer.cpp
   )
 
 target_link_libraries(clang-format-fuzzer
-  ${CLANG_FORMAT_LIB_DEPS})
+  ${CLANG_FORMAT_LIB_DEPS}
+  ${LIB_FUZZING_ENGINE}
+  )
Index: cfe/trunk/tools/clang-format/CMakeLists.txt
===
--- cfe/trunk/tools/clang-format/CMakeLists.txt
+++ cfe/trunk/tools/clang-format/CMakeLists.txt
@@ -15,7 +15,7 @@
   ${CLANG_FORMAT_LIB_DEPS}
   )
 
-if( LLVM_USE_SANITIZE_COVERAGE )
+if( LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
   add_subdirectory(fuzzer)
 endif()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51022: [libFuzzer] Port to Windows

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341082: [libFuzzer] Port to Windows (authored by morehouse, 
committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51022?vs=163338&id=163348#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51022

Files:
  lib/Driver/ToolChains/MSVC.cpp


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -365,6 +365,17 @@
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + 
ImplibName));
   }
 
+  if (TC.getSanitizerArgs().needsFuzzer()) {
+if (!Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(
+  Args.MakeArgString(std::string("-wholearchive:") +
+ TC.getCompilerRTArgString(Args, "fuzzer", 
false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation
+// arrays.
+CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
+  }
+
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1298,6 +1309,8 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
+  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -365,6 +365,17 @@
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + ImplibName));
   }
 
+  if (TC.getSanitizerArgs().needsFuzzer()) {
+if (!Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(
+  Args.MakeArgString(std::string("-wholearchive:") +
+ TC.getCompilerRTArgString(Args, "fuzzer", false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation
+// arrays.
+CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
+  }
+
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1298,6 +1309,8 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
+  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

You can probably get rid of the llvm-objcopy code and make this a lot simpler 
with something like:

1. Call `getSection()` on the Binary object to get the text section.
2. Read the `sh_offset` and `sh_size` of that section.
3. Copy `sh_size` bytes from the start of the binary buffer + `sh_offset` into 
your executable memory.
4. Run it.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:21
+  set(handle_llvm_deps)
+endif()
 

How are you doing your diff?  Some of these changes are already upstream.  
Please rebase 



Comment at: clang/tools/clang-fuzzer/handle-llvm/fuzzer_initialize.cpp:52
+  return 0;
+}

Can we use `fuzzer-initialize/` now?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:54
+
+static std::string OptIR;
+

Unless there's a good reason for global, let's make this a local.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:17
 
-# Depend on LLVM IR intrinsic generation.
+# Depend on LLVM IR instrinsic generation.
 set(handle_llvm_deps intrinsics_gen)

Typo introduced here.



Comment at: clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt:25
   handle_llvm.cpp
-
+  
   DEPENDS

Please don't add whitespace to a previously empty line.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:10
 //
-// Implements HandleLLVM for use by the Clang fuzzers. Mimics the llc tool to
-// compile an LLVM IR file to X86_64 assembly.
+// Implements HandleLLVM for use by the Clang fuzzers. First runs an loop
+// vectorizer optimization pass over the given IR code. Then mimics lli on both

an -> a



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:112
+// Mimics the opt tool to run an optimization pass over the provided IR
+std::string OptLLVM(const std::string IR, CodeGenOpt::Level &OLvl) {
+  InitEverything();

Maybe this should be `const std::string &IR` or `StringRef`.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:113
+std::string OptLLVM(const std::string IR, CodeGenOpt::Level &OLvl) {
+  InitEverything();
+ 

Does this need to be called every time we get a new input?  Can we call this 
from `LLVMFuzzerInitialize()`?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:123
+  args[1] = arg1.c_str();
+  cl::ParseCommandLineOptions(2, args);
 

This shouldn't be required.  You should be able to directly add the passes you 
want.  See `llvm/lib/Passes/PassBuilder.cpp`.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:132
+
+  Triple ModuleTriple(M->getTargetTriple());
+  TargetOptions Options = InitTargetOptionsFromCodeGenFlags();

Move this closer to where it's used.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:133
+  Triple ModuleTriple(M->getTargetTriple());
+  TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
+  std::string CPUStr;

`Options` is never used.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:136
+  std::string FeaturesStr;
+  std::unique_ptr TM(nullptr);
+  setFunctionAttributes(CPUStr, FeaturesStr, *M);

`TM` is never used.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:137
+  std::unique_ptr TM(nullptr);
+  setFunctionAttributes(CPUStr, FeaturesStr, *M);
+  

Does this do anything since `CPUStr` and `FeaturesStr` are empty?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:145
+  std::unique_ptr FPasses;
+  FPasses.reset(new legacy::FunctionPassManager(M.get()));
+  FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));

Can we just make `FPasses` on stack instead?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:190
+  builder.setMCJITMemoryManager(
+  std::unique_ptr(RTDyldMM));
+  builder.setOptLevel(OLvl);

These 3 lines can be combined to `builder.setMCJITMemoryManager(new 
SectionMemoryManager())`


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:190
+  builder.setMCJITMemoryManager(
+  std::unique_ptr(RTDyldMM));
+  builder.setOptLevel(OLvl);

emmettneyman wrote:
> morehouse wrote:
> > These 3 lines can be combined to `builder.setMCJITMemoryManager(new 
> > SectionMemoryManager())`
> I use RTDyldMM on line 208. Should I just keep these lines as they are?
Ah, missed that.  In that case, you can probably simplify this line to
`builder.setMCJITMemoryManager(RTDyldMM)`.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:190
+  builder.setMCJITMemoryManager(
+  std::unique_ptr(RTDyldMM));
+  builder.setOptLevel(OLvl);

emmettneyman wrote:
> morehouse wrote:
> > emmettneyman wrote:
> > > morehouse wrote:
> > > > These 3 lines can be combined to `builder.setMCJITMemoryManager(new 
> > > > SectionMemoryManager())`
> > > I use RTDyldMM on line 208. Should I just keep these lines as they are?
> > Ah, missed that.  In that case, you can probably simplify this line to
> > `builder.setMCJITMemoryManager(RTDyldMM)`.
> It looks like set `MCJITMemoryManager()` needs to take a `unique_ptr`. I'm 
> not sure how to clean it up any more than it already is.
Does it not implicitly cast?


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:190
+  builder.setMCJITMemoryManager(
+  std::unique_ptr(RTDyldMM));
+  builder.setOptLevel(OLvl);

emmettneyman wrote:
> morehouse wrote:
> > emmettneyman wrote:
> > > morehouse wrote:
> > > > emmettneyman wrote:
> > > > > morehouse wrote:
> > > > > > These 3 lines can be combined to `builder.setMCJITMemoryManager(new 
> > > > > > SectionMemoryManager())`
> > > > > I use RTDyldMM on line 208. Should I just keep these lines as they 
> > > > > are?
> > > > Ah, missed that.  In that case, you can probably simplify this line to
> > > > `builder.setMCJITMemoryManager(RTDyldMM)`.
> > > It looks like set `MCJITMemoryManager()` needs to take a `unique_ptr`. 
> > > I'm not sure how to clean it up any more than it already is.
> > Does it not implicitly cast?
> No, I think it only works in the other direction.
Ok, I see.  The constructor we need is marked explicit...



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:208
+
+  static_cast(RTDyldMM)->invalidateInstructionCache();
+

This cast shouldn't be necessary.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp:44
+  
+  PassRegistry &Registry = *llvm::PassRegistry::getPassRegistry();
+  initializeCore(Registry);

Unnecessary `llvm::`



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:89
+  Builder.Inliner = createFunctionInliningPass(OptLevel, SizeLevel, false);
+  Builder.LoopVectorize = true;
+  Builder.populateFunctionPassManager(FPM);

If we do this, do we need to explicitly add the vectorizer pass below?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:95
+// Mimics the opt tool to run an optimization pass over the provided IR
+std::string OptLLVM(const std::string &IR, CodeGenOpt::Level &OLvl) {
+  // Create a module that will run the optimization passes

`OLvl` is never used.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:105
+  std::string FeaturesStr = getFeaturesStr();
+  setFunctionAttributes(CPUStr, FeaturesStr, *M);
+  

Let's simplify to `setFunctionAttributes(getCPUStr(), getFeaturesStr(), *M)`.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:110
+  TargetLibraryInfoImpl TLII(ModuleTriple);
+  Passes.add(new TargetLibraryInfoWrapperPass(TLII));
+  Passes.add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));

Can simplify to `Passes.add(new 
TargetLibraryInfoWrapperPass(M->getTargetTriple))`.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:115
+  make_unique(M.get());
+  FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
+

Why do we add a `TargetTransformInfoWrapperPass` to both `Passes` and `FPasses`?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:115
+  make_unique(M.get());
+  FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
+

morehouse wrote:
> Why do we add a `TargetTransformInfoWrapperPass` to both `Passes` and 
> `FPasses`?
Do we need both `Passes` and `FPasses`?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:121
+ErrorAndExit("Cannot create IR pass");
+  Passes.add(P);
+  

Let's simplify to `Passes.add(createLoopVectorizePass())`.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:127
+  FPasses->doFinalization();
+  Passes.add(createVerifierPass());
+  

Why do we keep adding passes in different places?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:144
+   Context);
+  Module *M = Owner.get();
+  if (!M)

Why not just rename `Owner` to `M` and remove this line?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:158
+  builder.setMCJITMemoryManager(
+  std::unique_ptr(RTDyldMM));
+  builder.setOptLevel(OLvl);

Please simplify to 
`builder.setMCJITMemoryManager(std::make_unique())`.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:159
+  std::unique_ptr(RTDyldMM));
+  builder.setOptLevel(OLvl);
+  builder.setTargetOptions(InitTargetOptionsFromCodeGenFlags());

If the JIT does optimization already, do we need to call `OptLLVM` at all?  
Will the vectorizer kick in without `OptLLVM`?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:168
+  if (!EntryFunc)
+ErrorAndExit("Function not found in module");
 

Move this closer to where it's used.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:171
+  EE->finalizeObject();
+  EE->runStaticConstructorsDestructors(false);
+

Do we need to call this again to run destructors after we execute `f()` ?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:181
   
-  raw_null_ostream OS;
-  Target->addPassesToEmitFile(PM, OS, nullptr, TargetMachine::CGFT_ObjectFile,
-  false);
-  PM.run(*M);
+  (*f)(a, b, c, 1);
+}

I think `f(a, b, c, 1)` will also work.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:115
+  make_unique(M.get());
+  FPasses->add(createTargetTransformInfoWrapperPass(TargetIRAnalysis()));
+

emmettneyman wrote:
> morehouse wrote:
> > morehouse wrote:
> > > Why do we add a `TargetTransformInfoWrapperPass` to both `Passes` and 
> > > `FPasses`?
> > Do we need both `Passes` and `FPasses`?
> I think because we're just adding a loop vectorize pass, we don't need 
> `FPasses`. The loop vectorize pass lives within the regular 
> `ModulePassManager`, not the `FunctionPassManager`. I decided to put it in 
> the code so, in the future, it would be easier to add different kinds of 
> passes to the fuzz target. Should I remove it?
Let's remove it since it's currently unnecessary.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:135
+  std::unique_ptr M = parseIR(MemoryBufferRef(IR, "IR"), Err,
+   Context);
+  if (!M)

The indentation looks off.  Please fix.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:147
+  builder.setUseOrcMCJITReplacement(false);
+  builder.setMCJITMemoryManager(make_unique());
+  builder.setOptLevel(OLvl);

This uses `llvm:make_unique`, which was written when LLVM used C++11.  But 
since LLVM now uses C++14, I think we should use `std::make_unique` instead.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:169
+  int c[] = {1};
+  
+  f(a, b, c, 1);

Remove whitespace on empty lines.

Actually, you could probably remove this line altogether since the call to 
`f()` can be grouped with the parameter setup above.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:184
+  CodeGenOpt::Level OLvl;
+  getOptLevel(ExtraArgs, OLvl);
+  

We're allowing the JIT opt-level to be specified on the command line, but we're 
hard-coding OptLevel=3 for the optimization passes.

Do we need to allow the opt-level to be specified on the command line?


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Do we need to parse the arguments for opt-level, or can we just hardcode `-O2` 
and remove the argument parsing code?




Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:144
+   Context);
+  Module *M = Owner.get();
+  if (!M)

emmettneyman wrote:
> morehouse wrote:
> > Why not just rename `Owner` to `M` and remove this line?
> Reverted it back since the change caused the fuzzer to crash.
You will need to move any uses of `M` above the call to `std::move`, since that 
leaves `M` in an invalid state.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:90
+// Mimics the opt tool to run an optimization pass over the provided IR
+std::string OptLLVM(const std::string &IR, int OLvl) {
+  // Create a module that will run the optimization passes

Shouldn't `OLvl` be a `CodeGenOpt::Level`?


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:125
+  Context);
+  Module *M = Owner.get();
+  if (!M)

We should be able to get rid of this line now, and rename Owner again.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:152
+  typedef void (*func)(int*, int*, int*, int);
+  func f = (func) EE->getPointerToFunction(EntryFunc); 
+

Can we use `reinterpret_cast` here?


Repository:
  rC Clang

https://reviews.llvm.org/D49526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50194: LLVM Proto Fuzzer - Run Functions on Suite of Inputs

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:173
+  int s = getSize((char *) func_ptr);
+  memcpy(mem, func_ptr, s);
+}

Why do we need to copy the function somewhere else?  Looks very error-prone and 
unnecessary.  Also makes this patch larger than it needs to be.



Comment at: clang/tools/clang-fuzzer/handle-llvm/input_arrays.cpp:30
+  memcpy(b2, InputArrays[b_index], ArraySize * sizeof(int));
+  memcpy(c2, InputArrays[c_index], ArraySize * sizeof(int));
+}

Do the generated functions ever modify arrays a and b, or just c?  If just c, 
we can avoid lots of memcpys here.



Comment at: clang/tools/clang-fuzzer/handle-llvm/input_arrays.h:34
+// Define a corpus of possible inputs
+static int InputArrays[100][64] =
+{ {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},

Use the constants you just defined.


Repository:
  rC Clang

https://reviews.llvm.org/D50194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50194: LLVM Proto Fuzzer - Run Functions on Suite of Inputs

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:173
+  int s = getSize((char *) func_ptr);
+  memcpy(mem, func_ptr, s);
+}

emmettneyman wrote:
> morehouse wrote:
> > Why do we need to copy the function somewhere else?  Looks very error-prone 
> > and unnecessary.  Also makes this patch larger than it needs to be.
> I'm copying the functions because otherwise, the generated machine code gets 
> lost as soon as we exit that function's scope. So I'd have to run the 
> functions inside `CreateJITFunction` if I don't copy it.
> 
> I thought about doing it this way: moving the code from `RunFuncsOnInputs` to 
> the bottom of `CreateJITFunction` and then comparing the arrays after both 
> calls to `CreateJITFunction` inside `HandleLLVM`. Do you think that would be 
> cleaner?
Or just increase the scope of `EE`.


Repository:
  rC Clang

https://reviews.llvm.org/D50194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50194: LLVM Proto Fuzzer - Run Functions on Suite of Inputs

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:75
 
+// Helper function to print error message and stop the fuzzer
 void ErrorAndExit(std::string message) {

Unnecessary comment.   The naming and implementation of this function are 
intuitive.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:170
 
-  typedef void (*func)(int*, int*, int*, int);
-  func f = reinterpret_cast(EE->getPointerToFunction(EntryFunc)); 
+  LLVMFunc f = (LLVMFunc) EE->getPointerToFunction(EntryFunc); 
 

Does `reinterpret_cast` work here?



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:184
+  // Populate OptArrays and UnoptArrays with the arrays from InputArrays
+  memcpy(OptArrays, InputArrays, sizeof(int) * ArraySize * NumArrays);
+  memcpy(UnoptArrays, InputArrays, sizeof(int) * ArraySize * NumArrays);

The size here is reused a few times.  Let's create a variable for it.


Repository:
  rC Clang

https://reviews.llvm.org/D50194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50194: LLVM Proto Fuzzer - Run Functions on Suite of Inputs

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D50194#1187756, @emmettneyman wrote:

> An unrelated question:
>  Right now I have a mix of `static` and non-`static` functions in 
> `handle_llvm.cpp`. Should they all be `static`?


Any functions that are only used in the same file can and should be `static` or 
in an unnamed namespace.  Functions that implement something from a header file 
must not be `static`.


Repository:
  rC Clang

https://reviews.llvm.org/D50194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50194: LLVM Proto Fuzzer - Run Functions on Suite of Inputs

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:132
+// Takes a string of IR and compiles it using LLVM's JIT Engine
+static void CreateJITFunc(const std::string &IR, CodeGenOpt::Level OLvl) {
   SMDiagnostic Err;

`CreateAndRunJITFunc` describes this better.



Comment at: clang/tools/clang-fuzzer/handle-llvm/input_arrays.h:36
+  {1, 1, 2, 3, 2, 3, 0, 11, 10, 0, 7, 5, 3, 1, 18, 18, 18, 18, 0, 18, 18, 18, 
18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 
18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 
18, 18},
+  {0, 0, 2, 5, 4, 0, 2, 13, 12, 11, 0, 8, 6, 4, 2, 0, 20, 20, 20, 20, 0, 20, 
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 
20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 
20, 20},
+  {1, 1, 0, 1, 6, 2, 4, 1, 14, 13, 12, 0, 9, 7, 5, 3, 1, 22, 22, 22, 22, 22, 
0, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 
22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 22, 
22, 22},

emmettneyman wrote:
> kcc wrote:
> > constants are not diverse enough. 
> Yeah, I will completely redo the input arrays.
Please land today once you do.



Comment at: clang/tools/clang-fuzzer/handle-llvm/input_arrays.h:15
+#include 
+#include 
+

Are these includes needed?



Comment at: clang/tools/clang-fuzzer/handle-llvm/input_arrays.h:19
+static const int NumArrays = 100;
+static const int TotalSize = sizeof(int) * ArraySize * NumArrays;
+

Maybe prefix these names with `k` since they are constant.


Repository:
  rC Clang

https://reviews.llvm.org/D50194



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
morehouse added a reviewer: kcc.
Herald added a subscriber: hiraditya.

When building with libFuzzer, simplifyCFG reduces the coverage signal
available to libFuzzer when trying to find new inputs.  This patch
provides a way to disable simplifyCFG when building with libFuzzer.


https://reviews.llvm.org/D43423

Files:
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/no-simplify-cfg.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/SimplifyCFG/no-simplify-cfg.ll

Index: llvm/test/Transforms/SimplifyCFG/no-simplify-cfg.ll
===
--- /dev/null
+++ llvm/test/Transforms/SimplifyCFG/no-simplify-cfg.ll
@@ -0,0 +1,50 @@
+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+define i32 @foo(i32 %x) !no_simplify_cfg !{} {
+entry:
+  %x.addr = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  %0 = load i32, i32* %x.addr, align 4
+  %cmp = icmp sgt i32 %0, 16
+  br i1 %cmp, label %land.rhs, label %land.end
+
+land.rhs:
+  %1 = load i32, i32* %x.addr, align 4
+  %cmp1 = icmp slt i32 %1, 32
+  br label %land.end
+
+land.end:
+  %2 = phi i1 [ false, %entry ], [ %cmp1, %land.rhs ]
+  %conv = zext i1 %2 to i32
+  ret i32 %conv
+
+; CHECK-LABEL: define i32 @foo(i32 %x) !no_simplify_cfg
+; CHECK-LABEL: entry:
+; CHECK: br i1 %cmp, label %land.rhs, label %land.end
+; CHECK-LABEL: land.rhs:
+; CHECK: br label %land.end
+; CHECK-LABEL: land.end:
+; CHECK: phi {{.*}} %entry {{.*}} %land.rhs
+}
+
+define i32 @bar(i32 %x) {
+entry:
+  %x.addr = alloca i32, align 4
+  store i32 %x, i32* %x.addr, align 4
+  %0 = load i32, i32* %x.addr, align 4
+  %cmp = icmp sgt i32 %0, 16
+  br i1 %cmp, label %land.rhs, label %land.end
+
+land.rhs:
+  %1 = load i32, i32* %x.addr, align 4
+  %cmp1 = icmp slt i32 %1, 32
+  br label %land.end
+
+land.end:
+  %2 = phi i1 [ false, %entry ], [ %cmp1, %land.rhs ]
+  %conv = zext i1 %2 to i32
+  ret i32 %conv
+
+; CHECK-LABEL: define i32 @bar(i32 %x)
+; CHECK-NOT: br
+}
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6053,6 +6053,9 @@
 bool llvm::simplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI,
const SimplifyCFGOptions &Options,
SmallPtrSetImpl *LoopHeaders) {
+  const Function *Fn = BB->getParent();
+  if (Fn && Fn->getMetadata("no_simplify_cfg"))
+return false;
   return SimplifyCFGOpt(TTI, BB->getModule()->getDataLayout(), LoopHeaders,
 Options)
   .run(BB);
Index: clang/test/CodeGen/no-simplify-cfg.cpp
===
--- /dev/null
+++ clang/test/CodeGen/no-simplify-cfg.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s --check-prefix=SIMPLIFY
+// RUN: %clang_cc1 -fno-simplify-cfg -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: define i32 @_Z3foov() {{.*}} !no_simplify_cfg
+int foo() {
+  return 42;
+}
+
+// CHECK: define i32 @_Z3barv() {{.*}} !no_simplify_cfg
+int bar() {
+  return foo() * foo() + foo();
+}
+
+// CHECK: define i32 @_Z3bazii(i32 %x, i32 %y) {{.*}} !no_simplify_cfg
+int baz(int x, int y) {
+  int z = x / y;
+  return z + foo() - bar();
+}
+
+// SIMPLIFY-NOT: no_simplify_cfg
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -571,6 +571,7 @@
   Opts.MergeAllConstants = !Args.hasArg(OPT_fno_merge_all_constants);
   Opts.NoCommon = Args.hasArg(OPT_fno_common);
   Opts.NoImplicitFloat = Args.hasArg(OPT_no_implicit_float);
+  Opts.NoSimplifyCFG = Args.hasArg(OPT_fno_simplify_cfg);
   Opts.OptimizeSize = getOptimizationLevelSize(Args);
   Opts.SimplifyLibCalls = !(Args.hasArg(OPT_fno_builtin) ||
 Args.hasArg(OPT_ffreestanding));
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -862,6 +862,11 @@
   if (SanOpts.has(SanitizerKind::SafeStack))
 Fn->addFnAttr(llvm::Attribute::SafeStack);
 
+  // Apply no-simplify-cfg attribute to the function
+  if (CGM.getCodeGenOpts().NoSimplifyCFG)
+Fn->setMetadata("no_simplify_cfg",
+llvm::MDNode::get(CGM.getLLVMContext(), None));
+
   // Ignore TSan memory acesses from within ObjC/ObjC++ dealloc, initialize,
   // .cxx_destruct, __destroy_helper_block_ and all of their calees at run time.
   if (SanOpts.has(SanitizerKind::Thread)) {
Index: clang/include/clang/Frontend/CodeGenOptions.def
=

[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D43423#1011170, @davide wrote:

> Some high level comments:
>
> 1. This is something that GCC does relatively frequently (adding frontend 
> options to control optimization passes), but LLVM tends to not expose these 
> details. FWIW, I'd very much prefer the details of the optimizer wouldn't be 
> exposed as frontend flags.


We might not need the flag if we decide to always set `no_simplify_cfg` during 
codegen with coverage instrumentation.  But we're not sure if we want to do 
that yet.

> 2. I really don't think metadata is the correct way of conveying this 
> information to the optimizer. Among all the other problems, metadata can be 
> stripped willy-nilly. Maybe a function attribute will work better?

Should be simple enough to change this to a function attribute if you think 
that is more appropriate.  Wasn't sure whether metadata or an attribute made 
more sense.

> Some alternatives which I don't like, but I can't currently propose anything 
> better are:
> 
> 1. Having a separate optimization pipeline that you use in these cases

This seems like a lot more work.

> 2. Having an instrumentation pass that annotates your CFG to prevent passes 
> from destroying coverage signal. That said, I'm not really familiar to 
> comment on whether this is feasible or not.

This patch allows us to annotate our functions with `no_simplify_cfg` to do 
what you're suggesting.  Writing another instrumentation pass seems like 
overkill.

> Please note that completely disabling SimplifyCFG will result in non-trivial 
> performance hits (at least, in my experience), so you might want to evaluate 
> this carefully.
>  @chandlerc might have thoughts on how to address this.

We're still evaluating this with mixed results.  On one hand, disabling 
`simplifyCFG` makes most of our libFuzzer tests pass with `-O2`, and libFuzzer 
seems to perform at the same executions/sec rate.  On the other hand, 
executions/sec doesn't necessarily translate to more coverage/sec or bugs found.




Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:109
 
 static cl::opt MergeCondStores(
 "simplifycfg-merge-cond-stores", cl::Hidden, cl::init(true),

vitalybuka wrote:
> why metadata instead of cl:opt like these?
We want to be able to avoid `simplifyCFG` when we pass `-fsanitize=fuzzer` to 
the Clang driver.  AFAICT, the frontend does not explicitly invoke 
`simplifyCFG`, so it can't control the options here.


https://reviews.llvm.org/D43423



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 129318.
morehouse added a comment.

- Enable use-after-dtor instrumentation by default.
- Make sanitize-no-dtor-callback.cpp test fail with UAD instrumentation.
- Update test cases to reflect new default.


https://reviews.llvm.org/D37860

Files:
  clang/include/clang/Driver/SanitizerArgs.h
  clang/test/CodeGenCXX/sanitize-no-dtor-callback.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/msan/dtor-member.cc
  compiler-rt/test/msan/use-after-dtor.cc

Index: compiler-rt/test/msan/use-after-dtor.cc
===
--- compiler-rt/test/msan/use-after-dtor.cc
+++ compiler-rt/test/msan/use-after-dtor.cc
@@ -1,14 +1,17 @@
 // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out
+
+// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t > %t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out
 
 #include 
 #include 
@@ -32,14 +35,16 @@
   Simple *s = new(&buf) Simple();
   s->~Simple();
 
+  fprintf(stderr, "\n");  // Need output to parse for CHECK-UAD-OFF case
   return s->x_;
 
-  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
-  // CHECK: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]]
+  // CHECK-UAD: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK-UAD: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]]
 
   // CHECK-ORIGINS: Memory was marked as uninitialized
   // CHECK-ORIGINS: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK-ORIGINS: {{#1 0x.* in Simple::~Simple}}
 
-  // CHECK: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}}
+  // CHECK-UAD: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}}
+  // CHECK-UAD-OFF-NOT: SUMMARY: MemorySanitizer: use-of-uninitialized-value
 }
Index: compiler-rt/test/msan/dtor-member.cc
===
--- compiler-rt/test/msan/dtor-member.cc
+++ compiler-rt/test/msan/dtor-member.cc
@@ -7,7 +7,7 @@
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
-// RUN: %clangxx_msan %s -fsanitize=memory -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -fsanitize=memory -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
 
 // RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -184,11 +184,11 @@
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
 // CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
 // CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-field-pa

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

PTAL.  Patch has been updated.


https://reviews.llvm.org/D37860



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCRT31: [MSan] Enable use-after-dtor instrumentation by 
default. (authored by morehouse, committed by ).
Herald added a subscriber: Sanitizers.

Changed prior to commit:
  https://reviews.llvm.org/D37860?vs=129318&id=129322#toc

Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D37860

Files:
  test/msan/dtor-member.cc
  test/msan/use-after-dtor.cc


Index: test/msan/use-after-dtor.cc
===
--- test/msan/use-after-dtor.cc
+++ test/msan/use-after-dtor.cc
@@ -1,14 +1,17 @@
 // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o 
%t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory 
-fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not 
%run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory 
-fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not 
%run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory 
-fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && 
MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out
+
+// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && 
MSAN_OPTIONS=poison_in_dtor=1 not %run %t > %t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out
 
 #include 
 #include 
@@ -32,14 +35,16 @@
   Simple *s = new(&buf) Simple();
   s->~Simple();
 
+  fprintf(stderr, "\n");  // Need output to parse for CHECK-UAD-OFF case
   return s->x_;
 
-  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
-  // CHECK: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]]
+  // CHECK-UAD: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK-UAD: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]]
 
   // CHECK-ORIGINS: Memory was marked as uninitialized
   // CHECK-ORIGINS: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK-ORIGINS: {{#1 0x.* in Simple::~Simple}}
 
-  // CHECK: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}}
+  // CHECK-UAD: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}}
+  // CHECK-UAD-OFF-NOT: SUMMARY: MemorySanitizer: use-of-uninitialized-value
 }
Index: test/msan/dtor-member.cc
===
--- test/msan/dtor-member.cc
+++ test/msan/dtor-member.cc
@@ -7,7 +7,7 @@
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory 
-fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run 
%t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
-// RUN: %clangxx_msan %s -fsanitize=memory -o %t && 
MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -fsanitize=memory -fno-sanitize-memory-use-after-dtor 
-o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
 
 // RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o 
%t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1


Index: test/msan/use-after-dtor.cc
===
--- test/msan/use-after-dtor.cc
+++ test/msan/use-after-dtor.cc
@@ -1,14 +1,17 @@
 // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out
+
+// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t > %t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out
 
 #include 
 #include 
@@ -32,14 +35,16 @@
   Simple *s = new(&buf) Simple();
   s->~Simple();

[PATCH] D47666: Refactored clang-fuzzer and added new (copy) files

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: tools/clang-fuzzer/CMakeLists.txt:48
 ExampleClangProtoFuzzer.cpp
+FuzzerInitialize.cpp
 )

Rather than compiling `FuzzerInitialize.cpp` into the binary, can we make it a 
library like `handle-cxx` or `proto-to-cxx`?



Comment at: tools/clang-fuzzer/FuzzerInitialize.cpp:17
 
 #include "cxx_proto.pb.h"
 

Do we need this include?



Comment at: tools/clang-fuzzer/FuzzerInitialize.h:20
+
+#include 
+

Why do we need these includes in the header?  Doesn't look like they're used 
here.


Repository:
  rC Clang

https://reviews.llvm.org/D47666



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47666: Refactored clang-fuzzer and added new (copy) files

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp:23
 
 #include 
 

I think `cstring` is no longer used after this change.  So we can probably 
remove this include.



Comment at: tools/clang-fuzzer/fuzzer-initialize/CMakeLists.txt:3
+
+add_clang_library(clangFuzzerInit fuzzer_initialize.cpp)

Nit:  `clangFuzzerInitialize` would better follow the naming convention of the 
other libraries.



Comment at: tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h:15
+
+#include "src/libfuzzer/libfuzzer_macro.h"
+

I don't think this include is used in this file either.  Can we remove it?


Repository:
  rC Clang

https://reviews.llvm.org/D47666



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47666: Refactored clang-fuzzer and added new (copy) files

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D47666



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47666: Refactored clang-fuzzer and added new (copy) files

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333969: [clang-proto-fuzzer] Refactored LLVMFuzzerInitialize 
into its own file. (authored by morehouse, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47666?vs=149872&id=149876#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47666

Files:
  cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
  cfe/trunk/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp
  cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/CMakeLists.txt
  cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp
  cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h

Index: cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
===
--- cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
+++ cfe/trunk/tools/clang-fuzzer/CMakeLists.txt
@@ -40,6 +40,9 @@
   # Build the protobuf->C++ translation library and driver.
   add_clang_subdirectory(proto-to-cxx)
 
+  # Build the fuzzer initialization library.
+  add_clang_subdirectory(fuzzer-initialize)
+
   # Build the protobuf fuzzer
   add_clang_executable(clang-proto-fuzzer
 ${DUMMY_MAIN}
@@ -52,6 +55,7 @@
 ${PROTOBUF_LIBRARIES}
 ${LLVM_LIB_FUZZING_ENGINE}
 clangCXXProto
+clangFuzzerInitialize
 clangHandleCXX
 clangProtoToCXX
 )
Index: cfe/trunk/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp
===
--- cfe/trunk/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp
+++ cfe/trunk/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp
@@ -17,28 +17,12 @@
 #include "cxx_proto.pb.h"
 #include "handle-cxx/handle_cxx.h"
 #include "proto-to-cxx/proto_to_cxx.h"
-
+#include "fuzzer-initialize/fuzzer_initialize.h"
 #include "src/libfuzzer/libfuzzer_macro.h"
 
-#include 
-
 using namespace clang_fuzzer;
 
-static std::vector 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());
 }
Index: cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h
===
--- cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h
+++ cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.h
@@ -0,0 +1,19 @@
+//==-- fuzzer_initialize.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 
+
+namespace clang_fuzzer {
+const std::vector& GetCLArgs();
+}
Index: cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp
===
--- cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp
+++ cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp
@@ -0,0 +1,43 @@
+//===-- fuzzer_initialize.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 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 "fuzzer_initialize.h"
+#include 
+
+using namespace clang_fuzzer;
+
+
+namespace clang_fuzzer {
+
+static std::vector CLArgs;
+
+const std::vector& 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;
+}
Index: cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/CMakeLists.txt
===
--- cfe/trunk/tools/clang-fuzzer/fuzzer-initialize/CMakeLists.txt
+++ cfe/trunk/tools/cla

[PATCH] D47843: Introducing single for loop into clang_proto_fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

This contains changes from previous patch.  Please rebase.


Repository:
  rC Clang

https://reviews.llvm.org/D47843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47843: Introducing single for loop into clang_proto_fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: tools/clang-fuzzer/cxx_loop_proto.proto:93
+
+message Function {
+  required StatementSeq statements = 1;

Maybe call this `LoopFunction` to distinguish from the other protobuf.



Comment at: tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp:14
+/// the fuzzer with the correct command line arguments. 
 ///
 
//===--===//

Nit:  Try not to introduce whitespace at end of lines.  Depending on your 
configuration, this causes git to complain.



Comment at: tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp:115
+
+} // namespace clang_fuzzer

Right now this file duplicates a lot of code from `proto_to_cxx.cpp`.  But 
assuming this file will continue to diverge from `proto_to_cxx.cpp`, I'm fine 
with the duplication for now.



Comment at: tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.h:22
+std::string LoopProtoToCxx(const uint8_t *data, size_t size);
+}

Instead of making a whole new header for this, can we simply add  
`LoopProtoToCxx()` and `LoopFunctionToString()` to proto_to_cxx.h?  Then 
implement them in `loop_proto_to_cxx.cpp`?


Repository:
  rC Clang

https://reviews.llvm.org/D47843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47843: Introducing single for loop into clang_proto_fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: tools/clang-fuzzer/CMakeLists.txt:28
   protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_proto.proto)
+  protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS cxx_loop_proto.proto)
   set(LLVM_OPTIONAL_SOURCES ${LLVM_OPTIONAL_SOURCES} ${PROTO_SRCS})

I think it makes sense to use separate SRCS and HDRS variables for 
cxx_loop_proto.proto.  Otherwise each proto-fuzzer will be compiled with 
//both// protobufs even though each only uses one.



Comment at: tools/clang-fuzzer/CMakeLists.txt:58
   add_clang_subdirectory(fuzzer-initialize)
 
   # Build the protobuf fuzzer

Why is this here twice?



Comment at: tools/clang-fuzzer/CMakeLists.txt:90
+clangLoopProtoToCXX
+)
 endif()

Maybe you can cut down on some LOC here by creating a 
`COMMON_PROTO_FUZZ_LIBRARIES` variable with the dependencies that overlap 
between the proto-fuzzers.



Comment at: tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.h:22
+std::string LoopProtoToCxx(const uint8_t *data, size_t size);
+}

morehouse wrote:
> Instead of making a whole new header for this, can we simply add  
> `LoopProtoToCxx()` and `LoopFunctionToString()` to proto_to_cxx.h?  Then 
> implement them in `loop_proto_to_cxx.cpp`?
Can we remove this file completely?


Repository:
  rC Clang

https://reviews.llvm.org/D47843



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47920: Made loop_proto more "vectorizable"

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

Looks like a good start.


Repository:
  rC Clang

https://reviews.llvm.org/D47920



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47920: Made loop_proto more "vectorizable"

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC334252: [clang-fuzzer] Made loop_proto more 
"vectorizable". (authored by morehouse, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47920?vs=150435&id=150436#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47920

Files:
  tools/clang-fuzzer/cxx_loop_proto.proto
  tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp

Index: tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp
===
--- tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp
+++ tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp
@@ -7,10 +7,13 @@
 //
 //===--===//
 //
-// Implements functions for converting between protobufs and C++. Extends
+// Implements functions for converting between protobufs and C++. Differs from
 // proto_to_cxx.cpp by wrapping all the generated C++ code in a single for
 // loop. Also coutputs a different function signature that includes a
-// size_t parameter for the loop to use.
+// size_t parameter for the loop to use. The C++ code generated is meant to
+// stress the LLVM loop vectorizer.
+//
+// Still a work in progress.
 //
 //===--===//
 
@@ -33,19 +36,7 @@
 std::ostream &operator<<(std::ostream &os, const Const &x) {
   return os << "(" << x.val() << ")";
 }
-std::ostream &operator<<(std::ostream &os, const VarRef &x) {
-  if (x.is_loop_var()) {
-return os << "a[loop_ctr]";
-  } else {
-return os << "a[" << static_cast(x.varnum()) << " % s]";
-  }
-}
-std::ostream &operator<<(std::ostream &os, const Lvalue &x) {
-  return os << x.varref();
-}
 std::ostream &operator<<(std::ostream &os, const Rvalue &x) {
-  if (x.has_varref())
-return os << x.varref();
   if (x.has_cons())
 return os << x.cons();
   if (x.has_binop())
@@ -101,23 +92,18 @@
   return os << x.right() << ")";
 }
 std::ostream &operator<<(std::ostream &os, const AssignmentStatement &x) {
-  return os << x.lvalue() << "=" << x.rvalue();
+  return os << "a[i]=" << x.rvalue();
 }
 std::ostream &operator<<(std::ostream &os, const IfElse &x) {
   return os << "if (" << x.cond() << "){\n"
 << x.if_body() << "} else { \n"
 << x.else_body() << "}\n";
 }
-std::ostream &operator<<(std::ostream &os, const While &x) {
-  return os << "while (" << x.cond() << "){\n" << x.body() << "}\n";
-}
 std::ostream &operator<<(std::ostream &os, const Statement &x) {
   if (x.has_assignment())
 return os << x.assignment() << ";\n";
   if (x.has_ifelse())
 return os << x.ifelse();
-  if (x.has_while_loop())
-return os << x.while_loop();
   return os << "(void)0;\n";
 }
 std::ostream &operator<<(std::ostream &os, const StatementSeq &x) {
@@ -127,7 +113,7 @@
 }
 std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
   return os << "void foo(int *a, size_t s) {\n"
-<< "for (int loop_ctr = 0; loop_ctr < s; loop_ctr++){\n"
+<< "for (int i=0; i___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47964: Modified protobuf and converter to add new signature, remove conditionals.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: tools/clang-fuzzer/cxx_loop_proto.proto:67
-AssignmentStatement assignment = 1;
-IfElse  ifelse = 2;
-  }

Do you really want to get rid of if-else?



Comment at: tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp:123
 std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
-  return os << "void foo(int *a, size_t s) {\n"
+  return os << "#include \"foo.h\"\n" // This line is just for testing
+<< "void foo(int *a, int *b, int *__restrict__ c, size_t s) {\n"

What's the purpose of this include?


Repository:
  rC Clang

https://reviews.llvm.org/D47964



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47964: Modified protobuf and converter to add new signature, remove conditionals.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334421: [clang-fuzzer] Modified protobuf and converter to 
add new signature, remove… (authored by morehouse, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D47964

Files:
  cfe/trunk/tools/clang-fuzzer/cxx_loop_proto.proto
  cfe/trunk/tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp

Index: cfe/trunk/tools/clang-fuzzer/cxx_loop_proto.proto
===
--- cfe/trunk/tools/clang-fuzzer/cxx_loop_proto.proto
+++ cfe/trunk/tools/clang-fuzzer/cxx_loop_proto.proto
@@ -11,7 +11,7 @@
 ///  This file describes a subset of C++ as a protobuf. It is used to
 ///  more easily find interesting inputs for fuzzing Clang. This subset
 ///  differs from the one defined in cxx_proto.proto by eliminating while
-///  loops and Lvalues. The goal is that the C++ code generated will be
+///  loops and conditionals. The goal is that the C++ code generated will be
 ///  more likely to stress the LLVM loop vectorizer.
 ///
 //===--===//
@@ -22,6 +22,16 @@
   required int32 val = 1;
 }
 
+message VarRef {
+  // Add an enum for each array in function signature
+  enum Arr {
+ARR_A = 0;
+ARR_B = 1;
+ARR_C = 2;
+  };
+  required Arr arr = 1;
+}
+
 message BinaryOp {
   enum Op {
 PLUS = 0;
@@ -48,10 +58,12 @@
   oneof rvalue_oneof {
 Const cons = 1;
 BinaryOp binop = 2;
+VarRef varref = 3;
   }
 }
 
 message AssignmentStatement {
+  required VarRef varref = 1;
   required Rvalue rvalue = 2;
 }
 
@@ -62,10 +74,7 @@
 }
 
 message Statement {
-  oneof stmt_oneof {
-AssignmentStatement assignment = 1;
-IfElse  ifelse = 2;
-  }
+  required AssignmentStatement assignment = 1;
 }
 
 message StatementSeq {
Index: cfe/trunk/tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp
===
--- cfe/trunk/tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp
+++ cfe/trunk/tools/clang-fuzzer/proto-to-cxx/loop_proto_to_cxx.cpp
@@ -36,11 +36,23 @@
 std::ostream &operator<<(std::ostream &os, const Const &x) {
   return os << "(" << x.val() << ")";
 }
+std::ostream &operator<<(std::ostream &os, const VarRef &x) {
+  switch (x.arr()) {
+case VarRef::ARR_A:
+  return os << "a[i]";
+case VarRef::ARR_B:
+  return os << "b[i]";
+case VarRef::ARR_C:
+  return os << "c[i]";
+  }
+}
 std::ostream &operator<<(std::ostream &os, const Rvalue &x) {
   if (x.has_cons())
 return os << x.cons();
   if (x.has_binop())
 return os << x.binop();
+  if (x.has_varref())
+return os << x.varref();
   return os << "1";
 }
 std::ostream &operator<<(std::ostream &os, const BinaryOp &x) {
@@ -92,27 +104,23 @@
   return os << x.right() << ")";
 }
 std::ostream &operator<<(std::ostream &os, const AssignmentStatement &x) {
-  return os << "a[i]=" << x.rvalue();
+  return os << x.varref() << "=" << x.rvalue() << ";\n";
 }
 std::ostream &operator<<(std::ostream &os, const IfElse &x) {
   return os << "if (" << x.cond() << "){\n"
 << x.if_body() << "} else { \n"
 << x.else_body() << "}\n";
 }
 std::ostream &operator<<(std::ostream &os, const Statement &x) {
-  if (x.has_assignment())
-return os << x.assignment() << ";\n";
-  if (x.has_ifelse())
-return os << x.ifelse();
-  return os << "(void)0;\n";
+  return os << x.assignment();
 }
 std::ostream &operator<<(std::ostream &os, const StatementSeq &x) {
   for (auto &st : x.statements())
 os << st;
   return os;
 }
 std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
-  return os << "void foo(int *a, size_t s) {\n"
+  return os << "void foo(int *a, int *b, int *__restrict__ c, size_t s) {\n"
 << "for (int i=0; i___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Where is the fuzz target?




Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:33
+int ptr_ctr = 0;
+int val_ctr = 0;
+

I'd suggest wrapper functions that return unused variable names, so your code 
below won't need to juggle these globals.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:43
+std::ostream &operator<<(std::ostream &os, const VarRef &x) {
+  ptr_ctr++;
+  switch (x.arr()) {

The way these globals are being used seems confusing and error-prone.

Maybe what you want is to replace the operator<< overloads with functions that 
emit the LLVM and then return the variable name that the result is stored in.  
Then the parent node can use the returned variable name without juggling 
globals.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:48
+  << "%m_" << ptr_ctr << " = mul i64 8, %ct\n"
+  << "%i_" << ptr_ctr << " = add i64 %p2i_" << ptr_ctr << ", %m_"
+  << ptr_ctr << "\n"

`getelementptr` should make this much simpler.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:80
+<< "%val_" << val_ctr << " = load i32, i32* %pval_" << val_ctr
+<< "\n";
+}

When will this last return be executed?



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:132
+  int p = ptr_ctr + 1;
+  return os << x.varref() << x.rvalue() << "store i32 %val_" << val_ctr
+<< ", i32* %i2p_" << p << "\n";

Probably not a big deal, but maybe load the rvalue first?  I'd expect the 
frontend to do that normally to simplify register allocation.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:132
+  int p = ptr_ctr + 1;
+  return os << x.varref() << x.rvalue() << "store i32 %val_" << val_ctr
+<< ", i32* %i2p_" << p << "\n";

morehouse wrote:
> Probably not a big deal, but maybe load the rvalue first?  I'd expect the 
> frontend to do that normally to simplify register allocation.
Also, I think it is easier to read if you put each statement on its own line.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:144
+std::ostream &operator<<(std::ostream &os, const LoopFunction &x) {
+  return os << "define void @foo(i32* %a, i32*%b, i32* noalias %c, i64 %s) {\n"
+<< "%i = alloca i64\n"

Space before `%b`.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:153
+<< "body:\n"
+<< x.statements() << "%z = add i64 1, %ct\n"
+<< "store i64 %z, i64* %i\n"

Can we put `%z = ...` on the next line for consistency?



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:156
+<< "br label %loop\n"
+<< "\nendloop:\n"
+<< "ret void\n}\n";

Can we put the first newline on previous statement for consistency?


Repository:
  rC Clang

https://reviews.llvm.org/D48106



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D48106#1131625, @emmettneyman wrote:

> I wanted to implement the proto_to_llvm converter before the fuzz target.


The fuzz target should make testing your converter way easier.  I'd recommend 
adding it to this patch so that you're less likely to need a bug-fixing patch 
later.




Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:32
+// Counter variable to generate new LLVM IR variable names and wrapper function
+int ctr = 0;
+std::string get_var() {

You can hide this as a static int inside `get_var`.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:46
+   + "store i32 " + val + ", i32* " + alloca_var + "\n"
+   + load_var + " = load i32, i32* " + alloca_var + "\n";
+  return std::make_pair(insns, load_var);

What's the point of storing then loading the value?  Can you just return `val`?



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:49
+}
+std::pair  VarRefToString(const VarRef &x) {
+  std::string arr;

Returning a pair can be confusing (which element is which?).  I'd suggest 
passing `os` to these functions, writing the instructions to `os`, and then 
returning just the result variable.



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:120
+op = "add";
+break;
+  }

If all these cases are the same, we can simplify the code to
```
case BinaryOp::EQ:
case BinaryOp::NE:
...
op = "add";
break;
```



Comment at: tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:129
+}
+std::string AssignmentStatementToString(const AssignmentStatement &x) {
+  std::pair  ref = VarRefToString(x.varref());

If `AssignmentStatementToString` has no extra return value, I think its more 
concise to just keep the `operator<<` overload.  (Same for below functions)


Repository:
  rC Clang

https://reviews.llvm.org/D48106



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse requested changes to this revision.
morehouse added inline comments.
This revision now requires changes to proceed.



Comment at: tools/clang-fuzzer/CMakeLists.txt:72
 
+  # Build the lllvm protobuf fuzzer
+  add_clang_executable(clang-llvm-proto-fuzzer

llvm



Comment at: tools/clang-fuzzer/CMakeLists.txt:83
 clangFuzzerInitialize
 clangHandleCXX
 )

Maybe remove `clangHandleCXX` here, so you can use this variable for linking 
`clang-llvm-proto-fuzzer`.



Comment at: tools/clang-fuzzer/handle-llvm/CMakeLists.txt:5
+  handle_llvm.cpp
+  )

There's fewer libraries linked here than in `handle-cxx/` (not saying this is 
wrong, but it could be).  Do you get link errors if you build 
`clang-llvm-proto-fuzzer` with shared libraries?



Comment at: tools/clang-fuzzer/handle-llvm/\:31
+#include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Support/raw_ostream.h"
+

Please sort includes alphabetically, with handle_llvm.h separate at the top.



Comment at: tools/clang-fuzzer/handle-llvm/\:42
+void InitializeEverything() {
+
+  InitializeAllTargets();

Nit:  avoid empty lines at beginning or end of a `{}` block (here and below)



Comment at: tools/clang-fuzzer/handle-llvm/\:62
+  initializeScavengerTestPass(*Registry);
+
+}

Does this initialization need to happen every time the fuzzer generates a new 
input, or can we call this from `LLVMFuzzerInitialize()` instead?



Comment at: tools/clang-fuzzer/handle-llvm/\:71
+  StringRef *ID = new StringRef("IR");
+  MemoryBufferRef *ir = new MemoryBufferRef(*IRString, *ID);
+

# Avoid using `new` when you could create the object on the stack instead.  
This will prevent you from introducing memory leaks if you forget to call 
`delete` later.
# I don't think you need to construct StringRefs or the MemoryBufferRef here.  
Instead you can probably do `parseIR(MemoryBufferRef(S, "IR"), Err, ...)` below.



Comment at: tools/clang-fuzzer/handle-llvm/\:79
+  SMDiagnostic Err;
+  std::unique_ptr M;
+  Triple TheTriple;

I'd move the `unique_ptr` definition to the same line as `parseIR`.



Comment at: tools/clang-fuzzer/handle-llvm/\:81
+  Triple TheTriple;
+  TheTriple.setTriple(sys::getDefaultTargetTriple());
+

What's the point of wrapping `sys::getDefaultTargetTriple()` if we always 
unwrap `TheTriple` below?



Comment at: tools/clang-fuzzer/handle-llvm/\:84
+  // Set the Module to include the the IR code to be compiled
+  M = parseIR(*ir, Err, Context, false);
+

What does `UpgradeDebugInfo=false` do here?  The documentation warns about 
setting this bool to false.



Comment at: tools/clang-fuzzer/handle-llvm/\:84
+  // Set the Module to include the the IR code to be compiled
+  M = parseIR(*ir, Err, Context, false);
+

morehouse wrote:
> What does `UpgradeDebugInfo=false` do here?  The documentation warns about 
> setting this bool to false.
What if `parseIR` returns nullptr?



Comment at: tools/clang-fuzzer/handle-llvm/\:88
+  std::string Error;
+  const Target *TheTarget = TargetRegistry::lookupTarget(TheTriple.getTriple(),
+ Error);

What if `lookupTarget` returns nullptr?



Comment at: tools/clang-fuzzer/handle-llvm/\:90
+ Error);
+  std::string CPUStr = getCPUStr(), FeaturesStr = getFeaturesStr();
+

Please separate to 2 statements.



Comment at: tools/clang-fuzzer/handle-llvm/\:98
+  switch(A[2]) {  
+   case '0': OLvl = CodeGenOpt::None; break;
+case '1': OLvl = CodeGenOpt::Less; break;

Fix indentation here.



Comment at: tools/clang-fuzzer/handle-llvm/\:101
+case '2': OLvl = CodeGenOpt::Default; break;
+case '3': OLvl = CodeGenOpt::Aggressive; break;
+  }

Maybe add a default case here with an error message?



Comment at: tools/clang-fuzzer/handle-llvm/\:104
+}
+  }
+

It might make sense to move command line arg parsing to a helper function, and 
then call that function closer to the top.  That way if there's a bad argument 
we can quit before doing all the IR parsing.



Comment at: tools/clang-fuzzer/handle-llvm/\:113
+
+  legacy::PassManager PM;
+  TargetLibraryInfoImpl TLII(Triple(M->getTargetTriple()));

Any reason not to use the newer PassManager?



Comment at: tools/clang-fuzzer/handle-llvm/\:127
+
+  LLVMTargetMachine &LLVMTM = static_cast(*Target);
+  MachineModuleInfo *MMI = new MachineModuleInfo(&LLVMTM);
--

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: tools/clang-fuzzer/handle-llvm/CMakeLists.txt:5
+  handle_llvm.cpp
+  )

emmettneyman wrote:
> morehouse wrote:
> > There's fewer libraries linked here than in `handle-cxx/` (not saying this 
> > is wrong, but it could be).  Do you get link errors if you build 
> > `clang-llvm-proto-fuzzer` with shared libraries?
> It builds without any errors both with all the libraries from `handle-cxx/` 
> linked in and without any of the libraries linked in (as I have here). Which 
> is preferred?
I prefer what you have here, if shared libraries are OK.


Repository:
  rC Clang

https://reviews.llvm.org/D48106



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

If you haven't already, please apply for commit access:  
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

That way you can land this after it's accepted.




Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:23
+#include "llvm/IR/Module.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Verifier.h"

This should come before llvm/IR/Module.h.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:35
+
+void getOptLevel(const std::vector &ExtraArgs,
+  CodeGenOpt::Level &OLvl) {

Can make this static to avoid future namespace collisions.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:48
+default:
+  errs() << "error: opt level must be between 0 and 3.\n";
+  return;

Maybe exit() as well on error?  (here and below)

Exiting will cause the fuzz target to fail, so you can debug more easily.  
Otherwise, these error messages could go unnoticed.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:58
+  // Create a new Context
+  LLVMContext Context;
+  // Parse ExtraArgs to set the optimization level

It's easier to follow if you move this closer to where Context is first used.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:66
+  std::unique_ptr M = parseIR(MemoryBufferRef(S, "IR"), Err,
+  Context, true);
+  if (!M) {

You can actually omit the last argument here since you're using the default.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:81
+  std::string CPUStr = getCPUStr();
+  std::string FeaturesStr = getFeaturesStr();
+

Easier to follow if you move these two lines closer to where `CPUStr` and 
`FeaturesStr` are used.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:105
+  raw_null_ostream OS;
+  MachineModuleInfo *MMI = new MachineModuleInfo(Target.get());
+  Target->addPassesToEmitFile(PM, OS, nullptr, TargetMachine::CGFT_ObjectFile,

I believe you can avoid creating the MMI here since it looks like 
addPassesToEmitFile will do it for you.


Repository:
  rC Clang

https://reviews.llvm.org/D48106



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added inline comments.
This revision is now accepted and ready to land.



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:50
+  errs() << "error: opt level must be between 0 and 3.\n";
+  std::exit(0);
+  }

`exit(1)` will indicate an abnormal exit.  (here and below)



Comment at: tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:64
+  SMDiagnostic Err;
+  // Create a new Context
+  LLVMContext Context;

This comment is unnecessary.  Clearly a new Context is being created on the 
next line.


Repository:
  rC Clang

https://reviews.llvm.org/D48106



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48106: implemented proto to llvm

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Looks like `exit(0)` is still there.


Repository:
  rC Clang

https://reviews.llvm.org/D48106



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: docs/SanitizerCoverage.rst:149
+(``-Wl,-gc-sections``), thus resulting in a significant binary size overhead.
+See `Bug 34636 `_ for more info.
+

Maybe add "for linkers other than LLD".  Dead stripping should work 100% for 
LLD now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57474



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

[PATCH] D54805: [Driver] Use --push/pop-state with Sanitizer link deps

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D54805



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

This patch caused the Windows sanitizer bot to break:  
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/44238

Please take a look.

  FAILED: 
projects/compiler-rt/lib/fuzzer/tests/FuzzerTestObjects.gtest-all.cc.x86_64.o 
  cmd.exe /C "cd /D 
C:\b\slave\sanitizer-windows\build\build\stage1\projects\compiler-rt\lib\fuzzer\tests
 && C:\b\slave\sanitizer-windows\build\build\stage1\.\bin\clang.exe -DWIN32 
-D_WINDOWS -Wno-unknown-warning-option -DGTEST_NO_LLVM_RAW_OSTREAM=1 
-DGTEST_HAS_RTTI=0 
-IC:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest/include 
-IC:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest 
-Wno-deprecated-declarations 
-IC:/b/slave/sanitizer-windows/build/llvm.src/projects/compiler-rt/lib/fuzzer 
-fno-rtti -O2 -c -o FuzzerTestObjects.gtest-all.cc.x86_64.o 
C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest/src/gtest-all.cc"
  Stack dump:
  
  0.Program arguments: 
C:\b\slave\sanitizer-windows\build\build\stage1\bin\clang.exe -cc1 -triple 
x86_64-pc-windows-msvc19.16.27026 -emit-obj -mincremental-linker-compatible 
-disable-free -main-file-name gtest-all.cc -mrelocation-model pic -pic-level 2 
-mthread-model posix -fmath-errno -masm-verbose -mconstructor-aliases 
-munwind-tables -target-cpu x86-64 -dwarf-column-info -momit-leaf-frame-pointer 
-coverage-notes-file 
C:\b\slave\sanitizer-windows\build\build\stage1\projects\compiler-rt\lib\fuzzer\tests\FuzzerTestObjects.gtest-all.cc.x86_64.gcno
 -resource-dir C:\b\slave\sanitizer-windows\build\build\stage1\lib\clang\9.0.0 
-D WIN32 -D _WINDOWS -D GTEST_NO_LLVM_RAW_OSTREAM=1 -D GTEST_HAS_RTTI=0 -I 
C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest/include 
-I C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest -I 
C:/b/slave/sanitizer-windows/build/llvm.src/projects/compiler-rt/lib/fuzzer 
-internal-isystem 
C:\b\slave\sanitizer-windows\build\build\stage1\lib\clang\9.0.0\include 
-internal-isystem C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Professional\VC\Tools\MSVC\14.16.27023\ATLMFC\include 
-internal-isystem C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Professional\VC\Tools\MSVC\14.16.27023\include -internal-isystem 
C:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um -internal-isystem 
C:\Program Files (x86)\Windows Kits\10\include\10.0.17763.0\ucrt 
-internal-isystem C:\Program Files (x86)\Windows 
Kits\10\include\10.0.17763.0\shared -internal-isystem C:\Program Files 
(x86)\Windows Kits\10\include\10.0.17763.0\um -internal-isystem C:\Program 
Files (x86)\Windows Kits\10\include\10.0.17763.0\winrt -internal-isystem 
C:\Program Files (x86)\Windows Kits\10\include\10.0.17763.0\cppwinrt -O2 
-Wno-unknown-warning-option -Wno-deprecated-declarations -fdeprecated-macro 
-fdebug-compilation-dir 
C:\b\slave\sanitizer-windows\build\build\stage1\projects\compiler-rt\lib\fuzzer\tests
 -ferror-limit 19 -fmessage-length 0 -fno-rtti -fno-use-cxa-atexit 
-fms-extensions -fms-compatibility -fms-compatibility-version=19.16.27026 
-std=c++14 -fdelayed-template-parsing -fobjc-runtime=gcc -fcxx-exceptions 
-fexceptions -fdiagnostics-show-option -vectorize-loops -vectorize-slp -o 
FuzzerTestObjects.gtest-all.cc.x86_64.o -x c++ 
C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest/src/gtest-all.cc
 -faddrsig 
  
  1.
C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest\src/gtest-death-test.cc:79:1:
 current parser token 'namespace'
  
  2.
C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest\src/gtest.cc:149:11:
 LLVM IR generation of declaration 'testing'
  
  3.
C:/b/slave/sanitizer-windows/build/llvm.src/utils/unittest/googletest\src/gtest.cc:361:15:
 Generating code for declaration 'testing::internal::AssertHelper::AssertHelper'
  
   #0 0x7ff761d1de49 
clang::CodeGen::CGDebugInfo::addHeapAllocSiteMetadata(class llvm::Instruction 
*,class clang::QualType,class clang::SourceLocation) 
c:\b\slave\sanitizer-windows\build\llvm.src\tools\clang\lib\codegen\cgdebuginfo.cpp:1967:0
  
   #1 0x7ff761d9d462 clang::CodeGen::CodeGenFunction::EmitCall(class 
clang::CodeGen::CGFunctionInfo const &,class clang::CodeGen::CGCallee const 
&,class clang::CodeGen::ReturnValueSlot,class clang::CodeGen::CallArgList const 
&,class llvm::CallBase * *,class clang::SourceLocation) 
c:\b\slave\sanitizer-windows\build\llvm.src\tools\clang\lib\codegen\cgcall.cpp:4392:0
  
   #2 0x7ff76202a144 EmitNewDeleteCall 
c:\b\slave\sanitizer-windows\build\llvm.src\tools\clang\lib\codegen\cgexprcxx.cpp:1288:0
  
   #3 0x7ff762026a55 clang::CodeGen::CodeGenFunction::EmitCXXNewExpr(class 
clang::CXXNewExpr const *) 
c:\b\slave\sanitizer-windows\build\llvm.src\tools\clang\lib\codegen\cgexprcxx.cpp:1617:0
  
   #4 0x7ff761fdf0a6 clang::StmtVisitorBase::Visit 
c:\b\slave\sanitizer-windows\build\build\stage1\tools\clang\include\clang\ast\stmtnodes.inc:699:0
  
 

[PATCH] D57474: Update SanitizerCoverage doc regarding the issue with pc-table and gc-sections.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D57474



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

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: hiraditya.

- Don't sanitize __sancov_lowest_stack.
- Don't instrument leaf functions.
- Add CoverageStackDepth to Fuzzer and FuzzerNoLink.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -173,6 +174,13 @@
   return Options;
 }
 
+bool IsLeafFunc(const Function &F) {
+  for (const BasicBlock &BB : F.getBasicBlockList())
+for (const Instruction &Insn : BB.getInstList())
+  if (isa(Insn) && !isa(Insn)) return false;
+  return true;
+}
+
 class SanitizerCoverageModule : public ModulePass {
 public:
   SanitizerCoverageModule(
@@ -731,6 +739,22 @@
 
   IRBuilder<> IRB(&*IP);
   IRB.SetCurrentDebugLocation(EntryLoc);
+  if (Options.StackDepth && IsEntryBB && !IsLeafFunc(F)) {
+// Check stack depth.  If it's the deepest so far, record it.
+Function *GetFrameAddr =
+Intrinsic::getDeclaration(F.getParent(), Intrinsic::frameaddress);
+auto FrameAddrPtr =
+IRB.CreateCall(GetFrameAddr, {Constant::getNullValue(Int32Ty)});
+auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
+auto LowestStack = IRB.CreateLoad(SanCovLowestStack);
+auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
+auto ThenTerm = SplitBlockAndInsertIfThen(IsStackLower, &*IP, false);
+IRBuilder<> ThenIRB(ThenTerm);
+auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
+SetNoSanitizeMetadata(LowestStack);
+SetNoSanitizeMetadata(Store);
+IRB.SetInsertPoint(&*IP);
+  }
   if (Options.TracePC) {
 IRB.CreateCall(SanCovTracePC); // gets the PC using GET_CALLER_PC.
 IRB.CreateCall(EmptyAsm, {}); // Avoids callback merge.
@@ -753,19 +777,6 @@
 SetNoSanitizeMetadata(Load);
 SetNoSanitizeMetadata(Store);
   }
-  if (Options.StackDepth && IsEntryBB) {
-// Chec

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D37156#852780, @kcc wrote:

> Did you check this on something other than the unit tests? 
>  E.g. a couple of benchmarks from fuzzer-test-suite?


Just tested on the proj4 and lcms benchmarks and no issues came up.


https://reviews.llvm.org/D37156



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112739.
morehouse added a comment.

- Use existing linear scan, and check for InvokeInst.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
   void CreatePCArray(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast(&Inst))
   GepTraceTargets.push_back(GEP);
-   }
+  if (Options.StackDepth)
+if (isa(Inst) ||
+(isa(Inst) && !isa(Inst)))
+  IsLeafFunc = false;
+}
   }
 
-  InjectCoverage(F, BlocksToInstrument);
+  InjectCoverag

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL311801: [SanitizeCoverage] Enable stack-depth coverage for 
-fsanitize=fuzzer (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D37156?vs=112739&id=112746#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37156

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  compiler-rt/trunk/test/fuzzer/deep-recursion.test
  llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
   void CreatePCArray(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast(&Inst))
   GepTrace

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse reopened this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

Turns out I should have been testing the benchmarks with 
`FUZZING_ENGINE=fsanitize_fuzzer`.  My mistake.

After adding the weak reference to SanitizerCoverage.cpp, both lcms and proj4 
build with `fsanitize_fuzzer`.


Repository:
  rL LLVM

https://reviews.llvm.org/D37156



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112756.
morehouse added a comment.

- Add weak reference in SanitizerCoverage.cpp


https://reviews.llvm.org/D37156

Files:
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp


Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -78,6 +78,9 @@
 
 static const char *const SanCovLowestStackName = "__sancov_lowest_stack";
 
+__attribute__((tls_model("initial-exec")))
+thread_local uintptr_t __sancov_lowest_stack;
+
 static cl::opt ClCoverageLevel(
 "sanitizer-coverage-level",
 cl::desc("Sanitizer Coverage. 0: none, 1: entry block, 2: all blocks, "


Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -78,6 +78,9 @@
 
 static const char *const SanCovLowestStackName = "__sancov_lowest_stack";
 
+__attribute__((tls_model("initial-exec")))
+thread_local uintptr_t __sancov_lowest_stack;
+
 static cl::opt ClCoverageLevel(
 "sanitizer-coverage-level",
 cl::desc("Sanitizer Coverage. 0: none, 1: entry block, 2: all blocks, "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112759.
morehouse added a comment.

Full diff.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -78,6 +79,9 @@
 
 static const char *const SanCovLowestStackName = "__sancov_lowest_stack";
 
+__attribute__((tls_model("initial-exec")))
+thread_local uintptr_t __sancov_lowest_stack;
+
 static cl::opt ClCoverageLevel(
 "sanitizer-coverage-level",
 cl::desc("Sanitizer Coverage. 0: none, 1: entry block, 2: all blocks, "
@@ -200,13 +204,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
   void CreatePCArray(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +497,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
@@ -515,10 +522,14 @@
   if (Options.TraceGep)
 if (G

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 112923.
morehouse added a comment.
Herald added a subscriber: kubamracek.

- Add weak definition of __sancov_lowest_stack to runtime.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
   void CreatePCArray(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast(&Inst))
   GepTraceTargets.push_back(GEP);
-   }
+  if (Options.StackDepth)
+if (isa(Inst) ||
+(isa(Inst) && !is

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 113129.
morehouse added a comment.

- Disable stack depth tracking on Mac.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
  compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   GlobalVariable *CreatePCArray(Function &F, ArrayRef AllBlocks);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast(&Inst))
   GepTraceTargets.push_back(GEP);
-   }
+  if (Options.StackDepth)
+if (isa(Inst) ||
+(isa

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 113133.
morehouse added a comment.

- Eliminate "#if".
- Replace uintptr_t with uptr.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
  compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   GlobalVariable *CreatePCArray(Function &F, ArrayRef AllBlocks);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
@@ -515,10 +519,14 @@
   if (Options.TraceGep)
 if (GetElementPtrInst *GEP = dyn_cast(&Inst))
   GepTraceTargets.push_back(GEP);
-   }
+  if (Options.StackDepth)
+if (isa(Inst) ||
+ 

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312026: [SanitizeCoverage] Enable stack-depth coverage for 
-fsanitize=fuzzer (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D37156?vs=113133&id=113136#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37156

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h
  compiler-rt/trunk/test/fuzzer/deep-recursion.test
  llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   GlobalVariable *CreatePCArray(Function &F, ArrayRef AllBlocks);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldInstrumentBlock

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 113177.
morehouse added a comment.

- Only enable stack depth tracking on Linux.
- Ignore __sancov_lowest_stack in interface symbols tests.


https://reviews.llvm.org/D37156

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
  compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
  compiler-rt/test/asan/TestCases/Darwin/interface_symbols_darwin.c
  compiler-rt/test/asan/TestCases/Linux/interface_symbols_linux.c
  compiler-rt/test/asan/TestCases/Windows/interface_symbols_windows.c
  compiler-rt/test/fuzzer/deep-recursion.test
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   GlobalVariable *CreatePCArray(Function &F, ArrayRef AllBlocks);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std::pair
@@ -491,6 +494,7 @@
   &getAnalysis(F).getDomTree();
   const PostDominatorTree *PDT =
   &getAnalysis(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
 if (shouldIns

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL312185: [SanitizeCoverage] Enable stack-depth coverage for 
-fsanitize=fuzzer (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D37156?vs=113177&id=113329#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37156

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_internal_defs.h
  compiler-rt/trunk/test/asan/TestCases/Darwin/interface_symbols_darwin.c
  compiler-rt/trunk/test/asan/TestCases/Linux/interface_symbols_linux.c
  compiler-rt/trunk/test/asan/TestCases/Windows/interface_symbols_windows.c
  compiler-rt/trunk/test/fuzzer/deep-recursion.test
  llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll

Index: llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/trunk/test/Instrumentation/SanitizerCoverage/stack-depth.ll
@@ -1,9 +1,9 @@
 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN: -S | FileCheck %s --enable-var-scope
+; RUN: -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,13 +14,8 @@
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
@@ -30,12 +25,12 @@
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: :[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
Index: llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
@@ -200,13 +201,15 @@
  ArrayRef GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
 ArrayRef SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef AllBlocks,
+  bool IsLeafFunc = true);
   GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
 Function &F, Type *Ty,
 const char *Section);
   GlobalVariable *CreatePCArray(Function &F, ArrayRef AllBlocks);
   void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+ bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
Type *Ty, const char *Section);
   std

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.

Enable the compile-time flag -fsanitize-memory-use-after-dtor by
default. Note that the run-time option MSAN_OPTIONS=poison_in_dtor=1
still needs to be enabled for destructors to be poisoned.


https://reviews.llvm.org/D37860

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/msan/dtor-base-access.cc
  compiler-rt/test/msan/dtor-bit-fields.cc
  compiler-rt/test/msan/dtor-derived-class.cc
  compiler-rt/test/msan/dtor-member.cc
  compiler-rt/test/msan/dtor-multiple-inheritance-nontrivial-class-members.cc
  compiler-rt/test/msan/dtor-multiple-inheritance.cc
  compiler-rt/test/msan/dtor-trivial-class-members.cc
  compiler-rt/test/msan/dtor-trivial.cpp
  compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cc
  compiler-rt/test/msan/dtor-vtable.cc
  compiler-rt/test/msan/use-after-dtor.cc

Index: compiler-rt/test/msan/use-after-dtor.cc
===
--- compiler-rt/test/msan/use-after-dtor.cc
+++ compiler-rt/test/msan/use-after-dtor.cc
@@ -1,13 +1,13 @@
-// RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -O0 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
-// RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -O1 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
-// RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -O2 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
-// RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -O1 -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
 
 #include 
Index: compiler-rt/test/msan/dtor-vtable.cc
===
--- compiler-rt/test/msan/dtor-vtable.cc
+++ compiler-rt/test/msan/dtor-vtable.cc
@@ -1,16 +1,16 @@
-// RUN: %clangxx_msan %s -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
+// RUN: %clangxx_msan %s -O0 -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
 
-// RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
+// RUN: %clangxx_msan %s -O1 -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
 
-// RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
+// RUN: %clangxx_msan %s -O2 -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
 
-// RUN: %clangxx_msan %s -DVPTRA=1 -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
+// RUN: %clangxx_msan %s -DVPTRA=1 -O2 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
 
-// RUN: %clangxx_msan %s -DVPTRCA=1 -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
+// RUN: %clangxx_msan %s -DVPTRCA=1 -O2 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
 
-// RUN: %clangxx_msan %s -DVPTRCB=1 -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
+// RUN: %clangxx_msan %s -DVPTRCB=1 -O2 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
 
-// RUN: %clangxx_msan %s -DVPTRC=1 -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
+// RUN: %clangxx_msan %s -DVPTRC=1 -O2 -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t
 
 // Expected to quit due to invalid access when invoking
 // function using vtable.
Index: compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cc
===
--- compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cc
+++ compiler-rt/test/msan/dtor-vtable-multiple-inheritance.cc
@@ -1,14 +1,14 @@
-// RUN: %clangxx_msan %s -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
+// RUN: %clangxx_msan %s -O0 -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
 
-// RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
+// RUN: %clangxx_msan %s -O1 -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t
 
-// RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dto

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/include/clang/Driver/Options.td:854
  HelpText<"Enable use-after-destroy 
detection in MemorySanitizer">;
+def fno_sanitize_memory_use_after_dtor : Flag<["-"], 
"fno-sanitize-memory-use-after-dtor">,
+ Group,

vitalybuka wrote:
> I'd recommend to split patch in two:
> 1. Add no-sanitize with tests
> 2. Flip default
Can do.



Comment at: clang/test/Driver/fsanitize.c:175
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MSAN-USE-AFTER-DTOR
-// CHECK-MSAN-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR

vitalybuka wrote:
> Somewhere we need to have test that default is ON or OFF.
> Maybe here?
Line 177 tests that default is ON.



Comment at: compiler-rt/test/msan/use-after-dtor.cc:13
 
 #include 
 #include 

vitalybuka wrote:
> Probably we need one test which check that we stop detecting bugs with 
> -fno-sanitize-memory-track-origins. Maybe this one or in separate file.
Do you mean `-fno-sanitize-memory-use-after-dtor`?  See `dtor-member.cc` test.


https://reviews.llvm.org/D37860



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37867: [MSan] Add flag to disable use-after-dtor.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.

Flag is -fno-sanitize-use-after-dtor.


https://reviews.llvm.org/D37867

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -172,8 +172,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-track-origins=3 -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-TRACK-ORIGINS-3
 // CHECK-TRACK-ORIGINS-3: error: invalid value '3' in 
'-fsanitize-memory-track-origins=3'
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MSAN-USE-AFTER-DTOR
-// CHECK-MSAN-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
+// CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-field-padding=0 %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-FIELD-PADDING-0
 // CHECK-ASAN-FIELD-PADDING-0-NOT: -fsanitize-address-field-padding
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -828,6 +828,11 @@
   Args.hasArg(OPT_fsanitize_coverage_stack_depth);
   Opts.SanitizeMemoryTrackOrigins =
   getLastArgIntValue(Args, OPT_fsanitize_memory_track_origins_EQ, 0, 
Diags);
+  if (Arg *A = Args.getLastArg(OPT_fsanitize_memory_use_after_dtor,
+   OPT_fno_sanitize_memory_use_after_dtor)) {
+Opts.SanitizeMemoryUseAfterDtor =
+A->getOption().matches(OPT_fsanitize_memory_use_after_dtor);
+  }
   Opts.SanitizeMemoryUseAfterDtor =
   Args.hasArg(OPT_fsanitize_memory_use_after_dtor);
   Opts.SanitizeMinimalRuntime = Args.hasArg(OPT_fsanitize_minimal_runtime);
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -489,7 +489,9 @@
   }
 }
 MsanUseAfterDtor =
-Args.hasArg(options::OPT_fsanitize_memory_use_after_dtor);
+Args.hasFlag(options::OPT_fsanitize_memory_use_after_dtor,
+ options::OPT_fno_sanitize_memory_use_after_dtor,
+ false);
 NeedPIE |= !(TC.getTriple().isOSLinux() &&
  TC.getTriple().getArch() == llvm::Triple::x86_64);
   }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -851,6 +851,9 @@
 def fsanitize_memory_use_after_dtor : Flag<["-"], 
"fsanitize-memory-use-after-dtor">,
  Group,
  HelpText<"Enable use-after-destroy 
detection in MemorySanitizer">;
+def fno_sanitize_memory_use_after_dtor : Flag<["-"], 
"fno-sanitize-memory-use-after-dtor">,
+ Group,
+ HelpText<"Disable use-after-destroy 
detection in MemorySanitizer">;
 def fsanitize_address_field_padding : Joined<["-"], 
"fsanitize-address-field-padding=">,
 Group,
 HelpText<"Level of field padding for 
AddressSanitizer">;


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -172,8 +172,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-track-origins=3 -pie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TRACK-ORIGINS-3
 // CHECK-TRACK-ORIGINS-3: error: invalid value '3' in '-fsanitize-memory-tr

[PATCH] D37867: [MSan] Add flag to disable use-after-dtor.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 115295.
morehouse added a comment.

- Use hasFlag() in CompilerInvocation.cpp as well.


https://reviews.llvm.org/D37867

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -172,8 +172,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-track-origins=3 -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-TRACK-ORIGINS-3
 // CHECK-TRACK-ORIGINS-3: error: invalid value '3' in 
'-fsanitize-memory-track-origins=3'
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MSAN-USE-AFTER-DTOR
-// CHECK-MSAN-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
+// CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-field-padding=0 %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-FIELD-PADDING-0
 // CHECK-ASAN-FIELD-PADDING-0-NOT: -fsanitize-address-field-padding
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -829,7 +829,9 @@
   Opts.SanitizeMemoryTrackOrigins =
   getLastArgIntValue(Args, OPT_fsanitize_memory_track_origins_EQ, 0, 
Diags);
   Opts.SanitizeMemoryUseAfterDtor =
-  Args.hasArg(OPT_fsanitize_memory_use_after_dtor);
+  Args.hasFlag(OPT_fsanitize_memory_use_after_dtor,
+   OPT_fno_sanitize_memory_use_after_dtor,
+   false);
   Opts.SanitizeMinimalRuntime = Args.hasArg(OPT_fsanitize_minimal_runtime);
   Opts.SanitizeCfiCrossDso = Args.hasArg(OPT_fsanitize_cfi_cross_dso);
   Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -489,7 +489,9 @@
   }
 }
 MsanUseAfterDtor =
-Args.hasArg(options::OPT_fsanitize_memory_use_after_dtor);
+Args.hasFlag(options::OPT_fsanitize_memory_use_after_dtor,
+ options::OPT_fno_sanitize_memory_use_after_dtor,
+ false);
 NeedPIE |= !(TC.getTriple().isOSLinux() &&
  TC.getTriple().getArch() == llvm::Triple::x86_64);
   }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -851,6 +851,9 @@
 def fsanitize_memory_use_after_dtor : Flag<["-"], 
"fsanitize-memory-use-after-dtor">,
  Group,
  HelpText<"Enable use-after-destroy 
detection in MemorySanitizer">;
+def fno_sanitize_memory_use_after_dtor : Flag<["-"], 
"fno-sanitize-memory-use-after-dtor">,
+ Group,
+ HelpText<"Disable use-after-destroy 
detection in MemorySanitizer">;
 def fsanitize_address_field_padding : Joined<["-"], 
"fsanitize-address-field-padding=">,
 Group,
 HelpText<"Level of field padding for 
AddressSanitizer">;


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -172,8 +172,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-track-origins=3 -pie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TRACK-ORIGINS-3
 // CHECK-TRACK-ORIGINS-3: error: invalid value '3' in '-fsan

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 115312.
morehouse edited the summary of this revision.
morehouse added a comment.

- Move the new flag to https://reviews.llvm.org/D37867
- Address Vitaly's comments.


https://reviews.llvm.org/D37860

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGenCXX/sanitize-no-dtor-callback.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/test/msan/dtor-member.cc
  compiler-rt/test/msan/use-after-dtor.cc

Index: compiler-rt/test/msan/use-after-dtor.cc
===
--- compiler-rt/test/msan/use-after-dtor.cc
+++ compiler-rt/test/msan/use-after-dtor.cc
@@ -1,14 +1,17 @@
 // RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK-UAD < %t.out
 
 // RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -fsanitize-memory-track-origins -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
-// RUN: FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: FileCheck %s --check-prefixes=CHECK-UAD,CHECK-ORIGINS < %t.out
+
+// RUN: %clangxx_msan %s -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-UAD-OFF < %t.out
 
 #include 
 #include 
@@ -32,14 +35,17 @@
   Simple *s = new(&buf) Simple();
   s->~Simple();
 
+  fprintf(stderr, "\n");  // Need output to parse for CHECK-UAD-OFF case
   return s->x_;
 
-  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
-  // CHECK: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]]
+  // CHECK-UAD: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK-UAD: {{#0 0x.* in main.*use-after-dtor.cc:}}[[@LINE-3]]
+  // CHECK-UAD-OFF-NOT: WARNING: MemorySanitizer: use-of-uninitialized-value
 
   // CHECK-ORIGINS: Memory was marked as uninitialized
   // CHECK-ORIGINS: {{#0 0x.* in __sanitizer_dtor_callback}}
   // CHECK-ORIGINS: {{#1 0x.* in Simple::~Simple}}
 
-  // CHECK: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}}
+  // CHECK-UAD: SUMMARY: MemorySanitizer: use-of-uninitialized-value {{.*main}}
+  // CHECK-UAD-OFF-NOT: SUMMARY: MemorySanitizer: use-of-uninitialized-value
 }
Index: compiler-rt/test/msan/dtor-member.cc
===
--- compiler-rt/test/msan/dtor-member.cc
+++ compiler-rt/test/msan/dtor-member.cc
@@ -7,7 +7,7 @@
 // RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
-// RUN: %clangxx_msan %s -fsanitize=memory -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
+// RUN: %clangxx_msan %s -fsanitize=memory -fno-sanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
 
 // RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -174,11 +174,11 @@
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
 // CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
 // CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=addr

[PATCH] D37860: [MSan] Enable use-after-dtor instrumentation by default.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

In https://reviews.llvm.org/D37860#871368, @eugenis wrote:

> Looking at __sanitizer_dtor_callback implementation, this change will add a 
> (fast) stack unwind in every destructor. In extreme cases (like a tight loop 
> doing string operations) it could be bad for performance. I've seen ~15% 
> AFAIR.


I haven't looked at performance, but I'll take a look at your internal bug.  
Until that is resolved, this can be on hold.


https://reviews.llvm.org/D37860



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37867: [MSan] Add flag to disable use-after-dtor.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313314: [MSan] Add flag to disable use-after-dtor. (authored 
by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D37867?vs=115295&id=115317#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37867

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Driver/fsanitize.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -851,6 +851,9 @@
 def fsanitize_memory_use_after_dtor : Flag<["-"], 
"fsanitize-memory-use-after-dtor">,
  Group,
  HelpText<"Enable use-after-destroy 
detection in MemorySanitizer">;
+def fno_sanitize_memory_use_after_dtor : Flag<["-"], 
"fno-sanitize-memory-use-after-dtor">,
+ Group,
+ HelpText<"Disable use-after-destroy 
detection in MemorySanitizer">;
 def fsanitize_address_field_padding : Joined<["-"], 
"fsanitize-address-field-padding=">,
 Group,
 HelpText<"Level of field padding for 
AddressSanitizer">;
Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -172,8 +172,14 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-track-origins=3 -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-TRACK-ORIGINS-3
 // CHECK-TRACK-ORIGINS-3: error: invalid value '3' in 
'-fsanitize-memory-track-origins=3'
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -pie %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MSAN-USE-AFTER-DTOR
-// CHECK-MSAN-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor -fsanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR
+// CHECK-USE-AFTER-DTOR: -cc1{{.*}}-fsanitize-memory-use-after-dtor
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fno-sanitize-memory-use-after-dtor %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory 
-fsanitize-memory-use-after-dtor -fno-sanitize-memory-use-after-dtor %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-USE-AFTER-DTOR-OFF
+// CHECK-USE-AFTER-DTOR-OFF-NOT: -cc1{{.*}}memory-use-after-dtor
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-field-padding=0 %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-FIELD-PADDING-0
 // CHECK-ASAN-FIELD-PADDING-0-NOT: -fsanitize-address-field-padding
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -830,7 +830,9 @@
   Opts.SanitizeMemoryTrackOrigins =
   getLastArgIntValue(Args, OPT_fsanitize_memory_track_origins_EQ, 0, 
Diags);
   Opts.SanitizeMemoryUseAfterDtor =
-  Args.hasArg(OPT_fsanitize_memory_use_after_dtor);
+  Args.hasFlag(OPT_fsanitize_memory_use_after_dtor,
+   OPT_fno_sanitize_memory_use_after_dtor,
+   false);
   Opts.SanitizeMinimalRuntime = Args.hasArg(OPT_fsanitize_minimal_runtime);
   Opts.SanitizeCfiCrossDso = Args.hasArg(OPT_fsanitize_cfi_cross_dso);
   Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -489,7 +489,9 @@
   }
 }
 MsanUseAfterDtor =
-Args.hasArg(options::OPT_fsanitize_memory_use_after_dtor);
+Args.hasFlag(options::OPT_fsanitize_memory_use_after_dtor,
+ options::OPT_fno_sanitize_memory_use_after_dtor,
+ false);
 NeedPIE |= !(TC.getTriple().isOSLinux() &&
  TC.getTriple().getArch() == llvm::Triple::x86_64);
   }


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options

[PATCH] D38063: [MSan] Disable sanitization for __sanitizer_dtor_callback.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse created this revision.
Herald added a subscriber: hiraditya.

Eliminate unnecessary instrumentation at __sanitizer_dtor_callback
call sites.  Fixes https://github.com/google/sanitizers/issues/861.


https://reviews.llvm.org/D38063

Files:
  clang/lib/CodeGen/CGClass.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -1588,7 +1588,9 @@
llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
llvm::Value *Fn =
CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
-   CGF.EmitNounwindRuntimeCall(Fn, Args);
+   llvm::CallInst *I = CGF.EmitNounwindRuntimeCall(Fn, Args);
+   I->setMetadata("nosanitize", llvm::MDNode::get(CGF.getLLVMContext(),
+  llvm::None));
  }
 
   class SanitizeDtorMembers final : public EHScopeStack::Cleanup {


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -1588,7 +1588,9 @@
llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
llvm::Value *Fn =
CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
-   CGF.EmitNounwindRuntimeCall(Fn, Args);
+   llvm::CallInst *I = CGF.EmitNounwindRuntimeCall(Fn, Args);
+   I->setMetadata("nosanitize", llvm::MDNode::get(CGF.getLLVMContext(),
+  llvm::None));
  }
 
   class SanitizeDtorMembers final : public EHScopeStack::Cleanup {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38063: [MSan] Disable sanitization for __sanitizer_dtor_callback.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 116072.
morehouse added a comment.

- Add test case.
- Use SanitizerScope.


https://reviews.llvm.org/D38063

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
===
--- clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
+++ clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
@@ -55,16 +55,19 @@
 // to confirm that all invoked dtors have member poisoning
 // instrumentation inserted.
 // CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}InlinedD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}Defaulted_Non_TrivialD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -1577,6 +1577,7 @@
 
  static void EmitSanitizerDtorCallback(CodeGenFunction &CGF, llvm::Value *Ptr,
  CharUnits::QuantityType PoisonSize) {
+   CodeGenFunction::SanitizerScope SanScope(&CGF);
// Pass in void pointer and size of region as arguments to runtime
// function
llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, CGF.VoidPtrTy),


Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
===
--- clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
+++ clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
@@ -55,16 +55,19 @@
 // to confirm that all invoked dtors have member poisoning
 // instrumentation inserted.
 // CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}InlinedD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}Defaulted_Non_TrivialD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -1577,6 +1577,7 @@
 
  static void EmitSanitizerDtorCallback(CodeGenFunction &CGF, llvm::Value *Ptr,
  CharUnits::QuantityType PoisonSize) {
+   CodeGenFunction::SanitizerScope SanScope(&CGF);
// Pass in void pointer and size of region as arguments to runtime
// function
llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, CGF.VoidPtrTy),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38063: [MSan] Disable sanitization for __sanitizer_dtor_callback.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 116100.
morehouse added a comment.

- Add LLVM test.


https://reviews.llvm.org/D38063

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/call-nosanitize.ll


Index: llvm/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
@@ -0,0 +1,16 @@
+; Verify that calls with !nosanitize are not instrumented by MSan.
+; RUN: opt < %s -msan -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar(i32 %x)
+
+define void @foo() {
+  call void @bar(i32 7), !nosanitize !{}
+  ret void
+}
+
+; CHECK-LABEL: define void @foo
+; CHECK-NOT: store i{{[0-9]+}} 0, {{.*}} @__msan_param_tls
+; CHECK: call void @bar
+; CHECK: ret void
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
===
--- clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
+++ clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
@@ -55,16 +55,19 @@
 // to confirm that all invoked dtors have member poisoning
 // instrumentation inserted.
 // CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}InlinedD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}Defaulted_Non_TrivialD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -1577,6 +1577,7 @@
 
  static void EmitSanitizerDtorCallback(CodeGenFunction &CGF, llvm::Value *Ptr,
  CharUnits::QuantityType PoisonSize) {
+   CodeGenFunction::SanitizerScope SanScope(&CGF);
// Pass in void pointer and size of region as arguments to runtime
// function
llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, CGF.VoidPtrTy),


Index: llvm/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
@@ -0,0 +1,16 @@
+; Verify that calls with !nosanitize are not instrumented by MSan.
+; RUN: opt < %s -msan -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar(i32 %x)
+
+define void @foo() {
+  call void @bar(i32 7), !nosanitize !{}
+  ret void
+}
+
+; CHECK-LABEL: define void @foo
+; CHECK-NOT: store i{{[0-9]+}} 0, {{.*}} @__msan_param_tls
+; CHECK: call void @bar
+; CHECK: ret void
Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
===
--- clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
+++ clang/test/CodeGenCXX/sanitize-dtor-callback.cpp
@@ -55,16 +55,19 @@
 // to confirm that all invoked dtors have member poisoning
 // instrumentation inserted.
 // CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}InlinedD2Ev
+// CHECK-NOT: store i{{[0

[PATCH] D38063: [MSan] Disable sanitization for __sanitizer_dtor_callback.

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313831: [MSan] Disable sanitization for 
__sanitizer_dtor_callback. (authored by morehouse).

Changed prior to commit:
  https://reviews.llvm.org/D38063?vs=116100&id=116103#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38063

Files:
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp
  llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll


Index: llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
===
--- llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
+++ llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
@@ -0,0 +1,16 @@
+; Verify that calls with !nosanitize are not instrumented by MSan.
+; RUN: opt < %s -msan -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar(i32 %x)
+
+define void @foo() {
+  call void @bar(i32 7), !nosanitize !{}
+  ret void
+}
+
+; CHECK-LABEL: define void @foo
+; CHECK-NOT: store i{{[0-9]+}} 0, {{.*}} @__msan_param_tls
+; CHECK: call void @bar
+; CHECK: ret void
Index: llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -1577,6 +1577,7 @@
 
  static void EmitSanitizerDtorCallback(CodeGenFunction &CGF, llvm::Value *Ptr,
  CharUnits::QuantityType PoisonSize) {
+   CodeGenFunction::SanitizerScope SanScope(&CGF);
// Pass in void pointer and size of region as arguments to runtime
// function
llvm::Value *Args[] = {CGF.Builder.CreateBitCast(Ptr, CGF.VoidPtrTy),
Index: cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp
===
--- cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp
+++ cfe/trunk/test/CodeGenCXX/sanitize-dtor-callback.cpp
@@ -55,16 +55,19 @@
 // to confirm that all invoked dtors have member poisoning
 // instrumentation inserted.
 // CHECK-LABEL: define {{.*}}SimpleD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}InlinedD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void
 
 // CHECK-LABEL: define {{.*}}Defaulted_Non_TrivialD2Ev
+// CHECK-NOT: store i{{[0-9]+}} 0, {{.*}}@__msan_param_tls
 // CHECK: call void @__sanitizer_dtor_callback
 // CHECK-NOT: call void @__sanitizer_dtor_callback
 // CHECK: ret void


Index: llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
===
--- llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
+++ llvm/trunk/test/Instrumentation/MemorySanitizer/call-nosanitize.ll
@@ -0,0 +1,16 @@
+; Verify that calls with !nosanitize are not instrumented by MSan.
+; RUN: opt < %s -msan -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar(i32 %x)
+
+define void @foo() {
+  call void @bar(i32 7), !nosanitize !{}
+  ret void
+}
+
+; CHECK-LABEL: define void @foo
+; CHECK-NOT: store i{{[0-9]+}} 0, {{.*}} @__msan_param_tls
+; CHECK: call void @bar
+; CHECK: ret void
Index: llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2588,6 +2588,7 @@
 
   void visitCallSite(CallSite CS) {
 Instruction &I = *CS.getInstruction();
+if (I.getMetadata("nosanitize")) return;
 assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
 if (CS.isCall()) {
   CallInst *Call = cast(&I);
Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -1577,6 +1577,7 @@
 
  sta

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61923



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

[PATCH] D65029: [Driver] Support for disabling sanitizer runtime linking

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D65029



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

[PATCH] D63616: Implement `-fsanitize-coverage-whitelist` and `-fsanitize-coverage-blacklist` for clang

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

Thanks for the patch!  Seems like a useful feature for targeted fuzzing.




Comment at: clang/docs/SanitizerCoverage.rst:310
+
+In most cases, the whitelist will list the folders or source files for which 
you want
+instrumentation and allow all function names, while the blacklist will opt out 
some specific

The wording makes it sound like there may be exceptions to the expected 
whitelist/blacklist behavior.  But IIUC the paragraph is meant to explain the 
typical use case.  Can we make this more explicit?

e.g.,
```
A common use case is to have the whitelist list folders and source files ... 
while the blacklist ...
```

Or maybe we don't need this paragraph at all...



Comment at: clang/include/clang/Driver/Options.td:978
+def fsanitize_coverage_blacklist : Separate<["-"], 
"fsanitize-coverage-blacklist">,
+Group, Flags<[CoreOption, DriverOption]>, 
Alias;
 def fsanitize_memory_track_origins_EQ : Joined<["-"], 
"fsanitize-memory-track-origins=">,

For `fsanitize_blacklist` we only support `-fsanitize-blacklist=`.  Let's do 
the same for these lists to keep things simple.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:218
   Opts.StackDepth = CGOpts.SanitizeCoverageStackDepth;
-  PM.add(createSanitizerCoverageModulePass(Opts));
+  PM.add(createSanitizerCoverageModulePass(Opts, 
CGOpts.SanitizeCoverageWhitelistFiles, CGOpts.SanitizeCoverageBlacklistFiles));
 }

Please run `clang-format --style=LLVM` on the patch.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:743
+  }
+}
+  }

The two cases have lots of overlapping code.  Let's try to coalesce.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:759
+  D.Diag(clang::diag::err_drv_malformed_sanitizer_coverage_blacklist) << 
BLError;
+  }
+

Let's try to coalesce here too.  Maybe a helper function?  Then we could also 
use it for the sanitizer blacklist.



Comment at: 
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_whitelist_blacklist.cc:60
+// RUN: %clangxx -O0 %s -S -o - -emit-llvm -fsanitize-coverage=trace-pc 
-fsanitize-coverage-whitelist=wl_bar.txt  
-fsanitize-coverage-blacklist=bl_bar.txt   2>&1 | not grep "call void 
@__sanitizer_cov_trace_pc"
+
+// RUN: rm wl_*.txt

Can we also test with `-fsanitize=inline-8bit-counters`, since that is what 
libFuzzer uses by default?



Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:192
+  Blacklist = SpecialCaseList::createOrDie(BlacklistFiles);
+}
 initializeSanitizerCoverageModulePass(*PassRegistry::getPassRegistry());

Nit:  Preferred style is no curly braces for one-statement ifs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63616



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

[PATCH] D63616: Implement `-fsanitize-coverage-whitelist` and `-fsanitize-coverage-blacklist` for clang

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63616



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

[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a comment.

This diff is helpful to get an overall idea of how things fit together, but it 
is very difficult to review thoroughly.  Let's start splicing off pieces for 
individual review.

I suggest:

- Individual reviews for each prereq (mutex, random, etc.)
- Review for base GPA + unit tests
- Review for "optional" options parsing, etc.
- Review for documentation
- Review for Scudo integration + e2e tests

Submitting this in pieces (especially the Scudo integration) will also make 
things simpler if we need to revert something.




Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.h:97
+  // Return whether the allocation should be randomly chosen for sampling.
+  ALWAYS_INLINE bool shouldSample() {
+// NextSampleCounter == 0 means we "should regenerate the counter".

hctim wrote:
> morehouse wrote:
> > The assembly you posted for this function looks way too slow.  Is it 
> > actually optimized with `-O2`?
> > 
> > - The fast path (counter > 1) has *two* branches that check if 
> > `NextSampleCounter`'s TLS init function has run and one actual call to the 
> > TLS init function.  Ouch!  We shouldn't need these.
> >   - Maybe we need to make the counter function-local or use `__thread` 
> > instead of `thread_local`.
> > - The counter decrement of `NextSampleCounter` requires 4 instructions?  
> > Load TLS offset from global, read TLS, increment, write TLS.  Yikes.  For 
> > reference, TCMalloc does this operation in 1 instruction.
> > - The `+ 1` operation on the `NextSampleCounter == 0` branch is done with a 
> > mov-add, rather than an lea.
> > 
> > Again, please triple-check that the binary you compiled is actually 
> > optimized.  Make sure it is compiled with `clang++ -O2 ...`.
> > 
> > 
> As per offline discussion, declaring `NextSampleCounter` with `__thread` 
> instead of `thread_local` fixed this. Also added a note in `definitions.h`.
What is the assembly now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60593



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