[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-07-01 Thread Max Moroz via Phabricator via cfe-commits
Dor1s added a comment.

If we still decide to proceed with this, would it make sense to expand it to 
`sanitizer_coverage` based on any sancov instrumentation being enabled? As you 
mentioned in the description, there might be users who manually enable certain 
sancov flags. I think it's good to be able to support those usecases too   
(e.g. other fuzzing engines).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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


[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-07-01 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse added a comment.

In D82926#2125950 , @hctim wrote:

> So - the `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` flag is a property of the 
> build system and not that of the compiler. There are some places (android) 
> where enabling `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` globally changes 
> the behaviour of large amounts of libraries in ways that break the build 
> system.
>
> Having this flag allows us to make targeted compile-time changes to libc 
> based on sancov that don't require enabling 
> `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` across the entire build system.


Shouldn't we be fixing the fuzzing build for Android then?  
`FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` is what we use everywhere else, so I 
don't like the idea of making a new flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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


[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-07-01 Thread Mitch Phillips via Phabricator via cfe-commits
hctim added a comment.

So - the `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` flag is a property of the 
build system and not that of the compiler. There are some places (android) 
where enabling `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` globally changes the 
behaviour of large amounts of libraries in ways that break the build system.

Having this flag allows us to make targeted compile-time changes to libc based 
on sancov that don't require enabling 
`FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` across the entire build system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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


[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-07-01 Thread Max Moroz via Phabricator via cfe-commits
Dor1s added a comment.

What usecase(s) do you have for this in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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


[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-06-30 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse added a comment.

Can we just use `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` 
 instead?  I'm 
a little wary of introducing a new way to do conditional compilation since 
fuzzers that don't use sancov can't rely on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926



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


[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-06-30 Thread Mitch Phillips via Phabricator via cfe-commits
hctim updated this revision to Diff 274656.
hctim added a comment.

Changed the filename and fixed up the inverted test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82926

Files:
  clang/include/clang/Basic/Features.def
  clang/test/Lexer/has_feature_fuzzing_coverage.cpp


Index: clang/test/Lexer/has_feature_fuzzing_coverage.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_fuzzing_coverage.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer-no-link %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+
+#if __has_feature(fuzzing_coverage)
+int FuzzingCoverageEnabled();
+#else
+int FuzzingCoverageDisabled();
+#endif
+
+// CHECK-NONE: FuzzingCoverageDisabled
+// CHECK-ENABLED: FuzzingCoverageEnabled
Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -49,6 +49,9 @@
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(undefined_behavior_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
+FEATURE(fuzzing_coverage,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Fuzzer |
+   SanitizerKind::FuzzerNoLink))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)


Index: clang/test/Lexer/has_feature_fuzzing_coverage.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_fuzzing_coverage.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer-no-link %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+
+#if __has_feature(fuzzing_coverage)
+int FuzzingCoverageEnabled();
+#else
+int FuzzingCoverageDisabled();
+#endif
+
+// CHECK-NONE: FuzzingCoverageDisabled
+// CHECK-ENABLED: FuzzingCoverageEnabled
Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -49,6 +49,9 @@
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(undefined_behavior_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
+FEATURE(fuzzing_coverage,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Fuzzer |
+   SanitizerKind::FuzzerNoLink))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82926: [libfuzzer] [clang] Add __has_feature(fuzzing_coverage)

2020-06-30 Thread Mitch Phillips via Phabricator via cfe-commits
hctim created this revision.
hctim added reviewers: morehouse, Dor1s.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Probably a useful feature for checking whether a file was built with
-fsanitize=fuzzer or -fsanitize=fuzzer-no-link.

N.B. __has_feature(fuzzing_coverage) doesn't cover instances where users
manually specify -fsanitize-coverage=... (even if the flags are
identical to ToT -fsanitize=fuzzer-no-link). IMHO this is WAI -
-fsanitize=fuzzer-no-link is not a stable set of flags and people that
want sancov for fuzzing should use fuzzer-no-link.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82926

Files:
  clang/include/clang/Basic/Features.def
  clang/test/Lexer/has_feature_libfuzzer.cpp


Index: clang/test/Lexer/has_feature_libfuzzer.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_libfuzzer.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer-no-link %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+
+#if __has_feature(fuzzing_coverage)
+int FuzzingCoverageEnabled();
+#else
+int FuzzingCoverageDisabled();
+#endif
+
+// CHECK-NONE: FuzzingCoverageEnabled
+// CHECK-ENABLED: FuzzingCoverageDisabled
Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -49,6 +49,9 @@
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(undefined_behavior_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
+FEATURE(fuzzing_coverage,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Fuzzer |
+   SanitizerKind::FuzzerNoLink))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)


Index: clang/test/Lexer/has_feature_libfuzzer.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_libfuzzer.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+// RUN: %clang_cc1 -E -fsanitize=fuzzer-no-link %s -o - \
+// RUN:   | FileCheck --check-prefix=CHECK-ENABLED %s
+
+#if __has_feature(fuzzing_coverage)
+int FuzzingCoverageEnabled();
+#else
+int FuzzingCoverageDisabled();
+#endif
+
+// CHECK-NONE: FuzzingCoverageEnabled
+// CHECK-ENABLED: FuzzingCoverageDisabled
Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -49,6 +49,9 @@
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(undefined_behavior_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
+FEATURE(fuzzing_coverage,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Fuzzer |
+   SanitizerKind::FuzzerNoLink))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits