Clang 16 adds ODR checking for C, just hit an issue with the multiple Bionic timespec definitions

[finagolfin]

Description

I run an Android CI that builds the latest versions of Swift for Android, and I started seeing issues since Swift trunk recently rebranched to use a new LLVM, a bit more info at swiftlang/swift#64321. While this is currently a Swift trunk issue, the behavior comes from LLVM, so it's in the just-released clang 16, which will affect upcoming NDKs.

The issue stems from the two definitions of timespec in Android's Bionic C library: one in linux/time.h that is normally included through time.h and a few other headers, and another in bits/timespec.h that is included in signal.h and two other headers.

Both should be identical but use different typedefs for tv_sec that always resolve to the same type. However, clang 16 and now Swift's ClangImporter have turned on some ODR checking that errors if the typedefs are different in different C modules, even if it knows the underlying types are the same, llvm/llvm-project@160bc16.

This usually makes sense for user libraries, but causes problems like this for the rats nests' of C library headers. If time.h is included first in one module and signal.h in another, Swift's ClangImporter now complains that they are different definitions, even though it accepted them as identical definitions before this commit. If both headers are included in the same module, whichever comes first is the one used, because of the include guards in each header.

I was able to use that latter feature to work around the problem on my Android CI yesterday, by including signal.h before wherever time.h or unistd.h was included, to make sure the same header definition is always used. That required modifying libdispatch when building the Android SDK and headers in llbuild, swift-tools-support-core, and swift-crypto to build those Swift repos without causing an ODR conflict.

This isn't sustainable though, as any random C module may include one or the other and then cause these clashes again. Since Swift just uses C modules, this will hit those using C modules in non-Swift code with future NDKs that contain clang 16 too.

I'm letting the NDK devs know to see what you want to do about this upcoming issue. The easiest fix would be to update Bionic so all these multiple definitions of the same struct use the same typedefs too.

I don't know enough about these C header hell issues, so I'd like the NDK devs to weigh in.

Affected versions

Canary

Canary version

None yet that I tried, just reporting this early for feedback

Host OS

Linux

Host OS version

Ubuntu 22.04

Affected ABIs

armeabi-v7a, arm64-v8a

Build system

Other (specify below)

Other build system

Swift toolchain build and package manager

minSdkVersion

24

Device API level

Build issue, so not relevant

[enh-google]

I don't know enough about these C header hell issues, so I'd like the NDK devs to weigh in.

heh, no worries --- great bug report. you only needed the second paragraph really :-)

fixed by https://android-review.googlesource.com/c/platform/bionic/+/2497375