On Thu, Aug 13, 2015 at 1:08 PM, Markus Trippelsdorf <mar...@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 <l...@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 <l...@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
