[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this landed in b32d40417e25bad79e9b5bc0f0a29d7ba0222ad9 but the 
review feedback was never addressed.

With a more fine-grained thing, we could use this to add a compile-time error 
to libcxxabi for D97323 .


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52386

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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.
Herald added a project: LLVM.

This is needed in the NetBSD kernel, more fine-grained checks would be 
acceptable too, but one global feature detection is what I need.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52386



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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> This sounds like something that would be better handled in the build system 
> rather than in source code. In particular, I think you don't actually want to 
> detect "is this translation unit being compiled with ubsan enabled?", you 
> instead want to detect "is the rest of musl libc being compiled with ubsan 
> enabled?" -- you should not compile the stubs themselves with ubsan enabled, 
> because ubsan might insert calls to its runtime at arbitrary places within 
> that translation unit, which means you'll get infinite recursion when one of 
> the ubsan callbacks ends up calling itself.

UBSan doesn't actually instrument these stubs though. We have them initially 
defined to be weakly linked and just call __builtin_trap(), which is be 
replaced when the UBSan runtime eventually gets linked when compiling something 
that uses this libc.

I also imagine it would be convenient for there to be some `__has_feature` to 
detect if any of the UBSan checks are being used.


Repository:
  rL LLVM

https://reviews.llvm.org/D52386



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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D52386#1254483, @leonardchan wrote:

> In https://reviews.llvm.org/D52386#1254437, @rsmith wrote:
>
> > I'm not at all convinced that this is a good thing. There isn't such a 
> > thing as "undefined behavior sanitizer". Rather, there are a whole bunch of 
> > different checks that fall under the same umbrella. This test seems on the 
> > surface to be about as meaningless as `__has_feature(warnings)` would be: 
> > it's useless to ask the question without knowing *which* warnings you're 
> > talking about. But perhaps there's some use case I've overlooked (and your 
> > description of the patch doesn't mention why you want this). What is the 
> > use case you're trying to address with this change?
>
>
> This is part of enabling UBSan for Zircon (the Fuchsia kernel) 
> (https://fuchsia-review.googlesource.com/c/zircon/+/197017). We enable UBSan 
> when building musl libc, but libc is dynamically linked first before 
> sanitizer runtimes, so we need to stub them out in the beginning before the 
> UBSan runtime is linked. This `__has_feature` is there to check if we build 
> with `-fsanitize=undefined` since we only want to define these stubs if libc 
> is built with UBSan.


This sounds like something that would be better handled in the build system 
rather than in source code. In particular, I think you don't actually want to 
detect "is this translation unit being compiled with ubsan enabled?", you 
instead want to detect "is the rest of musl libc being compiled with ubsan 
enabled?" -- you should not compile the stubs themselves with ubsan enabled, 
because ubsan might insert calls to its runtime at arbitrary places within that 
translation unit, which means you'll get infinite recursion when one of the 
ubsan callbacks ends up calling itself.


Repository:
  rL LLVM

https://reviews.llvm.org/D52386



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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In https://reviews.llvm.org/D52386#1254437, @rsmith wrote:

> I'm not at all convinced that this is a good thing. There isn't such a thing 
> as "undefined behavior sanitizer". Rather, there are a whole bunch of 
> different checks that fall under the same umbrella. This test seems on the 
> surface to be about as meaningless as `__has_feature(warnings)` would be: 
> it's useless to ask the question without knowing *which* warnings you're 
> talking about. But perhaps there's some use case I've overlooked (and your 
> description of the patch doesn't mention why you want this). What is the use 
> case you're trying to address with this change?


This is part of enabling UBSan for Zircon (the Fuchsia kernel) 
(https://fuchsia-review.googlesource.com/c/zircon/+/197017). We enable UBSan 
when building musl libc, but libc is dynamically linked first before sanitizer 
runtimes, so we need to stub them out in the beginning before the UBSan runtime 
is linked. This `__has_feature` is there to check if we build with 
`-fsanitize=undefined` since we only want to define these stubs if libc is 
built with UBSan.

> If we want to change this, you'll also need to update 
> `getPPTransparentSanitizers` to exclude all the UBSan checks, because it's no 
> longer the case that those sanitizers only affect code generation.

I can update this in another patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D52386



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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I'm not at all convinced that this is a good thing. There isn't such a thing as 
"undefined behavior sanitizer". Rather, there are a whole bunch of different 
checks that fall under the same umbrella. This test seems on the surface to be 
about as meaningless as `__has_feature(warnings)` would be: it's useless to ask 
the question without knowing *which* warnings you're talking about. But perhaps 
there's some use case I've overlooked (and your description of the patch 
doesn't mention why you want this). What is the use case you're trying to 
address with this change?

If we want to change this, you'll also need to update 
`getPPTransparentSanitizers` to exclude all the UBSan checks, because it's no 
longer the case that those sanitizers only affect code generation.


Repository:
  rL LLVM

https://reviews.llvm.org/D52386



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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342793: [Lexer] Add udefined_behavior_sanitizer feature 
(authored by leonardchan, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D52386

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


Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -38,6 +38,8 @@
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
+FEATURE(undefined_behavior_sanitizer,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
===
--- test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
+++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck 
--check-prefix=CHECK-UBSAN %s
+// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck 
--check-prefix=CHECK-ALIGNMENT %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s
+
+#if __has_feature(undefined_behavior_sanitizer)
+int UBSanEnabled();
+#else
+int UBSanDisabled();
+#endif
+
+// CHECK-UBSAN: UBSanEnabled
+// CHECK-ALIGNMENT: UBSanEnabled
+// CHECK-NO-UBSAN: UBSanDisabled


Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -38,6 +38,8 @@
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
+FEATURE(undefined_behavior_sanitizer,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
===
--- test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
+++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s
+// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s
+
+#if __has_feature(undefined_behavior_sanitizer)
+int UBSanEnabled();
+#else
+int UBSanDisabled();
+#endif
+
+// CHECK-UBSAN: UBSanEnabled
+// CHECK-ALIGNMENT: UBSanEnabled
+// CHECK-NO-UBSAN: UBSanDisabled
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342793: [Lexer] Add udefined_behavior_sanitizer feature 
(authored by leonardchan, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52386?vs=166585=166587#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52386

Files:
  cfe/trunk/include/clang/Basic/Features.def
  cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp


Index: cfe/trunk/include/clang/Basic/Features.def
===
--- cfe/trunk/include/clang/Basic/Features.def
+++ cfe/trunk/include/clang/Basic/Features.def
@@ -38,6 +38,8 @@
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
+FEATURE(undefined_behavior_sanitizer,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
Index: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
===
--- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
+++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck 
--check-prefix=CHECK-UBSAN %s
+// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck 
--check-prefix=CHECK-ALIGNMENT %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s
+
+#if __has_feature(undefined_behavior_sanitizer)
+int UBSanEnabled();
+#else
+int UBSanDisabled();
+#endif
+
+// CHECK-UBSAN: UBSanEnabled
+// CHECK-ALIGNMENT: UBSanEnabled
+// CHECK-NO-UBSAN: UBSanDisabled


Index: cfe/trunk/include/clang/Basic/Features.def
===
--- cfe/trunk/include/clang/Basic/Features.def
+++ cfe/trunk/include/clang/Basic/Features.def
@@ -38,6 +38,8 @@
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
+FEATURE(undefined_behavior_sanitizer,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
Index: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
===
--- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
+++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s
+// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s
+
+#if __has_feature(undefined_behavior_sanitizer)
+int UBSanEnabled();
+#else
+int UBSanDisabled();
+#endif
+
+// CHECK-UBSAN: UBSanEnabled
+// CHECK-ALIGNMENT: UBSanEnabled
+// CHECK-NO-UBSAN: UBSanDisabled
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM but you should probably a wait a day to see if sanitizer owners are fine 
with with this as well.


Repository:
  rC Clang

https://reviews.llvm.org/D52386



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

[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, kcc, echristo, vitalybuka, morehouse.
leonardchan added a project: clang.

This can be used to detect whether the code is being built with UBSan using the 
__has_feature(undefined_behavior_sanitizer) predicate.


Repository:
  rC Clang

https://reviews.llvm.org/D52386

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


Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
===
--- /dev/null
+++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck 
--check-prefix=CHECK-UBSAN %s
+// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck 
--check-prefix=CHECK-ALIGNMENT %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s
+
+#if __has_feature(undefined_behavior_sanitizer)
+int UBSanEnabled();
+#else
+int UBSanDisabled();
+#endif
+
+// CHECK-UBSAN: UBSanEnabled
+// CHECK-ALIGNMENT: UBSanEnabled
+// CHECK-NO-UBSAN: UBSanDisabled
Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -38,6 +38,8 @@
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
+FEATURE(undefined_behavior_sanitizer,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)


Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
===
--- /dev/null
+++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s
+// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s
+// RUN: %clang -E  %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s
+
+#if __has_feature(undefined_behavior_sanitizer)
+int UBSanEnabled();
+#else
+int UBSanDisabled();
+#endif
+
+// CHECK-UBSAN: UBSanEnabled
+// CHECK-ALIGNMENT: UBSanEnabled
+// CHECK-NO-UBSAN: UBSanDisabled
Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -38,6 +38,8 @@
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
 FEATURE(xray_instrument, LangOpts.XRayInstrument)
+FEATURE(undefined_behavior_sanitizer,
+LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits