Re: [ARM] Make _kill() a noreturn function (was: [ARM] Add endless loop to avoid a compiler warning on noreturn functions.)

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On Tue, 2 Oct 2018 at 11:08, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>
> On 02/10/18 09:59, Christophe Lyon wrote:
> > On Tue, 2 Oct 2018 at 10:55, Richard Earnshaw (lists)
> > <Richard.Earnshaw@arm.com> wrote:
> >>
> >> On 02/10/18 07:52, Christophe Lyon wrote:
> >>> On Tue, 2 Oct 2018 at 00:25, Craig Howland <howland@lgsinnovations.com> 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.
>

OK, here is an updated version.

> 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
> >>
>

Attachment: newlib-1.txt
Description: Text document