From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>, Sergey Bronnikov <estetus@gmail.com> Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 1/2] Fix embedded bytecode loader. Date: Tue, 1 Aug 2023 12:56:07 +0300 [thread overview] Message-ID: <caca1092-3bb0-ab8e-d11c-789cacb705d9@tarantool.org> (raw) In-Reply-To: <5t3vejxwlcpd3fczyenum4527mtwtmt6qd4agwrxhnkt3zdtob@62fmw5pdbhqg> 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@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>
next prev parent reply other threads:[~2023-08-01 9:56 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-07-25 16:34 [Tarantool-patches] [PATCH 0/2] " Sergey Bronnikov via Tarantool-patches 2023-07-25 16:36 ` [Tarantool-patches] [PATCH 1/2] " Sergey Bronnikov via Tarantool-patches 2023-07-31 12:01 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 9:56 ` Sergey Bronnikov via Tarantool-patches [this message] 2023-08-06 11:09 ` Sergey Kaplun via Tarantool-patches 2023-08-15 8:51 ` Maxim Kokryashkin via Tarantool-patches 2023-07-25 16:37 ` [Tarantool-patches] [PATCH 2/2] Followup fix for " Sergey Bronnikov via Tarantool-patches 2023-07-31 12:20 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 9:53 ` Sergey Bronnikov via Tarantool-patches 2023-08-14 8:14 ` Sergey Bronnikov via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=caca1092-3bb0-ab8e-d11c-789cacb705d9@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=estetus@gmail.com \ --cc=m.kokryashkin@tarantool.org \ --cc=max.kokryashkin@gmail.com \ --cc=sergeyb@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/2] Fix embedded bytecode loader.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox