Re: [Tarantool-patches] [PATCH luajit v2 1/1] Fix saved bytecode encapsulated in ELF objects.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Maxim, Sergey!
Thanks for the comments!

On 31.05.23, Sergey Bronnikov wrote:
> Hello, Max! Thanks for comments!
> 
> Fixed all comments except checks for constant size and offset, fixes 
> force-pushed.
> 
> 
> Sergey
> 
> On 5/30/23 12:05, Maxim Kokryashkin wrote:
> > Hi, Sergey!
> > Thanks for the patch!
> > LGTM, except for a few comments below.

<snipped>

> >         +
> >         +test:is(strtab_size, expected_strtab_size, 'check .strtab size')
> >         +test:is(strtab_offset, expected_strtab_offset, 'check .strtab
> >         offset')
> >
> >     As I already said offline, I don’t like that approach. ELF is made
> >     to by
> >     as flexible as possible, so, generally, you have no guarantees about
> >     order and, hence, offsets of any sections. The only reason why we are
> >     getting away with that here, is because this ELF is generated
> >     in `bcsave.lua` in a particular manner. It is ok for now, but no one
> >     knows if this will ever change. I think that the symbol name search
> >     below is sufficient, because it is unlikely to pass with an
> >     incorrect offset.

But, the finding the symbol, doesn't guarantee the correct offset
emitting. I suggest to leave double check here, too, as well as Sergey.
Also, we can track the changes in <jit/bcsave.lua> better:).

> >
> Sure, I remember your thoughts regarding checking with constant size and 
> offset.
> 
> I left this for double-checking change proposed in patch.
> 
> I'll leave this as as without changes and will wait review from Sergey
> 
> and then we will decide remove these checks or not.
> 
> >     All of the checks are valid for now, though, so feel free to ignore.

<snipped>

> >     --
> >     Best regards,
> >     Maxim Kokryashkin
> >

-- 
Best regards,
Sergey Kaplun