From: Sasha Levin <sasha.levin@oracle.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC v2 1/2] compiler: use compiler to detect integer overflows
Date: Wed, 26 Nov 2014 20:37:29 -0500 [thread overview]
Message-ID: <54768059.1080406@oracle.com> (raw)
In-Reply-To: <CA+55aFwjTTcGFbbEPWckxZHya5PFWo+MFbK9zMzoqZorXV21nQ@mail.gmail.com>
On 11/26/2014 07:33 PM, Linus Torvalds wrote:
> On Wed, Nov 26, 2014 at 3:58 PM, Sasha Levin <sasha.levin@oracle.com> wrote:
>> > We've used to detect integer overflows by causing an overflow and testing the
>> > result. For example, to test for addition overflow we would:
> So I don't like this, for a very simple reason: it doesn't work for
> older gcc versions.
>
> Your "check_add_overflow()" doesn't actually do it. It just
> perpetuates any bugs you find. For unsigned additions, it's pointless,
> and for signed additions it remains as buggy as it was before.
I understand your point. It doesn't fix bugs but rather hides them on
newer compilers.
Since the way to fix this is by properly checking for overflow rather than
the old broken (a + b > a) conditional, how about something like the following
for the non-gcc5 case:
#define IS_UNSIGNED(A) (((typeof(A))-1) >= 0)
#define TYPE_MAX(A) ((typeof(A))(~0U>>1))
#define TYPE_MIN(A) (-TYPE_MAX(A) - 1)
#define check_add_overflow(A, B) \
({ \
typeof(A) __a = (A); \
typeof(B) __b = (B); \
typeof(sizeof(__a) > sizeof(__b) ? __a : __b) __min, __max; \
if (IS_UNSIGNED(__a) || IS_UNSIGNED(__b)) \
0; \
__min = TYPE_MIN(__min); \
__max = TYPE_MAX(__max); \
(((__a > 0) && (typeof(__max))__b > (__max - ((typeof(__max))__a))) ||\
((__a < 0) && (typeof(__max))__b < (__min - ((typeof(__max))__a))));\
})
> Also, your commit message is still *very*wrong*. You can't claim that
> integer addition overflow is undefined. It's undefined onyl for
> _signed_ integer types, and that's a big big difference.
Understood. I'll be specific it's only for signed integer overflows.
Thanks,
Sasha
next prev parent reply other threads:[~2014-11-27 1:37 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-26 23:58 [RFC v2 1/2] compiler: use compiler to detect integer overflows Sasha Levin
2014-11-26 23:58 ` [RFC v2 2/2] fs: correctly check for signed integer overflow in vfs_fallocate Sasha Levin
2014-11-27 0:33 ` [RFC v2 1/2] compiler: use compiler to detect integer overflows Linus Torvalds
2014-11-27 1:37 ` Sasha Levin [this message]
2014-11-27 3:13 ` Linus Torvalds
2014-11-29 15:07 ` Sasha Levin
2014-11-29 18:08 ` Linus Torvalds
2014-11-30 3:47 ` Sasha Levin
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=54768059.1080406@oracle.com \
--to=sasha.levin@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=torvalds@linux-foundation.org \
/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