Re: [ARM] Add endless loop to avoid a compiler warning on noreturn functions.

On 02/10/18 09:59, Christophe Lyon wrote:
> On Tue, 2 Oct 2018 at 10:55, Richard Earnshaw (lists)
> <[email protected]> wrote:
>>
>> On 02/10/18 07:52, Christophe Lyon wrote:
>>> On Tue, 2 Oct 2018 at 00:25, Craig Howland <[email protected]> wrote:
>>>>
>>>> On 10/01/2018 05:27 PM, Christophe Lyon wrote:
>>>>> Hi,
>>>>>
>>>>> While building newlib for ARM, I noticed GCC warnings for _exit() that
>>>>> the compiler thinks they return a value despite being noreturn.
>>>>>
>>>>> Like other targets, this small adds an endless loop to avoid the warning.
>>>>>
>>>>> OK?
>>>>>
>>>>> Christophe
>>>> The proper fix (for both places) is to add noreturn to the _kill() prototype in
>>>> the file.  (Which presumably is true, otherwise _exit() will return.  I did test
>>>> that it fixes the warning.)  (It would not be surprising if it also needed to be
>>>> added to the _kill() source, itself.)
>>>
>>> Well, when compiled with ARM_RDI_MONITOR, _kill does seem to return:
>>> #if SEMIHOST_V2
>>> if (_has_ext_exit_extended ())
>>>   return do_AngelSWI (insn, block);
>>> else
>>> #endif
>>>   return do_AngelSWI (insn, (void*)block[0]);
>>>
>>
>> do_AngelSWI is a multi-purpose call that will normally return, so that
>> can't be marked no-return.
> Indeed.
> 
>>
>> I think the right fix here is to remove the "return" from the statements
>> and add __builtin_unreachable () at the end of the function.
>>
> By "function", do you mean _kill or _exit ?
> IIUC the patch should:
> - remove "return" from _kill
> - add _builtin_unreachable to both _kill and _exit
> - add "noreturn" to _kill prototype
> 
> Unless there are cases where this version of _kill can kill
> another thread/process and thus actually return to its caller?
> 

Well if _kill can be properly marked as no-return then _exit can also
and you don't need to have a second _bi_unreachable(): that's only
needed when GCC cannot deduce the control flow from semantic analysis of
the code.

don't forget that this code is duplicated across both newlib/sys/arm and
libgloss, so please update both instances.

R.

> 
>> R.
>>
>>> I guess the noreturn should not be added to
>>> newlib/libc/include/sys/signal.h
>>> because it depends on the actual target implementation of _kill?
>>>
>>>> Craig
>>