_FoundationUnicode: apply symbol renaming for non-Darwin targets

[compnerd]

When building on non-Darwin targets, we are not guaranteed that there is no ICU library on the system (Windows ships an ICU since Windows 1703 (RedStone 2), SDK 10.0.15063. This now is implicitly pulled in OneCore.lib which vends the memory API contract; Linux normally ships a copy of ICU; android ships a copy of ICU since Android 6.0 (API Level 23) but is not part of the supported APIs). The availability of ICU on the system and in Swift causes a conflict. As there is no stable ABI for ICU, this is a problem and can cause spurious failures. Such a failure has been observed on Windows when statically linking Foundation.

Unfortunately, the symbol renaming support in ICU relies on the U_ICU_ENTRY_POINT_RENAME macro which is defined thusly:

#define U_DEF_ICU_ENTRY_POINT_RENAME(x, y, z) x ## y ## z
#define U_DEF2_ICU_ENTRY_POINT_RENAME(x, y, z) U_DEF_ICU_ENTRY_POINT_RENAME(x, y, z)
#define U_ICU_ENTRY_POINT_RENAME(name) U_DEF2_ICU_ENTRY_POINT_RENAME(name, ICU_VERSION_SUFFIX, U_LIB_SUFFIX_C_NAME)

This macro is too complex for the clang importer to import into Swift, making this not possible to use directly. Instead, we manually perform the expansion of the CPP macros, using the swift_ prefix instead to isolate and namespace our implementation.

Unfortunately, even the aliasing macro is too complex for the clang importer to import directly. Because changing all the call sites would not be tractable (nor maintainable), this will require an additional change (swiftlang/swift#81840) to support aliasing macros to be imported into Swift.

Although in theory only the dynamically linked version should be susceptible to the interpositioning issue, if any of the system libraries causes ICU to be included (e.g. a linker script on Unix or an import library pulling in another version), we can also hit this issue in the statically linked scenario. When dynamically linking, on Windows at least, it would not be a problem if the symbol is bound properly at link time as the symbolic resolution involves two-level namespacing, which is not available on ELF hosts.

On Darwin, we cannot perform the renaming as the library is vended by the system and this would constitute an ABI break.

[compnerd]

swiftlang/swift-foundation#1340 is the associated change for Foundation to make some of the implicit nullability handling explicit.

[finagolfin]

I'm skeptical this would hit anywhere other than Windows, as you note that we are statically linking this ICU into Foundation, so if we are careful with the CMake config could just use the current config without a problem.

However, I don't build for Windows and CMake can certainly be finicky, so up to you all.

[compnerd]

I'm not sure how you can be certain that nothing else in the address space can dlopen or link against the ICU system library. If that is loaded and you are dynamically linking ICU, that would cause an issue. However, the other described scenario (linker script replacement adding in ICU) is also possible. I don't see any of this being a CMake issue, but rather just how systems work.

[finagolfin]

So the fear is that some Swift app also dynamically links against the system libicu at runtime? It seemed you were talking more about build-time issues, eg the linker script, hence my reference to CMake.

I don't know how often something like that happens. I just checked and it looks like Android started making icu4c available since API 31 a couple years ago, so it may be possible there.

[parkera]

So, this library is actually used for building and runtime for the FoundationInternationalization package on Darwin. The system ICU is used when building FOUNDATION_FRAMEWORK, and is not the same as this repository. This repository is exported from that one.

[compnerd]

So, this library is actually used for building and runtime for the FoundationInternationalization package on Darwin. The system ICU is used when building FOUNDATION_FRAMEWORK, and is not the same as this repository. This repository is exported from that one.

Right, and the change accounts for that by disabling the renaming in FOUNDATION_FRAMEWORK mode. The system ICU is only a concern on non-Darwin platforms (which is what the previous conversation was about). The symbols being exported collide with the system ones as neither are namespaced. This change allows namespacing on non-Darwin platforms to avoid that collision.

[parkera]

@iCharlesHu can you take a look?

[iCharlesHu]

@compnerd thanks for the change and it makes sense to me. Could you elaborate on the steps you took to generate urename.h? This step will have to be done every time we upgrade ICU, right?

[compnerd]

Could you elaborate on the steps you took to generate urename.h? This step will have to be done every time we upgrade ICU, right?

Sadly it was rather "dumb". I simply duplicated the block beneath and did a replacement for ICU_ENTRY_POINT_RENAME(...) to swift_.... This will need to be done on the updates. Sadly improving the ClangImporter to work with the function like macros is a much larger hurdle and I didn't really see a straightforward way to handle that.

Basically, in vim, this amounts to: :'<'>s/U_ICU_ENTRY_POINT_RENAME(\(.*\))/swift_\1/

[compnerd]

Cross repo tested at https://ci-external.swift.org/job/swift-PR-windows/42872/