r223852 - AST: Don't assume two zero sized objects live at different addresses

[Richard Smith]

On Tue, Dec 9, 2014 at 3:32 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> Author: majnemer
> Date: Tue Dec  9 17:32:34 2014
> New Revision: 223852
>
> URL: http://llvm.org/viewvc/llvm-project?rev=223852&view=rev
> Log:
> AST: Don't assume two zero sized objects live at different addresses
>
> Zero sized objects may overlap with each other or any other object.
>
> This fixes PR21786.
>
> Modified:
>     cfe/trunk/lib/AST/ExprConstant.cpp
>     cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=223852&r1=223851&r2=223852&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Dec  9 17:32:34 2014
> @@ -1422,6 +1422,12 @@ static bool IsWeakLValue(const LValue &V
>    return Decl && Decl->isWeak();
>  }
>
> +static bool isZeroSized(const LValue &Value) {
> +  const ValueDecl *Decl = GetLValueBaseDecl(Value);
> +  return Decl && isa<VarDecl>(Decl) &&
> +         Decl->getASTContext().getTypeSize(Decl->getType()) == 0;
> +}
> +
>  static bool EvalPointerValueAsBool(const APValue &Value, bool &Result) {
>    // A null base expression indicates a null pointer.  These are always
>    // evaluatable, and they are false unless the offset is zero.
> @@ -6979,6 +6985,10 @@ bool IntExprEvaluator::VisitBinaryOperat
>              (RHSValue.Base && RHSValue.Offset.isZero() &&
>               isOnePastTheEndOfCompleteObject(Info.Ctx, LHSValue)))
>            return Error(E);
> +        // We can't tell whether an object is at the same address as
> another
> +        // zero sized object.
> +        if (isZeroSized(LHSValue) || isZeroSized(RHSValue))
> +          return Error(E);
>

We can do better here: one of the pointers must be to a zero-sized object,
and the other must be a past-the-end pointer (where a pointer to a
zero-sized object is considered to be a past-the-end pointer).

         // Pointers with different bases cannot represent the same object.
>          // (Note that clang defaults to -fmerge-all-constants, which can
>          // lead to inconsistent results for comparisons involving the
> address
>
> Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=223852&r1=223851&r2=223852&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original)
> +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Tue Dec  9
> 17:32:34 2014
> @@ -1955,3 +1955,9 @@ namespace EmptyClass {
>    constexpr E2 e2b(e2); // expected-error {{constant expression}}
> expected-note{{read of non-const}} expected-note {{in call}}
>    constexpr E3 e3b(e3);
>  }
> +
> +namespace PR21786 {
> +  extern void (*start[])();
> +  extern void (*end[])();
> +  static_assert(&start != &end, ""); // expected-error {{constant
> expression}}
> +}
>

This testcase looks like valid C++ code to me; the comparison is a constant
expression under the C++ rules and evaluates to true. I don't think we can
apply this check in this case, only when we have a complete type that is
zero-sized. That means we'll constant-fold equality comparisons to 'false'
even if they turn out to be true, but that seems to be unavoidable.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141211/c1167472/attachment.html>