r182681 - Add some safety checks in a couple of SourceManager functions.

[Jordan Rose]

I see that we're fixing crashes, but why isn't it just part of the precondition for this function that it is never called with an invalid FileID?

Also, you can't change isBeforeInTranslationUnit that way; the sort's not consistent. Invalid locs should be always before or always after valid locs, or you should assert that this doesn't happen.

Jordan


On May 24, 2013, at 15:24 , Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:

> Author: akirtzidis
> Date: Fri May 24 17:24:04 2013
> New Revision: 182681
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=182681&view=rev
> Log:
> Add some safety checks in a couple of SourceManager functions.
> 
> This is to address crash in rdar://13932308
> 
> Modified:
>    cfe/trunk/lib/Basic/SourceManager.cpp
> 
> Modified: cfe/trunk/lib/Basic/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=182681&r1=182680&r2=182681&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
> +++ cfe/trunk/lib/Basic/SourceManager.cpp Fri May 24 17:24:04 2013
> @@ -1958,6 +1958,9 @@ SourceManager::getMacroArgExpandedLocati
> 
> std::pair<FileID, unsigned>
> SourceManager::getDecomposedIncludedLoc(FileID FID) const {
> +  if (FID.isInvalid())
> +    return std::make_pair(FileID(), 0);
> +
>   // Uses IncludedLocMap to retrieve/cache the decomposed loc.
> 
>   typedef std::pair<FileID, unsigned> DecompTy;
> @@ -1969,11 +1972,14 @@ SourceManager::getDecomposedIncludedLoc(
>     return DecompLoc; // already in map.
> 
>   SourceLocation UpperLoc;
> -  const SrcMgr::SLocEntry &Entry = getSLocEntry(FID);
> -  if (Entry.isExpansion())
> -    UpperLoc = Entry.getExpansion().getExpansionLocStart();
> -  else
> -    UpperLoc = Entry.getFile().getIncludeLoc();
> +  bool Invalid = false;
> +  const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &Invalid);
> +  if (!Invalid) {
> +    if (Entry.isExpansion())
> +      UpperLoc = Entry.getExpansion().getExpansionLocStart();
> +    else
> +      UpperLoc = Entry.getFile().getIncludeLoc();
> +  }
> 
>   if (UpperLoc.isValid())
>     DecompLoc = getDecomposedLoc(UpperLoc);
> @@ -2033,6 +2039,9 @@ bool SourceManager::isBeforeInTranslatio
>   std::pair<FileID, unsigned> LOffs = getDecomposedLoc(LHS);
>   std::pair<FileID, unsigned> ROffs = getDecomposedLoc(RHS);
> 
> +  if (LOffs.first.isInvalid() || ROffs.first.isInvalid())
> +    return false;
> +
>   // If the source locations are in the same file, just compare offsets.
>   if (LOffs.first == ROffs.first)
>     return LOffs.second < ROffs.second;
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130524/66df78e5/attachment.html>