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

[Evgeniy Temirgaleev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 6855 bytes --]

Hi, Sergey! Thanks for review!

> 
> From: Sergey Kaplun <skaplun@tarantool.org>
> To: Evgeniy Temirgaleev <e.temirgaleev@tarantool.org>
> Cc: Sergey Bronnikov <sergeyb@tarantool.org>, tarantool-patches@dev.tarantool.org
> 
> Date: Tuesday, August 4, 2026 3:56 PM +03:00
> 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?
> 

None. The reason to add this action is the saving the logic of similar ‘setup-sanitizer-linux’ action, which is defines compiler for next steps.

But I rewrite this action completely to allow compiler selection. I shall present it in the patch v3.

> 
> 
> > +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.
> 

Thanks for the notice. It’s done in the patch v3. The experience showed that the compiler version may have significant influence on the succession of ASAN building and testing.

> 
> 
> > 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?
> 

I tried with include. It doesn’t help to make things better.

> 
> 
> > + 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?
> 

For the builtin macOS compiler (clang 17) this option isn’t supported. I got an ‘not supported for the platform’ error.
But for newest versions of gcc/clang it works. I set this option to ‘on’ in the patch v3.

> 
> 
> > + 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
> 

--
Best regards,
Evgeniy Temirgaleev

[-- Attachment #2: Type: text/html, Size: 9460 bytes --]