[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Artem Dergachev via Phabricator via cfe-commits]

NoQ commandeered this revision.
NoQ added a reviewer: k-wisniewski.
NoQ added a comment.

Seems to become outdated with https://reviews.llvm.org/D27090.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Artem Dergachev via cfe-commits]

NoQ added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

zaks.anna wrote:
> NoQ wrote:
> > zaks.anna wrote:
> > > k-wisniewski wrote:
> > > > NoQ wrote:
> > > > > zaks.anna wrote:
> > > > > > LocationContext can be obtained by calling 
> > > > > > CallEvent::getLocationContext(). I do not think that adding another 
> > > > > > parameter here buys us much. Am I missing something?
> > > > > CallEvent* is optional here - it's there only for invalidations 
> > > > > through calls.
> > > > How about the situation when this callback is triggered by something 
> > > > other than a call, like variable assignment? I've tried using that 
> > > > location context and had lots of segfaults as in such cases it appeared 
> > > > to be null. Do you have some info that it should be non-null in such a 
> > > > case?
> > > Ok, makes sense. Have you looked into providing a checker context there? 
> > > How much more difficult would that be? If that is too difficult, adding 
> > > LocationContext is good as well.
> > > 
> > > If Call is optional and LocationContext is not, should the Call parameter 
> > > be last.
> > If we add a CheckerContext, then we may end up calling callbacks that split 
> > states within callbacks that split states, and that'd be quite a mess to 
> > support.
> > 
> > Right now a checker may trigger `checkRegionChanges()` within its callback 
> > by manipulating the Store, which already leads to callbacks within 
> > callbacks, but that's bearable as long as you can't add transitions within 
> > the inner callbacks.
> This argument by itself does not seem to be preventing us from providing 
> CheckerContext. For example, we could disallow splitting states (ex: by 
> setting some flag within the CheckerContext) but still provide most of the 
> convenience APIs. 
> 
> The advantage of providing CheckerContext is that it is a class that provides 
> access to different analyzer APIs that the checker writer might want to 
> access. It is also familiar and somewhat expected as it is available in most 
> other cases. It might be difficult to pipe enough information to construct 
> it... I suspect that is the reason we do not provide it in this case.
> 
> I am OK with the patch as is but it would be good to explore if we could 
> extend this API by providing the full CheckerContext.
Hmm, with this kind of plan we should probably move all transition 
functionality into a sub-class of `CheckerContext`(?) So that it was obvious 
from the signature that some of those are restricted, and some are 
full-featured.

This definitely sounds like a big task, useful though, maybe a separate patch 
over all callbacks might be a good idea.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

NoQ wrote:
> zaks.anna wrote:
> > k-wisniewski wrote:
> > > NoQ wrote:
> > > > zaks.anna wrote:
> > > > > LocationContext can be obtained by calling 
> > > > > CallEvent::getLocationContext(). I do not think that adding another 
> > > > > parameter here buys us much. Am I missing something?
> > > > CallEvent* is optional here - it's there only for invalidations through 
> > > > calls.
> > > How about the situation when this callback is triggered by something 
> > > other than a call, like variable assignment? I've tried using that 
> > > location context and had lots of segfaults as in such cases it appeared 
> > > to be null. Do you have some info that it should be non-null in such a 
> > > case?
> > Ok, makes sense. Have you looked into providing a checker context there? 
> > How much more difficult would that be? If that is too difficult, adding 
> > LocationContext is good as well.
> > 
> > If Call is optional and LocationContext is not, should the Call parameter 
> > be last.
> If we add a CheckerContext, then we may end up calling callbacks that split 
> states within callbacks that split states, and that'd be quite a mess to 
> support.
> 
> Right now a checker may trigger `checkRegionChanges()` within its callback by 
> manipulating the Store, which already leads to callbacks within callbacks, 
> but that's bearable as long as you can't add transitions within the inner 
> callbacks.
This argument by itself does not seem to be preventing us from providing 
CheckerContext. For example, we could disallow splitting states (ex: by setting 
some flag within the CheckerContext) but still provide most of the convenience 
APIs. 

The advantage of providing CheckerContext is that it is a class that provides 
access to different analyzer APIs that the checker writer might want to access. 
It is also familiar and somewhat expected as it is available in most other 
cases. It might be difficult to pipe enough information to construct it... I 
suspect that is the reason we do not provide it in this case.

I am OK with the patch as is but it would be good to explore if we could extend 
this API by providing the full CheckerContext.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Artem Dergachev via cfe-commits]

NoQ added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

zaks.anna wrote:
> k-wisniewski wrote:
> > NoQ wrote:
> > > zaks.anna wrote:
> > > > LocationContext can be obtained by calling 
> > > > CallEvent::getLocationContext(). I do not think that adding another 
> > > > parameter here buys us much. Am I missing something?
> > > CallEvent* is optional here - it's there only for invalidations through 
> > > calls.
> > How about the situation when this callback is triggered by something other 
> > than a call, like variable assignment? I've tried using that location 
> > context and had lots of segfaults as in such cases it appeared to be null. 
> > Do you have some info that it should be non-null in such a case?
> Ok, makes sense. Have you looked into providing a checker context there? How 
> much more difficult would that be? If that is too difficult, adding 
> LocationContext is good as well.
> 
> If Call is optional and LocationContext is not, should the Call parameter be 
> last.
If we add a CheckerContext, then we may end up calling callbacks that split 
states within callbacks that split states, and that'd be quite a mess to 
support.

Right now a checker may trigger `checkRegionChanges()` within its callback by 
manipulating the Store, which already leads to callbacks within callbacks, but 
that's bearable as long as you can't add transitions within the inner callbacks.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

k-wisniewski wrote:
> NoQ wrote:
> > zaks.anna wrote:
> > > LocationContext can be obtained by calling 
> > > CallEvent::getLocationContext(). I do not think that adding another 
> > > parameter here buys us much. Am I missing something?
> > CallEvent* is optional here - it's there only for invalidations through 
> > calls.
> How about the situation when this callback is triggered by something other 
> than a call, like variable assignment? I've tried using that location context 
> and had lots of segfaults as in such cases it appeared to be null. Do you 
> have some info that it should be non-null in such a case?
Ok, makes sense. Have you looked into providing a checker context there? How 
much more difficult would that be? If that is too difficult, adding 
LocationContext is good as well.

If Call is optional and LocationContext is not, should the Call parameter be 
last.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:333
+   ProgramStateRef state,
+   const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->wantsRegionChangeUpdate(state, LCtx);

k-wisniewski wrote:
> NoQ wrote:
> > zaks.anna wrote:
> > > What is the scenario in which you need the context here? The idea is that 
> > > the checker would be interested in region changes if it is already 
> > > tracking information in the state. For that check, the information in the 
> > > state is sufficient. What is your scenario?
> > > 
> > > Also, For any checker API change, we need to update the 
> > > CheckerDocumentation.cpp.
> > Environment, which is a significant portion of the state, is essentially 
> > unaccessible without a location context.
> > 
> > Call stack is also an important bit of information to any checker that does 
> > something special to support/exploit the inlining IPA - there are many 
> > checkers that scan the call stack, but so far none of them needed to 
> > subscribe to checkRegionChanges; their possible use case may look like "we 
> > want an update only if a certain condition was met within the current stack 
> > frame".
> > 
> > For the recursion checker, i think, the point is "if current frame is 
> > spoiled, we no longer want updates" (which should probably be fixed in the 
> > checker patch: even though the callback is broken, it'd take one less 
> > action to fix it if we decide to).
> Well I've added it to be consistent with checkRegionChanges, but AFAIK this 
> callback is no longer necessary and there was a discussion about getting rid 
> of it altogether. Maybe it's a good moment ot purge it?
I don't recall where that discussion has ended. If the callback is not used, 
I'd be fine with removing it. (That would need to be a separate patch.)


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Krzysztof Wiśniewski via cfe-commits]

k-wisniewski added a comment.

Hi again!

Thanks for review! I'll upload updated version of this patch today in the 
evening/tomorrow. As Anna suggested I'll split it into two parts - one 
regarding extraction of argument SVals for StackFrameCtx and another one adding 
LocationContext to check::RegionChanges interface. I have commented inline  on 
why I think this change is needed.

Cheers!




Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

zaks.anna wrote:
> LocationContext can be obtained by calling CallEvent::getLocationContext(). I 
> do not think that adding another parameter here buys us much. Am I missing 
> something?
How about the situation when this callback is triggered by something other than 
a call, like variable assignment? I've tried using that location context and 
had lots of segfaults as in such cases it appeared to be null. Do you have some 
info that it should be non-null in such a case?



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:333
+   ProgramStateRef state,
+   const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->wantsRegionChangeUpdate(state, LCtx);

zaks.anna wrote:
> What is the scenario in which you need the context here? The idea is that the 
> checker would be interested in region changes if it is already tracking 
> information in the state. For that check, the information in the state is 
> sufficient. What is your scenario?
> 
> Also, For any checker API change, we need to update the 
> CheckerDocumentation.cpp.
Well I've added it to be consistent with checkRegionChanges, but AFAIK this 
callback is no longer necessary and there was a discussion about getting rid of 
it altogether. Maybe it's a good moment ot purge it?



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:231
 
   ProgramStateRef bindLoc(Loc location,
   SVal V,

NoQ wrote:
> Because this API becomes more complicated, i think we should add some 
> docstrings here, explaining the meaning of the location context parameter, in 
> particular.
Will do!



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:741
+// because the call wasn't modeled in the first place.
+const VarDecl *ArgDecl = FunctionDecl->parameters()[ArgIdx];
+const Loc ArgLoc = getLValue(ArgDecl, SFC);

NoQ wrote:
> a.sidorin wrote:
> > Unfortunately, this code does not consider the fact that argument values 
> > may be overwritten. If we want to get initial values, we should find 
> > another way.
> Uhm, yeah, this is in fact a problem!
> 
> The purpose of this code is, in fact, to construct `SymbolRegionValue` for 
> the parameter, so it is equivalent to calling 
> `SValBuilder::getRegionValueSymbolVal()` over the parameter region, as long 
> as the current `StoreManager` implementation remains unquestioned.
> 
> We could also ask `StoreManager` to provide a binding for this region from an 
> empty store, maybe extend its API to allow such queries, this would remove 
> the layering violation.
> 
> Because this topic getting is rather complicated, it might have been better 
> to make a separate review for this change, originally (no problem now that 
> most of the interested people have already had a look).
Yeah, as Anna suggested I will split this change and the addition of 
LocationContext in check::RegionChanges and we should probably continue this 
discussion there


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Artem Dergachev via cfe-commits]

NoQ added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

zaks.anna wrote:
> LocationContext can be obtained by calling CallEvent::getLocationContext(). I 
> do not think that adding another parameter here buys us much. Am I missing 
> something?
CallEvent* is optional here - it's there only for invalidations through calls.



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:333
+   ProgramStateRef state,
+   const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->wantsRegionChangeUpdate(state, LCtx);

zaks.anna wrote:
> What is the scenario in which you need the context here? The idea is that the 
> checker would be interested in region changes if it is already tracking 
> information in the state. For that check, the information in the state is 
> sufficient. What is your scenario?
> 
> Also, For any checker API change, we need to update the 
> CheckerDocumentation.cpp.
Environment, which is a significant portion of the state, is essentially 
unaccessible without a location context.

Call stack is also an important bit of information to any checker that does 
something special to support/exploit the inlining IPA - there are many checkers 
that scan the call stack, but so far none of them needed to subscribe to 
checkRegionChanges; their possible use case may look like "we want an update 
only if a certain condition was met within the current stack frame".

For the recursion checker, i think, the point is "if current frame is spoiled, 
we no longer want updates" (which should probably be fixed in the checker 
patch: even though the callback is broken, it'd take one less action to fix it 
if we decide to).


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Hi and welcome to the project!

This patch definitely looks quite complex for a first contribution, so great 
job at digging through the analyzer internals!

One higher level comment I have is that you should try and split patches 
whenever possible. For example, in the description, you mention that the patch 
contains 2 changes:

- extending RegionChanges interface with LocationContext
- adding an easy way to obtain arguments' SVals from StackFrameCtx

Since they are independent changes, please submit them as separate patches. 
This simplifies the job of the reviewers and anyone who will ever look at this 
patch in commit history or on phabricator. Also, this ensures that each change 
has sufficient test coverage. The LLVM Dev policy has a great explanation of 
the reasoning behind this: 
http://llvm.org/docs/DeveloperPolicy.html#incremental-development.

How do you plan on testing these changes? The main 2 methods are unit tests and 
checker regression tests. Since no checker uses these maybe we should only 
commit them once such checker is added...

I've only started looking at the first change and would like to understand the 
motivation behind it a bit more.




Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+  const CallEvent *Call,
+  const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,

LocationContext can be obtained by calling CallEvent::getLocationContext(). I 
do not think that adding another parameter here buys us much. Am I missing 
something?



Comment at: include/clang/StaticAnalyzer/Core/Checker.h:333
+   ProgramStateRef state,
+   const LocationContext *LCtx) {
+return ((const CHECKER *) checker)->wantsRegionChangeUpdate(state, LCtx);

What is the scenario in which you need the context here? The idea is that the 
checker would be interested in region changes if it is already tracking 
information in the state. For that check, the information in the state is 
sufficient. What is your scenario?

Also, For any checker API change, we need to update the 
CheckerDocumentation.cpp.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Artem Dergachev via cfe-commits]

NoQ added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:735
+  const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();
+  unsigned NumArgs = FunctionDecl->getNumParams();
+  assert(ArgIdx < NumArgs && "Arg access out of range!");

a.sidorin wrote:
> Maybe we should put a check that requested StackFrame is our StackFrame or 
> our parent StackFrame here?
Hmm. We should probably add a similar check to `getSVal(Ex, LCtx)`!- and also 
check that `Ex` is an active expression.

Unfortunately, we do not know the current location context within any of these 
methods, not sure how to implement that.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Artem Dergachev via cfe-commits]

NoQ added a comment.

Welcome to phabricator! I agree that having the location context in this 
callback is useful, and i'm all for reducing boilerplate in various checkers 
through better API.




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:231
 
   ProgramStateRef bindLoc(Loc location,
   SVal V,

Because this API becomes more complicated, i think we should add some 
docstrings here, explaining the meaning of the location context parameter, in 
particular.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:733
+inline SVal ProgramState::getArgSVal(const StackFrameContext *SFC,
+  const unsigned ArgIdx) const {
+  const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();

Indent.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:741
+// because the call wasn't modeled in the first place.
+const VarDecl *ArgDecl = FunctionDecl->parameters()[ArgIdx];
+const Loc ArgLoc = getLValue(ArgDecl, SFC);

a.sidorin wrote:
> Unfortunately, this code does not consider the fact that argument values may 
> be overwritten. If we want to get initial values, we should find another way.
Uhm, yeah, this is in fact a problem!

The purpose of this code is, in fact, to construct `SymbolRegionValue` for the 
parameter, so it is equivalent to calling 
`SValBuilder::getRegionValueSymbolVal()` over the parameter region, as long as 
the current `StoreManager` implementation remains unquestioned.

We could also ask `StoreManager` to provide a binding for this region from an 
empty store, maybe extend its API to allow such queries, this would remove the 
layering violation.

Because this topic getting is rather complicated, it might have been better to 
make a separate review for this change, originally (no problem now that most of 
the interested people have already had a look).


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Aleksei Sidorin via cfe-commits]

a.sidorin added a comment.

Hi Krzysztof!
This change  seems useful: I can imagine the situation where we want to ask 
current `LocationContext` in this callback. The change looks pretty intrusive 
but I mostly agree with it. Initially, I have some questions about the 
implementation of `getArgSVal()` function (inline comments). I'll add more 
comments later.




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:735
+  const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();
+  unsigned NumArgs = FunctionDecl->getNumParams();
+  assert(ArgIdx < NumArgs && "Arg access out of range!");

Maybe we should put a check that requested StackFrame is our StackFrame or our 
parent StackFrame here?



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:741
+// because the call wasn't modeled in the first place.
+const VarDecl *ArgDecl = FunctionDecl->parameters()[ArgIdx];
+const Loc ArgLoc = getLValue(ArgDecl, SFC);

Unfortunately, this code does not consider the fact that argument values may be 
overwritten. If we want to get initial values, we should find another way.


https://reviews.llvm.org/D26588



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Krzysztof Wiśniewski via cfe-commits]

k-wisniewski created this revision.
k-wisniewski added reviewers: dergachev.a, dcoughlin, zaks.anna.
k-wisniewski added a subscriber: cfe-commits.

Hi,

I've been working on a checker that uses RegionChanges interface and needed to 
access to LocationContext.  Another change is an easy way to obtain arguments' 
SVals from StackFrameCtx, with which the function/method has been called. In my 
opinion having that might prove useful for creating future checkers so I 
publish it here for review and discussion. Obvoiusly, this needs some 
improvement, but I'll be more than happy to hear community's opinion about the 
current version, as I wasn't sure if my changes fit into architecture etc.

Also: this is my first public contribution to clang. so if there are any 
annoying aspects of it (like the list of subscribers being to broad, bad forma, 
wrong metadata etc.) - sorry!


https://reviews.llvm.org/D26588

Files:
  include/clang/StaticAnalyzer/Core/Checker.h
  include/clang/StaticAnalyzer/Core/CheckerManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  lib/StaticAnalyzer/Core/CheckerManager.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp

Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -111,24 +111,29 @@
   return ConstraintMgr->removeDeadBindings(Result, SymReaper);
 }
 
-ProgramStateRef ProgramState::bindLoc(Loc LV, SVal V, bool notifyChanges) const {
+ProgramStateRef ProgramState::bindLoc(Loc LV,
+  SVal V,
+  const LocationContext *LCtx,
+  bool notifyChanges) const {
   ProgramStateManager &Mgr = getStateManager();
   ProgramStateRef newState = makeWithStore(Mgr.StoreMgr->Bind(getStore(),
  LV, V));
   const MemRegion *MR = LV.getAsRegion();
   if (MR && Mgr.getOwningEngine() && notifyChanges)
-return Mgr.getOwningEngine()->processRegionChange(newState, MR);
+return Mgr.getOwningEngine()->processRegionChange(newState, MR, LCtx);
 
   return newState;
 }
 
-ProgramStateRef ProgramState::bindDefault(SVal loc, SVal V) const {
+ProgramStateRef ProgramState::bindDefault(SVal loc,
+  SVal V,
+  const LocationContext *LCtx) const {
   ProgramStateManager &Mgr = getStateManager();
   const MemRegion *R = loc.castAs().getRegion();
   const StoreRef &newStore = Mgr.StoreMgr->BindDefault(getStore(), R, V);
   ProgramStateRef new_state = makeWithStore(newStore);
   return Mgr.getOwningEngine() ?
-   Mgr.getOwningEngine()->processRegionChange(new_state, R) :
+   Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx) :
new_state;
 }
 
@@ -202,7 +207,7 @@
 }
 
 return Eng->processRegionChanges(newState, IS, TopLevelInvalidated,
- Invalidated, Call);
+ Invalidated, Call, LCtx);
   }
 
   const StoreRef &newStore =
Index: lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -115,11 +115,11 @@
 SymbolRef Sym = SymMgr.conjureSymbol(elem, LCtx, T,
  currBldrCtx->blockCount());
 SVal V = svalBuilder.makeLoc(Sym);
-hasElems = hasElems->bindLoc(elementV, V);
+hasElems = hasElems->bindLoc(elementV, V, LCtx);
 
 // Bind the location to 'nil' on the false branch.
 SVal nilV = svalBuilder.makeIntVal(0, T);
-noElems = noElems->bindLoc(elementV, nilV);
+noElems = noElems->bindLoc(elementV, nilV, LCtx);
   }
 
 // Create the new nodes.
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -317,7 +317,7 @@
 // actually make things worse. Placement new makes this tricky as well,
 // since it's then possible to be initializing one part of a multi-
 // dimensional array.
-State = State->bindDefault(loc::MemRegionVal(Target), ZeroVal);
+St