_FoundationUnicode: apply symbol renaming for non-Darwin targets #63
Conversation
|
swiftlang/swift-foundation#1340 is the associated change for Foundation to make some of the implicit nullability handling explicit. |
|
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. |
|
I'm not sure how you can be certain that nothing else in the address space can |
|
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. |
|
So, this library is actually used for building and runtime for the FoundationInternationalization package on Darwin. The system ICU is used when building |
Right, and the change accounts for that by disabling the renaming in |
|
@iCharlesHu can you take a look? |
There was a problem hiding this comment.
@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?
Sadly it was rather "dumb". I simply duplicated the block beneath and did a replacement for Basically, in vim, this amounts to: |
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.
|
Cross repo tested at https://ci-external.swift.org/job/swift-PR-windows/42872/ |
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_RENAMEmacro which is defined thusly: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.