From: Trent Piepho <tpiepho@gmail.com>
To: Oskar Schirmer <oskar@scara.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
Daniel Latypov <dlatypov@google.com>,
Yiyuan guo <yguoaz@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"andy@kernel.org" <andy@kernel.org>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: A divide by zero bug in lib/math/rational.c (with triggering input)
Date: Sun, 23 May 2021 20:20:00 -0700 [thread overview]
Message-ID: <CA+7tXigHEwwnVqjD=Hi1HBCxrBfeuvieK8v=W4nvgsm+ipaLmQ@mail.gmail.com> (raw)
In-Reply-To: <20210522190759.GA9755@basil.scara.com>
On Sat, May 22, 2021 at 12:08 PM Oskar Schirmer <oskar@scara.com> wrote:
> On Fri, May 21, 2021 at 12:53:27 +0300, Andy Shevchenko wrote:
> > On Fri, May 21, 2021 at 12:20 PM Trent Piepho <tpiepho@gmail.com> wrote:
> > > On Fri, May 21, 2021 at 12:55 AM Yiyuan guo <yguoaz@gmail.com> wrote:
>
> Sorry, it does not. E.g. with the given fraction of 31/1000
> and the registers restricted to 8 and 5 bits respectively, the
> proposed fixed function would still divide by zero, because
> n1 == 0. If it was for the division by d1, the test for !d1
Yes, values less than 1 less than the smallest allowed non-zero value
will divide by zero will finish on the 2nd iteration, with n1 == 0,
and divide by zero.
The finished patch I've since sent fixes this.
> Moreover, for a fraction of 33/1000, both the original and
> the latest version would produce 1/30, which is off by some
> 1.01%, but the proposed fixed version would result in 1/31,
> which is worse: 2.24% off.
Finished patch correctly produces 1/30 in this case.
> I think the original function was not so bad. And the code it
> produced was much shorter than the latest version, although
> this might not be an argument in times, where a simple OS
> kernel is beyond the 40MB.
I measured this. I've compared the original, which did not consider
semi-convergents nor out of range values, the current version, which
does semi-convergents but fails on out of range, and the patched
version, which handles that too.
Size in bytes:
X64 149 205 278
ARM 164 220 300
So 129 bytes on x64 and 136 bytes on ARM. Not all that much. I
didn't try writing a special check for large/small inputs in the older
code to see how large that was, for a more like-to-like comparison to
my latest patch.
next prev parent reply other threads:[~2021-05-24 3:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-21 5:09 A divide by zero bug in lib/math/rational.c (with triggering input) Yiyuan guo
[not found] ` <CAHp75Vf8kQ73w0R9ieDNjDVkxM-V83QRN9mc6BjRZA8xHpPNAA@mail.gmail.com>
[not found] ` <CAHp75Vft8pnA+m0C=Ok7nRyjERAd2uJJ4q6HcN460j0Hir6Kaw@mail.gmail.com>
2021-05-21 7:55 ` Yiyuan guo
2021-05-21 9:20 ` Trent Piepho
2021-05-21 9:53 ` Andy Shevchenko
2021-05-22 19:07 ` Oskar Schirmer
2021-05-24 3:20 ` Trent Piepho [this message]
2021-05-23 0:05 ` Trent Piepho
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='CA+7tXigHEwwnVqjD=Hi1HBCxrBfeuvieK8v=W4nvgsm+ipaLmQ@mail.gmail.com' \
--to=tpiepho@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andy.shevchenko@gmail.com \
--cc=andy@kernel.org \
--cc=dlatypov@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oskar@scara.com \
--cc=yguoaz@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).