[Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Igor Munkin via Tarantool-patches]

This patch fixes inaccuracy in Tarantool build configuration introduced
by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
size and preferred load address for the bundle are necessary only for
builds with 32-bit GC area on 64-bit host. The only case fitting these
conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
use 64-bit GC area unconditionally.

Part of #5983
Needed for #5629
Follows up #4862

Signed-off-by: Igor Munkin <imun@tarantool.org>
---

This patch partially fixes the build on M1. I tested it on tntmac07
alongside with the changes Nikita made for libcoro[1]. As a result
Tarantool has been successfully built, but fails to start. CI looks to
be OK[2] except the known problems with ASAN[3].

Issue: https://github.com/tarantool/tarantool/issues/5983
Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1

[1]: https://github.com/tarantool/tarantool/commit/309ce38
[2]: https://github.com/tarantool/tarantool/commit/5465d7b
[3]: https://github.com/tarantool/tarantool/issues/6031


 cmake/luajit.cmake | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
index 3d37164e8..9390d0dfd 100644
--- a/cmake/luajit.cmake
+++ b/cmake/luajit.cmake
@@ -73,11 +73,21 @@ if(ENABLE_ASAN)
     add_definitions(-DLUAJIT_USE_ASAN=1)
 endif()
 
-if(TARGET_OS_DARWIN)
-    # Necessary to make LuaJIT (and Tarantool) work on Darwin, see
-    # http://luajit.org/install.html.
-    set(CMAKE_EXE_LINKER_FLAGS
-        "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
+if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64)
+    # XXX: This is not the best idea to build LuaJIT on MacOS
+    # with GC64 disabled. But nobody will stop you from this.
+    # You are warned. For more info see the following issue.
+    # https://github.com/tarantool/tarantool/issues/2643
+    message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. "
+                    "See #2643, why this is not a good idea.")
+    # XXX: Necessary to make LuaJIT (and hence Tarantool) work on
+    # Darwin/x86_64, see the following links for more info:
+    # * http://luajit.org/install.html#embed
+    # * https://github.com/tarantool/luajit/blob/789820a/cmake/SetTargetFlags.cmake
+    if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
+        set(CMAKE_EXE_LINKER_FLAGS
+            "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
+    endif()
 endif()
 
 # Define the locations for LuaJIT sources and artefacts.
-- 
2.25.0

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the patch!
LGTM, except a few nits below.

On 28.04.21, Igor Munkin wrote:
> This patch fixes inaccuracy in Tarantool build configuration introduced
> by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> size and preferred load address for the bundle are necessary only for
> builds with 32-bit GC area on 64-bit host. The only case fitting these
> conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> use 64-bit GC area unconditionally.
> 
> Part of #5983
> Needed for #5629
> Follows up #4862
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> This patch partially fixes the build on M1. I tested it on tntmac07
> alongside with the changes Nikita made for libcoro[1]. As a result
> Tarantool has been successfully built, but fails to start. CI looks to
> be OK[2] except the known problems with ASAN[3].
> 
> Issue: https://github.com/tarantool/tarantool/issues/5983
> Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1
> 
> [1]: https://github.com/tarantool/tarantool/commit/309ce38
> [2]: https://github.com/tarantool/tarantool/commit/5465d7b
> [3]: https://github.com/tarantool/tarantool/issues/6031
> 
> 
>  cmake/luajit.cmake | 20 +++++++++++++++-----

Should we add corresponding ChangeLog entry?

>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> index 3d37164e8..9390d0dfd 100644
> --- a/cmake/luajit.cmake
> +++ b/cmake/luajit.cmake
> @@ -73,11 +73,21 @@ if(ENABLE_ASAN)
>      add_definitions(-DLUAJIT_USE_ASAN=1)
>  endif()
>  
> -if(TARGET_OS_DARWIN)
> -    # Necessary to make LuaJIT (and Tarantool) work on Darwin, see
> -    # http://luajit.org/install.html.
> -    set(CMAKE_EXE_LINKER_FLAGS
> -        "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> +if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64)
> +    # XXX: This is not the best idea to build LuaJIT on MacOS
> +    # with GC64 disabled. But nobody will stop you from this.
> +    # You are warned. For more info see the following issue.

Typo: s/For more info/For more info,/

> +    # https://github.com/tarantool/tarantool/issues/2643
> +    message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. "
> +                    "See #2643, why this is not a good idea.")

Nit: may be the full link is better here (like we do for coredumps).
Feel free to ignore.

> +    # XXX: Necessary to make LuaJIT (and hence Tarantool) work on
> +    # Darwin/x86_64, see the following links for more info:
> +    # * http://luajit.org/install.html#embed
> +    # * https://github.com/tarantool/luajit/blob/789820a/cmake/SetTargetFlags.cmake
> +    if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
> +        set(CMAKE_EXE_LINKER_FLAGS
> +            "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> +    endif()
>  endif()
>  
>  # Define the locations for LuaJIT sources and artefacts.
> -- 
> 2.25.0
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review!

On 28.04.21, Sergey Kaplun wrote:
> 
> Hi, Igor!
> 
> Thanks for the patch!
> LGTM, except a few nits below.

Added your tag:
| Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>

> 
> On 28.04.21, Igor Munkin wrote:
> > This patch fixes inaccuracy in Tarantool build configuration introduced
> > by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> > LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> > size and preferred load address for the bundle are necessary only for
> > builds with 32-bit GC area on 64-bit host. The only case fitting these
> > conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> > use 64-bit GC area unconditionally.
> > 
> > Part of #5983
> > Needed for #5629
> > Follows up #4862
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> > 
> > This patch partially fixes the build on M1. I tested it on tntmac07
> > alongside with the changes Nikita made for libcoro[1]. As a result
> > Tarantool has been successfully built, but fails to start. CI looks to
> > be OK[2] except the known problems with ASAN[3].
> > 
> > Issue: https://github.com/tarantool/tarantool/issues/5983
> > Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1
> > 
> > [1]: https://github.com/tarantool/tarantool/commit/309ce38
> > [2]: https://github.com/tarantool/tarantool/commit/5465d7b
> > [3]: https://github.com/tarantool/tarantool/issues/6031
> > 
> > 
> >  cmake/luajit.cmake | 20 +++++++++++++++-----
> 
> Should we add corresponding ChangeLog entry?

Ugh... At first, I have no idea what do we need to write for this patch.
This is a partial fix for the build on M1. There is more work to be
done. The issue is not resolved by it, so it's too odd to me to add the
ChangeLog entry for such changes. BTW, as we discussed in chat, the
target branch for these patches is not the default one, but
wip-m1/master[1]. I guess, we can add everything necessary, when
Tarantool passes the tests on M1.

> 
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> > index 3d37164e8..9390d0dfd 100644
> > --- a/cmake/luajit.cmake
> > +++ b/cmake/luajit.cmake
> > @@ -73,11 +73,21 @@ if(ENABLE_ASAN)
> >      add_definitions(-DLUAJIT_USE_ASAN=1)
> >  endif()
> >  
> > -if(TARGET_OS_DARWIN)
> > -    # Necessary to make LuaJIT (and Tarantool) work on Darwin, see
> > -    # http://luajit.org/install.html.
> > -    set(CMAKE_EXE_LINKER_FLAGS
> > -        "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> > +if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64)
> > +    # XXX: This is not the best idea to build LuaJIT on MacOS
> > +    # with GC64 disabled. But nobody will stop you from this.
> > +    # You are warned. For more info see the following issue.
> 
> Typo: s/For more info/For more info,/

OK, fixed.

> 
> > +    # https://github.com/tarantool/tarantool/issues/2643
> > +    message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. "
> > +                    "See #2643, why this is not a good idea.")
> 
> Nit: may be the full link is better here (like we do for coredumps).
> Feel free to ignore.

If it makes someone happier. Diff with both fixes is below:

================================================================================

diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
index 9390d0dfd..6a94327a7 100644
--- a/cmake/luajit.cmake
+++ b/cmake/luajit.cmake
@@ -76,10 +76,11 @@ endif()
 if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64)
     # XXX: This is not the best idea to build LuaJIT on MacOS
     # with GC64 disabled. But nobody will stop you from this.
-    # You are warned. For more info see the following issue.
+    # You are warned. For more info, see the following issue.
     # https://github.com/tarantool/tarantool/issues/2643
     message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. "
-                    "See #2643, why this is not a good idea.")
+                    "If one wants to know why this is not a good idea, "
+                    "see https://github.com/tarantool/tarantool/issues/2643.")
     # XXX: Necessary to make LuaJIT (and hence Tarantool) work on
     # Darwin/x86_64, see the following links for more info:
     # * http://luajit.org/install.html#embed

================================================================================

> 
> > +    # XXX: Necessary to make LuaJIT (and hence Tarantool) work on
> > +    # Darwin/x86_64, see the following links for more info:
> > +    # * http://luajit.org/install.html#embed
> > +    # * https://github.com/tarantool/luajit/blob/789820a/cmake/SetTargetFlags.cmake
> > +    if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
> > +        set(CMAKE_EXE_LINKER_FLAGS
> > +            "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> > +    endif()
> >  endif()
> >  
> >  # Define the locations for LuaJIT sources and artefacts.
> > -- 
> > 2.25.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

[1]: https://github.com/tarantool/tarantool/tree/wip-m1/master

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Nikita Pettik via Tarantool-patches]

On 28 Apr 01:46, Igor Munkin wrote:
> This patch fixes inaccuracy in Tarantool build configuration introduced
> by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> size and preferred load address for the bundle are necessary only for
> builds with 32-bit GC area on 64-bit host. The only case fitting these
> conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> use 64-bit GC area unconditionally.
> 
> Part of #5983
> Needed for #5629
> Follows up #4862
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

LGTM

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Igor Munkin via Tarantool-patches]

Nikita,

Thanks for your review!

On 28.04.21, Nikita Pettik wrote:
> On 28 Apr 01:46, Igor Munkin wrote:
> > This patch fixes inaccuracy in Tarantool build configuration introduced
> > by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> > LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> > size and preferred load address for the bundle are necessary only for
> > builds with 32-bit GC area on 64-bit host. The only case fitting these
> > conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> > use 64-bit GC area unconditionally.
> > 
> > Part of #5983
> > Needed for #5629
> > Follows up #4862
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> 
> LGTM

Added your tag:
| Reviewed-by: Nikita Pettik <korablev@tarantool.org>

> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Sergey Ostanevich via Tarantool-patches]

Thanks for the patch, LGTM.

Sergos


> On 28 Apr 2021, at 01:46, Igor Munkin <imun@tarantool.org> wrote:
> 
> This patch fixes inaccuracy in Tarantool build configuration introduced
> by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> size and preferred load address for the bundle are necessary only for
> builds with 32-bit GC area on 64-bit host. The only case fitting these
> conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> use 64-bit GC area unconditionally.
> 
> Part of #5983
> Needed for #5629
> Follows up #4862
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> This patch partially fixes the build on M1. I tested it on tntmac07
> alongside with the changes Nikita made for libcoro[1]. As a result
> Tarantool has been successfully built, but fails to start. CI looks to
> be OK[2] except the known problems with ASAN[3].
> 
> Issue: https://github.com/tarantool/tarantool/issues/5983
> Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1
> 
> [1]: https://github.com/tarantool/tarantool/commit/309ce38
> [2]: https://github.com/tarantool/tarantool/commit/5465d7b
> [3]: https://github.com/tarantool/tarantool/issues/6031
> 
> 
> cmake/luajit.cmake | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> index 3d37164e8..9390d0dfd 100644
> --- a/cmake/luajit.cmake
> +++ b/cmake/luajit.cmake
> @@ -73,11 +73,21 @@ if(ENABLE_ASAN)
>     add_definitions(-DLUAJIT_USE_ASAN=1)
> endif()
> 
> -if(TARGET_OS_DARWIN)
> -    # Necessary to make LuaJIT (and Tarantool) work on Darwin, see
> -    # http://luajit.org/install.html.
> -    set(CMAKE_EXE_LINKER_FLAGS
> -        "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> +if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64)
> +    # XXX: This is not the best idea to build LuaJIT on MacOS
> +    # with GC64 disabled. But nobody will stop you from this.
> +    # You are warned. For more info see the following issue.
> +    # https://github.com/tarantool/tarantool/issues/2643
> +    message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. "
> +                    "See #2643, why this is not a good idea.")
> +    # XXX: Necessary to make LuaJIT (and hence Tarantool) work on
> +    # Darwin/x86_64, see the following links for more info:
> +    # * http://luajit.org/install.html#embed
> +    # * https://github.com/tarantool/luajit/blob/789820a/cmake/SetTargetFlags.cmake
> +    if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
> +        set(CMAKE_EXE_LINKER_FLAGS
> +            "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> +    endif()
> endif()
> 
> # Define the locations for LuaJIT sources and artefacts.
> -- 
> 2.25.0
> 

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Igor Munkin via Tarantool-patches]

Sergos,

Thanks for your review!

On 28.04.21, Sergey Ostanevich wrote:
> Thanks for the patch, LGTM.

Added your tag:
| Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>

> 
> Sergos
> 
> 
> > On 28 Apr 2021, at 01:46, Igor Munkin <imun@tarantool.org> wrote:
> > 
> > This patch fixes inaccuracy in Tarantool build configuration introduced
> > by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> > LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> > size and preferred load address for the bundle are necessary only for
> > builds with 32-bit GC area on 64-bit host. The only case fitting these
> > conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> > use 64-bit GC area unconditionally.
> > 
> > Part of #5983
> > Needed for #5629
> > Follows up #4862
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> > 
> > This patch partially fixes the build on M1. I tested it on tntmac07
> > alongside with the changes Nikita made for libcoro[1]. As a result
> > Tarantool has been successfully built, but fails to start. CI looks to
> > be OK[2] except the known problems with ASAN[3].
> > 
> > Issue: https://github.com/tarantool/tarantool/issues/5983
> > Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1
> > 
> > [1]: https://github.com/tarantool/tarantool/commit/309ce38
> > [2]: https://github.com/tarantool/tarantool/commit/5465d7b
> > [3]: https://github.com/tarantool/tarantool/issues/6031
> > 
> > 
> > cmake/luajit.cmake | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake
> > index 3d37164e8..9390d0dfd 100644
> > --- a/cmake/luajit.cmake
> > +++ b/cmake/luajit.cmake
> > @@ -73,11 +73,21 @@ if(ENABLE_ASAN)
> >     add_definitions(-DLUAJIT_USE_ASAN=1)
> > endif()
> > 
> > -if(TARGET_OS_DARWIN)
> > -    # Necessary to make LuaJIT (and Tarantool) work on Darwin, see
> > -    # http://luajit.org/install.html.
> > -    set(CMAKE_EXE_LINKER_FLAGS
> > -        "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> > +if(TARGET_OS_DARWIN AND NOT LUAJIT_ENABLE_GC64)
> > +    # XXX: This is not the best idea to build LuaJIT on MacOS
> > +    # with GC64 disabled. But nobody will stop you from this.
> > +    # You are warned. For more info see the following issue.
> > +    # https://github.com/tarantool/tarantool/issues/2643
> > +    message(WARNING "LUAJIT_ENABLE_GC64 is disabled for MacOS. "
> > +                    "See #2643, why this is not a good idea.")
> > +    # XXX: Necessary to make LuaJIT (and hence Tarantool) work on
> > +    # Darwin/x86_64, see the following links for more info:
> > +    # * http://luajit.org/install.html#embed
> > +    # * https://github.com/tarantool/luajit/blob/789820a/cmake/SetTargetFlags.cmake
> > +    if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
> > +        set(CMAKE_EXE_LINKER_FLAGS
> > +            "${CMAKE_EXE_LINKER_FLAGS} -pagezero_size 10000 -image_base 100000000")
> > +    endif()
> > endif()
> > 
> > # Define the locations for LuaJIT sources and artefacts.
> > -- 
> > 2.25.0
> > 
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Igor Munkin via Tarantool-patches]

I've checked the patch into wip-m1/master.

On 28.04.21, Igor Munkin wrote:
> This patch fixes inaccuracy in Tarantool build configuration introduced
> by commit 07c83aab5c066ca75c149112b331b4dbb81b3f38 ('build: adjust
> LuaJIT build system'). All those MacOS-related tweaks for __PAGEZERO
> size and preferred load address for the bundle are necessary only for
> builds with 32-bit GC area on 64-bit host. The only case fitting these
> conditions is x86_64 with no LUAJIT_ENABLE_GC64. All other 64-bit builds
> use 64-bit GC area unconditionally.
> 
> Part of #5983
> Needed for #5629
> Follows up #4862
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> This patch partially fixes the build on M1. I tested it on tntmac07
> alongside with the changes Nikita made for libcoro[1]. As a result
> Tarantool has been successfully built, but fails to start. CI looks to
> be OK[2] except the known problems with ASAN[3].
> 
> Issue: https://github.com/tarantool/tarantool/issues/5983
> Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-fix-build-on-m1
> 
> [1]: https://github.com/tarantool/tarantool/commit/309ce38
> [2]: https://github.com/tarantool/tarantool/commit/5465d7b
> [3]: https://github.com/tarantool/tarantool/issues/6031
> 
> 
>  cmake/luajit.cmake | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 

<snipped>

> -- 
> 2.25.0
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] build: fix linker flags for executable on MacOS

[Igor Munkin via Tarantool-patches]

After discussion with Nikita and Kirill also checked the patch into
master and 2.8.

-- 
Best regards,
IM