r294800 - Don't let EvaluationModes dictate whether an invalid base is OK

2017-02-10 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Fri Feb 10 16:52:29 2017
New Revision: 294800

URL: http://llvm.org/viewvc/llvm-project?rev=294800&view=rev
Log:
Don't let EvaluationModes dictate whether an invalid base is OK

What we want to actually control this behavior is something more local
than an EvalutationMode. Please see the linked revision for more
discussion on why/etc.

This fixes PR31843.

Differential Revision: https://reviews.llvm.org/D29469

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/object-size.c
cfe/trunk/test/Sema/builtin-object-size.c

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=294800&r1=294799&r2=294800&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb 10 16:52:29 2017
@@ -616,10 +616,12 @@ namespace {
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
 
-  /// Evaluate as a constant expression. Continue evaluating if either:
-  /// - We find a MemberExpr with a base that can't be evaluated.
-  /// - We find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it.
+  /// Evaluate as a constant expression. In certain scenarios, if:
+  /// - we find a MemberExpr with a base that can't be evaluated, or
+  /// - we find a variable initialized with a call to a function that has
+  ///   the alloc_size attribute on it
+  /// then we may consider evaluation to have succeeded.
+  ///
   /// In either case, the LValue returned shall have an invalid base; in 
the
   /// former, the base will be the invalid MemberExpr, in the latter, the
   /// base will be either the alloc_size CallExpr or a CastExpr wrapping
@@ -902,10 +904,6 @@ namespace {
   return KeepGoing;
 }
 
-bool allowInvalidBaseExpr() const {
-  return EvalMode == EM_OffsetFold;
-}
-
 class ArrayInitLoopIndex {
   EvalInfo &Info;
   uint64_t OuterIndex;
@@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev
 static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
 const LValue &This, const Expr *E,
 bool AllowNonLiteralTypes = false);
-static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info);
-static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo &Info);
+static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
+   bool InvalidBaseOK = false);
+static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo &Info,
+bool InvalidBaseOK = false);
 static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
   EvalInfo &Info);
 static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info);
@@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase
   : public ExprEvaluatorBase {
 protected:
   LValue &Result;
+  bool InvalidBaseOK;
   typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;
   typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
 
@@ -4843,9 +4844,14 @@ protected:
 return true;
   }
 
+  bool evaluatePointer(const Expr *E, LValue &Result) {
+return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);
+  }
+
 public:
-  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result) :
-ExprEvaluatorBaseTy(Info), Result(Result) {}
+  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool InvalidBaseOK)
+  : ExprEvaluatorBaseTy(Info), Result(Result),
+InvalidBaseOK(InvalidBaseOK) {}
 
   bool Success(const APValue &V, const Expr *E) {
 Result.setFrom(this->Info.Ctx, V);
@@ -4857,7 +4863,7 @@ public:
 QualType BaseTy;
 bool EvalOK;
 if (E->isArrow()) {
-  EvalOK = EvaluatePointer(E->getBase(), Result, this->Info);
+  EvalOK = evaluatePointer(E->getBase(), Result);
   BaseTy = 
E->getBase()->getType()->castAs()->getPointeeType();
 } else if (E->getBase()->isRValue()) {
   assert(E->getBase()->getType()->isRecordType());
@@ -4868,7 +4874,7 @@ public:
   BaseTy = E->getBase()->getType();
 }
 if (!EvalOK) {
-  if (!this->Info.allowInvalidBaseExpr())
+  if (!InvalidBaseOK)
 return false;
   Result.setInvalid(E);
   return true;
@@ -4962,8 +4968,8 @@ namespace {
 class LValueExprEvaluator
   : public LValueExprEvaluatorBase {
 public:
-  LValueExprEvaluator(EvalInfo &Info, LValue &Result) :
-LValueExprEvaluatorBaseTy(Info, Result) {}
+  LValueExprEvaluator(EvalInfo &Info, LValue &Result, bool InvalidBaseOK) :
+LValueExprEvaluatorBaseTy(Info, Result, InvalidBaseOK) {}
 
   bool VisitVarDecl(const Expr *E, const VarDecl *VD);
   bool VisitUnaryPreIncDec(const UnaryOperator *UO);
@@ -5016,10 +5022,11 @@ public:
 ///  * function desig

Re: r294800 - Don't let EvaluationModes dictate whether an invalid base is OK

2017-02-10 Thread George Burgess IV via cfe-commits
Hi Hans!

This fixes PR31843, which is a release blocker. Once the bots seem happy
with it, can we merge this into the 4.0 branch, please?

(Richard okayed this when he LGTM'ed the patch)

Thanks,
George

On Fri, Feb 10, 2017 at 2:52 PM, George Burgess IV via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: gbiv
> Date: Fri Feb 10 16:52:29 2017
> New Revision: 294800
>
> URL: http://llvm.org/viewvc/llvm-project?rev=294800&view=rev
> Log:
> Don't let EvaluationModes dictate whether an invalid base is OK
>
> What we want to actually control this behavior is something more local
> than an EvalutationMode. Please see the linked revision for more
> discussion on why/etc.
>
> This fixes PR31843.
>
> Differential Revision: https://reviews.llvm.org/D29469
>
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/test/CodeGen/object-size.c
> cfe/trunk/test/Sema/builtin-object-size.c
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCo
> nstant.cpp?rev=294800&r1=294799&r2=294800&view=diff
> 
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb 10 16:52:29 2017
> @@ -616,10 +616,12 @@ namespace {
>/// gets a chance to look at it.
>EM_PotentialConstantExpressionUnevaluated,
>
> -  /// Evaluate as a constant expression. Continue evaluating if
> either:
> -  /// - We find a MemberExpr with a base that can't be evaluated.
> -  /// - We find a variable initialized with a call to a function that
> has
> -  ///   the alloc_size attribute on it.
> +  /// Evaluate as a constant expression. In certain scenarios, if:
> +  /// - we find a MemberExpr with a base that can't be evaluated, or
> +  /// - we find a variable initialized with a call to a function that
> has
> +  ///   the alloc_size attribute on it
> +  /// then we may consider evaluation to have succeeded.
> +  ///
>/// In either case, the LValue returned shall have an invalid base;
> in the
>/// former, the base will be the invalid MemberExpr, in the latter,
> the
>/// base will be either the alloc_size CallExpr or a CastExpr
> wrapping
> @@ -902,10 +904,6 @@ namespace {
>return KeepGoing;
>  }
>
> -bool allowInvalidBaseExpr() const {
> -  return EvalMode == EM_OffsetFold;
> -}
> -
>  class ArrayInitLoopIndex {
>EvalInfo &Info;
>uint64_t OuterIndex;
> @@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev
>  static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
>  const LValue &This, const Expr *E,
>  bool AllowNonLiteralTypes = false);
> -static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info);
> -static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
> &Info);
> +static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
> +   bool InvalidBaseOK = false);
> +static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo &Info,
> +bool InvalidBaseOK = false);
>  static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
>EvalInfo &Info);
>  static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo
> &Info);
> @@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase
>: public ExprEvaluatorBase {
>  protected:
>LValue &Result;
> +  bool InvalidBaseOK;
>typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;
>typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
>
> @@ -4843,9 +4844,14 @@ protected:
>  return true;
>}
>
> +  bool evaluatePointer(const Expr *E, LValue &Result) {
> +return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);
> +  }
> +
>  public:
> -  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result) :
> -ExprEvaluatorBaseTy(Info), Result(Result) {}
> +  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool
> InvalidBaseOK)
> +  : ExprEvaluatorBaseTy(Info), Result(Result),
> +InvalidBaseOK(InvalidBaseOK) {}
>
>bool Success(const APValue &V, const Expr *E) {
>  Result.setFrom(this->Info.Ctx, V);
> @@ -4857,7 +4863,7 @@ public:
>  QualType BaseTy;
>  bool EvalOK;
>  if (E->isArrow()) {
> -  EvalOK = EvaluatePointer(E->getBase(), Result, this->Info);
> +  EvalOK = evaluatePointer(E->getBase(), Result);
>BaseTy = E->getBase()->getType()->castA
> s()->getPointeeType();
>  } else if (E->getBase()->isRValue()) {
>assert(E->getBase()->getType()->isRecordType());
> @@ -4868,7 +4874,7 @@ public:
>BaseTy = E->getBase()->getType();
>  }
>  if (!EvalOK) {
> -  if (!this->Info.allowInvalidBaseExpr())
> +  if (!InvalidBaseOK)
>  return false;
>Result.setInvalid(E);
>  

Re: r294800 - Don't let EvaluationModes dictate whether an invalid base is OK

2017-02-10 Thread Hans Wennborg via cfe-commits
Sgtm. Go ahead and merge when the bots have chewed on it for a bit,
otherwise I'll do it next week.

Thanks,
Hans

On Fri, Feb 10, 2017 at 3:06 PM, George Burgess IV
 wrote:
> Hi Hans!
>
> This fixes PR31843, which is a release blocker. Once the bots seem happy
> with it, can we merge this into the 4.0 branch, please?
>
> (Richard okayed this when he LGTM'ed the patch)
>
> Thanks,
> George
>
> On Fri, Feb 10, 2017 at 2:52 PM, George Burgess IV via cfe-commits
>  wrote:
>>
>> Author: gbiv
>> Date: Fri Feb 10 16:52:29 2017
>> New Revision: 294800
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=294800&view=rev
>> Log:
>> Don't let EvaluationModes dictate whether an invalid base is OK
>>
>> What we want to actually control this behavior is something more local
>> than an EvalutationMode. Please see the linked revision for more
>> discussion on why/etc.
>>
>> This fixes PR31843.
>>
>> Differential Revision: https://reviews.llvm.org/D29469
>>
>> Modified:
>> cfe/trunk/lib/AST/ExprConstant.cpp
>> cfe/trunk/test/CodeGen/object-size.c
>> cfe/trunk/test/Sema/builtin-object-size.c
>>
>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=294800&r1=294799&r2=294800&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb 10 16:52:29 2017
>> @@ -616,10 +616,12 @@ namespace {
>>/// gets a chance to look at it.
>>EM_PotentialConstantExpressionUnevaluated,
>>
>> -  /// Evaluate as a constant expression. Continue evaluating if
>> either:
>> -  /// - We find a MemberExpr with a base that can't be evaluated.
>> -  /// - We find a variable initialized with a call to a function that
>> has
>> -  ///   the alloc_size attribute on it.
>> +  /// Evaluate as a constant expression. In certain scenarios, if:
>> +  /// - we find a MemberExpr with a base that can't be evaluated, or
>> +  /// - we find a variable initialized with a call to a function that
>> has
>> +  ///   the alloc_size attribute on it
>> +  /// then we may consider evaluation to have succeeded.
>> +  ///
>>/// In either case, the LValue returned shall have an invalid base;
>> in the
>>/// former, the base will be the invalid MemberExpr, in the latter,
>> the
>>/// base will be either the alloc_size CallExpr or a CastExpr
>> wrapping
>> @@ -902,10 +904,6 @@ namespace {
>>return KeepGoing;
>>  }
>>
>> -bool allowInvalidBaseExpr() const {
>> -  return EvalMode == EM_OffsetFold;
>> -}
>> -
>>  class ArrayInitLoopIndex {
>>EvalInfo &Info;
>>uint64_t OuterIndex;
>> @@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev
>>  static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
>>  const LValue &This, const Expr *E,
>>  bool AllowNonLiteralTypes = false);
>> -static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo
>> &Info);
>> -static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>> &Info);
>> +static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
>> +   bool InvalidBaseOK = false);
>> +static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>> &Info,
>> +bool InvalidBaseOK = false);
>>  static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
>>EvalInfo &Info);
>>  static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo
>> &Info);
>> @@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase
>>: public ExprEvaluatorBase {
>>  protected:
>>LValue &Result;
>> +  bool InvalidBaseOK;
>>typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;
>>typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
>>
>> @@ -4843,9 +4844,14 @@ protected:
>>  return true;
>>}
>>
>> +  bool evaluatePointer(const Expr *E, LValue &Result) {
>> +return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);
>> +  }
>> +
>>  public:
>> -  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result) :
>> -ExprEvaluatorBaseTy(Info), Result(Result) {}
>> +  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool
>> InvalidBaseOK)
>> +  : ExprEvaluatorBaseTy(Info), Result(Result),
>> +InvalidBaseOK(InvalidBaseOK) {}
>>
>>bool Success(const APValue &V, const Expr *E) {
>>  Result.setFrom(this->Info.Ctx, V);
>> @@ -4857,7 +4863,7 @@ public:
>>  QualType BaseTy;
>>  bool EvalOK;
>>  if (E->isArrow()) {
>> -  EvalOK = EvaluatePointer(E->getBase(), Result, this->Info);
>> +  EvalOK = evaluatePointer(E->getBase(), Result);
>>BaseTy =
>> E->getBase()->getType()->castAs()->getPointeeType();
>>  } else if (E->getBase()->isRValue()) {
>>assert(E-

Re: r294800 - Don't let EvaluationModes dictate whether an invalid base is OK

2017-02-14 Thread Hans Wennborg via cfe-commits
Merged in r295087.

On Fri, Feb 10, 2017 at 5:00 PM, Hans Wennborg  wrote:
> Sgtm. Go ahead and merge when the bots have chewed on it for a bit,
> otherwise I'll do it next week.
>
> Thanks,
> Hans
>
> On Fri, Feb 10, 2017 at 3:06 PM, George Burgess IV
>  wrote:
>> Hi Hans!
>>
>> This fixes PR31843, which is a release blocker. Once the bots seem happy
>> with it, can we merge this into the 4.0 branch, please?
>>
>> (Richard okayed this when he LGTM'ed the patch)
>>
>> Thanks,
>> George
>>
>> On Fri, Feb 10, 2017 at 2:52 PM, George Burgess IV via cfe-commits
>>  wrote:
>>>
>>> Author: gbiv
>>> Date: Fri Feb 10 16:52:29 2017
>>> New Revision: 294800
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=294800&view=rev
>>> Log:
>>> Don't let EvaluationModes dictate whether an invalid base is OK
>>>
>>> What we want to actually control this behavior is something more local
>>> than an EvalutationMode. Please see the linked revision for more
>>> discussion on why/etc.
>>>
>>> This fixes PR31843.
>>>
>>> Differential Revision: https://reviews.llvm.org/D29469
>>>
>>> Modified:
>>> cfe/trunk/lib/AST/ExprConstant.cpp
>>> cfe/trunk/test/CodeGen/object-size.c
>>> cfe/trunk/test/Sema/builtin-object-size.c
>>>
>>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=294800&r1=294799&r2=294800&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb 10 16:52:29 2017
>>> @@ -616,10 +616,12 @@ namespace {
>>>/// gets a chance to look at it.
>>>EM_PotentialConstantExpressionUnevaluated,
>>>
>>> -  /// Evaluate as a constant expression. Continue evaluating if
>>> either:
>>> -  /// - We find a MemberExpr with a base that can't be evaluated.
>>> -  /// - We find a variable initialized with a call to a function that
>>> has
>>> -  ///   the alloc_size attribute on it.
>>> +  /// Evaluate as a constant expression. In certain scenarios, if:
>>> +  /// - we find a MemberExpr with a base that can't be evaluated, or
>>> +  /// - we find a variable initialized with a call to a function that
>>> has
>>> +  ///   the alloc_size attribute on it
>>> +  /// then we may consider evaluation to have succeeded.
>>> +  ///
>>>/// In either case, the LValue returned shall have an invalid base;
>>> in the
>>>/// former, the base will be the invalid MemberExpr, in the latter,
>>> the
>>>/// base will be either the alloc_size CallExpr or a CastExpr
>>> wrapping
>>> @@ -902,10 +904,6 @@ namespace {
>>>return KeepGoing;
>>>  }
>>>
>>> -bool allowInvalidBaseExpr() const {
>>> -  return EvalMode == EM_OffsetFold;
>>> -}
>>> -
>>>  class ArrayInitLoopIndex {
>>>EvalInfo &Info;
>>>uint64_t OuterIndex;
>>> @@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev
>>>  static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
>>>  const LValue &This, const Expr *E,
>>>  bool AllowNonLiteralTypes = false);
>>> -static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo
>>> &Info);
>>> -static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>>> &Info);
>>> +static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
>>> +   bool InvalidBaseOK = false);
>>> +static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>>> &Info,
>>> +bool InvalidBaseOK = false);
>>>  static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
>>>EvalInfo &Info);
>>>  static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo
>>> &Info);
>>> @@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase
>>>: public ExprEvaluatorBase {
>>>  protected:
>>>LValue &Result;
>>> +  bool InvalidBaseOK;
>>>typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;
>>>typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
>>>
>>> @@ -4843,9 +4844,14 @@ protected:
>>>  return true;
>>>}
>>>
>>> +  bool evaluatePointer(const Expr *E, LValue &Result) {
>>> +return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);
>>> +  }
>>> +
>>>  public:
>>> -  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result) :
>>> -ExprEvaluatorBaseTy(Info), Result(Result) {}
>>> +  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool
>>> InvalidBaseOK)
>>> +  : ExprEvaluatorBaseTy(Info), Result(Result),
>>> +InvalidBaseOK(InvalidBaseOK) {}
>>>
>>>bool Success(const APValue &V, const Expr *E) {
>>>  Result.setFrom(this->Info.Ctx, V);
>>> @@ -4857,7 +4863,7 @@ public:
>>>  QualType BaseTy;
>>>  bool EvalOK;
>>>  if (E->isArrow()) {
>>> -  EvalOK = EvaluatePointer(E->getBase(), Result,