[Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Igor Munkin via Tarantool-patches]

Originally there is nether a special option nor a variable to configure
check for instrunction/line hooks for compiled code via the build system
being used. We finally decided to use this feature in Tarantool, so for
convenient managing LUAJIT_ENABLE_CHECKHOOK option is added to the root
project CMakeLists.txt.

Needed for tarantool/tarantool#7762

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

Issue: https://github.com/tarantool/tarantool/issues/7762
Branch: https://github.com/tarantool/luajit/tree/imun/luajit-enable-checkhook
CI: https://github.com/tarantool/luajit/commit/701de8c

 CMakeLists.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8b49f9d7..c870cce2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -180,6 +180,11 @@ if(LUAJIT_ENABLE_GC64)
   AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_GC64)
 endif()
 
+option(LUAJIT_ENABLE_CHECKHOOK "Check instruction/line hooks for compiled code" OFF)
+if(LUAJIT_ENABLE_CHECKHOOK)
+  AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_CHECKHOOK)
+endif()
+
 # Disable memory profiler.
 option(LUAJIT_DISABLE_MEMPROF "LuaJIT memory profiler support" OFF)
 if(LUAJIT_DISABLE_MEMPROF)
-- 
2.34.0

Re: [Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the patch!
LGTM, except a single nit below.

Also, I suppose that the test will be added in the Tarantool patch with
version bump, as far as this option will be enabled by default for
Tarantool build.
Am I right?

On 02.10.22, Igor Munkin wrote:
> Originally there is nether a special option nor a variable to configure
> check for instrunction/line hooks for compiled code via the build system

Typo: s/check/the check/

> being used. We finally decided to use this feature in Tarantool, so for
> convenient managing LUAJIT_ENABLE_CHECKHOOK option is added to the root
> project CMakeLists.txt.
> 
> Needed for tarantool/tarantool#7762
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/7762
> Branch: https://github.com/tarantool/luajit/tree/imun/luajit-enable-checkhook
> CI: https://github.com/tarantool/luajit/commit/701de8c
> 
>  CMakeLists.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 8b49f9d7..c870cce2 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt

<snipped>

> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Maxim Kokryashkin via Tarantool-patches]

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


Hi, Igor!
Thanks for the patch!
LGTM, except for the nits, Sergey have already mentioned.
--
Best regards,
Maxim Kokryashkin
 
  
>Воскресенье, 2 октября 2022, 18:21 +03:00 от Igor Munkin <imun@tarantool.org>:
> 
>Originally there is nether a special option nor a variable to configure
>check for instrunction/line hooks for compiled code via the build system
>being used. We finally decided to use this feature in Tarantool, so for
>convenient managing LUAJIT_ENABLE_CHECKHOOK option is added to the root
>project CMakeLists.txt.
>
>Needed for tarantool/tarantool#7762
>
>Signed-off-by: Igor Munkin < imun@tarantool.org >
>---
>
>Issue:  https://github.com/tarantool/tarantool/issues/7762
>Branch:  https://github.com/tarantool/luajit/tree/imun/luajit-enable-checkhook
>CI:  https://github.com/tarantool/luajit/commit/701de8c
>
> CMakeLists.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/CMakeLists.txt b/CMakeLists.txt
>index 8b49f9d7..c870cce2 100644
>--- a/CMakeLists.txt
>+++ b/CMakeLists.txt
>@@ -180,6 +180,11 @@ if(LUAJIT_ENABLE_GC64)
>   AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_GC64)
> endif()
> 
>+option(LUAJIT_ENABLE_CHECKHOOK "Check instruction/line hooks for compiled code" OFF)
>+if(LUAJIT_ENABLE_CHECKHOOK)
>+ AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_CHECKHOOK)
>+endif()
>+
> # Disable memory profiler.
> option(LUAJIT_DISABLE_MEMPROF "LuaJIT memory profiler support" OFF)
> if(LUAJIT_DISABLE_MEMPROF)
>--
>2.34.0
 

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

Re: [Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review!

On 03.10.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> LGTM, except a single nit below.

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

> 
> Also, I suppose that the test will be added in the Tarantool patch with
> version bump, as far as this option will be enabled by default for
> Tarantool build.
> Am I right?

Partially. Honestly, I didn't plan to add the test at all (since we
don't write tests for the build options). However, if you insist, I'll
add one in scope of the PR enabling this option in 2.11 (NB: it will be
different PR than the one bumping LuaJIT submodule).

> 
> On 02.10.22, Igor Munkin wrote:
> > Originally there is nether a special option nor a variable to configure
> > check for instrunction/line hooks for compiled code via the build system
> 
> Typo: s/check/the check/

Fixed, thanks!

> 
> > being used. We finally decided to use this feature in Tarantool, so for
> > convenient managing LUAJIT_ENABLE_CHECKHOOK option is added to the root
> > project CMakeLists.txt.
> > 
> > Needed for tarantool/tarantool#7762
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> > 
> > Issue: https://github.com/tarantool/tarantool/issues/7762
> > Branch: https://github.com/tarantool/luajit/tree/imun/luajit-enable-checkhook
> > CI: https://github.com/tarantool/luajit/commit/701de8c
> > 
> >  CMakeLists.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > index 8b49f9d7..c870cce2 100644
> > --- a/CMakeLists.txt
> > +++ b/CMakeLists.txt
> 
> <snipped>
> 
> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Igor Munkin via Tarantool-patches]

Max,

Thanks for your review!

On 03.10.22, Maxim Kokryashkin wrote:
> 
> Hi, Igor!
> Thanks for the patch!
> LGTM, except for the nits, Sergey have already mentioned.

Added your tag:
| Reviewed-by: Maxim Kokryashkin <m.kokryashkin@tarantool.org>

> --
> Best regards,
> Maxim Kokryashkin

<snipped>

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] build: introduce LUAJIT_ENABLE_CHECKHOOK option

[Igor Munkin via Tarantool-patches]

I've checked the patch into all long-term branches in tarantool/luajit
and bumped a new version in master, 2.10 and 1.10.

On 02.10.22, Igor Munkin wrote:
> Originally there is nether a special option nor a variable to configure
> check for instrunction/line hooks for compiled code via the build system
> being used. We finally decided to use this feature in Tarantool, so for
> convenient managing LUAJIT_ENABLE_CHECKHOOK option is added to the root
> project CMakeLists.txt.
> 
> Needed for tarantool/tarantool#7762
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/7762
> Branch: https://github.com/tarantool/luajit/tree/imun/luajit-enable-checkhook
> CI: https://github.com/tarantool/luajit/commit/701de8c
> 
>  CMakeLists.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 8b49f9d7..c870cce2 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -180,6 +180,11 @@ if(LUAJIT_ENABLE_GC64)
>    AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_GC64)
>  endif()
>  
> +option(LUAJIT_ENABLE_CHECKHOOK "Check instruction/line hooks for compiled code" OFF)
> +if(LUAJIT_ENABLE_CHECKHOOK)
> +  AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_CHECKHOOK)
> +endif()
> +
>  # Disable memory profiler.
>  option(LUAJIT_DISABLE_MEMPROF "LuaJIT memory profiler support" OFF)
>  if(LUAJIT_DISABLE_MEMPROF)
> -- 
> 2.34.0
> 

-- 
Best regards,
IM