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

[Adrian Prantl]

> On Jan 20, 2015, at 11:46 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> 
>> On Jan 20, 2015, at 11:10 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Tue, Jan 20, 2015 at 10:58 AM, Adrian Prantl <aprantl at apple.com <mailto: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 <mailto:dexonsmith at apple.com>>
>> > Date: January 20, 2015 at 10:53:38 AM PST
>> > Cc: llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> > To: Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>>
>> >
>> >
>> >> On 2015 Jan 20, at 10:03, Adrian Prantl <aprantl at apple.com <mailto: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 <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 <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).
>> 
>> I don't believe that's the intention of the LLVM style guide - generally we try to remove conditionals like this and replace the conditional with the assertion so the dead code isn't exercised in release builds.
> 
> Isn’t the point of “unreachable” to let the compiler do whatever it wants in this case?
> I thought LLVM was free to just drop the test entirely in release mode?
> 

After going back to ErrorHandling.h it looks as if report_fatal_error() would be more appropriate.
llvm_unreachable() either becomes llvm_unreachable_internal() which invokes abort() and prints the message or __builtin_unreachable() which indeed drops the message.

-- adrian

> Mehdi
> 
> 
>>  
>> >
>> > 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.
>> 
>> -- adrian
>> 
>> >
>> >>        }
>> >>        if (!DISubprogram(Scope).describes(F))
>> >>          return false;
>> >>
>> >>
>> >> _______________________________________________
>> >> llvm-commits mailing list
>> >> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>> >
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150120/4ae7ae91/attachment.html>