build: Add missing prefix to BoringSSL delete_if stack symbols

simonjbeaumont commented 4 months ago

Motivation

When building projects that depend on both Swift Crypto and JWTKit on Linux, we hit a symbol clash. Both modules vendor in their own copy of BoringSSL and go to lengths to namespace the symbols, but there were some symbols that were not prefixed, which resulted in compilation failures due to clashing macro expansions, e.g.

...
/pwd/.build/checkouts/swift-crypto/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_x509.h:2964:1: error: 'sk_X509_OBJECT_delete_if' has different definitions in different modules; definition in module 'CCryptoBoringSSL' first difference is function body
DEFINE_STACK_OF(X509_OBJECT)
^
/pwd/.build/checkouts/swift-crypto/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_stack.h:100:31: note: expanded from macro 'DEFINE_STACK_OF'
#define DEFINE_STACK_OF(type) DEFINE_NAMED_STACK_OF(type, type)
                              ^
/pwd/.build/checkouts/swift-crypto/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_stack.h:93:3: note: expanded from macro 'DEFINE_NAMED_STACK_OF'
  BORINGSSL_DEFINE_STACK_OF_IMPL(name, type *, const type *) \
  ^
/pwd/.build/checkouts/swift-crypto/Sources/CCryptoBoringSSL/include/CCryptoBoringSSL_stack.h:486:23: note: expanded from macro 'BORINGSSL_DEFINE_STACK_OF_IMPL'
  OPENSSL_INLINE void sk_##name##_delete_if(                                   \
                      ^
<scratch space>:146:1: note: expanded from here
sk_X509_OBJECT_delete_if
^
/pwd/.build/checkouts/jwt-kit/Sources/CJWTKitBoringSSL/include/CJWTKitBoringSSL_x509.h:4332:1: note: but in 'CJWTKitBoringSSL' found a different body
DEFINE_STACK_OF(X509_OBJECT)
^
/pwd/.build/checkouts/jwt-kit/Sources/CJWTKitBoringSSL/include/CJWTKitBoringSSL_stack.h:100:31: note: expanded from macro 'DEFINE_STACK_OF'
#define DEFINE_STACK_OF(type) DEFINE_NAMED_STACK_OF(type, type)
                              ^
/pwd/.build/checkouts/jwt-kit/Sources/CJWTKitBoringSSL/include/CJWTKitBoringSSL_stack.h:93:3: note: expanded from macro 'DEFINE_NAMED_STACK_OF'
  BORINGSSL_DEFINE_STACK_OF_IMPL(name, type *, const type *) \
  ^
/pwd/.build/checkouts/jwt-kit/Sources/CJWTKitBoringSSL/include/CJWTKitBoringSSL_stack.h:486:23: note: expanded from macro 'BORINGSSL_DEFINE_STACK_OF_IMPL'
  OPENSSL_INLINE void sk_##name##_delete_if(                                   \
                      ^
<scratch space>:110:1: note: expanded from here
sk_X509_OBJECT_delete_if
^
<unknown>:0: error: too many errors emitted, stopping now

This appears to happen for the following symbols:

root@674ddc61f160:/pwd# less -R build.log | grep -a error: | cut -d' ' -f2-3 | sort | uniq -c
     26 error: 'sk_ASN1_INTEGER_delete_if'
     26 error: 'sk_ASN1_OBJECT_delete_if'
     26 error: 'sk_ASN1_TYPE_delete_if'
     26 error: 'sk_ASN1_VALUE_delete_if'
     26 error: 'sk_BIO_delete_if'
     26 error: 'sk_CONF_VALUE_delete_if'
     26 error: 'sk_GENERAL_NAME_delete_if'
     26 error: 'sk_OPENSSL_STRING_delete_if'
     26 error: 'sk_X509_ALGOR_delete_if'
     26 error: 'sk_X509_ATTRIBUTE_delete_if'
     26 error: 'sk_X509_CRL_delete_if'
     26 error: 'sk_X509_EXTENSION_delete_if'
     26 error: 'sk_X509_INFO_delete_if'
     26 error: 'sk_X509_NAME_ENTRY_delete_if'
     26 error: 'sk_X509_NAME_delete_if'
     26 error: 'sk_X509_OBJECT_delete_if'
     26 error: 'sk_X509_REVOKED_delete_if'
     26 error: 'sk_X509_delete_if'
     26 error: 'sk_void_delete_if'

Looks like all the *_delete_if functions generated by the many uses of DEFINE_STACK_OF macro were not prefixed.

Modifications

Update the vendor script and rerun to ensure delete_if stack functions are prefixed.

Result

Can depend on both Swift Crypto and JWTKit in the same project.

Notes

Following this fix we should take the time to see if we can statically audit for non-prefixed symbols.

simonjbeaumont commented 4 months ago

The contents of this file are automatically generated. We need to understand why the most recent run of the vendoring script didn’t catch these.

Because you since approved the PR, I'm assuming you missed the part of the patch that updated the vendor script and are now satisfied that the patch is sound.

Lukasa commented 4 months ago

That assumption is exactly right. I blame the beach being too bright for me to see my phone screen. 🙈

apple / swift-crypto