On Fri, Mar 1, 2013 at 3:11 PM, Argyrios Kyrtzidis <[email protected]> wrote: > On Mar 1, 2013, at 10:03 AM, David Blaikie <[email protected]> wrote: > >> On Fri, Mar 1, 2013 at 9:10 AM, Jordan Rose <[email protected]> wrote: >>> >>> On Feb 28, 2013, at 19:43 , Argyrios Kyrtzidis <[email protected]> wrote: >>> >>>> Author: akirtzidis >>>> Date: Thu Feb 28 21:43:33 2013 >>>> New Revision: 176333 >>>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=176333&view=rev >>>> Log: >>>> Add one more sanity check in SourceManager::getFileIDLoaded(). >>>> >>>> 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=176333&r1=176332&r2=176333&view=diff >>>> ============================================================================== >>>> --- cfe/trunk/lib/Basic/SourceManager.cpp (original) >>>> +++ cfe/trunk/lib/Basic/SourceManager.cpp Thu Feb 28 21:43:33 2013 >>>> @@ -863,6 +863,11 @@ FileID SourceManager::getFileIDLoaded(un >>>> return Res; >>>> } >>>> >>>> + // Sanity checking, otherwise a bug may lead to hanging in release >>>> build. >>>> + if (LessIndex == MiddleIndex) { >>>> + assert(0 && "binary search missed the entry"); >>> >>> llvm_unreachable? >> >> Or actually just remove the if/return/etc: >> >> assert(LessIndex == MiddleIndex) >> >> We really don't make a habit of writing asserts with fallbacks "just in >> case". > > ...except when seeing actual infinite loops occurring, in which case you > defend against it; particularly when the infinite loops occur in a process > using libclang.
Are we seeing multiple bugs of this kind? (eg: we fixed a bug that hit an inf loop due to this case, then later on we hit another, different bug that exhibited as an infinite loop here again) If it's just one so far: I assume we /fix/ the bug & move on, no? > >> >> (unreachable inside the 'if' would have the same 'problem' as removing >> the if/adding the obvious assert - in release builds we'd still >> optimize away the whole block & "a bug may lead to hanging in release >> build") >> >>>> + return FileID(); >>>> + } >>>> LessIndex = MiddleIndex; >>>> } >>>> } >>>> >>>> >>>> _______________________________________________ >>>> 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 > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
