<div class="gmail_quote">On Mon, Jan 30, 2012 at 1:10 AM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Jan 29, 2012, at 1:16 PM, Nico Weber wrote:<br>
> Chandler / Matthieu: I tried that - it gets rid of the copy<br>
> constructor error, but since the volatile is casted away, the<br>
> Wnull-dereference warning reappears.<br>
><br>
> John: abort() works, and that's what I'm using locally for now. I<br>
> don't know if all compilers are smart enough to not warn about a<br>
> missing return statement if the function body contains nothing but<br>
> abort() – but I suppose that would be other compiler's bug then.<br>
><br>
> Another example with the same problem was in a container class, which<br>
> looked like:<br>
><br>
> template<class T> class container {<br>
>  T Get(int index) {  if (i < 0 || i >= size_) return *(const T*)NULL;<br>
> return *((T*)storage_)[i]; }<br>
> };<br>
><br>
> The idea here is to turn a potentially exploitable invalid read into<br>
> at worst a harmless invalid read. Here the semantics change slightly<br>
> with the abort() from 'maybe crash on invalid index' to 'definitely<br>
> crash on invalid index' (at least for Chromium, that's fine).<br>
><br>
> So is abort() the recommended solution? Should the diagnostic text<br>
> recommend abort() when this warning occurs as part of a return<br>
> expression?<br>
<br>
</div>If you want a portable, reliable execution-time failure, abort()'s the tool<br>
for the job.  I know there are compilers that don't have a generic<br>
no-return annotation that nonetheless recognize abort().  __builtin_trap()<br>
has a slight advantage in that it generally makes the debugger stop<br>
in the current function instead of somewhere inside abort() (or even<br>
further downwind, e.g. in kill()).<br>
<br>
We could add abort() as a third, more portable option to the diagnostic;<br>
I don't see any reason to only suggest it in a return-statement context.<br>
This is one of those places where it would be nice to have the diagnostic<br>
link to a webpage with a longer discussion of the benefits of the<br>
different alternatives.</blockquote><div><br></div><div>While I agree, this comes up *very* rarely, so it may not be worth it to slave over the diagnostic.</div><div><br></div><div>Maybe it'd be worth having a blurb about this on the existing Clang docs that we can point people to when the question arises, but I genuinely hope we're not growing new uses of this at anything like a rapid pace.</div>
</div>