[cfe-commits] r165273 - in /cfe/trunk: lib/CodeGen/CodeGenFunction.cpp test/CodeGen/catch-undef-behavior.c test/CodeGenCXX/catch-undef-behavior.cpp test/CodeGenCXX/return.cpp

[Chandler Carruth]

On Wed, Jan 23, 2013 at 10:44 AM, Chandler Carruth <chandlerc at google.com>wrote:

> On Wed, Jan 23, 2013 at 10:27 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>
>>
>>
>>
>> On Tue, Jan 22, 2013 at 5:55 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>
>>> On Tue, Jan 22, 2013 at 5:42 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com
>>> > wrote:
>>>
>>>> On Jan 22, 2013, at 5:23 PM, Chandler Carruth <chandlerc at google.com>
>>>> wrote:
>>>>
>>>> On Tue, Jan 22, 2013 at 4:15 PM, Daniel Dunbar <daniel at zuster.org>wrote:
>>>>
>>>>> Hi Richard,
>>>>>
>>>>> I object pretty strongly to using unreachable here for C++ code bases.
>>>>>
>>>>
>>>> There was some discussion surrounding this, and I'm still pretty
>>>> strongly in favor of it...
>>>>
>>>>
>>>> IMHO you still haven't given enough justification; your proposed
>>>> optimization opportunity in
>>>> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-October/025326.html is
>>>> specific to a full covered switch, which can be handled with an
>>>> optimization that targets this case.
>>>>
>>>
>>> It is applicable to any circumstance where there is a programmer-known
>>> invariant that all paths terminate in a return which is not statically
>>> determinable. I've seen both predicated code with if/else chains of this
>>> form, and switches.
>>>
>>
>> The right way to handle this is with appropriate annotations. This is
>> simply not a good argument for emitting unreachable.
>>
>> But I actually think we're approaching this from the wrong direction. The
>>> C++ standard is extremely clear that this is UB. Given that, I think we
>>> should aggressively tell users about this problem with their code. If the
>>> optimization benefits aren't worthwhile, then I would argue for emitting
>>> llvm.trap in *all* cases rather than just in non-optimized builds. While I
>>> think the optimization benefits are both easy to get and worth it, I don't
>>> have a collection of benchmarks that rely on it so I'm not going to fight
>>> tooth and nail for it. I just don't understand sacrificing it when we don't
>>> have to.
>>>
>>
>> I'd much prefer llvm.trap in all cases instead of unreachable, although I
>> still don't see much value in forcing users to fix code that from their
>> perspective "works".
>>
>
> The fundamental question is: do you want to try to define as a language
> extension the behavior here? If so, we have a process for that, but I think
> it will be a painful and fruitless one as I think this language behavior
> was very well considered by the C++ committee. I do not think this behavior
> was chosen in ignorance but in a desire that the language work in a certain
> way.
>
> If we don't want to define the behavior here, we do a disservice to the
> users and to our selves to allow it to persist in the code. Eventually it
> *will* be exploited to the users detriment and we should tell the user
> about it quickly and effectively. This is the argument for using llvm.trap.
> This is the argument I feel *very* strongly about.
>
> The argument to switch from llvm.trap to unreachable in an optimized build
> is simply one of principles: the user told us to optimize, and the language
> gave us an opportunity. There isn't some practical, useful behavior being
> blocked here. We should optimize these cases just as we optimize signed
> integer overflow even if in the particular situation it didn't help the
> performance or code size signficantly.
>
>
> If you want to think about the extent to which this matters, consider
> inlining. Once inlined, the undefined nature of this pattern is more likely
> to result in an active bug, and thus the user will more greatly appreciate
> the trap diagnosing it for them. Also, once inlined the unreachable has
> much more power and can result in significant simplifications.
>
> To sum up:
> - if we aren't going to define the behavior, we should trap. This will
> make both programs and future optimizations much more predictable.
> - if we are going to trap, then when optimizing we should leverage that in
> the hope of secondary and tertiary simplifications
>

One more point: I'd be really happy to have -fsanitize=undefined-return or
some such which can be *combined* with optimizations to help track down
buggy code.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130123/fc34a812/attachment.html>