Re: powerpc Linux scv support and scv system call ABI proposal

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 30/01/2020 10:50, Segher Boessenkool wrote:
> Hi again,
> 
> On Thu, Jan 30, 2020 at 01:03:53PM +0100, Florian Weimer wrote:
>>> This is why that *is* the only supported use.  The documentation could
>>> use a touch-up, I think.  Unless we still have problems here?
>>
>> I really don't know.  GCC still has *some* support for the old behavior,
>> though.
> 
> No.  No support.  It still does some of the same things, but that can
> change (and probably should).  But this hasn't been supported since the
> dark ages, and the documentation has become gradually more explicit
> about it.
> 

I think this might be related to an odd sparc32 issue I am seeing with 
newer clock_nanosleep.  The expanded code is:

--
  register long err __asm__("g1");                                   // INTERNAL_SYSCALL_DECL  (err)
  r = ({                                                             // r = INTERNAL_SYSCALL_CANCEL (...)
	 long int sc_ret;
         if (SINGLE_THREAD_P)
	   sc_ret = INTERNAL_SYSCALL_CALL (__VA_ARGS__);
         else
           {
	     int sc_cancel_oldtype = __libc_enable_asynccancel ();
	     sc_ret = INTERNAL_SYSCALL_CALL (__VA_ARGS__);          // It issues the syscall with the asm (...)
	     __librt_disable_asynccancel (sc_cancel_oldtype);
	   }
         sc_ret;
       });
  if ((void) (val), __builtin_expect((err) != 0, 0))                // if (! INTERNAL_SYSCALL_ERROR_P (r, err))
    return 0;
  if ((-(val)) != ENOSYS)                                           // if (INTERNAL_SYSCALL_ERRNO (r, err) != ENOSYS)
    return ((-(val)));                                              //   return INTERNAL_SYSCALL_ERRNO (r, err);

  [...]

  r = ({                                                             // r = INTERNAL_SYSCALL_CANCEL (...)
       [...]
      )}
  if ((void) (val), __builtin_expect((err) != 0, 0))                // if (! INTERNAL_SYSCALL_ERROR_P (r, err))
    {
      [...]
    }
  return ((void) (val), __builtin_expect((err) != 0, 0))            // return (INTERNAL_SYSCALL_ERROR_P (r, err)
         ? ((-(val))) : 0;                                          //        ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
--

It requires that 'err' (assigned to 'g1') be value propagated over
functions calls and over different scopes, which I take from your 
explanation is not supported and fragile. It also seems that if I 
move the __libc_enable_* calls before 'err' initialization and after
its usage the code seems to works, but again it seems this usage
is not really supported on gcc.

So it seems that the current usage of 'INTERNAL_SYSCALL_DECL' and
'INTERNAL_SYSCALL_ERROR_P' are fragile if the architecture *does*
use the 'err' variable and it is defined a register alias (which 
its the case only for sparc currently).

Although a straightforward for sparc would be redefine 
INTERNAL_SYSCALL_DECL to not use a register alias, I still think
we should just follow Linux kernel ABI convention where value in 
the range between -4095 and -1 indicates an error and handle any 
specific symbols that might not strictly follow it with an 
arch-specific implementation (as we do for lseek on x32 and
mips64n32).  It would allow cleanup a lot of code and avoid such
pitfalls.