[perl #52454] [PATCH] t/compilers/imcc/syn/regresssions_2 divide by zero

# New Ticket Created by  Andy Dougherty 
# Please include the string:  [perl #52454]
# in the subject line of all future correspondence about this issue. 
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=52454 >


The test in t/compilers/imcc/syn/regressions_2.pir is failing, at
least on SPARC, with the following error message.

error:imcc:The opcode 'div_i_ic_ic' (div<3>) was not found. Check the type and number of the arguments
	in file 't/compilers/imcc/syn/regressions_2.pir' line 3

It apparently is passing on Linux/x86.

The test itself is as follows:

    .sub fold_by_zero
      push_eh ok
	$I1 = 1/0
      pop_eh
     ok:
      say "ok"
    .end


What is going on is a combination of several things.

First, deep inside IMCC_subst_constants() in compilers/imcc/optimizer.c,
when the divide by zero fails, it returns a NULL value, but forgets to
also set the undocumented "return" value for 'ok'.  The appended patch
sets it to 0, figuring all is not ok if we encountered an exception.

This is a problem because in compilers/imcc/parser_util.c, inside INS(),
the ok variable is never initialized.  Apparently, on x86, it ends up
filled with some non-zero value, but on SPARC it ends up as a zero.
The appended patch initializes the variable.

What to do with the return values, however, is a different question.
After this patch is applied, the information that an exception occured
has now correctly propagated up to the INS() function, but it's not
at all clear to me what to do with it from here.

Finally, this hasn't shown up on x86 because the test itself is
misleading.  Because the uninitialized variable caused the 1/0 to
appear to succeed, no error handler ever needed to be invoked.
Parrot simply continued stepping through the function to the
C< say "ok" > line, just as it would have if it had calculated 1/1
instead of 1/0.  This patch fixes that problem.

In the end, this levels the playing field -- this test now fails for
everyone.  I think the next step forward is for someone more familiar with
the PIR design than me to decide what behavior is actually wanted when
'1/0' appears in the source.  I'm also completely unfamiliar with
exception handlers in parrot, so I don't know where they are supposed to
figure in either.


diff -r -u parrot-svn/compilers/imcc/optimizer.c parrot-andy/compilers/imcc/optimizer.c
--- parrot-svn/compilers/imcc/optimizer.c	2008-03-31 15:14:39.000000000 -0400
+++ parrot-andy/compilers/imcc/optimizer.c	2008-04-03 13:14:52.000000000 -0400
@@ -976,8 +976,10 @@
      * from the result
      */
     branched = eval_ins(interp, op, found, r);
-    if (branched == -1)
+    if (branched == -1) {
+        *ok = 0; /* XXX Is this return value sensible? */
          return NULL;
+    }
     /*
      * for math ops result is in I0/N0
      * if it was a branch with constant args, the result is
diff -r -u parrot-svn/compilers/imcc/parser_util.c parrot-andy/compilers/imcc/parser_util.c
--- parrot-svn/compilers/imcc/parser_util.c	2008-03-31 15:14:40.000000000 -0400
+++ parrot-andy/compilers/imcc/parser_util.c	2008-04-03 13:17:44.000000000 -0400
@@ -642,14 +642,18 @@
         op = try_find_op(interp, unit, name, r, n, keyvec, emit);
 
     if (op < 0) {
-        int ok;
+        int ok = 0;
 
         /* check mixed constants */
         ins = IMCC_subst_constants_umix(interp, unit, name, r, n + 1);
         if (ins)
             goto found_ins;
 
-        /* and finally multiple constants */
+        /* and finally multiple constants
+           Returns ins==NULL, ok == 0 if an exception occured.
+           XXX The logic here isn't quite right, as imcc may then
+           report the instruction was not found.
+        */
         ins = IMCC_subst_constants(interp, unit, name, r, n + 1, &ok);
 
         if (ok) {
diff -r -u parrot-svn/t/compilers/imcc/syn/regressions.t parrot-andy/t/compilers/imcc/syn/regressions.t
--- parrot-svn/t/compilers/imcc/syn/regressions.t	2008-04-02 10:23:14.000000000 -0400
+++ parrot-andy/t/compilers/imcc/syn/regressions.t	2008-04-03 13:18:44.000000000 -0400
@@ -20,6 +20,8 @@
 .sub fold_by_zero
   push_eh ok
     $I1 = 1/0
+    say "Not ok"
+    end
   pop_eh
  ok:
   say "ok"

-- 
    Andy Dougherty		doughera@lafayette.edu

• [perl #52454] [PATCH] t/compilers/imcc/syn/regresssions_2 divide by zero by Andy Dougherty
• Re: [perl #52454] [PATCH] t/compilers/imcc/syn/regresssions_2 divide by zero by chromatic