From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 848BC6465D; Thu, 21 Jan 2021 18:32:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 848BC6465D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1611243176; bh=rg/ENjI6PVuhjQ750GK63OY+ju7aElEAArhrtJ17tQg=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=tZWAOBhAbe0vGwh9LlhkBi+42GTIPxYfu+/SREuguvYPdAqB0TNL8Rb4A5TjjUQT7 II0tUFijRL3N6Tc2wX7hHReR+Le9GGn4qfvz7dQs7EWzZxOwHp25EnYaemhRIgTPtp 5X9GOZlBzCxvAKWawmUIrvvGE7PVQWohAad51Vh0= Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 768D66465D for ; Thu, 21 Jan 2021 18:32:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 768D66465D Received: by smtp49.i.mail.ru with esmtpa (envelope-from ) id 1l2bx8-0002sG-Dd; Thu, 21 Jan 2021 18:32:54 +0300 Date: Thu, 21 Jan 2021 18:31:09 +0300 To: Sergey Nikiforov Message-ID: <20210121153109.5rgx6wttrkkmcrc5@tkn_work_nb> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9FAB20BF8F0759036E9D016CAEA9DE5625EC7EDBD8999FF7A182A05F5380850409FDAC03A33818B371F29DE6A044B90144F51D8AB67E097E024EF63EDF9EF2F6D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A40BF27E8345D582EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637775FFFCA96730EC9EA1F7E6F0F101C674E70A05D1297E1BBC6CDE5D1141D2B1C0F2D049882B02DE5F3FF324B29EA7E95ACA60F62A1031E7F9FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD182CC0D3CB04F14752D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE7B96B19DC4093321E7815D572A9D6896D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE350C865FA6708688F9735652A29929C6CC4224003CC836476EA7A3FFF5B025636A7F4EDE966BC389F9E8FC8737B5C22498A0907C648FB30DE089D37D7C0E48F6CCF19DD082D7633A0E7DDDDC251EA7DABAAAE862A0553A39223F8577A6DFFEA7CA819EB9AE8EA3DE343847C11F186F3C5E7DDDDC251EA7DABCC89B49CDF41148F1C646545D3098A473C9F3DD0FB1AF5EB4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A5A7B2018C2F23CC42DEDD65C9BD40B33F796C5D2E3B636D37D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA754DBC578B17CB11D78E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D343DCFC3BEDFB0242D3FC3195DBC308E1EC712E1733D90AF2843363A9D134F42CB218B09367085DDE41D7E09C32AA3244C6EA5A7EE8AFFC6C7FE80B0CC2B7057D2D9ADFF0C0BDB8D1F927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojYT//mZRaUejvIgsl70s7TQ== X-Mailru-Sender: FFAA8E4AEE17E37C3731A083A1A85ADE731876E102C45165DE35F77A284479EDB7EA9FE7735C3DBFC664A44C781FCEA7C77752E0C033A69EDF9F2CE1E9CF805D8CD356D4F938FF726C18EFA0BB12DBB0 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v6 2/2] base64: improve decoder performance X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Alexander Turenko via Tarantool-patches Reply-To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Mon, Jan 11, 2021 at 12:45:01PM +0300, Sergey Nikiforov wrote: > Unnecessary checks were removed from internal loops. > Benchmark shows that performance is now ~1.19 times higher > (release build, Intel Core I7-9700K, only one thread). > --- > > Branch: https://github.com/tarantool/tarantool/tree/void234/gh-3069-fix-base64-memory-overrun-v6 > > third_party/base64.c | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/third_party/base64.c b/third_party/base64.c > index 3ecd0843a..74260971b 100644 > --- a/third_party/base64.c > +++ b/third_party/base64.c > @@ -258,10 +258,10 @@ base64_decode_block(const char *in_base64, int in_len, > { > case step_a: > do { > - if (in_pos == in_end || out_pos >= out_end) > + if (in_pos >= in_end) Okay, it is the loop invariant and so we can move it outside the loop. But why we sink it down rather than hoisting up? We know that there is no space in the output buffer -- so there is no sense to read the input, right? Or it will break 'correctness' of the hypotetical use of the `state`? So we should imagine how the state would be used if it would be possible. From which input position the processing would be resumed if the code would provide such ability? How a caller supposed to distinguish end of the output buffer from successful processing the whole input? Aside of this, skipping unrecognized input bytes looks as another incorrectness. We should be ready to work with untrusted input, perform appropriate validation and report errors. We can skip newlines, but I guess there is no sense to do this in a loop: only one newline is a row is allowed (but it worth to verify the guess with the standard). It is hard to discuss an optimization until we'll define how the result should look in the corner cases like 'short output buffer', 'unexpected end of an input buffer' and so on. Current code does not report errors: it silently skips invalid input, accepts trimmed input, silently trims output at the buffer boundary. I don't understand how it supposed to work with untrusted input. I propose to stop attempts to do some 'quick fixups' and either work on the code or don't. If we would define behaviour first, I would spend MUCH less time trying to deduce it. That's really hard to discuss correctness of dead code. To be honest, it looks easier to me to finish the chained processing implementation, test it and remove than rather than trying to interpter the code (both existing and imaginary) in the mind. BTW, it seems we able to entirely eliminate the output buffer length checks for the first (out_len * 3 / 4) input bytes and than process the tail with full checks. (After we'll done with strict and correct API.)