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