[Tarantool-patches] [PATCH 1/2] Fix embedded bytecode loader.

[Sergey Bronnikov]

Hi, Max


thanks for review! see my comments


On 7/31/23 15:01, Maxim Kokryashkin via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> Please consider my comments below.
>
> On Tue, Jul 25, 2023 at 07:36:24PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
>> From: sergeyb at tarantool.org
>>
>> (cherry-picked from commit 820339960123dc78a7ce03edf53fcf4fdae0e55d)
>>
>> Original problem is specific for x32 and is as follows: when a chunk
> Typo: s/Original/The original/
> Typo: s/for x32/to x32/
Fixed.
>> with bytecode library is loaded into memory, and the address is higher
> Typo: s/with bytecode/with a bytecode/
Fixed.
>> than 0x80000100, the LexState->pe, that contains an address of the end
> Typo: s/the LexState/LexState/
Fixed.
>> of bytecode chunk in the memory, will wrap around and become smaller
> Typo: s/of bytecode/of the bytecode/
Fixed.
>> than address in LexState->p, that contains an address of the beginning
> Typo: s/than address/than the address/
Fixed.
>> of bytecode chunk in the memory. In bcread_fill() called by
>> bcread_want(), memcpy() is called with a very large size and causes bus
>> error on x86 and segmentation fault on ARM android.
> Typo: s/android/Android/
Fixed.
>> The problem cannot be reproduced on platforms supported by Tarantool
>> (ARM64, x86_64), so test doesn't reproduce a problem without a patch and
>> tests patch partially.
> Typo: s/tests the patch/
Fixed.
>
> Well, I've tried to run that test on x32 machine, and nothing happened.
> I think we should backport this patch without tests then, since this
> test seems irrelevant to the problem. What do you think?
It is relevant, because covers a part of the patch.
>
> Also, it is kinda strange, that you are talking about a test in a patch
> without any tests. You need to either mention that the test is present in
> the next commit, or move that test here.
I'll update commit message, thanks.
<snipped>