htscodecs icon indicating copy to clipboard operation
htscodecs copied to clipboard

Undefined symbols ___cpuid_count ___get_cpuid_max building on Mac OS X 10.7

Open ryandesign opened this issue 1 year ago • 8 comments

htslib 1.20 fails to build on Mac OS X 10.7.5 with the version of Apple's fork of the clang compiler (Apple LLVM version 4.2 (clang-425.0.28) (based on LLVM 3.2svn)) that comes with Xcode 4.6.3 (the last version compatible with Mac OS X 10.7) due to:

Undefined symbols for architecture x86_64:
  "___cpuid_count", referenced from:
      _htscodecs_tls_cpu_init in rANS_static4x16pr.o
  "___get_cpuid_max", referenced from:
      _htscodecs_tls_cpu_init in rANS_static4x16pr.o
ld: symbol(s) not found for architecture x86_64

As far as I know, versions of Apple clang less than 503 have this deficiency. Using a newer version of clang is a workaround—either a newer version of Apple clang on newer versions of macOS, or a newer version of llvm.org clang installed manually.

ryandesign avatar Apr 19 '24 19:04 ryandesign

Odd that the toolchain apparently declares them but does not provide them.

Presumably you have encountered this in your capacity as a MacPorts maintainer rather than because you are actually using this ancient platform version and ancient Xcode version. I see you have already worked around this in macports/macports-ports@319a368e862501dfd75ab8c75bb0b65422f8499d.

Can you suggest a preprocessor check to detect Apple clang being less than 503? Perhaps there would be values for __APPLE_CC__ or __apple_build_version__ that could be compared against in a suitably placed #if?

Alternatively if someone really cared this could be probed in configure.ac.

jmarshall avatar Apr 19 '24 21:04 jmarshall

Presumably you have encountered this in your capacity as a MacPorts maintainer

That's right, I just noticed the failure on our automated build system and wanted to do something about it.

I see you have already worked around this in macports/macports-ports@319a368.

Right, I just copied that from another port where a similar problem existed.

Can you suggest a preprocessor check to detect Apple clang being less than 503? Perhaps there would be values for __APPLE_CC__ or __apple_build_version__ that could be compared against in a suitably placed #if?

I don't know the exact version number where cpuid started working. I know Apple clang 503 works, 425 doesn't work, and I haven't tried versions in between.

The value of __APPLE_CC__ isn't used much anymore because it is 6000 in Apple clang 503 and has been that way ever since, even today in Apple clang 1500. In Apple clang 425 its value was 5621.

Most projects that need to inspect the Apple clang version number do it with __apple_build_version__ and if you know the Apple clang version number you can approximately guess what its __apple_build_version__ will be. Apple clang 425's __apple_build_version__ is 4250028 and 503's is 5030040. So if you check if __apple_build_version__ is defined and its value is less than 5030040 then you know that the cpuid stuff doesn't work and you can either use an alternative or print an error.

Alternatively if someone really cared this could be probed in configure.ac.

In my view, testing for the functionality is preferable to checking compiler version numbers, but I'm not skilled at writing configure.ac tests.

ryandesign avatar Apr 19 '24 22:04 ryandesign

testing for the functionality is preferable to checking compiler version numbers

I meant to add that another reason for this is that the problem might not relate to the compiler version but to the OS version. Xcode 4.6.3 was compatible with OS X 10.7 and 10.8 and it contains both the OS X 10.7 and 10.8 SDKs. It could be that this clang is using the 10.8 SDK by default, even when building on 10.7, and that this function exists in the 10.8 SDK but not in the 10.7 SDK. This is a common source of friction between the way that Apple's SDKs work and open source projects not originally designed for macOS not expecting this behavior. This is why testing for functionality is better.

ryandesign avatar Apr 19 '24 22:04 ryandesign

Certainly it's better to probe in general. But the real question is how much work is worth doing for the benefit of an elderly Xcode released in 2013.

I still use a 2012-era Mac mini, but — like I suspect most other users of such vintage machines — it's running a more recent macOS (10.15) and similar Xcode so wouldn't exhibit this issue. So it may be that the entire audience for this issue has already worked around it in their port file… :smile:

jmarshall avatar Apr 19 '24 22:04 jmarshall

the problem might not relate to the compiler version but to the OS version

With a little grepping around the SDKs I think I have assured myself that's not the case. __get_cpuid_max is an inline function and __cpuid_count is a macro, both in a clang header, so it is indeed tied to the compiler version.

Odd that the toolchain apparently declares them but does not provide them.

Oh, and it doesn't declare them. Earlier in the log:

htscodecs/htscodecs/rANS_static4x16pr.c:863:17: warning: implicit declaration of function '__get_cpuid_max' is invalid in C99 [-Wimplicit-function-declaration]
    int level = __get_cpuid_max(0, NULL);
                ^
htscodecs/htscodecs/rANS_static4x16pr.c:864:5: warning: implicit declaration of function '__cpuid_count' is invalid in C99 [-Wimplicit-function-declaration]
    __cpuid_count(0, 0, eax, ebx, ecx, edx);
    ^

Here's the full log on 10.7 and for comparison the log on 10.8.

ryandesign avatar Apr 19 '24 23:04 ryandesign

In addition to assuming that cpuid.h declares __get_cpuid_max and __cpuid_count, you're assuming that all versions of gcc and clang on x86_64 provide cpuid.h:

https://github.com/samtools/htscodecs/blob/ffda7310c4b3292955561d6c3b1743cb82bfe26b/htscodecs/rANS_static4x16pr.c#L827-L829

Is that a valid assumption?

ryandesign avatar Apr 19 '24 23:04 ryandesign

In addition to assuming that cpuid.h declares __get_cpuid_max and __cpuid_count, you're assuming that all versions of gcc and clang on x86_64 provide cpuid.h:

https://github.com/samtools/htscodecs/blob/ffda7310c4b3292955561d6c3b1743cb82bfe26b/htscodecs/rANS_static4x16pr.c#L827-L829

Is that a valid assumption?

Possibly not, but generally it's not worth trying to tie it down to specific compilers until we find genuine cases of users failing to compile. As you've already noted, figuring out which explicit compiler version to validate on is hard, complicated further by vendors changing a standard compiler to have their own version numbers. Hence it's less error prone to not add checks until we know they're really required.

Edit: That said, an AC_TRY_LINK on cpuid count/max calls would be a more robust way of detecting compiler support irrespective of bizarreness of apple versioning.

jkbonfield avatar Apr 22 '24 07:04 jkbonfield

Can you please test if #116 fixes this? I gave it a punt given it simplifies the actual code checks a little and helps make them more robust.

Of course, this doesn't really help htslib, so I may need some additional way of handling it there or add an additional AC_DEFINE(CPUID_CHECKED,...) so the code can rely on HAVE_CPUID when we checked and fall back to a less robust method when not (as htslib doesn't build htscodecs as a library and instead uses its own Makefile for building this code). That's a trickier problem, but I'll punt that for now! (Eg: Htslib could punt the existing ifdefs to config.h/Makefile so it builds without configure)

jkbonfield avatar Apr 22 '24 08:04 jkbonfield