[Tarantool-patches] [PATCH luajit 1/2][v2] FFI: Treat cdata finalizer table as a GC root.
Sergey Kaplun
skaplun at tarantool.org
Mon Aug 12 16:32:28 MSK 2024
Hi, Sergey!
Thanks for the fixes!
On 23.07.24, Sergey Bronnikov wrote:
> Hi,
>
> please see comments below. Fixes applied and force-pushed.
>
> Sergey
>
> On 10.07.2024 16:13, Sergey Kaplun wrote:
> > Hi, Sergey!
> > Thanks for the fixes!
> > Please consider my minor nits about comments below.
> >
> > On 09.07.24, Sergey Bronnikov wrote:
> >> Hi, Sergey,
> >>
> >> thanks for review. Fixes applied and force-pushed.
> >>
> >> Sergey
> >>
> >>
> >> On 09.07.2024 14:52, Sergey Kaplun via Tarantool-patches wrote:
> >>> Hi, Sergey!
> >>> Thanks for the patch!
> >>> Please consider my comments below.
> >>>
> >>> On 09.07.24, Sergey Bronnikov wrote:
> >>>> From: Mike Pall <mike>
> >>>>
<snipped>
> >
> > It is more correct to say, that "`lua_State` is marked after the
> > function is removed from it" (since we stop the GC before chunk
> > loading and starts after).
My bad:
Typo: s/starts/start/
> >
<snipped>
> >>> Also, it is worth mentioning that the problem was partially solved, the
> >>> complete fix will be applied in the next patch.
> Added.
> > Please, add its description to the commit message too.
I would rephrase this part as the following:
| The patch fixes the problem partially by marking the finalizer table
| on the start of the GC cycle.
| The complete fix will be applied in the next patch by turning the
| finalizer table into the proper GC root.
> >
> >>>> Sergey Bronnikov:
> >>>> * added the description and the tests for the problem
> >>>>
> >>>> Part of tarantool/tarantool#10199
> >>>> ---
> >>>> src/lj_gc.c | 3 +
> >>>> ...free-on-access-to-CTState-finalizer.test.c | 66 +++++++++++++++++++
> >>>> ...ee-on-access-to-CTState-finalizer.test.lua | 18 +++++
> >>>> 3 files changed, 87 insertions(+)
> >>>> create mode 100644 test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c
> >>>> create mode 100644 test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua
> > <snipped>
> >
> >>
> >>>> + * has the finalizer table as its environment. But, there is no
> >>>> + * FFI module table anywhere to anchor the `ffi.gc` itself, and
> >>>> + * the `lua_State` object was marked before the function is
> > It is more correct to say, that "`lua_State` is marked after the
> > function is removed from it" (since we stop the GC before chunk
> > loading and starts after).
My bad:
Typo: s/starts/start/
> >
<snipped>
> >>>> + if (luaL_loadbufferx(L, buff, sizeof(buff) - 1, "chunk", "t") != LUA_OK)
> >>> Why do we need to omit the ending zero byte?
Please add a comment that the terminating '\0' is considered by parser
as part of the input, so we must chomp it.
> >
> > <snipped>
> >
> >>>> diff --git a/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua b/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua
> >>>> new file mode 100644
> >>>> index 00000000..fca5ec76
> >>>> --- /dev/null
> >>>> +++ b/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua
<snipped>
> >>>> 2.34.1
> >>>>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list