Re: [OpenBIOS] [PATCH 9/10] Divide by zero check

[Andreas Färber]

Am 10.08.2011 um 17:48 schrieb William Hahne:
> On Wed, Aug 10, 2011 at 6:00 AM, Mark Cave-Ayland <[e-mail]
> > wrote:
> > On 09/08/11 22:55, William Hahne wrote:
> > > It is possible for BootX to purposefully divide by zero expecting  
> > > 0 as
> > > the result.
> > > Index: kernel/forth.c
> > >
> > > --- kernel/forth.c (revision 1041)
> > > +++ kernel/forth.c (working copy)
> > > @@ -1157,6 +1157,12 @@
> > >  {
> > > const ucell b = POP();
> > > const ducell a = DPOP();
> > > if (b == 0) { // can't divide by zero
> > >        PUSH(0);
> > >        DPUSH(0);
> > >        return;
> > > }
> > >
> > > #ifdef NEED_FAKE_INT128_T
> > >          if (a.hi != 0) {
> > >              fprintf(stderr, "mudivmod called (0x%016llx %016llx /
> > > 0x%016llx)\n",
> >
> > There is something wrong here: nothing will purposefully divide by  
> > zero as it will invoke a trap somewhere. Again, I think this is  
> > hiding another bug somewhere and is not necessarily the correct fix.
>
> You would be surprised then, if you type "0 0 /" into a OF mac you  
> will get 0. I thought it was a bug as well at first but after  
> carefully looking at the source code for BootX I found that it  
> wasn't. This is probably just a peculiarity of Apple's OF  
> implementation.

I'd rather think this is an architectural difference: On ppc, divwo  
sets the OV bit in XER but divw doesn't, PowerISA 2.06B saying that  
the result of the division is undefined in both cases.

On POWER5 I do get zero as well, so it's not just Apple.

0 > 0 0 /  ok
1 > . 0  ok

I'd suggest to stick the new code fragment into an appropriate ppc  
#ifdef if sparc does it differently.

Andreas

[PATCH 9/10] Divide by zero check

[William Hahne]

On Wed, Aug 10, 2011 at 11:54 AM, Andreas Färber wrote:
> Am 10.08.2011 um 17:48 schrieb William Hahne:
>
> On Wed, Aug 10, 2011 at 6:00 AM, Mark Cave-Ayland <
> > [e-mail].**uk >
> > wrote:
> > > On 09/08/11 22:55, William Hahne wrote:
> > >
> > > It is possible for BootX to purposefully divide by zero expecting 0 as
> > > > the result.
> > > >
> > > > Index: kernel/forth.c
> > > > ==============================**==============================**=======
> > > > --- kernel/forth.c (revision 1041)
> > > > +++ kernel/forth.c (working copy)
> > > > @@ -1157,6 +1157,12 @@
> > > >  {
> > > > const ucell b = POP();
> > > > const ducell a = DPOP();
> > > > if (b == 0) { // can't divide by zero
> > > >        PUSH(0);
> > > >        DPUSH(0);
> > > >        return;
> > > > }
> > > >
> > > > #ifdef NEED_FAKE_INT128_T
> > > >         if (a.hi != 0) {
> > > >             fprintf(stderr, "mudivmod called (0x%016llx %016llx /
> > > > 0x%016llx)\n",
> > >
> > > There is something wrong here: nothing will purposefully divide by zero
> > > as it will invoke a trap somewhere. Again, I think this is hiding another
> > > bug somewhere and is not necessarily the correct fix.
> >
> > You would be surprised then, if you type "0 0 /" into a OF mac you will
> > get 0. I thought it was a bug as well at first but after carefully looking
> > at the source code for BootX I found that it wasn't. This is probably just a
> > peculiarity of Apple's OF implementation.
>
> I'd rather think this is an architectural difference: On ppc, divwo sets
> the OV bit in XER but divw doesn't, PowerISA 2.06B saying that the result of
> the division is undefined in both cases.
>
> On POWER5 I do get zero as well, so it's not just Apple.
>
> 0 > 0 0 /  ok
> 1 > . 0  ok
>
> I'd suggest to stick the new code fragment into an appropriate ppc #ifdef
> if sparc does it differently.
>
> Andreas

That makes sense. Here is a patch with an appropriate #ifdef added.

Index: kernel/forth.c
===================================================================
--- kernel/forth.c      (revision 1046)
+++ kernel/forth.c      (working copy)
@@ -1157,6 +1157,17 @@
 {
        const ucell b = POP();
        const ducell a = DPOP();
+
+#if defined(CONFIG_PPC) || defined(CONFIG_PPC64)
+       /* PPC OF provides a result of 0 after a division by zero
+           rather than an exception */
+        if (b == 0) {
+            PUSH(0);
+            DPUSH(0);
+            return;
+       }
+#endif
+
 #ifdef NEED_FAKE_INT128_T
         if (a.hi != 0) {
             fprintf(stderr, "mudivmod called (0x%016llx %016llx /
0x%016llx)\n",
>

William Hahne