Re: [Tarantool-patches] [PATCH luajit v2 2/3] ci: enabled sanitizer tests for macOS

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Evgeniy!
Thanks for the patch!
Please, consider my questions below.

On 31.07.26, Evgeniy Temirgaleev wrote:
> From: Temir Galeev <temir.galeev@bk.ru>
> 
> The arm64 and x86_64 architectures with clang compiler were added
> to the matrix.
> ---
>  .../README.md                                 |  0
>  .../action.yml                                |  0
>  .../actions/setup-sanitizers-macos/README.md  | 12 ++++
>  .../actions/setup-sanitizers-macos/action.yml | 18 +++++
>  .github/workflows/sanitizers-testing.yml      | 65 ++++++++++++++++---
>  5 files changed, 86 insertions(+), 9 deletions(-)
>  rename .github/actions/{setup-sanitizers => setup-sanitizers-linux}/README.md (100%)
>  rename .github/actions/{setup-sanitizers => setup-sanitizers-linux}/action.yml (100%)
>  create mode 100644 .github/actions/setup-sanitizers-macos/README.md
>  create mode 100644 .github/actions/setup-sanitizers-macos/action.yml
> 
> diff --git a/.github/actions/setup-sanitizers/README.md b/.github/actions/setup-sanitizers-linux/README.md
> similarity index 100%
> rename from .github/actions/setup-sanitizers/README.md
> rename to .github/actions/setup-sanitizers-linux/README.md
> diff --git a/.github/actions/setup-sanitizers/action.yml b/.github/actions/setup-sanitizers-linux/action.yml
> similarity index 100%
> rename from .github/actions/setup-sanitizers/action.yml
> rename to .github/actions/setup-sanitizers-linux/action.yml
> diff --git a/.github/actions/setup-sanitizers-macos/README.md b/.github/actions/setup-sanitizers-macos/README.md
> new file mode 100644
> index 00000000..7061c042
> --- /dev/null
> +++ b/.github/actions/setup-sanitizers-macos/README.md

<snipped>

> diff --git a/.github/actions/setup-sanitizers-macos/action.yml b/.github/actions/setup-sanitizers-macos/action.yml
> new file mode 100644
> index 00000000..441285f9
> --- /dev/null
> +++ b/.github/actions/setup-sanitizers-macos/action.yml
> @@ -0,0 +1,18 @@
> +name: Setup CI environment for testing with sanitizers on macOS
> +description: Common part to tweak macOS CI runner environment for sanitizers

For macOS this looks weird, since we just use the default compiler anyway.
Is it possible to specify compiler version on macOS?

> +inputs:
> +  cc_name:
> +    description: C compiler name (for example, gcc-12)
> +    required: false
> +    default: clang
> +runs:
> +  using: composite
> +  steps:
> +    - name: Setup CI environment on macOS
> +      uses: ./.github/actions/setup-macos
> +    - name: Set specific C compiler as a default toolchain
> +      run: |
> +        echo CC=${CC_NAME} | tee -a $GITHUB_ENV
> +      shell: bash
> +      env:
> +        CC_NAME: ${{ inputs.cc_name }}
> diff --git a/.github/workflows/sanitizers-testing.yml b/.github/workflows/sanitizers-testing.yml
> index 4bf7d023..aa9eae16 100644
> --- a/.github/workflows/sanitizers-testing.yml
> +++ b/.github/workflows/sanitizers-testing.yml
> @@ -31,17 +31,33 @@ jobs:
>      strategy:
>        fail-fast: false
>        matrix:
> -        # XXX: Let's start with only Linux/x86_64
> +        ARCH: [ARM64, x86_64]

Side note: For local x86_64 macOS, I can't build LuaJIT with enabled
since ASAN just can't be initialized. Hence it leads to buildvm
crashes. I suppose there is some issue with modern clang. So, the
version specification is desirable.

>          BUILDTYPE: [Debug, Release]
> -        CC: [gcc-10, clang-11]
> +        OS: [Linux, macOS]
> +        # Versioned compilers are for Linux.
> +        CC: [gcc-10, clang-11, clang]
>          include:
>            - BUILDTYPE: Debug
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>            - BUILDTYPE: Release
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    runs-on: [self-hosted, regular, Linux, x86_64]
> +        exclude:
> +          - OS: macOS
> +            CC: gcc-10
> +          - OS: macOS
> +            CC: clang-11
> +          - OS: Linux
> +            CC: clang
> +          # On current runners with Linux/ARM64 environment and
> +          # with LUAJIT_USE_SYSMALLOC=ON the system allocator returns addresses
> +          # with 48-bit set. Thus checkptrGC() fails with new Lua state pointer
> +          # and luajit fails to start with 'cannot create state: not enough memory'
> +          # error. So, we exclude this case.
> +          - ARCH: ARM64
> +            OS: Linux

These excludes looks too drammatic. Have you considering include option instead?
Does it look better/more compact?

> +    runs-on: [self-hosted, regular, '${{ matrix.OS }}', '${{ matrix.ARCH }}']
>      name: >
> -      LuaJIT with ASan and UBSan (Linux/x86_64)
> +      LuaJIT with ASan and UBSan (${{ matrix.OS }}/${{ matrix.ARCH }})
>        ${{ matrix.BUILDTYPE }}
>        CC:${{ matrix.CC }}
>        GC64:ON SYSMALLOC:ON

<snipped>

>            ASAN_OPTIONS: "                    \
>              detect_invalid_pointer_pairs=1:  \
>              detect_leaks=1:                  \
> @@ -95,5 +119,28 @@ jobs:
>            UBSAN_OPTIONS: "
>              print_stacktrace=1 \
>            "
> +        run: |
> +          echo ASAN_OPTIONS=${ASAN_OPTIONS} | tee -a $GITHUB_ENV
> +          echo UBSAN_OPTIONS=${UBSAN_OPTIONS} | tee -a $GITHUB_ENV
> +      - name: setup sanitizer options for macOS
> +        if: ${{ matrix.OS == 'macOS' }}
> +        env:
> +          ASAN_OPTIONS: "                    \
> +            detect_invalid_pointer_pairs=1:  \
> +            detect_leaks=0:                  \

Why do we need to exclude detect_leaks for macOS?

> +            detect_stack_use_after_return=1: \
> +            dump_instruction_bytes=1:        \
> +            heap_profile=0:                  \
> +            print_suppressions=0:            \
> +            symbolize=1:                     \
> +            unmap_shadow_on_exit=1:          \
> +          "
> +          UBSAN_OPTIONS: "
> +            print_stacktrace=1 \
> +          "
> +        run: |
> +          echo ASAN_OPTIONS=${ASAN_OPTIONS} | tee -a $GITHUB_ENV
> +          echo UBSAN_OPTIONS=${UBSAN_OPTIONS} | tee -a $GITHUB_ENV
> +      - name: test
>          run: cmake --build . --parallel --target LuaJIT-test
>          working-directory: ${{ env.BUILDDIR }}
> -- 
> 2.49.0
> 

-- 
Best regards,
Sergey Kaplun