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

[Richard Smith]

On Thu, Dec 11, 2014 at 12:20 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> On Thu, Dec 11, 2014 at 11:47 AM, David Majnemer <david.majnemer at gmail.com
> > wrote:
>
>> On Thu, Dec 11, 2014 at 11:28 AM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>>
>>> 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).
>>>
>>
>> Ah, clever.
>>
>
> Actually, must the other be a past-the-end pointer?
>
> #include <stdio.h>
> extern "C" void (*__preinit_array_start[])();
> void __attribute((section(".preinit_array"))) f() {}
> int main() {
>   printf("%p\n", &__preinit_array_start[0]);
>   printf("%p\n", &f);
> }
>
> gives me:
> 0x600de0
> 0x600de0
>

Good point. We can say: one must be zero-sized and the other must be a
pointer to either the start or the end of a complete object. (A zero-sized
complete object can't be in the middle of another object.)


>          // 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.
>>>
>>
>> I don't quite understand why we should fold that comparison to false, GCC
>> and ICC both consider that expression to be non-constant.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141211/aef7d944/attachment.html>