On Thu, Dec 11, 2014 at 7:14 PM, David Majnemer <[email protected]> wrote: > > On Thu, Dec 11, 2014 at 7:08 PM, Richard Smith <[email protected]> > wrote: > >> On Thu, Dec 11, 2014 at 6:50 PM, David Majnemer <[email protected] >> > wrote: >> >>> On Thu, Dec 11, 2014 at 1:12 PM, Richard Smith <[email protected]> >>> wrote: >>> >>>> On Thu, Dec 11, 2014 at 1:02 PM, David Majnemer < >>>> [email protected]> wrote: >>>> >>>>> On Thu, Dec 11, 2014 at 12:45 PM, Richard Smith <[email protected] >>>>> > wrote: >>>>> >>>>>> On Thu, Dec 11, 2014 at 11:47 AM, David Majnemer < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> On Thu, Dec 11, 2014 at 11:28 AM, Richard Smith < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> On Tue, Dec 9, 2014 at 3:32 PM, David Majnemer < >>>>>>>> [email protected]> 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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
