[llvm] r226588 - Add an assertion and prefer a crash over an infinite loop.

[Duncan P. N. Exon Smith]

> On 2015 Jan 20, at 10:58, Adrian Prantl <aprantl at apple.com> wrote:
> 
> 
> 
>> Begin forwarded message:
>> 
>> Subject: Re: [llvm] r226588 - Add an assertion and prefer a crash over an infinite loop.
>> From: "Duncan P. N. Exon Smith" <dexonsmith at apple.com>
>> Date: January 20, 2015 at 10:53:38 AM PST
>> Cc: llvm-commits at cs.uiuc.edu
>> To: Adrian Prantl <aprantl at apple.com>
>> 
>> 
>>> On 2015 Jan 20, at 10:03, Adrian Prantl <aprantl at apple.com> wrote:
>>> 
>>> Author: adrian
>>> Date: Tue Jan 20 12:03:37 2015
>>> New Revision: 226588
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=226588&view=rev
>>> Log:
>>> Add an assertion and prefer a crash over an infinite loop.
>>> 
>>> Modified:
>>>  llvm/trunk/lib/IR/DebugInfo.cpp
>>> 
>>> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=226588&r1=226587&r2=226588&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
>>> +++ llvm/trunk/lib/IR/DebugInfo.cpp Tue Jan 20 12:03:37 2015
>>> @@ -527,12 +527,15 @@ bool DISubprogram::Verify() const {
>>>       while ((IA = DL.getInlinedAt()))
>>>         DL = DebugLoc::getFromDILocation(IA);
>>>       DL.getScopeAndInlinedAt(Scope, IA);
>>> +        assert(Scope && "debug location has no scope");
>>>       assert(!IA);
>>>       while (!DIDescriptor(Scope).isSubprogram()) {
>>>         DILexicalBlockFile D(Scope);
>>>         Scope = D.isLexicalBlockFile()
>>>                     ? D.getScope()
>>>                     : DebugLoc::getFromDILexicalBlock(Scope).getScope();
>>> +          if (!Scope)
>>> +            llvm_unreachable("lexical block file has no scope");
>> 
>> It's strange that this isn't simply an assertion:
>> 
>>   assert(Scope && "lexical block file has no scope");
>> 
>> But I guess that's so that it crashes even when NDEBUG?  (Is that
>> reliable?)
> 
> Yes, that’s the intention (and llvm_unreachable is meant to be used that way).
>> 
>> If that's the reason, you should probably add a comment to that
>> effect since otherwise I'm likely to switch it to an assert next
>> time I look at it ;).
>> 
>> (Also, do you know when this situation arises?)
> 
> Happened to me when I compiled an old .ll file without the MDLocation upgrade.

Ah.  Is it worth testing this?

Also, can you merge this into the 3.6 branch?

> 
> -- adrian
> 
>> 
>>>       }
>>>       if (!DISubprogram(Scope).describes(F))
>>>         return false;
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits