[PATCH 0/5] Synchronize <sys/cdefs.h> and <stdatomic.h> with FreeBSD

[PATCH 0/5] Synchronize <sys/cdefs.h> and <stdatomic.h> with FreeBSD

[Sebastian Huber]

This patch set adds some changes after the last synchronization point with
FreeBSD to Newlib.

dab (1):
  Don't sanitize linker_set

jah (1):
  amd64: prevent KCSan false positives on LAPIC mapping

jhb (1):
  Make the system C11 atomics headers fully compatible with external
    GCC.

mjg (1):
  sys: clean up empty lines in .c and .h files

rlibby (1):
  gcc: quiet Wattribute for no_sanitize("address")

 newlib/libc/include/stdatomic.h | 26 +++++++++++---------------
 newlib/libc/include/sys/cdefs.h | 15 ++++++++++++++-
 2 files changed, 25 insertions(+), 16 deletions(-)

-- 
2.26.2

[PATCH 1/5] Make the system C11 atomics headers fully compatible with external GCC.

[Sebastian Huber]

From: jhb <jhb@FreeBSD.org>

The <sys/cdefs.h> and <stdatomic.h> headers already included support for
C11 atomics via intrinsincs in modern versions of GCC, but these versions
tried to "hide" atomic variables inside a wrapper structure.  This wrapper
is not compatible with GCC's internal <stdatomic.h> header, so that if
GCC's <stdatomic.h> was used together with <sys/cdefs.h>, use of C11
atomics would fail to compile.  Fix this by not hiding atomic variables
in a structure for modern versions of GCC.  The headers already avoid
using a wrapper structure on clang.

Note that this wrapper was only used if C11 was not enabled (e.g.
via -std=c99), so this also fixes compile failures if a modern version
of GCC was used with -std=c11 but with FreeBSD's <stdatomic.h> instead
of GCC's <stdatomic.h> and this change fixes that case as well.

Reported by:	Mark Millard
Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D16585
---
 newlib/libc/include/stdatomic.h | 25 +++++++++++--------------
 newlib/libc/include/sys/cdefs.h |  2 +-
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/newlib/libc/include/stdatomic.h b/newlib/libc/include/stdatomic.h
index 09c0cf73e..1441c9e3b 100644
--- a/newlib/libc/include/stdatomic.h
+++ b/newlib/libc/include/stdatomic.h
@@ -169,12 +169,9 @@ atomic_signal_fence(memory_order __order __unused)
 /* Atomics in kernelspace are always lock-free. */
 #define	atomic_is_lock_free(obj) \
 	((void)(obj), (_Bool)1)
-#elif defined(__CLANG_ATOMICS)
+#elif defined(__CLANG_ATOMICS) || defined(__GNUC_ATOMICS)
 #define	atomic_is_lock_free(obj) \
 	__atomic_is_lock_free(sizeof(*(obj)), obj)
-#elif defined(__GNUC_ATOMICS)
-#define	atomic_is_lock_free(obj) \
-	__atomic_is_lock_free(sizeof((obj)->__val), &(obj)->__val)
 #else
 #define	atomic_is_lock_free(obj) \
 	((void)(obj), sizeof((obj)->__val) <= sizeof(void *))
@@ -260,28 +257,28 @@ typedef _Atomic(uintmax_t)		atomic_uintmax_t;
 #elif defined(__GNUC_ATOMICS)
 #define	atomic_compare_exchange_strong_explicit(object, expected,	\
     desired, success, failure)						\
-	__atomic_compare_exchange_n(&(object)->__val, expected,		\
+	__atomic_compare_exchange_n(object, expected,			\
 	    desired, 0, success, failure)
 #define	atomic_compare_exchange_weak_explicit(object, expected,		\
     desired, success, failure)						\
-	__atomic_compare_exchange_n(&(object)->__val, expected,		\
+	__atomic_compare_exchange_n(object, expected,			\
 	    desired, 1, success, failure)
 #define	atomic_exchange_explicit(object, desired, order)		\
-	__atomic_exchange_n(&(object)->__val, desired, order)
+	__atomic_exchange_n(object, desired, order)
 #define	atomic_fetch_add_explicit(object, operand, order)		\
-	__atomic_fetch_add(&(object)->__val, operand, order)
+	__atomic_fetch_add(object, operand, order)
 #define	atomic_fetch_and_explicit(object, operand, order)		\
-	__atomic_fetch_and(&(object)->__val, operand, order)
+	__atomic_fetch_and(object, operand, order)
 #define	atomic_fetch_or_explicit(object, operand, order)		\
-	__atomic_fetch_or(&(object)->__val, operand, order)
+	__atomic_fetch_or(object, operand, order)
 #define	atomic_fetch_sub_explicit(object, operand, order)		\
-	__atomic_fetch_sub(&(object)->__val, operand, order)
+	__atomic_fetch_sub(object, operand, order)
 #define	atomic_fetch_xor_explicit(object, operand, order)		\
-	__atomic_fetch_xor(&(object)->__val, operand, order)
+	__atomic_fetch_xor(object, operand, order)
 #define	atomic_load_explicit(object, order)				\
-	__atomic_load_n(&(object)->__val, order)
+	__atomic_load_n(object, order)
 #define	atomic_store_explicit(object, desired, order)			\
-	__atomic_store_n(&(object)->__val, desired, order)
+	__atomic_store_n(object, desired, order)
 #else
 #define	__atomic_apply_stride(object, operand) \
 	(((__typeof__((object)->__val))0) + (operand))
diff --git a/newlib/libc/include/sys/cdefs.h b/newlib/libc/include/sys/cdefs.h
index ccb47ea40..9a0466fff 100644
--- a/newlib/libc/include/sys/cdefs.h
+++ b/newlib/libc/include/sys/cdefs.h
@@ -288,7 +288,7 @@
 #endif
 
 #if !defined(__cplusplus) && !__has_extension(c_atomic) && \
-    !__has_extension(cxx_atomic)
+	!__has_extension(cxx_atomic) && !__GNUC_PREREQ__(4, 7)
 /*
  * No native support for _Atomic(). Place object in structure to prevent
  * most forms of direct non-atomic access.
-- 
2.26.2

[PATCH 2/5] Don't sanitize linker_set

[Sebastian Huber]

From: dab <dab@FreeBSD.org>

The assumptions of linker_set don't play nicely with
AddressSanitizer. AddressSanitizer adds a 'redzone' of zeros around
globals (including those in named sections), whereas linker_set
assumes they are all packed consecutively like a pointer array. So:
let's annotate linker_set so that AddressSanitizer ignores it.

Submitted by:	Matthew Bryan <matthew.bryan@isilon.com>
Reviewed by:	kib, rang_acm.org
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D22239
---
 newlib/libc/include/sys/cdefs.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/newlib/libc/include/sys/cdefs.h b/newlib/libc/include/sys/cdefs.h
index 9a0466fff..1f2ce063d 100644
--- a/newlib/libc/include/sys/cdefs.h
+++ b/newlib/libc/include/sys/cdefs.h
@@ -709,6 +709,13 @@
 /* Function should not be analyzed. */
 #define	__no_lock_analysis	__lock_annotate(no_thread_safety_analysis)
 
+/* Function or variable should not be sanitized, ie. by AddressSanitizer */
+#if __has_attribute(no_sanitize)
+#define __nosanitizeaddress	__attribute__((no_sanitize("address")))
+#else
+#define __nosanitizeaddress
+#endif
+
 /* Guard variables and structure members by lock. */
 #define	__guarded_by(x)		__lock_annotate(guarded_by(x))
 #define	__pt_guarded_by(x)	__lock_annotate(pt_guarded_by(x))
-- 
2.26.2

[PATCH 3/5] gcc: quiet Wattribute for no_sanitize("address")

[Sebastian Huber]

From: rlibby <rlibby@FreeBSD.org>

This is an unfortunate instance where the __has_attribute check does
not function usefully.  Gcc does have the attribute, but for gcc it only
applies to functions, not variables, and trying to apply it to a
variable generates Wattribute.  So far we only apply the attribute to
variables.  Only enable the attribute for clang, for now.

Reviewed by:	Anton Rang <rang at acm.org>
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D22875
---
 newlib/libc/include/sys/cdefs.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/newlib/libc/include/sys/cdefs.h b/newlib/libc/include/sys/cdefs.h
index 1f2ce063d..1be994ffd 100644
--- a/newlib/libc/include/sys/cdefs.h
+++ b/newlib/libc/include/sys/cdefs.h
@@ -709,8 +709,12 @@
 /* Function should not be analyzed. */
 #define	__no_lock_analysis	__lock_annotate(no_thread_safety_analysis)
 
-/* Function or variable should not be sanitized, ie. by AddressSanitizer */
-#if __has_attribute(no_sanitize)
+/*
+ * Function or variable should not be sanitized, i.e. by AddressSanitizer.
+ * GCC has the nosanitize attribute, but as a function attribute only, and
+ * warns on use as a variable attribute.
+ */
+#if __has_attribute(no_sanitize) && defined(__clang__)
 #define __nosanitizeaddress	__attribute__((no_sanitize("address")))
 #else
 #define __nosanitizeaddress
-- 
2.26.2

[PATCH 4/5] sys: clean up empty lines in .c and .h files

[Sebastian Huber]

From: mjg <mjg@FreeBSD.org>

---
 newlib/libc/include/stdatomic.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/newlib/libc/include/stdatomic.h b/newlib/libc/include/stdatomic.h
index 1441c9e3b..d58e795eb 100644
--- a/newlib/libc/include/stdatomic.h
+++ b/newlib/libc/include/stdatomic.h
@@ -373,7 +373,6 @@ __extension__ ({							\
 typedef struct {
 	atomic_bool	__flag;
 } atomic_flag;
-
 #define	ATOMIC_FLAG_INIT		{ ATOMIC_VAR_INIT(0) }
 
 static __inline _Bool
-- 
2.26.2

[PATCH 5/5] amd64: prevent KCSan false positives on LAPIC mapping

[Sebastian Huber]

From: jah <jah@FreeBSD.org>

For configurations without x2APIC support (guests, older hardware), the global
LAPIC MMIO mapping will trigger false-positive KCSan reports as it will appear
that multiple CPUs are concurrently reading and writing the same address.
This isn't actually true, as the underlying physical access will be performed
on the local CPU's APIC. Additionally, because LAPIC access can happen during
event timer configuration, the resulting KCSan printf can produce a panic due
to attempted recursion on event timer resources.

Add a __nosanitizethread preprocessor define to prevent the compiler from
inserting TSan hooks, and apply it to the x86 LAPIC accessors.

PR:		249149
Reported by:	gbe
Reviewed by:	andrew, kib
Tested by:	gbe
Differential Revision:	https://reviews.freebsd.org/D26354
---
 newlib/libc/include/sys/cdefs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/newlib/libc/include/sys/cdefs.h b/newlib/libc/include/sys/cdefs.h
index 1be994ffd..da7290921 100644
--- a/newlib/libc/include/sys/cdefs.h
+++ b/newlib/libc/include/sys/cdefs.h
@@ -716,8 +716,10 @@
  */
 #if __has_attribute(no_sanitize) && defined(__clang__)
 #define __nosanitizeaddress	__attribute__((no_sanitize("address")))
+#define __nosanitizethread	__attribute__((no_sanitize("thread")))
 #else
 #define __nosanitizeaddress
+#define __nosanitizethread
 #endif
 
 /* Guard variables and structure members by lock. */
-- 
2.26.2

Re: [PATCH 0/5] Synchronize <sys/cdefs.h> and <stdatomic.h> with FreeBSD

[Corinna Vinschen]

On Oct 19 17:55, Sebastian Huber wrote:
> This patch set adds some changes after the last synchronization point with
> FreeBSD to Newlib.
> 
> dab (1):
>   Don't sanitize linker_set
> 
> jah (1):
>   amd64: prevent KCSan false positives on LAPIC mapping
> 
> jhb (1):
>   Make the system C11 atomics headers fully compatible with external
>     GCC.
> 
> mjg (1):
>   sys: clean up empty lines in .c and .h files
> 
> rlibby (1):
>   gcc: quiet Wattribute for no_sanitize("address")
> 
>  newlib/libc/include/stdatomic.h | 26 +++++++++++---------------
>  newlib/libc/include/sys/cdefs.h | 15 ++++++++++++++-
>  2 files changed, 25 insertions(+), 16 deletions(-)
> 
> -- 
> 2.26.2

LGTM, please push.


Thanks,
Corinna