[cfe-commits] r141048 - /cfe/trunk/lib/Basic/SourceManager.cpp

[Argyrios Kyrtzidis]

On Oct 3, 2011, at 5:03 PM, Eli Friedman wrote:

> On Mon, Oct 3, 2011 at 4:43 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> Author: akirtzidis
>> Date: Mon Oct  3 18:43:01 2011
>> New Revision: 141048
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=141048&view=rev
>> Log:
>> Make sure SourceManager::getFileIDLoaded doesn't hang in release build because of invalid passed parameter.
>> rdar://10210140
>> 
>> 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=141048&r1=141047&r2=141048&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
>> +++ cfe/trunk/lib/Basic/SourceManager.cpp Mon Oct  3 18:43:01 2011
>> @@ -732,6 +732,10 @@
>>  FileID SourceManager::getFileIDLoaded(unsigned SLocOffset) const {
>>   assert(SLocOffset >= CurrentLoadedOffset && "Bad function choice");
>> 
>> +  // Sanity checking, otherwise a bug may lead to hanging in release build.
>> +  if (SLocOffset < CurrentLoadedOffset)
>> +    return FileID();
>> +
>>   // Essentially the same as the local case, but the loaded array is sorted
>>   // in the other direction.
> 
> Having an assert followed by an if statement checking the exact same
> condition doesn't really make sense: it means that either the assert
> is incorrect or the check is useless.

Talked to Eli; the idea here is that if the invariant breaks the assertion should trigger, but in case there is another bug we should really really not hang in release. (The underlying bug for the hang in rdar://10210140 is already fixed in r138813).

> 
> -Eli