r223852 - AST: Don't assume two zero sized objects live at different addresses
David Majnemer
david.majnemer at gmail.com
Sun Dec 14 00:42:34 PST 2014
On Thu, Dec 11, 2014 at 7:14 PM, David Majnemer <david.majnemer at gmail.com>
wrote:
>
> On Thu, Dec 11, 2014 at 7:08 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Thu, Dec 11, 2014 at 6:50 PM, David Majnemer <david.majnemer at gmail.com
>> > wrote:
>>
>>> 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).
>>>
>>
>> OK, so I think the compromise position is:
>>
>> An entity is considered as being possibly-zero-sized if either:
>> 1) The type is incomplete and we're in C, or
>> 2) The type is an array of unknown bound and we're in C++, or
>> 3) The type is complete and its size is zero.
>>
>> We refuse to constant-fold an address comparison if one operand is
>> possibly-zero-sized, and the other is either possibly-zero-sized or
>> evaluates to the address of the start or end of an object of a complete
>> type.
>>
>> Does that make sense to you? I think that's as close as we can get to the
>> standard behavior in C and C++ without miscompiling address comparisons
>> against zero-sized objects.
>>
>
> I think this makes sense. I don't think we want to enshrine that struct S
> { int x[0]; }; will reliably give you a zero sized type in C++ (it doesn't
> in the MS ABI, it has size four) which is exactly what point #2 prevents.
>
I've updated clang to implement the discussed behavior in r224215.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141214/9cf5660a/attachment.html>
More information about the cfe-commits
mailing list