Re: [LTP] [PATCH] syscalls/waitid10: Fix on ARM, PPC and possibly others

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> >> While integer division by zero does trap on x86_64 and causes the SIGFPE
>> >> signal to be delivered it's not the case on all architecutes. At least
>> >> on ARM and PPC64LE division by zero simply returns undefined result
>> >> instead.
>> 
>> Nit picking: even with this patch we are still testing undefined
>> behaviour.
>> 
>>    There are six signals that can be delivered as a consequence of a
>>    hardware exception: SIGBUS, SIGEMT, SIGFPE, SIGILL, SIGSEGV, and
>>    SIGTRAP.  Which of these signals is delivered, for any given
>>    hard- ware exception, is not documented and does not always make
>>    sense.
>> 
>> If dividing by zero produces SIGEMT then it's still valid according to
>> the specification. FPE does stand for floating point exception, but we
>> are dividing integers.
>
> Actually as far as I can tell the POSIX says that for integer division
> by zero you shall get SIGFPE (and si_code in siginfo se tto FPE_INTDIV)
> if the operation traps. It seems to be pretty well defined:
>
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html
>
>> >> 
>> >> This patch adds raise(SIGFPE) at the end of the child as a fallback to
>> >> make sure the process is killed with the right signal on all
>> >> architectures.
>> >> 
>> >> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>> >> ---
>> >>  testcases/kernel/syscalls/waitid/waitid10.c | 5 ++++-
>> >>  1 file changed, 4 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/testcases/kernel/syscalls/waitid/waitid10.c b/testcases/kernel/syscalls/waitid/waitid10.c
>> >> index 869ef18bd..8c351d120 100644
>> >> --- a/testcases/kernel/syscalls/waitid/waitid10.c
>> >> +++ b/testcases/kernel/syscalls/waitid/waitid10.c
>> >> @@ -28,7 +28,10 @@ static void run(void)
>> >>  		volatile int a, zero = 0;
>> >>  
>> >>  		a = 1 / zero;
>> >> -		exit(a);
>> >> +
>> >> +		tst_res(TINFO, "Division by zero didn't trap, raising SIGFPE");
>> >
>> > This patch inroduces 'set but not used' warning for the a variable so
>> > maybe the message should look like:
>> >
>> > 		tst_res(TINFO, "1/0 = %i raising SIGFPE", a);
>> >
>> >> +		raise(SIGFPE);
>> 
>> I'm wondering if we should branch on the architecture. If it's x86[_64]
>> then we only do divide by zero as it's reasonable to think that if the
>> signal is not raised then this is a bug.
>
> That would work too I guess.

I would still use #ifdef to remove raise(SIGFPE) on x86. I think this
makes it a more solid test on x86. As it is defined in the spec I guess
you could do the divide by zero on other arches and we can review the
results to see if any also raise SIGFPE.

With that:
Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp