On Sep 19, 2011, at 11:27 PM, Ted Kremenek wrote:
> On Sep 19, 2011, at 6:51 PM, Anna Zaks wrote:
>
>> + FullSourceLoc genLocation(const ParentMap *PM=0) const;
>> + PathDiagnosticRange genRange(const ParentMap *PM=0) const;
>
>
> Hi Anna,
>
> Instead of passing the ParentMap, how about having genLocation() lazily query
> LocationContext for one? That way it only gets created when we need it.
>
> Actually, it it looks like we are still using the LC member variable in
> genRange(), instead of the passed ParentMap:
>
> case Stmt::ObjCForCollectionStmtClass: {
> SourceLocation L = getValidSourceLocation(S, LC->getParentMap());
> return SourceRange(L, L);
>
> Since LocationContext isn't guaranteed to live for the lifetime of
> PathDiagnosticLocation, I suggest:
>
> 1) Remove the LC member variable (fix potential use-after-free).
There was still a dependency remaining. Removed in r140146.
> 2) Pass (possibly null?) LocationContext * to genLocation() and genRange()
Thanks for catching this! LocationContext's constructor is protected so we
cannot construct one from AnalysisContext directly (passing ParentMap would
solve the issue, I forgot that it's constructed on demand). We could require
either LocationContextManager or AnalysisManager to be passed in by the users
that do not have LocationContext. AnalysisManager is probably better since
that's what is passed to the checkers.
> 3) Lazily query that LocationContext for a ParentMap as needed.
> LocationContext (or really, AnalysisContext) will make sure the ParentMap is
> constructed once.
>
Will do.
> What do you think?
>
> Cheers,
> Ted
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
