<div dir="ltr">Looks fine to me. (At one point we were deliberately not following GCC in making -fwrapv affect left shift, because it's a bit operation rather than an arithmetic one, but this is ultimately supposed to be a GCC-compatible flag so behaving as they do seems reasonable.)<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 16, 2016 at 9:38 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm not Richard, but I agree with merging it into 3.9 nonetheless. :-)<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Tue, Aug 16, 2016 at 12:30 PM, Hans Wennborg via cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
> Seems reasonable to me if Richard agrees.<br>
><br>
> On Tue, Aug 16, 2016 at 3:01 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk">james@jamesmolloy.co.uk</a>> wrote:<br>
>> Hi Hans,<br>
>><br>
>> [cc. Richard as code owner]<br>
>><br>
>> I'd like to nominate this commit for 3.9. The actual code churn is tiny, and<br>
>> this fixes a problem noticed by and causing pain to the qemu project.<br>
>><br>
>> Cheers,<br>
>><br>
>> James<br>
>><br>
>> On Tue, 16 Aug 2016 at 10:53 James Molloy via cfe-commits<br>
>> <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
>>><br>
>>> Author: jamesm<br>
>>> Date: Tue Aug 16 04:45:36 2016<br>
>>> New Revision: 278786<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=278786&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=278786&view=rev</a><br>
>>> Log:<br>
>>> Left shifts of negative values are defined if -fwrapv is set<br>
>>><br>
>>> This means we shouldn't emit ubsan detection code or warn.<br>
>>> Fixes PR25552.<br>
>>><br>
>>> Added:<br>
>>> cfe/trunk/test/CodeGen/wrapv-<wbr>lshr-sanitize.c<br>
>>> cfe/trunk/test/Sema/negative-<wbr>shift-wrapv.c<br>
>>> Modified:<br>
>>> cfe/trunk/lib/CodeGen/<wbr>CGExprScalar.cpp<br>
>>> cfe/trunk/lib/Sema/SemaExpr.<wbr>cpp<br>
>>><br>
>>> Modified: cfe/trunk/lib/CodeGen/<wbr>CGExprScalar.cpp<br>
>>> URL:<br>
>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=278786&r1=278785&r2=278786&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/CodeGen/<wbr>CGExprScalar.cpp?rev=278786&<wbr>r1=278785&r2=278786&view=diff</a><br>
>>><br>
>>> ==============================<wbr>==============================<wbr>==================<br>
>>> --- cfe/trunk/lib/CodeGen/<wbr>CGExprScalar.cpp (original)<br>
>>> +++ cfe/trunk/lib/CodeGen/<wbr>CGExprScalar.cpp Tue Aug 16 04:45:36 2016<br>
>>> @@ -2714,7 +2714,8 @@ Value *ScalarExprEmitter::EmitShl(<wbr>const<br>
>>> RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false,<br>
>>> "sh_prom");<br>
>>><br>
>>> bool SanitizeBase = CGF.SanOpts.has(SanitizerKind:<wbr>:ShiftBase) &&<br>
>>> - Ops.Ty-><wbr>hasSignedIntegerRepresentation<wbr>();<br>
>>> + Ops.Ty-><wbr>hasSignedIntegerRepresentation<wbr>() &&<br>
>>> + !CGF.getLangOpts().<wbr>isSignedOverflowDefined();<br>
>>> bool SanitizeExponent = CGF.SanOpts.has(SanitizerKind:<wbr>:ShiftExponent);<br>
>>> // OpenCL 6.3j: shift values are effectively % word size of LHS.<br>
>>> if (CGF.getLangOpts().OpenCL)<br>
>>><br>
>>> Modified: cfe/trunk/lib/Sema/SemaExpr.<wbr>cpp<br>
>>> URL:<br>
>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=278786&r1=278785&r2=278786&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>SemaExpr.cpp?rev=278786&r1=<wbr>278785&r2=278786&view=diff</a><br>
>>><br>
>>> ==============================<wbr>==============================<wbr>==================<br>
>>> --- cfe/trunk/lib/Sema/SemaExpr.<wbr>cpp (original)<br>
>>> +++ cfe/trunk/lib/Sema/SemaExpr.<wbr>cpp Tue Aug 16 04:45:36 2016<br>
>>> @@ -8670,7 +8670,7 @@ static void DiagnoseBadShiftValues(Sema&<br>
>>><br>
>>> // If LHS does not have a signed type and non-negative value<br>
>>> // then, the behavior is undefined. Warn about it.<br>
>>> - if (Left.isNegative()) {<br>
>>> + if (Left.isNegative() && !S.getLangOpts().<wbr>isSignedOverflowDefined()) {<br>
>>> S.DiagRuntimeBehavior(Loc, LHS.get(),<br>
>>> S.PDiag(diag::warn_shift_lhs_<wbr>negative)<br>
>>> << LHS.get()->getSourceRange());<br>
>>><br>
>>> Added: cfe/trunk/test/CodeGen/wrapv-<wbr>lshr-sanitize.c<br>
>>> URL:<br>
>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/wrapv-lshr-sanitize.c?rev=278786&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>CodeGen/wrapv-lshr-sanitize.c?<wbr>rev=278786&view=auto</a><br>
>>><br>
>>> ==============================<wbr>==============================<wbr>==================<br>
>>> --- cfe/trunk/test/CodeGen/wrapv-<wbr>lshr-sanitize.c (added)<br>
>>> +++ cfe/trunk/test/CodeGen/wrapv-<wbr>lshr-sanitize.c Tue Aug 16 04:45:36 2016<br>
>>> @@ -0,0 +1,12 @@<br>
>>> +// RUN: %clang_cc1 -fsanitize=shift-base -emit-llvm %s -o - -triple<br>
>>> x86_64-linux-gnu -fwrapv | FileCheck %s<br>
>>> +<br>
>>> +// CHECK-LABEL: @lsh_overflow<br>
>>> +int lsh_overflow(int a, int b) {<br>
>>> + // CHECK-NOT: br<br>
>>> + // CHECK-NOT: call void @__ubsan_<br>
>>> + // CHECK-NOT: call void @llvm.trap<br>
>>> +<br>
>>> + // CHECK: %[[RET:.*]] = shl i32<br>
>>> + // CHECK-NEXT: ret i32 %[[RET]]<br>
>>> + return a << b;<br>
>>> +}<br>
>>><br>
>>> Added: cfe/trunk/test/Sema/negative-<wbr>shift-wrapv.c<br>
>>> URL:<br>
>>> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/negative-shift-wrapv.c?rev=278786&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Sema/<wbr>negative-shift-wrapv.c?rev=<wbr>278786&view=auto</a><br>
>>><br>
>>> ==============================<wbr>==============================<wbr>==================<br>
>>> --- cfe/trunk/test/Sema/negative-<wbr>shift-wrapv.c (added)<br>
>>> +++ cfe/trunk/test/Sema/negative-<wbr>shift-wrapv.c Tue Aug 16 04:45:36 2016<br>
>>> @@ -0,0 +1,9 @@<br>
>>> +// RUN: %clang_cc1 -Wall -ffreestanding -fsyntax-only -fwrapv -verify %s<br>
>>> +<br>
>>> +int test() {<br>
>>> + int i;<br>
>>> + i = -1 << 1; // no-warning<br>
>>> + return i;<br>
>>> +}<br>
>>> +<br>
>>> +// expected-no-diagnostics<br>
>>><br>
>>><br>
>>> ______________________________<wbr>_________________<br>
>>> cfe-commits mailing list<br>
>>> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
> ______________________________<wbr>_________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>