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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 16 18:46:57 PDT 2016


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160816/f3abfeed/attachment-0001.html>


More information about the cfe-commits mailing list