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

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,
> Matthew
> 
> ###############     Attachment also inlined for ease of reply    ###############
> 
> 
> diff --git a/libgloss/arm/_exit.c b/libgloss/arm/_exit.c
> index 4a071df58e4c1bc48c64e62d2eae62d33224e806..180541eceb8dbac69e160fdda24af2696a5599a2 100644
> --- a/libgloss/arm/_exit.c
> +++ b/libgloss/arm/_exit.c
> @@ -1,15 +1,16 @@
>  #include <_ansi.h>
> +#include "swi.h"
>  
> -int _kill (int, int) __attribute__((__noreturn__));
> +int _kill_shared (int, int, int) __attribute__((__noreturn__));
>  void _exit (int);
>  
>  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);
>  }
> diff --git a/libgloss/arm/_kill.c b/libgloss/arm/_kill.c
> index 34a6ffddfa0e458cb8db3633249d533fbab98a67..69f9856537a9312ff4d6bc1ccdca38e22e7de50f 100644
> --- a/libgloss/arm/_kill.c
> +++ b/libgloss/arm/_kill.c
> @@ -2,16 +2,27 @@
>  #include <signal.h>
>  #include "swi.h"
>  
> -int _kill (int, int) __attribute__((__noreturn__));
> +int _kill_shared (int, int, int) __attribute__((__noreturn__));
> +int _kill (int, int);
>  
>  int
>  _kill (int pid, int sig)
>  {
> +  if (sig == SIGABRT)
> +    _kill_shared (pid, sig, ADP_Stopped_RunTimeError);
> +  else
> +    _kill_shared (pid, sig, ADP_Stopped_ApplicationExit);
> +}
> +
> +int
> +_kill_shared (int pid, int sig, int reason)
> +{
>    (void) pid; (void) sig;
>  #ifdef ARM_RDI_MONITOR
>    /* Note: The pid argument is thrown away.  */
>    int block[2];
>    block[1] = sig;
> +  block[0] = reason;
>    int insn;
>  
>  #if SEMIHOST_V2
> @@ -25,20 +36,6 @@ _kill (int pid, int sig)
>        insn = AngelSWI_Reason_ReportException;
>      }
>  
> -  switch (sig)
> -    {
> -    case SIGABRT:
> -      {
> -	block[0] = ADP_Stopped_RunTimeError;
> -	break;
> -      }
> -    default:
> -      {
> -	block[0] = ADP_Stopped_ApplicationExit;
> -	break;
> -      }
> -    }
> -
>  #if SEMIHOST_V2
>  if (_has_ext_exit_extended ())
>    do_AngelSWI (insn, block);
> 
> 
> arm-kill-exit.patch
> 
> diff --git a/libgloss/arm/_exit.c b/libgloss/arm/_exit.c
> index 4a071df58e4c1bc48c64e62d2eae62d33224e806..180541eceb8dbac69e160fdda24af2696a5599a2 100644
> --- a/libgloss/arm/_exit.c
> +++ b/libgloss/arm/_exit.c
> @@ -1,15 +1,16 @@
>  #include <_ansi.h>
> +#include "swi.h"
>  
> -int _kill (int, int) __attribute__((__noreturn__));
> +int _kill_shared (int, int, int) __attribute__((__noreturn__));
>  void _exit (int);
>  
>  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);
>  }
> diff --git a/libgloss/arm/_kill.c b/libgloss/arm/_kill.c
> index 34a6ffddfa0e458cb8db3633249d533fbab98a67..69f9856537a9312ff4d6bc1ccdca38e22e7de50f 100644
> --- a/libgloss/arm/_kill.c
> +++ b/libgloss/arm/_kill.c
> @@ -2,16 +2,27 @@
>  #include <signal.h>
>  #include "swi.h"
>  
> -int _kill (int, int) __attribute__((__noreturn__));
> +int _kill_shared (int, int, int) __attribute__((__noreturn__));
> +int _kill (int, int);
>  
>  int
>  _kill (int pid, int sig)
>  {
> +  if (sig == SIGABRT)
> +    _kill_shared (pid, sig, ADP_Stopped_RunTimeError);
> +  else
> +    _kill_shared (pid, sig, ADP_Stopped_ApplicationExit);
> +}
> +
> +int
> +_kill_shared (int pid, int sig, int reason)
> +{
>    (void) pid; (void) sig;
>  #ifdef ARM_RDI_MONITOR
>    /* Note: The pid argument is thrown away.  */
>    int block[2];
>    block[1] = sig;
> +  block[0] = reason;
>    int insn;
>  
>  #if SEMIHOST_V2
> @@ -25,20 +36,6 @@ _kill (int pid, int sig)
>        insn = AngelSWI_Reason_ReportException;
>      }
>  
> -  switch (sig)
> -    {
> -    case SIGABRT:
> -      {
> -	block[0] = ADP_Stopped_RunTimeError;
> -	break;
> -      }
> -    default:
> -      {
> -	block[0] = ADP_Stopped_ApplicationExit;
> -	break;
> -      }
> -    }
> -
>  #if SEMIHOST_V2
>  if (_has_ext_exit_extended ())
>    do_AngelSWI (insn, block);
>