Missing yk tracing locations on function prototype

[Pavel-Durov]

Some function prototypes don't have to trace locations when it Lua interpreter frees their references.

Solution:

We need to find a way to store trace locations in a way that will persist through GC cycles so that we can remove (aka drop) them from tracing when functions are freed by GC.

Context:
Lua bytecodes for a function are stored inside this "prototype" struct and therefore it made sense to put our locations in there too so that's what we did as the first implementation.
When GC frees functions, we cannot obtain references to trace locations and cannot drop them via yk_location_drop.

Related Issues: #31, #25

[ltratt]

[I don't think this has anything to do with the trace compiler per se, although that happens to be the way that we notice that we've not found the right part of yklua to alter.]

[Pavel-Durov]

@ltratt re-worded

[ltratt]

Works for me!

[Pavel-Durov]

From my understanding, GC clears all the heap-allocated objects that have no reference to them.

In this issue we have f->yklocs with a surprising value of null, and we assumed that it was collected by the GC.

But if it was collected by the GC I would expect f to be cleared as well since f holds a reference to yklocs.

And in the callstack I can see that when we get Segment fault, the GC is just about to clear the f reference 0x929990:

@@ f: 0x929990, f->yklocs: (nil) <------------------- printf just before Segmentation fault.

Program received signal SIGSEGV, Segmentation fault.
0x000000000080808e in luaF_freeproto (L=<optimised out>, f=0x929990) at lfunc.c:280
280	        printf("@@ f->yklocs[i]: %p,\n", f->yklocs[i]);
(gdb) bt
#0  0x000000000080808e in luaF_freeproto (L=<optimised out>, f=0x929990) at lfunc.c:280
#1  0x000000000080a9fe in freeobj (L=0x917e38, o=0x929990) at lgc.c:767
#2  0x00000000008158a7 in deletelist (L=0x917e38, p=0x929990, limit=<optimised out>) at lgc.c:1494
#3  0x00000000008156b4 in luaC_freeallobjects (L=0x917e38) at lgc.c:1512
#4  0x000000000085052d in close_state (L=0x917e38) at lstate.c:276
#5  0x0000000000850bae in lua_close (L=0x917e38) at lstate.c:421
#6  0x00000000007d9cd0 in main (argc=<optimised out>, argv=<optimised out>) at lua.c:663

So it doesn't look like a GC issue to me 🤔

[ltratt]

My suggestion is that we put to one side that this an interpreter with a VM: we've got a double free and so we need to find all the paths that can call free and work out where we should be calling free ourselves. For example, setting a breakpoint on the function that does the double free might be a start.

[Pavel-Durov]

Yklua loads the same function twice but in two different ways.

The first time it loads it as text using luaY_parser and the second time as "pre-compiled" binary using luaU_undump (prototypes are dumped/saved prior to that).

See condition: https://github.com/ykjit/yklua/blob/main/src/ldo.c#L965

When prototype binaries are loaded/undumped - tracing locations are not loaded cause we didn't add any code to save them here: https://github.com/ykjit/yklua/blob/main/src/ldump.c#L213

It looks less and less like a GC issue (I was able to reproduce the same error with GC disabled) :)

[ltratt]

The first time it loads it as text using luaY_parser and the second time as "pre-compiled" binary using luaU_undump (prototypes are dumped/saved prior to that).

Aha, that sounds promising!

[Pavel-Durov]

Another thing to note 🤓

We've seen this behaviour when running lua tests (all.lua file) which shadows the standard dofile function: https://github.com/ykjit/yklua/blob/main/tests/all.lua#L143 - it causes this pre-compiled loading behaviour.

For example:

-- NO missing locations on function prototype Segmentation fault
dofile('main.lua')

-- This code will have missing locations on function prototype Segmentation fault
local dofile = function (n, strip)
  local f = assert(loadfile(n))
  local b = string.dump(f, strip)
  f = assert(load(b))
  return f()
end

dofile('main.lua')

[Pavel-Durov]

Was fixed in #36 and #64

[Pavel-Durov]

Was fixed in #36 and #64