Hi Bruce, Mike,

> On Fri, Nov 11, 2016 at 3:18 AM, Mike Stump <mikest...@comcast.net> wrote:
>>
>> No objections from me.
>>
> Or me.  Thanks!

as discussed at length in the PR, the fixincludes route alone isn't
enough to get libsanitizer to build on Darwin 15: we stay with undefined
references to a function which had to fixinclude'd out due to use of the
Blocks extension.  On closer inspection, it turns out that the Darwin 16
<os/trace.h> effectively disables os_trace unless using clang >= 7.3.
So there's no point in jumping through hoops to achieve the same with a
large effort.

OTOH, the fixes in my fixincludes patch fix real incompatibilities
between Darwin headers an GCC, so we agreed to keep them even if
libsanitizer would build without.

So the current suggestion is to combine my fixincludes patch and Jack's
patch to disable <os/trace.h> use if !__BLOCKS__.

Bootstrapped without regressions on Darwin 16 by myself, and on 15 and
14 by Jack.

I guess this is ok for mainline now to restore bootstrap?

It may be that we add some more fixincludes fixes later, even if not
required for libsanitizer.

        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2016-11-10  Rainer Orth  <r...@cebitec.uni-bielefeld.de>

        PR sanitizer/78267
        * inclhack.def (darwin_availabilityinternal, darwin_os_trace_1)
        (darwin_os_trace_2, darwin_os_trace_3): New fixes.
        (hpux_stdint_least_fast): Remove spurious _EOFix_.
        * fixincl.x: Regenerate.
        * tests/bases/AvailabilityInternal.h: New file.
        * tests/bases/os/trace.h: New file.

2016-11-14  Jack Howarth  <howarth.at....@gmail.com>

        PR sanitizer/78267
        * sanitizer_common/sanitizer_mac.cc: Include <os/trace.h> only if
        compiler supports blocks extension.

# HG changeset patch
# Parent  14eb543b5cb5e7a2bdea7430e3bf3b771bf0e54f
Fix macOS 10.12 <AvailabilityInternal.h> and <os/trace.h> (PR sanitizer/78267)

diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -1338,6 +1338,32 @@ fix = {
 };
 
 /*
+ *  macOS 10.12 <AvailabilityInternal.h> uses __attribute__((availability))
+ *  unconditionally.
+ */
+fix = {
+    hackname  = darwin_availabilityinternal;
+    mach      = "*-*-darwin*";
+    files     = AvailabilityInternal.h;
+    select    = "#define[ \t]+(__API_[ADU]\\([^)]*\\)).*";
+    c_fix     = format;
+    c_fix_arg = <<- _EOFix_
+	#if defined(__has_attribute)
+	  #if __has_attribute(availability)
+	%0
+	  #else
+	    #define %1
+	  #endif
+	#else
+	    #define %1
+	#endif
+	_EOFix_;
+
+    test_text = "#define __API_A(x) __attribute__((availability(__API_AVAILABLE_PLATFORM_##x)))\n"
+		"#define __API_D(msg,x) __attribute__((availability(__API_DEPRECATED_PLATFORM_##x,message=msg)))";
+};
+
+/*
  *  For the AAB_darwin7_9_long_double_funcs fix to be useful,
  *  you have to not use "" includes.
  */
@@ -1410,6 +1436,62 @@ fix = {
 };
 
 /*
+ *  Mac OS X 10.11 <os/trace.h> uses attribute on function definition.
+ */
+fix = {
+  hackname  = darwin_os_trace_1;
+  mach      = "*-*-darwin*";
+  files     = os/trace.h;
+  select    = "^(_os_trace_verify_printf.*) (__attribute__.*)";
+  c_fix     = format;
+  c_fix_arg = "%1";
+  test_text = "_os_trace_verify_printf(const char *msg, ...) __attribute__((format(printf, 1, 2)))";
+};
+
+/*
+ *  Mac OS X 10.1[012] <os/trace.h> os_trace_payload_t typedef uses Blocks
+ *  extension without guard.
+ */
+fix = {
+  hackname  = darwin_os_trace_2;
+  mach      = "*-*-darwin*";
+  files     = os/trace.h;
+  select    = "typedef.*\\^os_trace_payload_t.*";
+  c_fix     = format;
+  c_fix_arg = "#if __BLOCKS__\n%0\n#endif";
+  test_text = "typedef void (^os_trace_payload_t)(xpc_object_t xdict);";
+};
+
+/*
+ *  In Mac OS X 10.1[012] <os/trace.h>, need to guard users of
+ *  os_trace_payload_t typedef, too.
+ */
+fix = {
+  hackname  = darwin_os_trace_3;
+  mach      = "*-*-darwin*";
+  files     = os/trace.h;
+  select    = <<- _EOSelect_
+	__(API|OSX)_.*
+	OS_EXPORT.*
+	.*
+	_os_trace.*os_trace_payload_t payload);
+	_EOSelect_;
+  c_fix     = format;
+  c_fix_arg = "#if __BLOCKS__\n%0\n#endif";
+  test_text = <<- _EOText_
+	__API_AVAILABLE(macosx(10.10), ios(8.0), watchos(2.0), tvos(8.0))
+	OS_EXPORT OS_NOTHROW OS_NOT_TAIL_CALLED
+	void
+	_os_trace_with_buffer(void *dso, const char *message, uint8_t type, const void *buffer, size_t buffer_size, os_trace_payload_t payload);
+
+	__OSX_AVAILABLE_STARTING(__MAC_10_12, __IPHONE_10_0)
+	OS_EXPORT OS_NOTHROW
+	void
+	_os_trace_internal(void *dso, uint8_t type, const char *format, const uint8_t *buf, size_t buf_size, os_trace_payload_t payload);
+	_EOText_;
+};
+
+/*
  *  __private_extern__ doesn't exist in FSF GCC.  Even if it did,
  *  why would you ever put it in a system header file?
  */
@@ -2638,7 +2720,6 @@ fix = {
     c-fix-arg = "#  define	UINT_%164_MAX	__UINT64_MAX__";
     test-text = "#  define       UINT_FAST64_MAX        ULLONG_MAX\n"
 		"#  define       UINT_LEAST64_MAX        ULLONG_MAX\n";
-	_EOFix_;
 };
 
 /*
diff --git a/fixincludes/tests/base/AvailabilityInternal.h b/fixincludes/tests/base/AvailabilityInternal.h
new file mode 100644
--- /dev/null
+++ b/fixincludes/tests/base/AvailabilityInternal.h
@@ -0,0 +1,31 @@
+/*  DO NOT EDIT THIS FILE.
+
+    It has been auto-edited by fixincludes from:
+
+	"fixinc/tests/inc/AvailabilityInternal.h"
+
+    This had to be done to correct non-standard usages in the
+    original, manufacturer supplied header file.  */
+
+
+
+#if defined( DARWIN_AVAILABILITYINTERNAL_CHECK )
+#if defined(__has_attribute)
+  #if __has_attribute(availability)
+#define __API_A(x) __attribute__((availability(__API_AVAILABLE_PLATFORM_##x)))
+  #else
+    #define __API_A(x)
+  #endif
+#else
+    #define __API_A(x)
+#endif
+#if defined(__has_attribute)
+  #if __has_attribute(availability)
+#define __API_D(msg,x) __attribute__((availability(__API_DEPRECATED_PLATFORM_##x,message=msg)))
+  #else
+    #define __API_D(msg,x)
+  #endif
+#else
+    #define __API_D(msg,x)
+#endif
+#endif  /* DARWIN_AVAILABILITYINTERNAL_CHECK */
diff --git a/fixincludes/tests/base/os/trace.h b/fixincludes/tests/base/os/trace.h
new file mode 100644
--- /dev/null
+++ b/fixincludes/tests/base/os/trace.h
@@ -0,0 +1,38 @@
+/*  DO NOT EDIT THIS FILE.
+
+    It has been auto-edited by fixincludes from:
+
+	"fixinc/tests/inc/os/trace.h"
+
+    This had to be done to correct non-standard usages in the
+    original, manufacturer supplied header file.  */
+
+
+
+#if defined( DARWIN_OS_TRACE_1_CHECK )
+_os_trace_verify_printf(const char *msg, ...)
+#endif  /* DARWIN_OS_TRACE_1_CHECK */
+
+
+#if defined( DARWIN_OS_TRACE_2_CHECK )
+#if __BLOCKS__
+typedef void (^os_trace_payload_t)(xpc_object_t xdict);
+#endif
+#endif  /* DARWIN_OS_TRACE_2_CHECK */
+
+
+#if defined( DARWIN_OS_TRACE_3_CHECK )
+#if __BLOCKS__
+__API_AVAILABLE(macosx(10.10), ios(8.0), watchos(2.0), tvos(8.0))
+OS_EXPORT OS_NOTHROW OS_NOT_TAIL_CALLED
+void
+_os_trace_with_buffer(void *dso, const char *message, uint8_t type, const void *buffer, size_t buffer_size, os_trace_payload_t payload);
+#endif
+
+#if __BLOCKS__
+__OSX_AVAILABLE_STARTING(__MAC_10_12, __IPHONE_10_0)
+OS_EXPORT OS_NOTHROW
+void
+_os_trace_internal(void *dso, uint8_t type, const char *format, const uint8_t *buf, size_t buf_size, os_trace_payload_t payload);
+#endif
+#endif  /* DARWIN_OS_TRACE_3_CHECK */
diff --git a/libsanitizer/sanitizer_common/sanitizer_mac.cc b/libsanitizer/sanitizer_common/sanitizer_mac.cc
--- a/libsanitizer/sanitizer_common/sanitizer_mac.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_mac.cc
@@ -34,7 +34,7 @@
 extern char **environ;
 #endif
 
-#if defined(__has_include) && __has_include(<os/trace.h>)
+#if defined(__has_include) && __has_include(<os/trace.h>) && defined(__BLOCKS__)
 #define SANITIZER_OS_TRACE 1
 #include <os/trace.h>
 #else

Reply via email to