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

[David Majnemer]

On Thu, Dec 11, 2014 at 1:12 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On Thu, Dec 11, 2014 at 1:02 PM, David Majnemer <david.majnemer at gmail.com>
> wrote:
>
>> On Thu, Dec 11, 2014 at 12:45 PM, Richard Smith <richard at metafoo.co.uk>
>> 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.
>>>>
>>>>
>>>>>
>>>>>          // 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.
>>>>
>>>
>>> That doesn't make them right. =) C++ does not have zero-sized types, nor
>>> the possibility of the above objects being at the same address. Per its
>>> constant evaluation rules, the above expression *is* a constant expression,
>>> and we are required to treat it as such. In this regard, zero-sized types
>>> are not a conforming extension.
>>>
>>
>> They are both (potentially) one-past-the-end objects though.  I think our
>> hands are tied, seeing as how we use the constant expression evaluation to
>> CodeGen if conditions and what-not.
>>
>
> I don't think it's so clear. No valid C or C++ program can have an array
> of zero bound, and I think we should generally prioritize doing the right
> thing on conforming code over giving better semantics to a language
> extension. I think the question is, does any real code rely on this not
> being constant-folded for incomplete arrays that turn out to have a bound
> of zero?
>

I'm not entirely sure how we can answer that but I found the following
after a minute of digging around the linux kernel:

kernel_memsize = kernel_size + (_end - _edata);

_end and _edata are two linker generated symbols.  If people are
subtracting these things, I can imagine that they are also comparing them.


>
> In any case, the incomplete-type case should be restricted to incomplete
> arrays, since incomplete class types can never have zero size in C++.
>

I completely agree. In an ideal world, I'd stuff this zero-sized
mumbo-jumbo under a hypothetical -fgcc-compatibility (or something similar).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141211/7e0e7ea8/attachment.html>