Inline functions are apparently really complicated.
C and C++ handle inline functions differently. In C++, an inline function is
defined in just the header file, potentially emitted in multiple compilation
units (in cases the compiler did not inline), but each copy must be identical
to satsify ODR. In C, a non-static inline must be manually emitted in exactly
one compilation unit with a separate extern inline declaration.
In both languages, exported inline functions referencing file-local symbols are
problematic. C forbids this altogether (though GCC and Clang seem not to
enforce it). It works in C++, but ODR requires the definitions be identical,
including all names in the definitions resolving to the "same entity". In
practice, this is unlikely to be a problem, but an inline function that returns
a pointer to a file-local symbol could compile oddly.
Historically, we used static inline in headers. However, to satisfy ODR, use
plain inline in C++, to allow inline consumer functions to call our header
functions. Plain inline would also work better with C99 inline, but that is not
used much in practice, extern inline is tedious, and there are conflicts with
the old gnu89 model: https://stackoverflow.com/questions/216510/extern-inline
For dual C/C++ code, use a macro to dispatch between these. For C++-only
code, stop using static inline and just use plain inline.
Update-Note: If you see weird C++ compile or link failures in header
functions, this change is probably to blame. Though this change
doesn't affect C and non-static inline is extremely common in C++,
so I would expect this to be fine.
Change-Id: Ibb0bf8ff57143fc14e10342854e467f85a5e4a82
Reviewed-on: https://boringssl-review.googlesource.com/32116
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/base.h b/include/openssl/base.h
index 10f6e41..5a4bb66 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -246,6 +246,35 @@
#define OPENSSL_UNUSED
#endif
+// C and C++ handle inline functions differently. In C++, an inline function is
+// defined in just the header file, potentially emitted in multiple compilation
+// units (in cases the compiler did not inline), but each copy must be identical
+// to satsify ODR. In C, a non-static inline must be manually emitted in exactly
+// one compilation unit with a separate extern inline declaration.
+//
+// In both languages, exported inline functions referencing file-local symbols
+// are problematic. C forbids this altogether (though GCC and Clang seem not to
+// enforce it). It works in C++, but ODR requires the definitions be identical,
+// including all names in the definitions resolving to the "same entity". In
+// practice, this is unlikely to be a problem, but an inline function that
+// returns a pointer to a file-local symbol
+// could compile oddly.
+//
+// Historically, we used static inline in headers. However, to satisfy ODR, use
+// plain inline in C++, to allow inline consumer functions to call our header
+// functions. Plain inline would also work better with C99 inline, but that is
+// not used much in practice, extern inline is tedious, and there are conflicts
+// with the old gnu89 model:
+// https://stackoverflow.com/questions/216510/extern-inline
+#if defined(__cplusplus)
+#define OPENSSL_INLINE inline
+#else
+// Add OPENSSL_UNUSED so that, should an inline function be emitted via macro
+// (e.g. a |STACK_OF(T)| implementation) in a source file without tripping
+// clang's -Wunused-function.
+#define OPENSSL_INLINE static inline OPENSSL_UNUSED
+#endif
+
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE) && \
!defined(BORINGSSL_UNSAFE_DETERMINISTIC_MODE)
#define BORINGSSL_UNSAFE_DETERMINISTIC_MODE
@@ -506,16 +535,16 @@
template <typename T>
using UniquePtr = std::unique_ptr<T, internal::Deleter<T>>;
-#define BORINGSSL_MAKE_UP_REF(type, up_ref_func) \
- static inline UniquePtr<type> UpRef(type *v) { \
- if (v != nullptr) { \
- up_ref_func(v); \
- } \
- return UniquePtr<type>(v); \
- } \
- \
- static inline UniquePtr<type> UpRef(const UniquePtr<type> &ptr) { \
- return UpRef(ptr.get()); \
+#define BORINGSSL_MAKE_UP_REF(type, up_ref_func) \
+ inline UniquePtr<type> UpRef(type *v) { \
+ if (v != nullptr) { \
+ up_ref_func(v); \
+ } \
+ return UniquePtr<type>(v); \
+ } \
+ \
+ inline UniquePtr<type> UpRef(const UniquePtr<type> &ptr) { \
+ return UpRef(ptr.get()); \
}
BSSL_NAMESPACE_END
diff --git a/include/openssl/cpu.h b/include/openssl/cpu.h
index bb847f9..b2759fe 100644
--- a/include/openssl/cpu.h
+++ b/include/openssl/cpu.h
@@ -96,7 +96,7 @@
#if defined(BORINGSSL_FIPS)
const uint32_t *OPENSSL_ia32cap_get(void);
#else
-static inline const uint32_t *OPENSSL_ia32cap_get(void) {
+OPENSSL_INLINE const uint32_t *OPENSSL_ia32cap_get(void) {
return OPENSSL_ia32cap_P;
}
#endif
@@ -119,7 +119,7 @@
// CRYPTO_is_NEON_capable returns true if the current CPU has a NEON unit. If
// this is known statically then it returns one immediately.
-static inline int CRYPTO_is_NEON_capable(void) {
+OPENSSL_INLINE int CRYPTO_is_NEON_capable(void) {
// Only statically skip the runtime lookup on aarch64. On arm, one CPU is
// known to have a broken NEON unit which is known to fail with on some
// hand-written NEON assembly. For now, continue to apply the workaround even
@@ -152,7 +152,7 @@
#else
-static inline int CRYPTO_is_NEON_capable(void) {
+OPENSSL_INLINE int CRYPTO_is_NEON_capable(void) {
#if defined(OPENSSL_STATIC_ARMCAP_NEON) || defined(__ARM_NEON__)
return 1;
#else
@@ -160,7 +160,7 @@
#endif
}
-static inline int CRYPTO_is_ARMv8_AES_capable(void) {
+OPENSSL_INLINE int CRYPTO_is_ARMv8_AES_capable(void) {
#if defined(OPENSSL_STATIC_ARMCAP_AES) || defined(__ARM_FEATURE_CRYPTO)
return 1;
#else
@@ -168,7 +168,7 @@
#endif
}
-static inline int CRYPTO_is_ARMv8_PMULL_capable(void) {
+OPENSSL_INLINE int CRYPTO_is_ARMv8_PMULL_capable(void) {
#if defined(OPENSSL_STATIC_ARMCAP_PMULL) || defined(__ARM_FEATURE_CRYPTO)
return 1;
#else
diff --git a/include/openssl/stack.h b/include/openssl/stack.h
index 82869cf..7feecee 100644
--- a/include/openssl/stack.h
+++ b/include/openssl/stack.h
@@ -276,10 +276,6 @@
#define BORINGSSL_DEFINE_STACK_TRAITS(name, type, is_const)
#endif
-// Stack functions must be tagged unused to support file-local stack types.
-// Clang's -Wunused-function only allows unused static inline functions if they
-// are defined in a header.
-
#define BORINGSSL_DEFINE_STACK_OF_IMPL(name, ptrtype, constptrtype) \
DECLARE_STACK_OF(name) \
\
@@ -287,117 +283,112 @@
typedef ptrtype (*stack_##name##_copy_func)(ptrtype); \
typedef int (*stack_##name##_cmp_func)(constptrtype *a, constptrtype *b); \
\
- static inline OPENSSL_UNUSED void sk_##name##_call_free_func( \
- stack_free_func free_func, void *ptr) { \
+ OPENSSL_INLINE void sk_##name##_call_free_func(stack_free_func free_func, \
+ void *ptr) { \
((stack_##name##_free_func)free_func)((ptrtype)ptr); \
} \
\
- static inline OPENSSL_UNUSED void *sk_##name##_call_copy_func( \
- stack_copy_func copy_func, void *ptr) { \
+ OPENSSL_INLINE void *sk_##name##_call_copy_func(stack_copy_func copy_func, \
+ void *ptr) { \
return (void *)((stack_##name##_copy_func)copy_func)((ptrtype)ptr); \
} \
\
- static inline OPENSSL_UNUSED int sk_##name##_call_cmp_func( \
+ OPENSSL_INLINE int sk_##name##_call_cmp_func( \
stack_cmp_func cmp_func, const void **a, const void **b) { \
constptrtype a_ptr = (constptrtype)*a; \
constptrtype b_ptr = (constptrtype)*b; \
return ((stack_##name##_cmp_func)cmp_func)(&a_ptr, &b_ptr); \
} \
\
- static inline OPENSSL_UNUSED STACK_OF(name) * \
+ OPENSSL_INLINE STACK_OF(name) * \
sk_##name##_new(stack_##name##_cmp_func comp) { \
return (STACK_OF(name) *)sk_new((stack_cmp_func)comp); \
} \
\
- static inline OPENSSL_UNUSED STACK_OF(name) *sk_##name##_new_null(void) { \
+ OPENSSL_INLINE STACK_OF(name) *sk_##name##_new_null(void) { \
return (STACK_OF(name) *)sk_new_null(); \
} \
\
- static inline OPENSSL_UNUSED size_t sk_##name##_num( \
- const STACK_OF(name) *sk) { \
+ OPENSSL_INLINE size_t sk_##name##_num(const STACK_OF(name) *sk) { \
return sk_num((const _STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED void sk_##name##_zero(STACK_OF(name) *sk) { \
+ OPENSSL_INLINE void sk_##name##_zero(STACK_OF(name) *sk) { \
sk_zero((_STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED ptrtype sk_##name##_value( \
- const STACK_OF(name) *sk, size_t i) { \
+ OPENSSL_INLINE ptrtype sk_##name##_value(const STACK_OF(name) *sk, \
+ size_t i) { \
return (ptrtype)sk_value((const _STACK *)sk, i); \
} \
\
- static inline OPENSSL_UNUSED ptrtype sk_##name##_set(STACK_OF(name) *sk, \
- size_t i, ptrtype p) { \
+ OPENSSL_INLINE ptrtype sk_##name##_set(STACK_OF(name) *sk, size_t i, \
+ ptrtype p) { \
return (ptrtype)sk_set((_STACK *)sk, i, (void *)p); \
} \
\
- static inline OPENSSL_UNUSED void sk_##name##_free(STACK_OF(name) *sk) { \
+ OPENSSL_INLINE void sk_##name##_free(STACK_OF(name) * sk) { \
sk_free((_STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED void sk_##name##_pop_free( \
- STACK_OF(name) *sk, stack_##name##_free_func free_func) { \
+ OPENSSL_INLINE void sk_##name##_pop_free( \
+ STACK_OF(name) * sk, stack_##name##_free_func free_func) { \
sk_pop_free_ex((_STACK *)sk, sk_##name##_call_free_func, \
(stack_free_func)free_func); \
} \
\
- static inline OPENSSL_UNUSED size_t sk_##name##_insert( \
- STACK_OF(name) *sk, ptrtype p, size_t where) { \
+ OPENSSL_INLINE size_t sk_##name##_insert(STACK_OF(name) *sk, ptrtype p, \
+ size_t where) { \
return sk_insert((_STACK *)sk, (void *)p, where); \
} \
\
- static inline OPENSSL_UNUSED ptrtype sk_##name##_delete(STACK_OF(name) *sk, \
- size_t where) { \
+ OPENSSL_INLINE ptrtype sk_##name##_delete(STACK_OF(name) *sk, \
+ size_t where) { \
return (ptrtype)sk_delete((_STACK *)sk, where); \
} \
\
- static inline OPENSSL_UNUSED ptrtype sk_##name##_delete_ptr( \
- STACK_OF(name) *sk, constptrtype p) { \
+ OPENSSL_INLINE ptrtype sk_##name##_delete_ptr(STACK_OF(name) *sk, \
+ constptrtype p) { \
return (ptrtype)sk_delete_ptr((_STACK *)sk, (const void *)p); \
} \
\
- static inline OPENSSL_UNUSED int sk_##name##_find( \
- const STACK_OF(name) *sk, size_t *out_index, constptrtype p) { \
+ OPENSSL_INLINE int sk_##name##_find(const STACK_OF(name) *sk, \
+ size_t * out_index, constptrtype p) { \
return sk_find((const _STACK *)sk, out_index, (const void *)p, \
sk_##name##_call_cmp_func); \
} \
\
- static inline OPENSSL_UNUSED ptrtype sk_##name##_shift(STACK_OF(name) *sk) { \
+ OPENSSL_INLINE ptrtype sk_##name##_shift(STACK_OF(name) *sk) { \
return (ptrtype)sk_shift((_STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED size_t sk_##name##_push(STACK_OF(name) *sk, \
- ptrtype p) { \
+ OPENSSL_INLINE size_t sk_##name##_push(STACK_OF(name) *sk, ptrtype p) { \
return sk_push((_STACK *)sk, (void *)p); \
} \
\
- static inline OPENSSL_UNUSED ptrtype sk_##name##_pop(STACK_OF(name) *sk) { \
+ OPENSSL_INLINE ptrtype sk_##name##_pop(STACK_OF(name) *sk) { \
return (ptrtype)sk_pop((_STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED STACK_OF(name) * \
- sk_##name##_dup(const STACK_OF(name) *sk) { \
+ OPENSSL_INLINE STACK_OF(name) * sk_##name##_dup(const STACK_OF(name) *sk) { \
return (STACK_OF(name) *)sk_dup((const _STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED void sk_##name##_sort(STACK_OF(name) *sk) { \
+ OPENSSL_INLINE void sk_##name##_sort(STACK_OF(name) *sk) { \
sk_sort((_STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED int sk_##name##_is_sorted( \
- const STACK_OF(name) *sk) { \
+ OPENSSL_INLINE int sk_##name##_is_sorted(const STACK_OF(name) *sk) { \
return sk_is_sorted((const _STACK *)sk); \
} \
\
- static inline OPENSSL_UNUSED stack_##name##_cmp_func \
- sk_##name##_set_cmp_func(STACK_OF(name) *sk, \
- stack_##name##_cmp_func comp) { \
+ OPENSSL_INLINE stack_##name##_cmp_func sk_##name##_set_cmp_func( \
+ STACK_OF(name) *sk, stack_##name##_cmp_func comp) { \
return (stack_##name##_cmp_func)sk_set_cmp_func((_STACK *)sk, \
(stack_cmp_func)comp); \
} \
\
- static inline OPENSSL_UNUSED STACK_OF(name) * \
+ OPENSSL_INLINE STACK_OF(name) * \
sk_##name##_deep_copy(const STACK_OF(name) *sk, \
ptrtype(*copy_func)(ptrtype), \
void (*free_func)(ptrtype)) { \
@@ -519,7 +510,7 @@
// PushToStack pushes |elem| to |sk|. It returns true on success and false on
// allocation failure.
template <typename Stack>
-static inline
+inline
typename std::enable_if<!internal::StackTraits<Stack>::kIsConst, bool>::type
PushToStack(Stack *sk,
UniquePtr<typename internal::StackTraits<Stack>::Type> elem) {
@@ -535,12 +526,12 @@
// Define begin() and end() for stack types so C++ range for loops work.
template <typename Stack>
-static inline bssl::internal::StackIterator<Stack> begin(const Stack *sk) {
+inline bssl::internal::StackIterator<Stack> begin(const Stack *sk) {
return bssl::internal::StackIterator<Stack>(sk, 0);
}
template <typename Stack>
-static inline bssl::internal::StackIterator<Stack> end(const Stack *sk) {
+inline bssl::internal::StackIterator<Stack> end(const Stack *sk) {
return bssl::internal::StackIterator<Stack>(
sk, sk_num(reinterpret_cast<const _STACK *>(sk)));
}
