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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
