Re: [Tarantool-patches] [PATCH luajit 2/3] macOS: Workaround for buggy XCode 15.0 - 15.2 linker.

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

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

Hi, Sergey,

Thanks for updates, LGTM

On 11/27/25 12:59, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
>
> See my answers below.
>
> On 26.11.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch! LGTM with a minor comment.
>>
>> Sergey
>>
>> On 11/24/25 23:26, Sergey Kaplun wrote:
>>> From: Mike Pall <mike>
>>>
>>> Thanks to Carlo Cabrera.
>>>
>>> (cherry picked from commit b2915e9ab55b999429b4d1931097064c4e17de53)
>>>
>>> The previous commit removes emitting the `.subsections_via_symbols` [1]
>>> directive for macOS. Nevertheless, the aforementioned Xcode linker
>>> versions produce incorrect code for the VM without this directive.
>>>
>>> As a fix for old versions of macOS, this patch adds emitting this
>> I would add exact macOS versions, according to [1] it is: 15000000
>> 1.
> As discussed offline, this is exactly the problem with the Xcode
> version, and this macro specifies the corresponding Xcode versions.
>
>> https://stackoverflow.com/questions/19387043/how-can-i-reliably-detect-the-version-of-clang-at-preprocessing-time#comment137533353_19391724 
>>
>>> directive in the buildvm.
>>>
>>> [1]:https://reviews.llvm.org/D79926
>>>
>>> Sergey Kaplun:
>>> * added the description for the problem
>>>
>>> Part of tarantool/tarantool#11691
> I've added the link to the machO format as you suggested:
>
> | macOS: Workaround for buggy XCode 15.0 - 15.2 linker.
> |
> | Thanks to Carlo Cabrera.
> |
> | (cherry picked from commit b2915e9ab55b999429b4d1931097064c4e17de53)
> |
> | The previous commit removes emitting the `.subsections_via_symbols`
> | [1][2] directive for macOS. Nevertheless, the aforementioned Xcode linker
> | versions produce incorrect code for the VM without this directive.
> |
> | As a fix for old versions of macOS, this patch adds emitting this
> | directive in the buildvm.
> |
> | [1]:https://reviews.llvm.org/D79926
> | [2]:https://github.com/aidansteele/osx-abi-macho-file-format-reference
> |
> | Sergey Kaplun:
> | * added the description for the problem
> |
> | Part of tarantool/tarantool#11691
>
>>> ---
>>>    src/host/buildvm_asm.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/src/host/buildvm_asm.c b/src/host/buildvm_asm.c
>>> index 43595b31..fc09d71a 100644
>>> --- a/src/host/buildvm_asm.c
>>> +++ b/src/host/buildvm_asm.c
>>> @@ -347,6 +347,10 @@ void emit_asm(BuildCtx *ctx)
>>>        fprintf(ctx->fp, "\t.ident \"%s\"\n", ctx->dasm_ident);
>>>        break;
>>>      case BUILD_machasm:
>>> +#if defined(__apple_build_version__) && __apple_build_version__ >= 15000000 && __apple_build_version__ < 15000300
>>> +    /* Workaround for XCode 15.0 - 15.2. */
>> __apple_build_version__ defines a macOS version. But macOS 15 includes
>> XCode 16, see [2], not 15.x
>>
>> So there is an inconsistency in condition and comment. I would list
>> macOS versions in a comment like this
>>
>> "Workaround for macOS 15, 15.1 and 15.2" to avoid misinterpretation due
>> to non-strict inequality with 15000300.
>>
>> Also, I would include a link to [2] to the commit message.
>>
>> 2.
>> https://developer.apple.com/documentation/macos-release-notes/macos-15-release-notes
> The same about the comment above.
>
>>> +    fprintf(ctx->fp, "\t.subsections_via_symbols\n");
>>> +#endif
>>>        fprintf(ctx->fp,
>>>          "\t.cstring\n"
>>>          "\t.ascii \"%s\\0\"\n", ctx->dasm_ident);

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