Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.

[Joe Simmons-Talbott <josimmon@redhat.com>]

On Tue, Apr 11, 2023 at 01:39:31PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 11/04/23 13:03, Florian Weimer wrote:
> > * Adhemerval Zanella Netto:
> > 
> >> On 11/04/23 10:50, Florian Weimer via Libc-alpha wrote:
> >>> * Joe Simmons-Talbott via Libc-alpha:
> >>>
> >>>>    ({ long _sys_result;						\
> >>>>       {								\
> >>>>         LOAD_ARGS_##nr (args)					\
> >>>>         register long _x8 asm ("x8") = (name);			\
> >>>> +       if (__builtin_constant_p(name))				\
> >>>> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
> >>>> +                       : /* no output */ : "i" (name) : "x8");	\
> >>>>         asm volatile ("svc	0	// syscall " # name     \
> >>>>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
> >>>>         _sys_result = _x0;					\
> >>>
> >>> I think you should do this in a single assembler statement, load the
> >>> constant only once.
> >>
> >> Is this required because compiler is free to reorganize the argument
> >> list? I think it should me it clear on the commit message.
> > 
> > Yes, that's the reason.  It's a bit tricky to recover the system call
> > number using static analysis otherwise.  I suggested to Joe that we
> > should put something into glibc, rather than improving that static
> > analysis tool so that it's fully reliable.
> 
> Direct syscalls are done by different projects, like sanitizer, libgomp,
> etc; so imho improving the static analysis tool could potentially catch
> a wide range of usages than trying to fix only on glibc.
> 

I agree that improving the static analysis tool would be helpful.  One
thing to keep in mind is that this patchset will also aid people doing
static analysis manually.  It also seems likely that other static
analysis tools would benefit from this change.

Thanks,
Joe