[libc++] Introduce ABI sensitive areas to avoid requiring _LIBCPP_HIDE_FROM_ABI everywhere #131156
Conversation
philnik777
changed the title
philnik777
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesPatch is 92.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131156.diff 102 Files Affected:
diff --git a/libcxx/include/__algorithm/shuffle.h b/libcxx/include/__algorithm/shuffle.h
index 7177fbb469ba7..37999364797c6 100644
--- a/libcxx/include/__algorithm/shuffle.h
+++ b/libcxx/include/__algorithm/shuffle.h
@@ -65,6 +65,7 @@ class _LIBCPP_EXPORTED_FROM_ABI __libcpp_debug_randomizer {
#if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_RANDOM_SHUFFLE) || defined(_LIBCPP_BUILDING_LIBRARY)
class _LIBCPP_EXPORTED_FROM_ABI __rs_default;
+_LIBCPP_BEGIN_ABI_SENSITIVE
_LIBCPP_EXPORTED_FROM_ABI __rs_default __rs_get();
class _LIBCPP_EXPORTED_FROM_ABI __rs_default {
@@ -90,6 +91,7 @@ class _LIBCPP_EXPORTED_FROM_ABI __rs_default {
};
_LIBCPP_EXPORTED_FROM_ABI __rs_default __rs_get();
+_LIBCPP_END_ABI_SENSITIVE
template <class _RandomAccessIterator>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_DEPRECATED_IN_CXX14 void
diff --git a/libcxx/include/__algorithm/sort.h b/libcxx/include/__algorithm/sort.h
index 8dd0721f2c65f..f3dc2e0d6c180 100644
--- a/libcxx/include/__algorithm/sort.h
+++ b/libcxx/include/__algorithm/sort.h
@@ -64,7 +64,7 @@ enum { __block_size = sizeof(uint64_t) * 8 };
// Ensures that __c(*__x, *__y) is true by swapping *__x and *__y if necessary.
template <class _Compare, class _RandomAccessIterator>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
+inline _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
__cond_swap(_RandomAccessIterator __x, _RandomAccessIterator __y, _Compare __c) {
// Note: this function behaves correctly even with proxy iterators (because it relies on `value_type`).
using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
@@ -78,7 +78,7 @@ __cond_swap(_RandomAccessIterator __x, _RandomAccessIterator __y, _Compare __c)
// Ensures that *__x, *__y and *__z are ordered according to the comparator __c,
// under the assumption that *__y and *__z are already ordered.
template <class _Compare, class _RandomAccessIterator>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
+inline _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
__partially_sorted_swap(_RandomAccessIterator __x, _RandomAccessIterator __y, _RandomAccessIterator __z, _Compare __c) {
// Note: this function behaves correctly even with proxy iterators (because it relies on `value_type`).
using value_type = typename iterator_traits<_RandomAccessIterator>::value_type;
@@ -97,7 +97,7 @@ template <class,
class _Compare,
class _RandomAccessIterator,
__enable_if_t<__use_branchless_sort<_Compare, _RandomAccessIterator>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
+inline _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
__sort3(_RandomAccessIterator __x1, _RandomAccessIterator __x2, _RandomAccessIterator __x3, _Compare __c) {
bool __swapped1 = std::__cond_swap<_Compare>(__x2, __x3, __c);
bool __swapped2 = std::__partially_sorted_swap<_Compare>(__x1, __x2, __x3, __c);
@@ -108,7 +108,7 @@ template <class _AlgPolicy,
class _Compare,
class _RandomAccessIterator,
__enable_if_t<!__use_branchless_sort<_Compare, _RandomAccessIterator>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
+inline _LIBCPP_CONSTEXPR_SINCE_CXX14 bool
__sort3(_RandomAccessIterator __x, _RandomAccessIterator __y, _RandomAccessIterator __z, _Compare __c) {
using _Ops = _IterOps<_AlgPolicy>;
@@ -140,7 +140,7 @@ template <class,
class _Compare,
class _RandomAccessIterator,
__enable_if_t<__use_branchless_sort<_Compare, _RandomAccessIterator>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI void
+inline void
__sort4(_RandomAccessIterator __x1,
_RandomAccessIterator __x2,
_RandomAccessIterator __x3,
@@ -157,7 +157,7 @@ template <class _AlgPolicy,
class _Compare,
class _RandomAccessIterator,
__enable_if_t<!__use_branchless_sort<_Compare, _RandomAccessIterator>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI void
+inline void
__sort4(_RandomAccessIterator __x1,
_RandomAccessIterator __x2,
_RandomAccessIterator __x3,
@@ -182,7 +182,7 @@ template <class _AlgPolicy,
class _Compare,
class _RandomAccessIterator,
__enable_if_t<__use_branchless_sort<_Compare, _RandomAccessIterator>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI void
+inline void
__sort5(_RandomAccessIterator __x1,
_RandomAccessIterator __x2,
_RandomAccessIterator __x3,
@@ -201,7 +201,7 @@ template <class _AlgPolicy,
class _Compare,
class _RandomAccessIterator,
__enable_if_t<!__use_branchless_sort<_Compare, _RandomAccessIterator>, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI void
+inline void
__sort5(_RandomAccessIterator __x1,
_RandomAccessIterator __x2,
_RandomAccessIterator __x3,
@@ -227,7 +227,7 @@ __sort5(_RandomAccessIterator __x1,
// Assumes size > 0
template <class _AlgPolicy, class _Compare, class _BidirectionalIterator>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void
+_LIBCPP_CONSTEXPR_SINCE_CXX14 void
__selection_sort(_BidirectionalIterator __first, _BidirectionalIterator __last, _Compare __comp) {
_BidirectionalIterator __lm1 = __last;
for (--__lm1; __first != __lm1; ++__first) {
@@ -240,7 +240,7 @@ __selection_sort(_BidirectionalIterator __first, _BidirectionalIterator __last,
// Sort the iterator range [__first, __last) using the comparator __comp using
// the insertion sort algorithm.
template <class _AlgPolicy, class _Compare, class _BidirectionalIterator>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX26 void
+_LIBCPP_CONSTEXPR_SINCE_CXX26 void
__insertion_sort(_BidirectionalIterator __first, _BidirectionalIterator __last, _Compare __comp) {
using _Ops = _IterOps<_AlgPolicy>;
@@ -270,8 +270,7 @@ __insertion_sort(_BidirectionalIterator __first, _BidirectionalIterator __last,
// Assumes that there is an element in the position (__first - 1) and that each
// element in the input range is greater or equal to the element at __first - 1.
template <class _AlgPolicy, class _Compare, class _RandomAccessIterator>
-_LIBCPP_HIDE_FROM_ABI void
-__insertion_sort_unguarded(_RandomAccessIterator const __first, _RandomAccessIterator __last, _Compare __comp) {
+void __insertion_sort_unguarded(_RandomAccessIterator const __first, _RandomAccessIterator __last, _Compare __comp) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;
@@ -298,8 +297,7 @@ __insertion_sort_unguarded(_RandomAccessIterator const __first, _RandomAccessIte
}
template <class _AlgPolicy, class _Comp, class _RandomAccessIterator>
-_LIBCPP_HIDE_FROM_ABI bool
-__insertion_sort_incomplete(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp __comp) {
+bool __insertion_sort_incomplete(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp __comp) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
@@ -352,7 +350,7 @@ __insertion_sort_incomplete(_RandomAccessIterator __first, _RandomAccessIterator
}
template <class _AlgPolicy, class _RandomAccessIterator>
-inline _LIBCPP_HIDE_FROM_ABI void __swap_bitmap_pos(
+inline void __swap_bitmap_pos(
_RandomAccessIterator __first, _RandomAccessIterator __last, uint64_t& __left_bitset, uint64_t& __right_bitset) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename std::iterator_traits<_RandomAccessIterator>::difference_type difference_type;
@@ -370,7 +368,7 @@ inline _LIBCPP_HIDE_FROM_ABI void __swap_bitmap_pos(
template <class _Compare,
class _RandomAccessIterator,
class _ValueType = typename iterator_traits<_RandomAccessIterator>::value_type>
-inline _LIBCPP_HIDE_FROM_ABI void
+inline void
__populate_left_bitset(_RandomAccessIterator __first, _Compare __comp, _ValueType& __pivot, uint64_t& __left_bitset) {
// Possible vectorization. With a proper "-march" flag, the following loop
// will be compiled into a set of SIMD instructions.
@@ -386,7 +384,7 @@ __populate_left_bitset(_RandomAccessIterator __first, _Compare __comp, _ValueTyp
template <class _Compare,
class _RandomAccessIterator,
class _ValueType = typename iterator_traits<_RandomAccessIterator>::value_type>
-inline _LIBCPP_HIDE_FROM_ABI void
+inline void
__populate_right_bitset(_RandomAccessIterator __lm1, _Compare __comp, _ValueType& __pivot, uint64_t& __right_bitset) {
// Possible vectorization. With a proper "-march" flag, the following loop
// will be compiled into a set of SIMD instructions.
@@ -403,7 +401,7 @@ template <class _AlgPolicy,
class _Compare,
class _RandomAccessIterator,
class _ValueType = typename iterator_traits<_RandomAccessIterator>::value_type>
-inline _LIBCPP_HIDE_FROM_ABI void __bitset_partition_partial_blocks(
+inline void __bitset_partition_partial_blocks(
_RandomAccessIterator& __first,
_RandomAccessIterator& __lm1,
_Compare __comp,
@@ -450,7 +448,7 @@ inline _LIBCPP_HIDE_FROM_ABI void __bitset_partition_partial_blocks(
}
template <class _AlgPolicy, class _RandomAccessIterator>
-inline _LIBCPP_HIDE_FROM_ABI void __swap_bitmap_pos_within(
+inline void __swap_bitmap_pos_within(
_RandomAccessIterator& __first, _RandomAccessIterator& __lm1, uint64_t& __left_bitset, uint64_t& __right_bitset) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename std::iterator_traits<_RandomAccessIterator>::difference_type difference_type;
@@ -491,7 +489,7 @@ inline _LIBCPP_HIDE_FROM_ABI void __swap_bitmap_pos_within(
// __bitset_partition uses bitsets for storing outcomes of the comparisons
// between the pivot and other elements.
template <class _AlgPolicy, class _RandomAccessIterator, class _Compare>
-_LIBCPP_HIDE_FROM_ABI std::pair<_RandomAccessIterator, bool>
+std::pair<_RandomAccessIterator, bool>
__bitset_partition(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename std::iterator_traits<_RandomAccessIterator>::value_type value_type;
@@ -583,7 +581,7 @@ __bitset_partition(_RandomAccessIterator __first, _RandomAccessIterator __last,
// the provided range is already sorted, false otherwise. We assume that the
// length of the range is at least three elements.
template <class _AlgPolicy, class _RandomAccessIterator, class _Compare>
-_LIBCPP_HIDE_FROM_ABI std::pair<_RandomAccessIterator, bool>
+std::pair<_RandomAccessIterator, bool>
__partition_with_equals_on_right(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
@@ -651,7 +649,7 @@ __partition_with_equals_on_right(_RandomAccessIterator __first, _RandomAccessIte
// Similar to the above function. Elements equivalent to the pivot are put to
// the left of the pivot. Returns the iterator to the pivot element.
template <class _AlgPolicy, class _RandomAccessIterator, class _Compare>
-_LIBCPP_HIDE_FROM_ABI _RandomAccessIterator
+_RandomAccessIterator
__partition_with_equals_on_left(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp) {
using _Ops = _IterOps<_AlgPolicy>;
typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
@@ -829,7 +827,7 @@ void __introsort(_RandomAccessIterator __first,
}
template <typename _Number>
-inline _LIBCPP_HIDE_FROM_ABI _Number __log2i(_Number __n) {
+inline _Number __log2i(_Number __n) {
if (__n == 0)
return 0;
if (sizeof(__n) <= sizeof(unsigned))
@@ -847,6 +845,7 @@ inline _LIBCPP_HIDE_FROM_ABI _Number __log2i(_Number __n) {
return __log2;
}
+_LIBCPP_BEGIN_ABI_SENSITIVE
template <class _Comp, class _RandomAccessIterator>
void __sort(_RandomAccessIterator, _RandomAccessIterator, _Comp);
@@ -875,9 +874,10 @@ extern template _LIBCPP_EXPORTED_FROM_ABI void __sort<__less<float>&, float*>(fl
extern template _LIBCPP_EXPORTED_FROM_ABI void __sort<__less<double>&, double*>(double*, double*, __less<double>&);
extern template _LIBCPP_EXPORTED_FROM_ABI void
__sort<__less<long double>&, long double*>(long double*, long double*, __less<long double>&);
+_LIBCPP_END_ABI_SENSITIVE
template <class _AlgPolicy, class _RandomAccessIterator, class _Comp>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
+_LIBCPP_CONSTEXPR_SINCE_CXX20 void
__sort_dispatch(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp& __comp) {
typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
difference_type __depth_limit = 2 * std::__log2i(__last - __first);
@@ -914,20 +914,20 @@ using __sort_is_specialized_in_library _LIBCPP_NODEBUG = __is_any_of<
long double>;
template <class _AlgPolicy, class _Type, __enable_if_t<__sort_is_specialized_in_library<_Type>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI void __sort_dispatch(_Type* __first, _Type* __last, __less<>&) {
+void __sort_dispatch(_Type* __first, _Type* __last, __less<>&) {
__less<_Type> __comp;
std::__sort<__less<_Type>&, _Type*>(__first, __last, __comp);
}
template <class _AlgPolicy, class _Type, __enable_if_t<__sort_is_specialized_in_library<_Type>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI void __sort_dispatch(_Type* __first, _Type* __last, less<_Type>&) {
+void __sort_dispatch(_Type* __first, _Type* __last, less<_Type>&) {
__less<_Type> __comp;
std::__sort<__less<_Type>&, _Type*>(__first, __last, __comp);
}
#if _LIBCPP_STD_VER >= 14
template <class _AlgPolicy, class _Type, __enable_if_t<__sort_is_specialized_in_library<_Type>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI void __sort_dispatch(_Type* __first, _Type* __last, less<>&) {
+void __sort_dispatch(_Type* __first, _Type* __last, less<>&) {
__less<_Type> __comp;
std::__sort<__less<_Type>&, _Type*>(__first, __last, __comp);
}
@@ -935,14 +935,14 @@ _LIBCPP_HIDE_FROM_ABI void __sort_dispatch(_Type* __first, _Type* __last, less<>
#if _LIBCPP_STD_VER >= 20
template <class _AlgPolicy, class _Type, __enable_if_t<__sort_is_specialized_in_library<_Type>::value, int> = 0>
-_LIBCPP_HIDE_FROM_ABI void __sort_dispatch(_Type* __first, _Type* __last, ranges::less&) {
+void __sort_dispatch(_Type* __first, _Type* __last, ranges::less&) {
__less<_Type> __comp;
std::__sort<__less<_Type>&, _Type*>(__first, __last, __comp);
}
#endif
template <class _AlgPolicy, class _RandomAccessIterator, class _Comp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
+inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void
__sort_impl(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp& __comp) {
std::__debug_randomize_range<_AlgPolicy>(__first, __last);
@@ -956,14 +956,13 @@ __sort_impl(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp&
}
template <class _RandomAccessIterator, class _Comp>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
+inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void
sort(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp __comp) {
std::__sort_impl<_ClassicAlgPolicy>(std::move(__first), std::move(__last), __comp);
}
template <class _RandomAccessIterator>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-sort(_RandomAccessIterator __first, _RandomAccessIterator __last) {
+inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void sort(_RandomAccessIterator __first, _RandomAccessIterator __last) {
std::sort(__first, __last, __less<>());
}
diff --git a/libcxx/include/__atomic/atomic_sync.h b/libcxx/include/__atomic/atomic_sync.h
index 0dae448d649be..344dd261eb94d 100644
--- a/libcxx/include/__atomic/atomic_sync.h
+++ b/libcxx/include/__atomic/atomic_sync.h
@@ -58,6 +58,7 @@ struct __atomic_waitable< _Tp,
#if _LIBCPP_STD_VER >= 20
# if _LIBCPP_HAS_THREADS
+_LIBCPP_BEGIN_ABI_SENSITIVE
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_EXPORTED_FROM_ABI void __cxx_atomic_notify_one(void const volatile*) _NOEXCEPT;
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_EXPORTED_FROM_ABI void __cxx_atomic_notify_all(void const volatile*) _NOEXCEPT;
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_EXPORTED_FROM_ABI __cxx_contention_t
@@ -73,6 +74,7 @@ _LIBCPP_AVAILABILITY_SYNC _LIBCPP_EXPORTED_FROM_ABI __cxx_contention_t
__libcpp_atomic_monitor(__cxx_atomic_contention_t const volatile*) _NOEXCEPT;
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_EXPORTED_FROM_ABI void
__libcpp_atomic_wait(__cxx_atomic_contention_t const volatile*, __cxx_contention_t) _NOEXCEPT;
+_LIBCPP_END_ABI_SENSITIVE
template <class _AtomicWaitable, class _Poll>
struct __atomic_wait_backoff_impl {
diff --git a/libcxx/include/__charconv/from_chars_floating_point.h b/libcxx/include/__charconv/from_chars_floating_point.h
index 811e518a81db7..d93c98162930d 100644
--- a/libcxx/include/__charconv/from_chars_floating_point.h
+++ b/libcxx/include/__charconv/from_chars_floating_point.h
@@ -35,6 +35,7 @@ struct __from_chars_result {
errc __ec;
};
+_LIBCPP_BEGIN_ABI_SENSITIVE
template <class _Fp>
_LIBCPP_EXPORTED_FROM_ABI __from_chars_result<_Fp> __from_chars_floating_point(
_LIBCPP_NOESCAPE const char* __first, _LIBCPP_NOESCAPE const char* __last, chars_format __fmt);
@@ -44,6 +45,7 @@ extern template __from_chars_result<float> __from_chars_floating_point(
extern template __from_chars_result<double> __from_chars_floating_point(
_LIBCPP_NOESCAPE const char* __first, _LIBCPP_NOESCAPE const char* __last, chars_format __fmt);
+_LIBCPP_END_ABI_SENSITIVE
template <class _Fp>
_LIBCPP_HIDE_FROM_ABI from_chars_result
diff --git a/libcxx/include/__charconv/to_chars_floating_point.h b/libcxx/include/__charconv/to_chars_floating_point.h
index 118f316b21a10..d84373e1818b7 100644
--- a/libcxx/include/__charconv/to_chars_floating_point.h
+++ b/libcxx/include/__charconv/to_chars_floating_point.h
@@ -18,10 +18,11 @@
# pragma GCC system_header
#endif
-_LIBCPP_BEGIN_NAMESPACE_STD
-
#if _LIBCPP_STD_VER >= 17
+_LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_ABI_SENSITIVE
+
_LIBCPP_AVAILABILITY_TO_CHARS_FLOATING_POINT _LIBCPP_EXPORTED_FROM_ABI to_chars_result
to_chars(char* __first, char* __last, float __value);
@@ -48,8 +49,10 @@ to_chars(char* __first, char* __last, double __value, chars_format __fmt, int __
_LIBCPP_AVAILABILITY_TO_CHARS_FLOATING_POINT _LIBCPP_EXPORTED_FROM_ABI to_chars_result
to_chars(char* __first, char* __last, long double __value, chars_format __fmt, int __precision);
-#endif // _LIBCPP_STD_VER >= 17
+_LIBCPP_END_ABI_SENSITIVE
_LIBCPP_END_NAMESPACE_STD
+#endif // _LIBCPP_STD_VER >= 17
+
#endif // _LIBCPP___CHARCONV_TO_CHARS_FLOATING_POINT_H
diff --git a/libcxx/include/__chrono/file_clock.h b/libcxx/include/__chrono/file_clock.h
index b4b7e9dc14e70..6f4beb38ce65d 100644
--- a/libcxx/include/__chrono/file_clock.h
+++ b/libcxx/include/__chrono/file_clock.h
@@ -46,6 +46,7 @@ _LIBCPP_END_NAMESPACE_STD
#ifndef _LIBCPP_CXX03_LANG
_LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
+_LIBCPP_BEGIN_ABI_SENSITIVE
struct _FilesystemClock {
# if _LIBCPP_HAS_INT128
typedef __int128_t rep;
@@ -74,6 +75,7 @@ struct _FilesystemClock {
}
# endif // _LIBCPP_STD_VER >= 20
};
+_LIBCPP_END_ABI_SENSITIVE
_LIBCPP_END_NAMESPACE_FILESYSTEM
#endif // !_LIBCPP_CXX03_LANG
diff --git a/libcxx/include/__chrono/steady_clock.h b/libcxx/include/__chrono/steady_clock.h
index 1b247b2c28609..2a0c0797272af 100644
--- a/libcxx/include/__chrono/steady_clock.h
+++ b/libcxx/include/__chrono/steady_clock.h
@@ -19,6 +19,7 @@
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_ABI_SENSITIVE
namespace chrono {
@@ -37,6 +38,7 @@ class _LIBCPP_EXPORTED_FROM_ABI steady_clock {
} // namespace chrono
+_LIBCPP_END_ABI_SENSITIVE
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___CHRONO_STEADY_CLOCK_H
diff --git a/libcxx/include/__chrono/system_clock.h b/libcxx/include/__chrono/system_clock.h
index 5a9eb65bdae7a..fe5a5e5d18f47 100644
--- a/libcxx/include/__chrono/system_clock.h
+++ b/libcxx/include/__chrono/system_clock.h
@@ -20,6 +20,7 @@
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_ABI_SENSITIVE
namespace chrono {
@@ -47,6 +48,7 @@ using sys_days = sys_time<days>;
} // namespace chrono
+_LIBCPP_END_ABI_SENSITIVE
_LIBCPP_END_NAMESPACE_ST...
[truncated]
|
There was a problem hiding this comment.
This is an interesting patch but I think it would be important to tackle the GCC/visibility situation first to simplify things. Per our conversation just now, I would suggest this:
We drop all visibility/ODR guarantees on GCC and always make everything default vis. That's what libstdc++ does IIRC, so GCC users should be used to controlling the symbols they export themselves when they need to. This would allow us to get rid of _LIBCPP_TEMPLATE_VIS and _LIBCPP_ALWAYS_INLINE on GCC, which would also greatly improve our QOI on GCC for e.g. <format> and other things that are just broken right now.
Once that's done, the next step could be to use explicit function lists for all the class template instantiation declarations we have in the headers (like ofstream). And we keep the class template instantiation declaration itself only on Clang to control the vtable's visibility (until we have a syntax to do that more specifically).
Those steps are worth doing in themselves. And once that's done, I think the conversation here will be a lot simpler to have.
libcxx/include/__config
Outdated
|
|
||
| // Inline namespaces are available in Clang/GCC/MSVC regardless of C++ dialect. | ||
| # define _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_PUSH_EXTENSION_DIAGNOSTICS _LIBCPP_BEGIN_HIDE_FROM_ABI \ | ||
| namespace _LIBCPP_TYPE_VISIBILITY_DEFAULT _LIBCPP_HIDDEN std { \ |
There was a problem hiding this comment.
Why is _LIBCPP_HIDDEN required here? That seems surprising.
Answer: that's because we can't apply visibility attributes with the pragma push/pop, so we need to do it by applying the visibility attribute on the namespace.
philnik777
force-pushed
the
ranged_hide_from_abi
branch
from
27492b8 to
18faeef
Compare
philnik777
force-pushed
the
ranged_hide_from_abi
branch
2 times, most recently
from
ca222cf to
a5126ea
Compare
libcxx/include/__algorithm/shuffle.h
Outdated
There was a problem hiding this comment.
With the introduction of this mechanism, as you pointed out, it would be possible to introduce a symbol in the dylib while forgetting to use _LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS. The unfortunate result of that would be that we're exporting a symbol from the dylib which contains an ABI tag, and we'd likely only notice when we rev up the libc++ version number and notice the ABI list test failing. That's not great.
I think we can guard against that issue by ensuring that our ABI list never contains an ABI tag. I'd like that safeguard to be introduced before we make the change in this patch.
philnik777
force-pushed
the
ranged_hide_from_abi
branch
from
a5126ea to
de1400c
Compare
philnik777
force-pushed
the
ranged_hide_from_abi
branch
from
de1400c to
04fe986
Compare
This wasn't actually specific to the frozen headers, it just showed in a funny way there. I just forgot to surround the The main concern from my point of view is that this didn't show up in the abilist. Do we not have symbols from libc++abi in the abilist? If not, we should definitely add them or make abilists for libc++abi. I've manually diffed the |
philnik777
force-pushed
the
ranged_hide_from_abi
branch
6 times, most recently
from
99a0e39 to
2770f4e
Compare
philnik777
force-pushed
the
ranged_hide_from_abi
branch
from
2770f4e to
dc2fbd6
Compare
|
This broke code that sets a nondefault hardening mode on windows, if using the threading APIs. The ABI tag gets applied on symbols that are exported from the dynamic library, like |
|
@philnik777 Hey! It looks like this broke the lldb macOS incremental c++ expression tests: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/ I reverted locally and verified that this change is causing the problem. Let me know if you need help with this. If you don't have time to look at this shortly, let me know if I should revert it. |
|
@medismailben Could you check whether #140592 fixes your problem? |
|
We can maybe work around this. |
|
@dschuff AFAICT this only affects cases where you specialize a class from the |
|
I think that @philnik777 might be correct here - the C++ standard does permit specialization of template types for UDTs. That is defining and exporting |
I agree that this template specialization is something users would be doing; although it's not clear to me what you're implying about what we should do now. @philnik777 clearly doesn't think that the new behavior where such specializations can't be exported by default is a problem. Speaking of which, is there a way to change the visibility from the namespace scope (i.e. on the |
Given that this is causing disruption, I think we should probably revert while investigating. WDYT @compnerd @philnik777 ? |
I'd rather not revert unless there is actually significant disruption. FWIW we can work around the problem for specific classes in the library, which would be annoying, but definitely better than reverting. The main question I'd like answered before doing anything is whether we consider this a problem worth solving. If the answer is yes, we'll work around the problem in the library until all compilers we support have been fixed. |
We're rejecting correct code now, right? That's a conformance issue for which we'd generally revert.
If I'm correct about this rejecting valid code, then I think the answer is "yes, unless LWG says this is a defect". |
We're not. This is outside the realm of the standard, since it only concerns the visibility of a specialized |
Ah good point, this is visibility related which the committee doesn't need to care about, so revert pressure is a bit lessened. But if we do something in the compiler about how visibility is inherited from the namespace, it should probably not be specific to CC @ldionne on the revert question, I leave it in his capable hands as to whether that's something to do or not. |
Definitely. My question is mostly what behaviour we actually want. |
My intuition is that inheriting visibility from the namespace would be in line with how the standard functionality already works for linkage. e.g., users are used to "everything within this unnamed namespace has internal linkage" so "everything within this namespace with this marking about linkage has that linkage" is not a stretch. Whatever visibility is specified on the declaration within the namespace should take precedence over what's specified on the namespace. CC @erichkeane for more opinions as attributes code owner. |
This last bit is actually quite relevant, because it makes working around the issue more annoying and invasive, since as mentioned above you can't "correct" the bleed into userspace using the same scoped mechanism that caused it to leak in the first place. Also because regardless of what the standard says, this behavior change exposes users to differences in implementation between gcc and clang, that make workarounds more difficult. |
Use explicit visibility to force std::hash template specializations to be exported from libbinaryen dynamic library. Currently we are just using the default flags, causing most of our symbols to have "default" visibility, meaning they can all be used from the tool sources. However a recent libc++ change caused functions declared in namespace std (e.g. hash template specializations) to have hidden visibility, inherited. from the namespece (see llvm/llvm-project#131156). So this macro forces them to be exported. Currently it is only applied to the hash specializations that are defined in libbinaryen and used in the tool code. In the future if we want to compile libbinaryen with -fvisibility-hidden or use a DLL on Windows, we'll need to explicitly annotate everything we want to export. But that's probably only useful if we want external users to link against libbinaryen.so (currently we don't; it's only used to reduce our install size).
|
I believe this change does need to be reverted, unfortunately. I would submit that we are, in effect, "rejecting valid code" now. User code, without shared libraries in the picture, may write in a header and then define that function in one TU, The next question is: what if Users can of course do many things to explicitly break this default expectation (such as adding symbol visibility attributes, linker scripts with symbol visibility lists, etc etc.). Yet, without any of that, everything should work the same as you'd get in a program without shared libraries in play. This is a long-standing expectation (on ELF platforms), and many things depend on it. This change, unfortunately, breaks that expectation, by causing the definition of this user-defined function to be marked hidden -- without the user code having any such non-standard attributes or linker/compiler flags.
Defining custom specializations of std classes is common. Building shared libraries without using any symbol visibility attributes, and expecting the default behaviors is also common. Neither are a weird edge case. |
I don't think this can be worked around for only specific classes, because (somewhat sadly, IMHO) the standard permits user specializations of nearly everything in the stdlib (other than a small list of explicit prohibitions).
While I see there's some inconsistency between compilers on what to do about conflicting attributes, the core issue is that visibility is inherited from the first namespace decl to the second, if the second doesn't have any visibility attribute. Compilers agree upon that, and I don't think it's something that can or should be changed. That is, the following is the expected behavior from the compiler -- but, the resulting behavior of hiding a user-defined specialization is invalid. I believe this implies libc++ simply must not declare hidden visibility on public namespaces (it may on private implementation details namespaces). [edit: changed example to use generic namespace name for clarity] |
You say that, but there has been a single report of someone affected by this so far. That makes the claim that this is common somewhat questionable.
TBH I really don't care about this use-case. Users almost never define their own specializations of specific classes except for ones that are specifically designed for it. I'm quite certain that every implementation would break in spectacular ways if you actually tried to specialize quite a few classes. At least for libc++ that's certainly the case.
It's not though. See https://godbolt.org/z/GEbMnod1x. Note that
Edit: I forgot to mention: This example is incorrect. |
|
Oops, I failed to submit my previous comment ... I too believe that we should be reverting this. I do think that this is invasive enough and something user visible. While I can appreciate that not having to annotate everything with |
I am commenting because I also have breakage due to this, independently of dschuff. So, that's at least 2. I'm not sure how many more independent reports of breakage you could expect from a PR that's only a week old.
Ahhh, great point! I actually completely misunderstood how the visibility propagation works. (Due in part to failing to properly read the results of my testing). So, ignore the example code in my previous message, it totally bogus. What is actually happening is not propagation of the namespace decl -- we actually don't do that! (nor GCC). Rather, what we have here is propagation from the visibility of the template, to any explicit specialization of the template. The code which does that propagation is here. |
Thanks for the info. It wasn't clear to me whether this affected you or whether you were just curious about this breakage. FWIW, I usually get multiple people commenting within a couple of days if I break something significant. Do you have breakage with
That is quite interesting. I don't really know what the code does, but given the results it seems somehow wrong. |
|
We see another issue with this patch when using libc++ with GCC: I believe the issue is that |
The first issue encountered was actually on
I don't think it's wrong. Rather, I'd say: when the attribute is used the way libc++ has attempted, this intended-behavior of the compiler results in invalid outcomes. It's conceivable that with further investigation we could determine that the behavior of the compiler should be changed -- but I think it's much more likely we'd determine that there's good reason to keep the current behavior of the compiler, and that libc++ should just not attempt use the attribute in this manner. |
Whether or not it's used as intended in libc++ doesn't matter. It's inconsistent with itself, which makes it wrong no matter what the intended behvaiour is.
|
It's an extension, "wrong" is what we say it is. :-) FWIW, how we got there:
+1; I still think we should revert. Changing the compiler implementation is risky due to potential for silent breaking changes, so it's not something we should do quickly. And there are people impacted by the changes that are currently in tree, with reproducing code examples at hand. I think this pretty clearly meets our revert criteria. |
|
Chiming in after the fact, but I think I agree this should be reverted. Thanks for handling it folks. We'll figure out a path forward which will probably require a Clang change, but we can do that without pressure once this has been reverted. |
…BCPP_HIDE_FROM_ABI everywhere (#131156)" (#141756) This reverts commit c861fe8. Unfortunately, this use of hidden visibility attributes causes user-defined specializations of standard-library types to also be marked hidden by default, which is incorrect. See discussion thread on #131156. ...and also reverts the follow-up commits: Revert "[libc++] Add explicit ABI annotations to functions from the block runtime declared in <__functional/function.h> (#140592)" This reverts commit 3e4c9dc. Revert "[libc++] Make ABI annotations explicit for windows-specific code (#140507)" This reverts commit f73287e. Revert "[libc++][NFC] Replace a few "namespace std" with the correct macro (#140510)" This reverts commit 1d411f2.
…BCPP_HIDE_FROM_ABI everywhere (llvm#131156)" (llvm#141756) This reverts commit c861fe8. Unfortunately, this use of hidden visibility attributes causes user-defined specializations of standard-library types to also be marked hidden by default, which is incorrect. See discussion thread on llvm#131156. ...and also reverts the follow-up commits: Revert "[libc++] Add explicit ABI annotations to functions from the block runtime declared in <__functional/function.h> (llvm#140592)" This reverts commit 3e4c9dc. Revert "[libc++] Make ABI annotations explicit for windows-specific code (llvm#140507)" This reverts commit f73287e. Revert "[libc++][NFC] Replace a few "namespace std" with the correct macro (llvm#140510)" This reverts commit 1d411f2.
…E_FROM_ABI everywhere (llvm#131156) This patch introduces `_LIBCPP_{BEGIN,END}_EXPLICIT_ABI_ANNOTATIONS`, which allow us to have implicit annotations for most functions, and just where it's not "hide_from_abi everything" we add explicit annotations. This allows us to drop the `_LIBCPP_HIDE_FROM_ABI` macro from most functions in libc++.
This patch introduces
_LIBCPP_{BEGIN,END}_EXPLICIT_ABI_ANNOTATIONS, which allow us to have implicit annotations for most functions, and just where it's not "hide_from_abi everything" we add explicit annotations. This allows us to drop the_LIBCPP_HIDE_FROM_ABImacro from most functions in libc++.