diff mbox series

testsuite/ubsan/overflow-div-3.c: Use SIGTRAP for MIPS

Message ID 20240620071617.61040-1-syq@gcc.gnu.org
State New
Headers
Series testsuite/ubsan/overflow-div-3.c: Use SIGTRAP for MIPS |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

YunQiang Su June 20, 2024, 7:16 a.m. UTC 
  The DIV instructions of MIPS won't be trapped themself if the divisor
is zero.  The compiler will emit a conditional trap instruct for it.
So the signal will be SIGTRAP instead of SIGFPE.

gcc/testsuite
	* c-c++-common/ubsan/overflow-div-3.c: Use SIGTRAP for MIPS.
---
 gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Maciej W. Rozycki June 20, 2024, 10:57 a.m. UTC | #1 
On Thu, 20 Jun 2024, YunQiang Su wrote:

> The DIV instructions of MIPS won't be trapped themself if the divisor
> is zero.  The compiler will emit a conditional trap instruct for it.
> So the signal will be SIGTRAP instead of SIGFPE.

 It's an OS kernel bug if you get SIGTRAP for integer division by zero, so 
if a test case fails, it rightfully does.

  Maciej
Richard Biener June 20, 2024, 11:17 a.m. UTC | #2 
On Thu, Jun 20, 2024 at 12:57 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Thu, 20 Jun 2024, YunQiang Su wrote:
>
> > The DIV instructions of MIPS won't be trapped themself if the divisor
> > is zero.  The compiler will emit a conditional trap instruct for it.
> > So the signal will be SIGTRAP instead of SIGFPE.
>
>  It's an OS kernel bug if you get SIGTRAP for integer division by zero, so
> if a test case fails, it rightfully does.

MIPS doesn't trap on integer division by zero, instead it's a GCC bug that
we emit a conditional trapping instruction.  So I think the fix is fine and
accurate.

Richard.

>   Maciej
Maciej W. Rozycki June 20, 2024, 1:58 p.m. UTC | #3 
On Thu, 20 Jun 2024, Richard Biener wrote:

> > > The DIV instructions of MIPS won't be trapped themself if the divisor
> > > is zero.  The compiler will emit a conditional trap instruct for it.
> > > So the signal will be SIGTRAP instead of SIGFPE.
> >
> >  It's an OS kernel bug if you get SIGTRAP for integer division by zero, so
> > if a test case fails, it rightfully does.
> 
> MIPS doesn't trap on integer division by zero, instead it's a GCC bug that
> we emit a conditional trapping instruction.  So I think the fix is fine and
> accurate.

 Then GCC emits the wrong trap instruction, wherever it comes from and 
whatever has caused it.  The correct ones for integer division by zero 
(and FWIW overflow; and also the corresponding BREAK variants for MIPS I 
compatibility) have been designated in the ABI decades ago, documented in 
MIPS architecture handbooks, including printed ones, and they are supposed 
to cause the OS kernel to deliver SIGFPE (with the code correctly set) 
rather than SIGTRAP.

 It's been like this for 20+ years now, used to work last time I checked 
(but it may have been a decade ago, possibly when the microMIPS port was 
added, which sadly needlessly repeated IRIX compatibility breakage here), 
and suddenly requiring a SIGTRAP here would be a divergence from the ABI 
and a functional regression.  In fact I fixed bugs in this area in the 
Linux kernel myself, beacuse indeed it used to send SIGTRAP incorrectly, 
but again it was 20+ years ago.

 FAOD GAS gets the trap encoding right in output from the DIV, MULO, REM, 
etc. macros in the MIPS assembly dialect.

  Maciej
YunQiang Su June 20, 2024, 11:50 p.m. UTC | #4 
>
>  Then GCC emits the wrong trap instruction, wherever it comes from and
> whatever has caused it.  The correct ones for integer division by zero

Thanks so much. It is not the bug of Linux kernel or GCC.
It is a bug of me ;) and qemu.

Qemu didn't pass the code of TEQ correctly; and I haven't run this test on
real hardware.
Maciej W. Rozycki June 21, 2024, 12:12 p.m. UTC | #5 
On Fri, 21 Jun 2024, YunQiang Su wrote:

> >  Then GCC emits the wrong trap instruction, wherever it comes from and
> > whatever has caused it.  The correct ones for integer division by zero
> 
> Thanks so much. It is not the bug of Linux kernel or GCC.
> It is a bug of me ;) and qemu.
> 
> Qemu didn't pass the code of TEQ correctly; and I haven't run this test on
> real hardware.

 QEMU is a simulator only and has bugs or other discrepancies from real 
hardware.  Especially the user emulation mode combines issues with actual 
instruction simulation *and* the OS interface.  Therefore you can use QEMU 
as a valuable tool in the course of making your changes, but you need to 
always verify the final result with real hardware before submitting it 
upstream.

 Also submissions need to include details as to how the problem addressed 
has been reproduced.

 Otherwise this is just producing noise and wasting people's time.

  Maciej
diff mbox series

Patch

diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c b/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c
index 479dffb0304..eef58aca832 100644
--- a/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c
+++ b/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c
@@ -7,6 +7,11 @@ 
 #include <stdlib.h>
 
 int cnt;
+#ifdef __mips
+ int sig = SIGTRAP;
+#else
+ int sig = SIGFPE;
+#endif
 
 __attribute__((noipa)) int
 foo (int x, int y)
@@ -30,7 +35,7 @@  main (void)
   sigemptyset (&s.sa_mask);
   s.sa_handler = handler;
   s.sa_flags = 0;
-  sigaction (SIGFPE, &s, NULL);
+  sigaction (sig, &s, NULL);
   volatile int a = foo (42, 0);
   cnt++;
   volatile int b = foo (INT_MIN, -1);