Re: [Patch][arm] Enable return code with semi-hosting SYS_EXIT_EXTENDED

Pushed to master.

-- Jeff J.

On Mon, Nov 26, 2018 at 5:26 AM Matthew Malcomson <[email protected]>
wrote:

> On 20/11/18 11:16, Richard Earnshaw (lists) wrote:
> > On 16/11/2018 18:48, Matthew Malcomson wrote:
> >> The _exit function currently passes -1 as a "sig" to the _kill function
> as an
> >> invalid signal number so that _kill can distinguish between an abort
> and a
> >> standard exit.
> >>
> >> For boards using the SYS_EXIT_EXTENDED semi-hosting operation to return
> a
> >> status code, this means that the "status" paramter to _exit is ignored
> and the
> >> return code is always -1.
> >>
> https://developer.arm.com/docs/100863/latest/semihosting-operations/sys_exit_extended-0x20
> >>
> >> This patch puts shared code between _kill and _exit into a new function
> >> _kill_shared that takes the semi-hosting "reason" to use (if
> semi-hosting is
> >> available) as an argument.
> >>
> >> For semi-hosting _kill_shared provides that "reason".
> >>
> >> Without the "sig" argument being used to distinguish between a normal
> and
> >> abnormal exit, the _exit function can provide the return code to be
> used if the
> >> SYS_EXIT_EXTENDED operation is available.
> >>
> >> Hence the exit code can be returned.
> >>
> >> Tested with gcc testsuite on arm-none-eabi and no issues.
> >> Ok for master?
> >>
> >> PS. I don't have commit rights so if OK can someone commit for me?
> > This change needs to be applied to the similar code in newlib's sys
> > directory as well.  PLEASE keep this in sync.
> >
> > R.
> Thanks for pointing that out, and thanks for the help in testing too.
> The patch applying this same change to the sys/ directory is attached.
>
> If OK could someone commit for me? I don't have commit rights.
>
> Regards,
> Matthew
>
>
> #### Patch inlined for ease of reply ###
>
> diff --git a/newlib/libc/sys/arm/syscalls.c
> b/newlib/libc/sys/arm/syscalls.c
> index 87ddca7..b521074 100644
> --- a/newlib/libc/sys/arm/syscalls.c
> +++ b/newlib/libc/sys/arm/syscalls.c
> @@ -30,7 +30,7 @@ int    _stat        (const char *, struct stat *);
>   int    _fstat        (int, struct stat *);
>   void *    _sbrk        (ptrdiff_t);
>   pid_t    _getpid        (void);
> -int    _kill        (int, int) __attribute__((__noreturn__));
> +int    _kill        (int, int);
>   void    _exit        (int);
>   int    _close        (int);
>   int    _swiclose    (int);
> @@ -49,6 +49,7 @@ static int    error        (int);
>   static int    get_errno    (void);
>   static int    remap_handle    (int);
>   static int    findslot    (int);
> +static int    _kill_shared    (int, int, int)
> __attribute__((__noreturn__));
>
>   /* Register name faking - works in collusion with the linker.  */
>   register char * stack_ptr asm ("sp");
> @@ -424,21 +425,35 @@ _close (int file)
>     return wrap (_swiclose (file));
>   }
>
> -int
> -_kill (int pid, int sig)
> +static int
> +_kill_shared (int pid, int sig, int reason)
>   {
> -  (void)pid; (void)sig;
> +  (void) pid; (void) sig;
>   #ifdef ARM_RDI_MONITOR
>     /* Note: The pid argument is thrown away.  */
> -  switch (sig) {
> -      case SIGABRT:
> -          do_AngelSWI (AngelSWI_Reason_ReportException,
> -                   (void *) ADP_Stopped_RunTimeError);
> -          __builtin_unreachable();
> -      default:
> -          do_AngelSWI (AngelSWI_Reason_ReportException,
> -                   (void *) ADP_Stopped_ApplicationExit);
> -  }
> +  int block[2];
> +  block[1] = sig;
> +  block[0] = reason;
> +  int insn;
> +
> +#if SEMIHOST_V2
> +  if (_has_ext_exit_extended ())
> +    {
> +      insn = AngelSWI_Reason_ReportExceptionExtended;
> +    }
> +  else
> +#endif
> +    {
> +      insn = AngelSWI_Reason_ReportException;
> +    }
> +
> +#if SEMIHOST_V2
> +if (_has_ext_exit_extended ())
> +  do_AngelSWI (insn, block);
> +else
> +#endif
> +  do_AngelSWI (insn, (void*)block[0]);
> +
>   #else
>     asm ("swi %a0" :: "i" (SWI_Exit));
>   #endif
> @@ -446,15 +461,24 @@ _kill (int pid, int sig)
>     __builtin_unreachable();
>   }
>
> +int
> +_kill (int pid, int sig)
> +{
> +  if (sig == SIGABRT)
> +    _kill_shared (pid, sig, ADP_Stopped_RunTimeError);
> +  else
> +    _kill_shared (pid, sig, ADP_Stopped_ApplicationExit);
> +}
> +
>   void
>   _exit (int status)
>   {
> -  /* There is only one SWI for both _exit and _kill. For _exit, call
> -     the SWI with the second argument set to -1, an invalid value for
> -     signum, so that the SWI handler can distinguish the two calls.
> -     Note: The RDI implementation of _kill throws away both its
> -     arguments.  */
> -  _kill(status, -1);
> +  /* The same SWI is used for both _exit and _kill.
> +     For _exit, call the SWI with "reason" set to
> ADP_Stopped_ApplicationExit
> +     to mark a standard exit.
> +     Note: The RDI implementation of _kill_shared throws away all its
> +     arguments and all implementations ignore the first argument. */
> +  _kill_shared (-1, status, ADP_Stopped_ApplicationExit);
>   }
>
>   pid_t
> --
> 2.7.4
>
>
>
>