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
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
[-- 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 --]
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
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
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