Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Bronnikov <sergeyb@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 2/3] macOS: Workaround for buggy XCode 15.0 - 15.2 linker.
Date: Thu, 27 Nov 2025 12:59:51 +0300	[thread overview]
Message-ID: <aSghF2fyrhbrtysV@root> (raw)
In-Reply-To: <a706e699-d447-4e2c-82e9-996a201b13b0@tarantool.org>

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);

-- 
Best regards,
Sergey Kaplun

  reply	other threads:[~2025-11-27  9:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-24 20:26 [Tarantool-patches] [PATCH luajit 0/3] Various fixes for macOS Sergey Kaplun via Tarantool-patches
2025-11-24 20:26 ` [Tarantool-patches] [PATCH luajit 1/3] macOS: Fix macOS 15 / Clang 16 build Sergey Kaplun via Tarantool-patches
2025-11-25 13:56   ` Sergey Bronnikov via Tarantool-patches
2025-11-27  9:50     ` Sergey Kaplun via Tarantool-patches
2025-11-27 13:47       ` Sergey Bronnikov via Tarantool-patches
2025-11-24 20:26 ` [Tarantool-patches] [PATCH luajit 2/3] macOS: Workaround for buggy XCode 15.0 - 15.2 linker Sergey Kaplun via Tarantool-patches
2025-11-26 12:09   ` Sergey Bronnikov via Tarantool-patches
2025-11-27  9:59     ` Sergey Kaplun via Tarantool-patches [this message]
2025-11-27 13:55       ` Sergey Bronnikov via Tarantool-patches
2025-11-24 20:26 ` [Tarantool-patches] [PATCH luajit 3/3] macOS: Remove obsolete -single_module flag Sergey Kaplun via Tarantool-patches
2025-11-26 11:58   ` Sergey Bronnikov via Tarantool-patches
2025-11-27  9:44     ` Sergey Kaplun via Tarantool-patches
2025-11-27 14:06       ` Sergey Bronnikov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aSghF2fyrhbrtysV@root \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 2/3] macOS: Workaround for buggy XCode 15.0 - 15.2 linker.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox