Analyzer: SVal-building idioms

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Analyzer: SVal-building idioms

Jordy Rose

In trying to excise the remnants of getSizeInElements() from the code, I
realized we don't have a great place for SVal-building idioms. There's
SValuator::EvalEQ, but nothing else. In looking through the code, I see two
more useful idioms that it might be good to box up into methods:

// perhaps with LengthTy defaulting to SizeTy or ArrayIndexTy
SVal ConvertToCharUnits(const GRState *state, SVal Length,
                        QualType EleTy, QualType LengthTy) {
  // return Length * (LengthTy) sizeof(EleTy)
}

// To replace the rather useless AssumeInBound,
// since this could be used to change the state
SVal BuildBoundCheck(const GRState *state, SVal Index, SVal Limit,
                     QualType IndexTy = ArrayIndexTy) {
  // 0 <= Index < Limit
  // is the same as
  // Index+MIN < Limit+MIN
  // which the constraint manager can handle now
  // as long as either Index or Limit is constant
  // and Limit is positive.
  // return (Index + getMinValue(IndexTy)) < (Limit +
getMinValue(IndexTy))
}

It's not that these are so complex that they can't be used on their own,
but it would reduce duplicated code. But where would these methods go? On
SValuator, which Ted's already suggested is more of an SValBuilder? Or on
ValueManager, since the code makes use of its ArrayIndexTy? (Though we
could add a getIndexType() method very easily.)

Or are these not things that need to be packaged up? I feel like the
bounds-checking would be useful, at least, since that /is/ something used
in multiple places.

Jordy
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Analyzer: SVal-building idioms

Ted Kremenek
Another idea: combine ValueManager and SValuator into a single class, and put the functionality there?

I like the idea of moving most SVal construction to an SValBuilder class.  The base implementation would provide essential functionality (such as what is in ValueManager) and subclasses can implement functionality for more more advanced SVal construction.  There doesn't seem to have much value to keeping ValueManager and SValBuilder separate at this point, and most clients who have reference to one have a reference to the other.

By unifying them, at defines a consistent place to put this functionality.

On Jul 6, 2010, at 2:21 PM, Jordy Rose wrote:

>
> In trying to excise the remnants of getSizeInElements() from the code, I
> realized we don't have a great place for SVal-building idioms. There's
> SValuator::EvalEQ, but nothing else. In looking through the code, I see two
> more useful idioms that it might be good to box up into methods:
>
> // perhaps with LengthTy defaulting to SizeTy or ArrayIndexTy
> SVal ConvertToCharUnits(const GRState *state, SVal Length,
>                        QualType EleTy, QualType LengthTy) {
>  // return Length * (LengthTy) sizeof(EleTy)
> }
>
> // To replace the rather useless AssumeInBound,
> // since this could be used to change the state
> SVal BuildBoundCheck(const GRState *state, SVal Index, SVal Limit,
>                     QualType IndexTy = ArrayIndexTy) {
>  // 0 <= Index < Limit
>  // is the same as
>  // Index+MIN < Limit+MIN
>  // which the constraint manager can handle now
>  // as long as either Index or Limit is constant
>  // and Limit is positive.
>  // return (Index + getMinValue(IndexTy)) < (Limit +
> getMinValue(IndexTy))
> }
>
> It's not that these are so complex that they can't be used on their own,
> but it would reduce duplicated code. But where would these methods go? On
> SValuator, which Ted's already suggested is more of an SValBuilder? Or on
> ValueManager, since the code makes use of its ArrayIndexTy? (Though we
> could add a getIndexType() method very easily.)
>
> Or are these not things that need to be packaged up? I feel like the
> bounds-checking would be useful, at least, since that /is/ something used
> in multiple places.
>
> Jordy


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Analyzer: SVal-building idioms

Zhongxing Xu
I think these constructions are more appropriate to put into
ValueManager, since ValueManager constructs SVals, but SValuator
perform evaluations.

On Wed, Jul 7, 2010 at 6:08 AM, Ted Kremenek <[hidden email]> wrote:

> Another idea: combine ValueManager and SValuator into a single class, and put the functionality there?
>
> I like the idea of moving most SVal construction to an SValBuilder class.  The base implementation would provide essential functionality (such as what is in ValueManager) and subclasses can implement functionality for more more advanced SVal construction.  There doesn't seem to have much value to keeping ValueManager and SValBuilder separate at this point, and most clients who have reference to one have a reference to the other.
>
> By unifying them, at defines a consistent place to put this functionality.
>
> On Jul 6, 2010, at 2:21 PM, Jordy Rose wrote:
>
>>
>> In trying to excise the remnants of getSizeInElements() from the code, I
>> realized we don't have a great place for SVal-building idioms. There's
>> SValuator::EvalEQ, but nothing else. In looking through the code, I see two
>> more useful idioms that it might be good to box up into methods:
>>
>> // perhaps with LengthTy defaulting to SizeTy or ArrayIndexTy
>> SVal ConvertToCharUnits(const GRState *state, SVal Length,
>>                        QualType EleTy, QualType LengthTy) {
>>  // return Length * (LengthTy) sizeof(EleTy)
>> }
>>
>> // To replace the rather useless AssumeInBound,
>> // since this could be used to change the state
>> SVal BuildBoundCheck(const GRState *state, SVal Index, SVal Limit,
>>                     QualType IndexTy = ArrayIndexTy) {
>>  // 0 <= Index < Limit
>>  // is the same as
>>  // Index+MIN < Limit+MIN
>>  // which the constraint manager can handle now
>>  // as long as either Index or Limit is constant
>>  // and Limit is positive.
>>  // return (Index + getMinValue(IndexTy)) < (Limit +
>> getMinValue(IndexTy))
>> }
>>
>> It's not that these are so complex that they can't be used on their own,
>> but it would reduce duplicated code. But where would these methods go? On
>> SValuator, which Ted's already suggested is more of an SValBuilder? Or on
>> ValueManager, since the code makes use of its ArrayIndexTy? (Though we
>> could add a getIndexType() method very easily.)
>>
>> Or are these not things that need to be packaged up? I feel like the
>> bounds-checking would be useful, at least, since that /is/ something used
>> in multiple places.
>>
>> Jordy
>
>

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Analyzer: SVal-building idioms

Zhongxing Xu
I find SValuator also constructs SVals. It seems a bit hard to
separate construct from evaluate. We construct when we cannot
evaluate. Combining them is reasonable.

On Wed, Jul 7, 2010 at 8:17 AM, Zhongxing Xu <[hidden email]> wrote:

> I think these constructions are more appropriate to put into
> ValueManager, since ValueManager constructs SVals, but SValuator
> perform evaluations.
>
> On Wed, Jul 7, 2010 at 6:08 AM, Ted Kremenek <[hidden email]> wrote:
>> Another idea: combine ValueManager and SValuator into a single class, and put the functionality there?
>>
>> I like the idea of moving most SVal construction to an SValBuilder class.  The base implementation would provide essential functionality (such as what is in ValueManager) and subclasses can implement functionality for more more advanced SVal construction.  There doesn't seem to have much value to keeping ValueManager and SValBuilder separate at this point, and most clients who have reference to one have a reference to the other.
>>
>> By unifying them, at defines a consistent place to put this functionality.
>>
>> On Jul 6, 2010, at 2:21 PM, Jordy Rose wrote:
>>
>>>
>>> In trying to excise the remnants of getSizeInElements() from the code, I
>>> realized we don't have a great place for SVal-building idioms. There's
>>> SValuator::EvalEQ, but nothing else. In looking through the code, I see two
>>> more useful idioms that it might be good to box up into methods:
>>>
>>> // perhaps with LengthTy defaulting to SizeTy or ArrayIndexTy
>>> SVal ConvertToCharUnits(const GRState *state, SVal Length,
>>>                        QualType EleTy, QualType LengthTy) {
>>>  // return Length * (LengthTy) sizeof(EleTy)
>>> }
>>>
>>> // To replace the rather useless AssumeInBound,
>>> // since this could be used to change the state
>>> SVal BuildBoundCheck(const GRState *state, SVal Index, SVal Limit,
>>>                     QualType IndexTy = ArrayIndexTy) {
>>>  // 0 <= Index < Limit
>>>  // is the same as
>>>  // Index+MIN < Limit+MIN
>>>  // which the constraint manager can handle now
>>>  // as long as either Index or Limit is constant
>>>  // and Limit is positive.
>>>  // return (Index + getMinValue(IndexTy)) < (Limit +
>>> getMinValue(IndexTy))
>>> }
>>>
>>> It's not that these are so complex that they can't be used on their own,
>>> but it would reduce duplicated code. But where would these methods go? On
>>> SValuator, which Ted's already suggested is more of an SValBuilder? Or on
>>> ValueManager, since the code makes use of its ArrayIndexTy? (Though we
>>> could add a getIndexType() method very easily.)
>>>
>>> Or are these not things that need to be packaged up? I feel like the
>>> bounds-checking would be useful, at least, since that /is/ something used
>>> in multiple places.
>>>
>>> Jordy
>>
>>
>

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Analyzer: SVal-building idioms

Ted Kremenek
Right.  As "evaluation" becomes more lazy (e.g., by "assuming" SVals as constraints) the distinction between an SVal representing a "value" and one representing an "evaluation of an expression" should intentionally become blurred.  In truth no SVal really becomes evaluated until either a constrained is assumed by the ConstraintManager or a load/store is made by the StoreManager.

On Jul 6, 2010, at 5:21 PM, Zhongxing Xu wrote:

> I find SValuator also constructs SVals. It seems a bit hard to
> separate construct from evaluate. We construct when we cannot
> evaluate. Combining them is reasonable.
>
> On Wed, Jul 7, 2010 at 8:17 AM, Zhongxing Xu <[hidden email]> wrote:
>> I think these constructions are more appropriate to put into
>> ValueManager, since ValueManager constructs SVals, but SValuator
>> perform evaluations.
>>
>> On Wed, Jul 7, 2010 at 6:08 AM, Ted Kremenek <[hidden email]> wrote:
>>> Another idea: combine ValueManager and SValuator into a single class, and put the functionality there?
>>>
>>> I like the idea of moving most SVal construction to an SValBuilder class.  The base implementation would provide essential functionality (such as what is in ValueManager) and subclasses can implement functionality for more more advanced SVal construction.  There doesn't seem to have much value to keeping ValueManager and SValBuilder separate at this point, and most clients who have reference to one have a reference to the other.
>>>
>>> By unifying them, at defines a consistent place to put this functionality.
>>>
>>> On Jul 6, 2010, at 2:21 PM, Jordy Rose wrote:
>>>
>>>>
>>>> In trying to excise the remnants of getSizeInElements() from the code, I
>>>> realized we don't have a great place for SVal-building idioms. There's
>>>> SValuator::EvalEQ, but nothing else. In looking through the code, I see two
>>>> more useful idioms that it might be good to box up into methods:
>>>>
>>>> // perhaps with LengthTy defaulting to SizeTy or ArrayIndexTy
>>>> SVal ConvertToCharUnits(const GRState *state, SVal Length,
>>>>                        QualType EleTy, QualType LengthTy) {
>>>>  // return Length * (LengthTy) sizeof(EleTy)
>>>> }
>>>>
>>>> // To replace the rather useless AssumeInBound,
>>>> // since this could be used to change the state
>>>> SVal BuildBoundCheck(const GRState *state, SVal Index, SVal Limit,
>>>>                     QualType IndexTy = ArrayIndexTy) {
>>>>  // 0 <= Index < Limit
>>>>  // is the same as
>>>>  // Index+MIN < Limit+MIN
>>>>  // which the constraint manager can handle now
>>>>  // as long as either Index or Limit is constant
>>>>  // and Limit is positive.
>>>>  // return (Index + getMinValue(IndexTy)) < (Limit +
>>>> getMinValue(IndexTy))
>>>> }
>>>>
>>>> It's not that these are so complex that they can't be used on their own,
>>>> but it would reduce duplicated code. But where would these methods go? On
>>>> SValuator, which Ted's already suggested is more of an SValBuilder? Or on
>>>> ValueManager, since the code makes use of its ArrayIndexTy? (Though we
>>>> could add a getIndexType() method very easily.)
>>>>
>>>> Or are these not things that need to be packaged up? I feel like the
>>>> bounds-checking would be useful, at least, since that /is/ something used
>>>> in multiple places.
>>>>
>>>> Jordy
>>>
>>>
>>


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev