Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284)

[Jeff Law <law@redhat.com>]

On 08/21/2015 08:41 AM, Marek Polacek wrote:
> On Fri, Aug 21, 2015 at 03:37:38PM +0200, Richard Biener wrote:
>> On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> wrote:
>>> On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote:
>>>> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote:
>>>>> This fixes the libgo breakage.  Seems I really should have removed the
>>>>> edge after we split the block with null dereference after __builtin_trap
>>>>> statement so that it's unreachable.
>>>>>
>>>>> Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on
>>>>> aarch64-linux, ok for trunk?
>>>>
>>>> Hum.  I don't see why this is needed - CFG cleanup (which of course needs
>>>> to run!) should do this for you.  In fact stray unreachable blocks are usually
>>>> more of a problem.
>>>
>>> Aha.  It seems cleanup does that if I change the code to generate
>>> __builtin_unreachable instead of __builtin_trap.  A hint maybe? ;)
>>
>> Not really...
>>
>> static bool
>> cleanup_control_flow_bb (basic_block bb)
>> {
>> ...
>>    /* Check for indirect calls that have been turned into
>>       noreturn calls.  */
>>    else if (is_gimple_call (stmt)
>>             && gimple_call_noreturn_p (stmt)
>>             && remove_fallthru_edge (bb->succs))
>>      retval = true;
>>
>> and __builtin_trap is NORETURN.  But there is the hint where to debug.
>
> Yes, but gimple_call_noreturn_p is false on __builtin_trap.  That's quite
> confusing... but flags_from_decl_or_type really returns 0 for __builtin_trap.
>
> So can't we use __builtin_unreachable in isolate-path code?
No, we really want the trap to halt execution rather than executing 
whatever code is following in the instruction stream.  The latter is a 
significant security problem.



jeff