[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-19 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai added a comment.

In D61479#1586987 , @sberg wrote:

> eh, the summary here doesn't get updated from the actual git commit message :(
>
> thanks for the reviews!


You can use `arc diff --verbatim` to update the summary based on your local 
commit message. You'll want to have done an `arc amend` before that to get any 
extra reviewers, subscribers, etc. from Phabricator into your local commit 
message, otherwise the `arc diff --verbatim` will happily override those too.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-16 Thread Stephan Bergmann via Phabricator via cfe-commits 
sberg added a comment.

eh, the summary here doesn't get updated from the actual git commit message :(

thanks for the reviews!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-16 Thread Stephan Bergmann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366186: Finish Adapt -fsanitize=function to 
SANITIZER_NON_UNIQUE_TYPEINFO (authored by sberg, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D61479?vs=209865=210025#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61479

Files:
  cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/CodeGen/ubsan-function.cpp
  cfe/trunk/test/Driver/fsanitize.c
  compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.h
  compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
  compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp

Index: compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp
===
--- compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -1,11 +1,53 @@
-// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t
-// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC -shared -o %t-so.so
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK $(%run %t-unique UNIQUE)
 // Verify that we can disable symbolization if needed:
-// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
+// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM $(%run %t-unique NOSYM-UNIQUE)
 // XFAIL: windows-msvc
 // Unsupported function flag
 // UNSUPPORTED: openbsd
 
+#ifdef DETERMINE_UNIQUE
+
+#include 
+
+#include "../../../../../lib/sanitizer_common/sanitizer_platform.h"
+
+int main(int, char **argv) {
+  if (!SANITIZER_NON_UNIQUE_TYPEINFO)
+std::cout << "--check-prefix=" << argv[1];
+}
+
+#else
+
+struct Shared {};
+using FnShared = void (*)(Shared *);
+FnShared getShared();
+
+struct __attribute__((visibility("hidden"))) Hidden {};
+using FnHidden = void (*)(Hidden *);
+FnHidden getHidden();
+
+namespace {
+struct Private {};
+} // namespace
+using FnPrivate = void (*)(void *);
+FnPrivate getPrivate();
+
+#ifdef SHARED_LIB
+
+void fnShared(Shared *) {}
+FnShared getShared() { return fnShared; }
+
+void fnHidden(Hidden *) {}
+FnHidden getHidden() { return fnHidden; }
+
+void fnPrivate(Private *) {}
+FnPrivate getPrivate() { return reinterpret_cast(fnPrivate); }
+
+#else
+
 #include 
 
 void f() {}
@@ -64,12 +106,31 @@
   p2(0);
 }
 
+void check_cross_dso() {
+  getShared()(nullptr);
+
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnHidden(Hidden*) through pointer to incorrect function type 'void (*)(Hidden *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)(Hidden *)'
+  getHidden()(nullptr);
+
+  // TODO: Unlike GCC, Clang fails to prefix the typeinfo name for the function
+  // type with "*", so this erroneously only fails for "*UNIQUE":
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnPrivate((anonymous namespace)::Private*) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  reinterpret_cast(getPrivate())(nullptr);
+}
+
 int main(void) {
   make_valid_call();
   make_invalid_call();
   check_noexcept_calls();
+  check_cross_dso();
   // Check that no more errors will be printed.
   // CHECK-NOT: runtime error: call to function
   // NOSYM-NOT: runtime error: call to function
   make_invalid_call();
 }
+
+#endif
+
+#endif
Index: compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
+++ compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
@@ -21,8 +21,8 @@
 INTERFACE_FUNCTION(__ubsan_handle_dynamic_type_cache_miss_abort)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow_abort)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_abort)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1_abort)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion_abort)

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-15 Thread Evgenii Stepanov via Phabricator via cfe-commits 
eugenis accepted this revision.
eugenis added a comment.
This revision is now accepted and ready to land.

LGTM, but please update the summary:

  An alternative would be to disallow -fsanitize=function ...


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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-15 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

Looks reasonable to me -- mind waiting for another +1 on this to be safe? 
@eugenis, any thoughts?


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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-15 Thread Stephan Bergmann via Phabricator via cfe-commits 
sberg updated this revision to Diff 209865.
sberg added a comment.

- Disallowed -fsanitize=function in combination with -fsanitize-minimal-runtime 
now.

- Left the ubsan test's RUN lines and \#include magic as-is, if there's no 
clearly better way to avoid this unfortunate complexity.


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

https://reviews.llvm.org/D61479

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/ubsan-function.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
  compiler-rt/lib/ubsan/ubsan_interface.inc
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp

Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -1,11 +1,53 @@
-// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t
-// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC -shared -o %t-so.so
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK $(%run %t-unique UNIQUE)
 // Verify that we can disable symbolization if needed:
-// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
+// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM $(%run %t-unique NOSYM-UNIQUE)
 // XFAIL: windows-msvc
 // Unsupported function flag
 // UNSUPPORTED: openbsd
 
+#ifdef DETERMINE_UNIQUE
+
+#include 
+
+#include "../../../../../lib/sanitizer_common/sanitizer_platform.h"
+
+int main(int, char **argv) {
+  if (!SANITIZER_NON_UNIQUE_TYPEINFO)
+std::cout << "--check-prefix=" << argv[1];
+}
+
+#else
+
+struct Shared {};
+using FnShared = void (*)(Shared *);
+FnShared getShared();
+
+struct __attribute__((visibility("hidden"))) Hidden {};
+using FnHidden = void (*)(Hidden *);
+FnHidden getHidden();
+
+namespace {
+struct Private {};
+} // namespace
+using FnPrivate = void (*)(void *);
+FnPrivate getPrivate();
+
+#ifdef SHARED_LIB
+
+void fnShared(Shared *) {}
+FnShared getShared() { return fnShared; }
+
+void fnHidden(Hidden *) {}
+FnHidden getHidden() { return fnHidden; }
+
+void fnPrivate(Private *) {}
+FnPrivate getPrivate() { return reinterpret_cast(fnPrivate); }
+
+#else
+
 #include 
 
 void f() {}
@@ -64,12 +106,31 @@
   p2(0);
 }
 
+void check_cross_dso() {
+  getShared()(nullptr);
+
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnHidden(Hidden*) through pointer to incorrect function type 'void (*)(Hidden *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)(Hidden *)'
+  getHidden()(nullptr);
+
+  // TODO: Unlike GCC, Clang fails to prefix the typeinfo name for the function
+  // type with "*", so this erroneously only fails for "*UNIQUE":
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnPrivate((anonymous namespace)::Private*) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  reinterpret_cast(getPrivate())(nullptr);
+}
+
 int main(void) {
   make_valid_call();
   make_invalid_call();
   check_noexcept_calls();
+  check_cross_dso();
   // Check that no more errors will be printed.
   // CHECK-NOT: runtime error: call to function
   // NOSYM-NOT: runtime error: call to function
   make_invalid_call();
 }
+
+#endif
+
+#endif
Index: compiler-rt/lib/ubsan/ubsan_interface.inc
===
--- compiler-rt/lib/ubsan/ubsan_interface.inc
+++ compiler-rt/lib/ubsan/ubsan_interface.inc
@@ -21,8 +21,8 @@
 INTERFACE_FUNCTION(__ubsan_handle_dynamic_type_cache_miss_abort)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow_abort)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_abort)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1_abort)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion_abort)
 INTERFACE_FUNCTION(__ubsan_handle_invalid_builtin)
Index: compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
===
---

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-11 Thread Evgenii Stepanov via Phabricator via cfe-commits 
eugenis added inline comments.



Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:207
+equivalence by pointer, not also by name (even on platforms that would
+otherwise fall back to checking equivalence by name).
 

I don't think this is true. As far as I can see, 
__ubsan_handle_function_type_mismatch in the minimal runtime simply prints a 
report and exits, i.e. -fsanitize=function is incompatible and should be 
rejected by the driver.


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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-07-11 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

This looks reasonable to me. I'd prefer to disallow -fsanitize=function in 
combination with -fsanitize-minimal-runtime, as I'm not sure it's appropriate 
for the minimal runtime to handle SANITIZER_NON_UNIQUE_TYPEINFO incorrectly. If 
this has already been discussed elsewhere I'd appreciate a pointer.

I have a more minor concern about the complexity of the ubsan test's RUN lines 
and \#include magic. I'm worried that these factors could make the test brittle 
(w.r.t unexpected directory structures or platforms that don't support $(...) 
expressions). But I don't have a clearly superior alternative to suggest. One 
idea is to a) redefine `sanitizer_non_unique_typeinfo` in the lit config and b) 
define %clangxx to include a path to the sanitizer_common headers. That might 
make the test easier to read, but not less brittle. Do you see this as a 
problem and if so are there any alternatives you might suggest?


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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

2019-06-27 Thread Stephan Bergmann via Phabricator via cfe-commits 
sberg added a subscriber: cfe-commits.
sberg added a comment.

Any thoughts on this?  (cfe-commits had inadvertently been missing from 
subscribers, it touches clang as well as compiler-rt.)


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

https://reviews.llvm.org/D61479



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