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 <[email protected]> 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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

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

Reply via email to