Nomination for 3.9: r278786 - Left shifts of negative values are defined if -fwrapv is set

[Hans Wennborg via cfe-commits]

r278989.

Thanks,
Hans

On Tue, Aug 16, 2016 at 6:46 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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.)
>
>
> On Tue, Aug 16, 2016 at 9:38 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> I'm not Richard, but I agree with merging it into 3.9 nonetheless. :-)
>>
>> ~Aaron
>>
>> On Tue, Aug 16, 2016 at 12:30 PM, Hans Wennborg via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>> > Seems reasonable to me if Richard agrees.
>> >
>> > On Tue, Aug 16, 2016 at 3:01 AM, James Molloy <james at jamesmolloy.co.uk>
>> > wrote:
>> >> Hi Hans,
>> >>
>> >> [cc. Richard as code owner]
>> >>
>> >> I'd like to nominate this commit for 3.9. The actual code churn is
>> >> tiny, and
>> >> this fixes a problem noticed by and causing pain to the qemu project.
>> >>
>> >> Cheers,
>> >>
>> >> James
>> >>
>> >> On Tue, 16 Aug 2016 at 10:53 James Molloy via cfe-commits
>> >> <cfe-commits at lists.llvm.org> wrote:
>> >>>
>> >>> Author: jamesm
>> >>> Date: Tue Aug 16 04:45:36 2016
>> >>> New Revision: 278786
>> >>>
>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=278786&view=rev
>> >>> Log:
>> >>> Left shifts of negative values are defined if -fwrapv is set
>> >>>
>> >>> This means we shouldn't emit ubsan detection code or warn.
>> >>> Fixes PR25552.
>> >>>
>> >>> Added:
>> >>>     cfe/trunk/test/CodeGen/wrapv-lshr-sanitize.c
>> >>>     cfe/trunk/test/Sema/negative-shift-wrapv.c
>> >>> Modified:
>> >>>     cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>> >>>     cfe/trunk/lib/Sema/SemaExpr.cpp
>> >>>
>> >>> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>> >>> URL:
>> >>>
>> >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=278786&r1=278785&r2=278786&view=diff
>> >>>
>> >>>
>> >>> ==============================================================================
>> >>> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
>> >>> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Aug 16 04:45:36 2016
>> >>> @@ -2714,7 +2714,8 @@ Value *ScalarExprEmitter::EmitShl(const
>> >>>      RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false,
>> >>> "sh_prom");
>> >>>
>> >>>    bool SanitizeBase = CGF.SanOpts.has(SanitizerKind::ShiftBase) &&
>> >>> -                      Ops.Ty->hasSignedIntegerRepresentation();
>> >>> +                      Ops.Ty->hasSignedIntegerRepresentation() &&
>> >>> +                      !CGF.getLangOpts().isSignedOverflowDefined();
>> >>>    bool SanitizeExponent =
>> >>> CGF.SanOpts.has(SanitizerKind::ShiftExponent);
>> >>>    // OpenCL 6.3j: shift values are effectively % word size of LHS.
>> >>>    if (CGF.getLangOpts().OpenCL)
>> >>>
>> >>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> >>> URL:
>> >>>
>> >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=278786&r1=278785&r2=278786&view=diff
>> >>>
>> >>>
>> >>> ==============================================================================
>> >>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> >>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Aug 16 04:45:36 2016
>> >>> @@ -8670,7 +8670,7 @@ static void DiagnoseBadShiftValues(Sema&
>> >>>
>> >>>    // If LHS does not have a signed type and non-negative value
>> >>>    // then, the behavior is undefined. Warn about it.
>> >>> -  if (Left.isNegative()) {
>> >>> +  if (Left.isNegative() &&
>> >>> !S.getLangOpts().isSignedOverflowDefined()) {
>> >>>      S.DiagRuntimeBehavior(Loc, LHS.get(),
>> >>>                            S.PDiag(diag::warn_shift_lhs_negative)
>> >>>                              << LHS.get()->getSourceRange());
>> >>>
>> >>> Added: cfe/trunk/test/CodeGen/wrapv-lshr-sanitize.c
>> >>> URL:
>> >>>
>> >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/wrapv-lshr-sanitize.c?rev=278786&view=auto
>> >>>
>> >>>
>> >>> ==============================================================================
>> >>> --- cfe/trunk/test/CodeGen/wrapv-lshr-sanitize.c (added)
>> >>> +++ cfe/trunk/test/CodeGen/wrapv-lshr-sanitize.c Tue Aug 16 04:45:36
>> >>> 2016
>> >>> @@ -0,0 +1,12 @@
>> >>> +// RUN: %clang_cc1 -fsanitize=shift-base -emit-llvm %s -o - -triple
>> >>> x86_64-linux-gnu -fwrapv | FileCheck %s
>> >>> +
>> >>> +// CHECK-LABEL: @lsh_overflow
>> >>> +int lsh_overflow(int a, int b) {
>> >>> +  // CHECK-NOT: br
>> >>> +  // CHECK-NOT: call void @__ubsan_
>> >>> +  // CHECK-NOT: call void @llvm.trap
>> >>> +
>> >>> +  // CHECK:      %[[RET:.*]] = shl i32
>> >>> +  // CHECK-NEXT: ret i32 %[[RET]]
>> >>> +  return a << b;
>> >>> +}
>> >>>
>> >>> Added: cfe/trunk/test/Sema/negative-shift-wrapv.c
>> >>> URL:
>> >>>
>> >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/negative-shift-wrapv.c?rev=278786&view=auto
>> >>>
>> >>>
>> >>> ==============================================================================
>> >>> --- cfe/trunk/test/Sema/negative-shift-wrapv.c (added)
>> >>> +++ cfe/trunk/test/Sema/negative-shift-wrapv.c Tue Aug 16 04:45:36
>> >>> 2016
>> >>> @@ -0,0 +1,9 @@
>> >>> +// RUN: %clang_cc1 -Wall -ffreestanding -fsyntax-only -fwrapv -verify
>> >>> %s
>> >>> +
>> >>> +int test() {
>> >>> +  int i;
>> >>> +  i = -1 << 1; // no-warning
>> >>> +  return i;
>> >>> +}
>> >>> +
>> >>> +// expected-no-diagnostics
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> cfe-commits mailing list
>> >>> cfe-commits at lists.llvm.org
>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>