Skip to content

Assertion failure in test_capi test_alignof_max_align_t possible with macOS universal2 (multi-arch) builds #110820

Closed

Description

@ned-deily

macOS supports multi-architecture binaries (fat binaries) and CPython have long supported building them. Current releases of macOS support two system architectures: legacy x86_64 (for Intel Macs) and arm64 (for Apple Silicon Macs). The CPython ./configure supports building fat binaries containing both archs by use of the --with-universal-archs=universal2 option. To do so, it makes use of the muilt-arch support built into the Apple-supplied build tools (Xcode or Command Line Tools) which allows multi-arch compiles and links to happen via single calls (by adding --arch flags to the calls to compilers et al).

However, because configure's autoconf tests are only run in the architecture of the build machine, different results can be obtained depending on whether the universal2 build machine is an Intel or an Apple Silicon Mac. In particular, there are differences in the autoconf generated pyconfig.h as shown here with modified file names for clarity; in either case, there is only one pyconfig.h file that will be generated and used regardless of which arch the fat binaries are run on:

$ diff pyconfig_intel.h pyconfig_arm.h
24c24
< #define ALIGNOF_MAX_ALIGN_T 16
---
> #define ALIGNOF_MAX_ALIGN_T 8
439c439
< #define HAVE_GCC_ASM_FOR_X64 1
---
> /* #undef HAVE_GCC_ASM_FOR_X64 */
443c443
< #define HAVE_GCC_ASM_FOR_X87 1
---
> /* #undef HAVE_GCC_ASM_FOR_X87 */
1607c1607
< #define PY_SUPPORT_TIER 1
---
> #define PY_SUPPORT_TIER 2
1656c1656
< #define SIZEOF_LONG_DOUBLE 16
---
> #define SIZEOF_LONG_DOUBLE 8

The difference in ALIGNOF_MAX_ALIGN_T between the two archs leads to the following test failure when a universal2 binary is built on an Apple Silicon Mac but executed on an Intel Mac:

test_alignof_max_align_t (test.test_capi.test_misc.Test_testcapi.test_alignof_max_align_t) ... Assertion failed: (ALIGNOF_MAX_ALIGN_T >= _Alignof(long double)), function test_alignof_max_align_t, file heaptype_relative.c, line 309.
Fatal Python error: Aborted

The test does not crash in the opposite case: when built on an Intel Mac but run on an Apple Silicon Mac, because in this case 16 > 8.

This issue was noticed while testing changes to the process we use to build python.org python for macOS installers. Up to now, we have been using Intel Macs to build all installers but, when testing building installers on Apple Silicon Macs, the crash of test_capi was seen when running the resultant universal2 installer on Intel Macs.

It's not clear what is the best way to resolve this and, more importantly, whether there are other kinds of similar problems not yet discovered. Probably the safest approach would be to build each architecture separately on their native archs and lipo the resultant single-arch binaries into fat files. And/or it may be better to provide separate copies of arch-sensitive header files like pyconfig.h in separate directories and have the interpreter point to the correct one at run time for extension module builds. Or, in this case, perhaps adding some some conditional code to force the sizes to be the larger value in the case of macOS universal2 builds might be enough this time?

Linked PRs

Activity

  1. added
    buildThe build process and cross-build
    3.12only security fixes
    3.13only security fixes
    on Oct 13, 2023
  2. ronaldoussoren commented on Oct 14, 2023

    @ronaldoussoren
    Contributor

    The easiest fix here is to do something similar as was done for other CPU specific defines on macOS: use pymacconfig.h to hardcode the expected values.

  3. added a commit that references this issue on Dec 7, 2023
  4. ronaldoussoren commented on Dec 7, 2023

    @ronaldoussoren
    Contributor

    The attached PR synchronises the various macros with reality through pymacconfig.h, except for PY_SUPPORT_TIER.

    PY_SUPPORT_TIER could also be fixed, but through _osx_support or sysconfig as the macro is only used in the latter module. That would IMHO require a separate PR.

  5. ned-deily commented on Dec 7, 2023

    @ned-deily
    MemberAuthor

    PY_SUPPORT_TIER could also be fixed, but through _osx_support or sysconfig as the macro is only used in the latter module. That would IMHO require a separate PR.

    I wouldn't worry about PY_SUPPORT_TIER in the edge case of universal2 builds as we intend to officially move Apple Silicon support to tier 1 as soon as CI test resources are freely available (i.e. via GitHub Actions).

  6. added 2 commits that reference this issue on Dec 8, 2023
  7. ronaldoussoren commented on Dec 8, 2023

    @ronaldoussoren
    Contributor

    3.12 backport PR is #112864.

    This is a manual back port based on work by cherry_pick (but with significant manual work because I managed to confuse that tool somehow)

  8. added a commit that references this issue on Dec 9, 2023
  9. ronaldoussoren commented on Dec 9, 2023

    @ronaldoussoren
    Contributor

    As mentioned in #112828 it is my opinion that backporting to 3.11 is not useful, I'm therefore closing this issue. Please reopen if there is a reason to do a back port to 3.11.

    I propose to skip the back port to 3.11 because the impact there is likely lower:

    • ALIGNOF_MAX_ALIGN_T isn't present in 3.11( added for a feature in 3.12)
    • SIZEOF_LONG_DOUBLE is present, but isn't used by CPython itself
    • HAVE_GCC_ASM_FOR_X64 is present, but isn't used by CPython itself
    • HAVE_GCC_ASM_FOR_X87 is present and used, but X87_DOUBLE_ROUNDING is not set

    For the latter: I checked with the configure check for x87 double rounding that double rounding isn't detected when compiling for x86_64 using clang -arch x86_64.

    cpython/configure.ac

    Lines 5469 to 5492 in 1eac0aa

    AC_CACHE_CHECK([for x87-style double rounding], [ac_cv_x87_double_rounding], [
    # $BASECFLAGS may affect the result
    ac_save_cc="$CC"
    CC="$CC $BASECFLAGS"
    AC_RUN_IFELSE([AC_LANG_SOURCE([[
    #include <stdlib.h>
    #include <math.h>
    int main(void) {
    volatile double x, y, z;
    /* 1./(1-2**-53) -> 1+2**-52 (correct), 1.0 (double rounding) */
    x = 0.99999999999999989; /* 1-2**-53 */
    y = 1./x;
    if (y != 1.)
    exit(0);
    /* 1e16+2.99999 -> 1e16+2. (correct), 1e16+4. (double rounding) */
    x = 1e16;
    y = 2.99999;
    z = x + y;
    if (z != 1e16+4.)
    exit(0);
    /* both tests show evidence of double rounding */
    exit(1);
    }
    ]])],

  10. added a commit that references this issue on Feb 11, 2024
  11. added a commit that references this issue on Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.12only security fixes3.13only security fixesOS-macbuildThe build process and cross-build

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions