Re: [Valgrind-developers] [valgrind] sigaltstack syscall: improve error messages

[Mark W. <ma...@kl...>]

Hi Paul,

On Tue, 2024-10-08 at 11:21 +0000, Paul Floyd wrote:
> https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=fc20240c84b5f61e4bf590d69eb50475fa2c119c
> 
> commit fc20240c84b5f61e4bf590d69eb50475fa2c119c
> Author: Paul Floyd <pj...@wa...>
> Date:   Tue Oct 8 13:19:08 2024 +0200
> 
>     sigaltstack syscall: improve error messages
>     
>     Previously the same message was generated for the 3 members of
>     stack_t.
> [...]
> diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
> index 9093b831d6..5434a68369 100644
> --- a/coregrind/m_syswrap/syswrap-generic.c
> +++ b/coregrind/m_syswrap/syswrap-generic.c
> @@ -5033,9 +5033,9 @@ PRE(sys_sigaltstack)
>                   const vki_stack_t *, ss, vki_stack_t *, oss);
>     if (ARG1 != 0) {
>        const vki_stack_t *ss = (vki_stack_t *)(Addr)ARG1;
> -      PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_sp, sizeof(ss->ss_sp) );
> -      PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_flags, sizeof(ss->ss_flags) );
> -      PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_size, sizeof(ss->ss_size) );
> +      PRE_MEM_READ( "sigaltstack(ss->ss_sp)", (Addr)&ss->ss_sp, sizeof(ss->ss_sp) );
> +      PRE_MEM_READ( "sigaltstack(ss->ss_size)", (Addr)&ss->ss_size, sizeof(ss->ss_size) );
> +      PRE_MEM_READ( "sigaltstack(ss->ss_flags)", (Addr)&ss->ss_flags, sizeof(ss->ss_flags) );
>     }
>     if (ARG2 != 0) {
>        PRE_MEM_WRITE( "sigaltstack(oss)", ARG2, sizeof(vki_stack_t) );

This is clearly a bug fix and I agree with the new names. Thanks.

But you might want to explicitly mention this in NEWS since it does
change how suppressions work for sigaltstack.

Previously a suppression like

{
   my-weird-sigaltstack-call
   Memcheck:Param
   sigaltstack(ss)
   fun:sigaltstack
   fun:main
}

would work, now that needs to be rewritten to refer to one of the new
messages.

Most likely nobody did the above, but it is technically a user visible
change, so would be good to explicitly mention.

Also I accidentially changed some syscall Param messages in the past
without realizing it is a user visible change, so I just mention it to
remind myself to be careful here.

Cheers,

Mark