Hi Jordy,
I applied most of your patch, except that I adopted 'bindDefault' approach
to set the default value, since it does not touch GRExprEngine. Thanks for
working on this!
On Sun, May 30, 2010 at 3:46 PM, Jordy Rose <jedik...@belkadan.com> wrote:
>
> Binding a symbolic region whose type is a reference shows up when the
> reference is an argument, like so:
>
> char t3 (char& r) {
> r = 'c';
> if (r) return r;
> return '0';
> }
>
> The reason for the SymbolicRegion section in canHaveDirectBinding(),
> though, was originally more about having a way to set default values by
> taking advantages of a fact about SymbolicRegions (if you're accessing them
> directly, it's either *p or a reference, or an explicit call to Bind()),
> not enforcing a rule.
>
> For looking up super regions' direct bindings, I tried commenting out that
> entire section, but it makes the case you mentioned fail
> (no-outofbounds.c). Looking at each of the inside IFs:
> 1. SVN blame says this fixed a crash in misc-ps-region-store-x86_64.m.
> 2. Same as your example, but with x not yet defined:
> int x;
> char *p = &x;
> return p[0]; // expected-warning {{Undefined}}
> 3. LazyCompoundVal is LazyCompoundVal...i.e. I don't fully understand what
> circumstances they're used in. On the other hand, if I understand
> correctly, things with compound values should never have direct bindings
> /other/ than LazyCompoundVal.
>
> It seems like if there's any chance of the feature being useful (even
> UnknownVal vs UndefinedVal) this section has to stay. Between
> no-outofbounds.c and the commit message about misc-ps-region-store-x86_64.m
> crashing; I didn't pry much deeper.
>
>
> As for the calloc() part of the patch, it depends on how default values
> should be set from outside RegionStore.cpp, which calloc() needs to be
> properly modeled. (And malloc(), for that matter.) I came up with three
> options:
> 1. Add a new BindDefault() method to Store, make it a no-op in BasicStore
> and implement it for real in RegionStore.
> 2. Add a new parameter to SetExtent(). After all, anything with an
> explicit extent doesn't get the usual BindArray initialization, and so it's
> going to /need/ some kind of initialization. Downside: the rest of the
> Extent code isn't related (though the only extents right now are malloc
> regions and alloca regions, which could both use this).
> 3. Define binding to a symbolic region as setting a default value. Because
> my fix for PR7218 already changed how lookups worked for arrays, this
> didn't seem too far afield, but that's not really a great reason. This
> involved the part about changing *p to use ElementRegions ahead of time, so
> that the only SymbolicRegions that would make it to Bind() would be the
> kind that could have default values (...and references).
>
> I chose #3, but either of the other two would also work. #2 is actually my
> favorite, if we don't mind linking regions and default values conceptually.
>
> Guess I should have submitted this in two pieces. It's because I was
> working on calloc() first that I didn't think of that ahead of time. Let's
> get PR7218 nailed down first, then decide how calloc() can set a default
> value from outside RegionStore.cpp.
>
> Sorry...? And thanks for the review, and pointing me in the right
> direction.
> Jordy
> (IRC: jediknil)
>
>
> On Sat, 29 May 2010 14:55:08 +0800, Zhongxing Xu <xuzhongx...@gmail.com>
> wrote:
> > I'm thinking about the whole logic below. Does it make sense to try to
> get
> > the direct binding of the super region of an element region?
> >
> > I can only think of one case:
> >
> > int x = 1;
> > char *y = &x;
> > y[2];
> >
> > But this case only triggers 'return UnknownVal();' in the last. What
> cases
> > does the 3 'if' above deal with?
> >
> > RegionStore.cpp:1177
> >
> > // Check if the immediate super region has a direct binding.
> > if (const Optional<SVal> &V = getDirectBinding(B, superR)) {
> > if (SymbolRef parentSym = V->getAsSymbol()) {
> > return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R);
> > }
> >
> > if (V->isUnknownOrUndef())
> > return *V;
> >
> > // Handle LazyCompoundVals for the immediate super region. Other
> cases
> > // are handled in 'RetrieveFieldOrElementCommon'.
> > if (const nonloc::LazyCompoundVal *LCV =
> > dyn_cast<nonloc::LazyCompoundVal>(V)) {
> > R = MRMgr.getElementRegionWithSuper(R, LCV->getRegion());
> > return RetrieveElement(LCV->getStore(), R);
> > }
> >
> > // Other cases: give up.
> > return UnknownVal();
> > }
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
