[Tarantool-patches] [PATCH luajit v2 1/1] Fix saved bytecode encapsulated in ELF objects.
Sergey Kaplun
skaplun at tarantool.org
Tue Jun 6 15:55:26 MSK 2023
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
More information about the Tarantool-patches
mailing list