[cfe-commits] r154794 - in /cfe/trunk: lib/AST/ExprConstant.cpp test/Sema/const-eval-64.c test/Sema/const-eval.c

[Richard Smith]

Hi Eli,

This "breaks" pointer arithmetic for some cases with undefined behavior
(but which are probably common anyway):

  int a;
  bool b = &a - 1 < &a;

I suspect (without evidence) that this happens frequently in real
code. Perhaps we should refuse to fold a pointer comparison for a non-null
base if either (unsigned) offset is greater than the size of the base
object?

On Sun, Apr 15, 2012 at 9:30 PM, Eli Friedman <eli.friedman at gmail.com>wrote:

> Author: efriedma
> Date: Sun Apr 15 23:30:08 2012
> New Revision: 154794
>
> URL: http://llvm.org/viewvc/llvm-project?rev=154794&view=rev
> Log:
> Make constant evaluation for pointer comparisons work correctly for some
> uncommon cases.  <rdar://problem/10962435>.
>
>
> Added:
>    cfe/trunk/test/Sema/const-eval-64.c
> Modified:
>    cfe/trunk/lib/AST/ExprConstant.cpp
>    cfe/trunk/test/Sema/const-eval.c
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=154794&r1=154793&r2=154794&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Sun Apr 15 23:30:08 2012
> @@ -5088,14 +5088,26 @@
>         }
>       }
>
> +      // The comparison here must be unsigned, and performed with the same
> +      // width as the pointer.
> +      // FIXME: Knowing the base is the same for the LHS and RHS isn't
> enough
> +      // for relational operators.
> +      unsigned PtrSize = Info.Ctx.getTypeSize(LHSTy);
> +      uint64_t CompareLHS = LHSOffset.getQuantity();
> +      uint64_t CompareRHS = RHSOffset.getQuantity();
> +      assert(PtrSize <= 64 && "Unexpected pointer width");
> +      uint64_t Mask = ~0ULL >> (64 - PtrSize);
> +      CompareLHS &= Mask;
> +      CompareRHS &= Mask;
> +
>       switch (E->getOpcode()) {
>       default: llvm_unreachable("missing comparison operator");
> -      case BO_LT: return Success(LHSOffset < RHSOffset, E);
> -      case BO_GT: return Success(LHSOffset > RHSOffset, E);
> -      case BO_LE: return Success(LHSOffset <= RHSOffset, E);
> -      case BO_GE: return Success(LHSOffset >= RHSOffset, E);
> -      case BO_EQ: return Success(LHSOffset == RHSOffset, E);
> -      case BO_NE: return Success(LHSOffset != RHSOffset, E);
> +      case BO_LT: return Success(CompareLHS < CompareRHS, E);
> +      case BO_GT: return Success(CompareLHS > CompareRHS, E);
> +      case BO_LE: return Success(CompareLHS <= CompareRHS, E);
> +      case BO_GE: return Success(CompareLHS >= CompareRHS, E);
> +      case BO_EQ: return Success(CompareLHS == CompareRHS, E);
> +      case BO_NE: return Success(CompareLHS != CompareRHS, E);
>       }
>     }
>   }
>
> Added: cfe/trunk/test/Sema/const-eval-64.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/const-eval-64.c?rev=154794&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Sema/const-eval-64.c (added)
> +++ cfe/trunk/test/Sema/const-eval-64.c Sun Apr 15 23:30:08 2012
> @@ -0,0 +1,7 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s
> +
> +#define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char
> qq[expr];});
> +
> +// <rdar://problem/10962435>
> +EVAL_EXPR(1, ((char*)-1LL) + 1 == 0 ? 1 : -1)
> +EVAL_EXPR(2, ((char*)-1LL) + 1 < (char*) -1 ? 1 : -1)
>
> Modified: cfe/trunk/test/Sema/const-eval.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/const-eval.c?rev=154794&r1=154793&r2=154794&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/const-eval.c (original)
> +++ cfe/trunk/test/Sema/const-eval.c Sun Apr 15 23:30:08 2012
> @@ -121,3 +121,7 @@
>  // <rdar://problem/11205586>
>  // (Make sure we continue to reject this.)
>  EVAL_EXPR(44, "x"[0]); // expected-error {{variable length array}}
> +
> +// <rdar://problem/10962435>
> +EVAL_EXPR(45, ((char*)-1) + 1 == 0 ? 1 : -1)
> +EVAL_EXPR(46, ((char*)-1) + 1 < (char*) -1 ? 1 : -1)
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120415/2827c9ba/attachment.html>