On Mon, Oct 15, 2012 at 11:39 AM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com" target="_blank">kyrtzidis@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div><div class="h5"><div>On Oct 14, 2012, at 5:26 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br><blockquote type="cite">
On Sun, Oct 14, 2012 at 4:07 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div>On Fri, Oct 12, 2012 at 7:07 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com" target="_blank">kyrtzidis@apple.com</a>></span> wrote:<br></div><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>On Oct 4, 2012, at 4:56 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
<br>
> As of r165273, clang produces 'unreachable' if we flow off the end of a value-returning function (without returning a value) in C++. This is undefined behavior, but we used to just return undef, instead of trying to exploit it. Please let me know if this causes problems for you, and we can weaken it to an acceptable level.<br>



<br>
</div>FYI, this caused some libc++ tests to crash (or not terminate). To highlight one example, there was this method:<br>
<br>
> template <class _CharT, class _Traits><br>
> inline _LIBCPP_INLINE_VISIBILITY<br>
> basic_filebuf<_CharT, _Traits>&<br>
> basic_filebuf<_CharT, _Traits>::operator=(basic_filebuf&& __rhs)<br>
> {<br>
>     close();<br>
>     swap(__rhs);<br>
> }<br>
<br>
Notice the absence of a "return *this".<br>
<br>
At the test (test/input.output/file.streams/fstreams/filebuf.assign/move_assign.pass.cpp) the method was used like this:<br>
<br>
 std::filebuf f2;<br>
 f2 = move(f);<br>
<br>
Notice that the returned value from 'operator=' is ignored.<br>
<br>
There was a crash with a stack trace showing bad access inside "libunwind.dylib`_Unwind_Resume"; the stack trace didn't provide much hint at what the culprit is.<br>
<br>
I don't think producing 'unreachable' always by default is a good idea; this is too aggressive to do at the frontend level and it makes debugging (on a "-O0 -g" build) a nightmare.</blockquote><div>


<br></div></div><div>Yeah, we were worried that this might be a problem for people who don't use -Wreturn-type. How would you feel about inserting a call to llvm.trap before the unreachable if we're building without optimizations? </div>


</div>
</blockquote></div><br><div>I've implemented this in r165914. We can tune this further if there are still debugability problems.</div></blockquote><div><br></div></div></div><div>Stepping back a bit, could you elaborate on the real benefit of the initial change and with what kind of source code you saw it ?</div>
</div></div></blockquote><div><br></div><div>This change was made as a cleanup when adding the -fcatch-undefined-behavior check. Internal discussion concluded that returning undef is not helpful, and 'unreachable' is a more appropriate representation.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>Unless I'm missing something, this will benefit functions that are not checked with -Wreturn-type and are supposed to be unreachable in some path but are not marked as such.</div>
<div>I'd prefer that these functions are actually marked as 'unreachable' in source code, instead of depending on the compiler implicitly assuming that in order to get such an optimization.</div></div></div></blockquote>
<div><br></div><div>I agree, but if they're not marked 'unreachable' in the source code, what IR would you want to produce for code paths which fall off the end? @llvm.trap() at -O0 and unreachable otherwise seems reasonable to me; would you prefer something else? (Perhaps always emitting a call to @llvm.trap?)</div>
</div>