This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch] sext_hwi: Avoid left shift of negative value undefined behaviour
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Markus Trippelsdorf <markus at trippelsdorf dot de>
- Cc: Mikael Morin <mikael dot morin at sfr dot fr>, Jeff Law <law at redhat dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Thu, 13 Aug 2015 13:11:53 +0200
- Subject: Re: [Patch] sext_hwi: Avoid left shift of negative value undefined behaviour
- Authentication-results: sourceware.org; auth=none
- References: <20150812110724 dot GB403 at x4> <CAFiYyc3G2NM2iqM5b5TKs+yWZJZ5-S13n30Bz51a2uMYL7N6KA at mail dot gmail dot com> <55CB4B40 dot 6040902 at sfr dot fr> <55CB7C0C dot 7030203 at redhat dot com> <F89F5949-3938-4F58-9C31-8BC0B217533B at gmail dot com> <55CB8B51 dot 5080908 at redhat dot com> <21C33A3D-9976-4632-ACCC-082F2D618ED5 at gmail dot com> <55CB9343 dot 6000103 at redhat dot com> <87k2t06zkk dot fsf at googlemail dot com> <55CC77D9 dot 9040101 at sfr dot fr> <20150813110816 dot GB386 at x4>
On Thu, Aug 13, 2015 at 1:08 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2015.08.13 at 12:56 +0200, Mikael Morin wrote:
>> Le 12/08/2015 22:07, Richard Sandiford a Ãcrit :
>> > Jeff Law <law@redhat.com> writes:
>> >> On 08/12/2015 12:32 PM, Richard Biener wrote:
>> >>> On August 12, 2015 8:07:13 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
>> >>>> On 08/12/2015 11:12 AM, Richard Biener wrote:
>> >>>>
>> >>>>>
>> >>>>> Prec is almost never a constant and is heavily used from wide-int.
>> >>>>>
>> >>>>> We are not exploiting this undefined ness in C so I object to making
>> >>>> this so much slower.
>> >>>>>
>> >>>>> Can we instead do what we do for abs_hwi and add a checking assert so
>> >>>> we can move the tests to the callers that misbehave instead?
>> >>>> Given that ISO C++ is moving away from making shifting 1 into the sign
>> >>>> bit undefined behaviour, maybe we should make UBSan less strict in its
>> >>>> warning. That may eliminate the need for Mikael's patch.
>> >>>
>> >>> We can also use an logical left shift followed by an arithmetic right
>> >>> shift. Or is the latter invoking undefined behaviour as well in some
>> >>> cases we hit?
>> >> Hmm, why aren't we using logicals for the left shift to begin with?
>> >> That's the problem area. I don't think the right shifts are an issue at
>> >> all.
>> >
>> > Well, they're implementation-defined, at least in C. The C11 wording
>> > for E1 >> E2 is "If E1 has a signed type and a negative value, the
>> > resulting value is implementation-defined". Is C++ different?
>> > (I don't have the standard handy.)
>> >
>> > So...
>> >
>> >> It's strange that when I was researching this, consistently I saw folks
>> >> suggesting the LEFT-SHIFT followed by RIGHT-SHIFT, then it'd get shot
>> >> down as UB, then folks went to either Mikael's approach or another that
>> >> is very similar. Nobody suggested twidding the types to get a
>> >> left-logical-shift, then doing a right-arithmetic-shift.
>> >
>> > ...unless C++ is different, there's not a standard-level concept
>> > of arithmetic shift.
>> >
>> Still, implementation-defined behavior is a progress over undefined
>> behaviour.
>> Even if it's not set in stone, the good thing about
>> implementation-defined behaviour is that it's known to be non-random.
>> So it can be checked, either with autoconf, or with a macro.
>> Would it be acceptable to have both variants of the code and decide
>> among them with such a check?
>> It feels a bit overkill for such a little function, but it is the only
>> way that I see to achieve both need for speed and for well-defined-ness.
>> I guess it won't kill the bootstrap-ubsan errors though.
>
> For such cases you can use the ATTRIBUTE_NO_SANITIZE_UNDEFINED macro,
> that is defined in include/ansidecl.h.
Rather ubsan should not complain about implementation defined behavior (or
should separate those cases out with a different switch compared to undefined
behavior).
Richard.
> --
> Markus