Re: [OpenBIOS] [RFC 3/3] SPARC32/64: Mimic Sun's OBP behaviour if a divide by zero occurs.
On 07/01/13 16:47, Artyom Tarasenko wrote:
Hi Artyom,
Happy New Year!
As reported by Artyom Tarasenko, if anything is divided by zero then we should return no value:
0> 4 2 / u. 2 ok 0> 2 0 / ok
NAK. Either my memory is cheating on me or there is some other Artyom Tarasenko who reported it. OBP produces an exception in case of division by zero.
Here is the post I referenced sent by your imposter: http://lists.openbios.org/pipermail/openbios/2011-August/006626.html.
Also as noted in the first patch's commit message, most of the OpenBIOS platforms can't call Forth from a trap :( SPARC64 is the only exception here as I had to write something to enable va>tte-data to be called from an MMU trap, but that was just one platform and took me in the order of months. Hence why the patch takes the approach it does :/
(This time adding you to cc, since the mailing list bounces my mails saying my emai is graylisted - whatever it means).
Graylisting means that the first email from your domain (@gmail.com) in every period (say 24 hours) is temporarily delayed to prevent spam overload :)
ATB,
Mark.
On 2013-Jan-7 12:04 , Mark Cave-Ayland wrote:
NAK. Either my memory is cheating on me or there is some other Artyom Tarasenko who reported it. OBP produces an exception in case of division by zero.
Here is the post I referenced sent by your imposter: http://lists.openbios.org/pipermail/openbios/2011-August/006626.html.
Artyom's post says it returns no value _IFF_ he gets a trap 3e. A trap 3e (on SPARC) is a non-resumable trap; the world has gone to shit, all you can do is poke around in the debris and find out what blew up. In this case, it re-enters forth clobbering anything that might have been in progress.
I think Mark's patch is the correct one if this is indeed what PPC does; I'm suspicious because I see elsewhere that current PPCs do indeed trap on divide by zero. Rather than allowing divide by zero, I'd think it would make more sense to figure out where that is coming from and prevent whatever parameter is causing this from ever being set to values that will cause a trap.
On Jan 7, 2013, at 1:41 PM, Tarl Neustaedter wrote:
On 2013-Jan-7 12:04 , Mark Cave-Ayland wrote:
NAK. Either my memory is cheating on me or there is some other Artyom Tarasenko who reported it. OBP produces an exception in case of division by zero.
Here is the post I referenced sent by your imposter: http://lists.openbios.org/pipermail/openbios/2011-August/006626.html.
Artyom's post says it returns no value _IFF_ he gets a trap 3e. A trap 3e (on SPARC) is a non-resumable trap; the world has gone to shit, all you can do is poke around in the debris and find out what blew up. In this case, it re-enters forth clobbering anything that might have been in progress.
I think Mark's patch is the correct one if this is indeed what PPC does; I'm suspicious because I see elsewhere that current PPCs do indeed trap on divide by zero. Rather than allowing divide by zero, I'd think it would make more sense to figure out where that is coming from and prevent whatever parameter is causing this from ever being set to values that will cause a trap.
What PowerPC systems trap on divide by zero?
On 2013-Jan-7 13:44 , Programmingkid wrote:
What PowerPC systems trap on divide by zero?
PPC 8360
http://stackoverflow.com/questions/6460558/powerpc-how-to-make-div-0-return-...
On Mon, Jan 7, 2013 at 7:46 PM, Tarl Neustaedter tarl-b2@tarl.net wrote:
On 2013-Jan-7 13:44 , Programmingkid wrote:
What PowerPC systems trap on divide by zero?
PPC 8360 http://stackoverflow.com/questions/6460558/powerpc-how-to-make-div-0-return-...
Actually from this article I don't see that exception is happening in the Firmware. I'm very surprised, but our tests on RS/6000 (PReP) and Beige Rev 1 (thanks dreadbit for testing that!) show that PPC firmware allows division by zero:
0 > 1 0 / ok 1 > 0 0 / ok 2 > . 0 ok 1 > . 0 ok 0 >
It would be interesting to know what happens on a CHRP platform.
-- Regards, Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
What PowerPC systems trap on divide by zero?
PPC 8360
http://stackoverflow.com/questions/6460558/powerpc-how-to-make- div-0-return-zero-as-a-result
The 8360 is an e300 core, which implements the PowerPC architecture just fine, which means it will not generate an exception for any integer divide (or other integer insn) ever. The result of the divide instruction is not zero, but instead undefined (by the architecture; specific implementations have their own behaviour, of course).
The question you linked is about *floating point* division by zero. Those can be set up to generate an FP exception (not a trap exception!); if you want to be able to handle such exceptions, fudging things so it seems the instruction returned some specific value (as in the question) you will have to switch FP exceptions to be exact, which is very expensive performance-wise.
Anyway, divide by zero in Open Firmware. There are three things we want: a) Compliance to the standard; b) Traps if running on Sun hardware, since OBP does too; c) No traps if running on PowerPC hardware, since no existing system does, and Apple's bootloader depends on this behaviour.
So, how to implement this. Firstly, we should have the same behaviour for *all* divide insns (/ u/ mod umod /mod u/mod um/mod mu/mod sm/rem fm/mod and whatever I forgot), not just um/mod and those words derived from it. Secondly, we get what we want if we just use the machine's (or C's) divide operator. So it seems the actual problem (if there is one at all) is that OpenBIOS' mu/mod does not behave as the hardware would. Related, / and the other single-precision division words do not need to call mu/mod (which is very expensive), they can much cheaper be implemented as C words (cheaper in execution time that is -- it costs more code :-) )
Segher
On 11/01/13 18:22, Segher Boessenkool wrote:
So, how to implement this. Firstly, we should have the same behaviour for *all* divide insns (/ u/ mod umod /mod u/mod um/mod mu/mod sm/rem fm/mod and whatever I forgot), not just um/mod and those words derived from it. Secondly, we get what we want if we just use the machine's (or C's) divide operator. So it seems the actual problem (if there is one at all) is that OpenBIOS' mu/mod does not behave as the hardware would. Related, / and the other single-precision division words do not need to call mu/mod (which is very expensive), they can much cheaper be implemented as C words (cheaper in execution time that is -- it costs more code :-) )
Actually this got me thinking, since you're right in that integer division should require a manual trap. This took me into OpenBIOS's bundled version of libgcc and __udivmoddi4.c:
__uint128_t __udivmodti4(__uint128_t num, __uint128_t den, __uint128_t *rem_p) { __uint128_t quot = 0, qbit = 1;
if ( den == 0 ) { __divide_error(); return 0; /* If trap returns... */ }
.. .. }
A quick grep of the source shows that __divide_error() is manually set to the unexpected exception trap handler in start.S:
_GLOBAL(__divide_error): trap_error: mflr r3 LOAD_REG_FUNC(r4, unexpected_excep) mtctr r4 bctr
Heh. So actually all we need to do is provide an empty function for __divide_error() and we magically get the behaviour we want. Revised patch to follow shortly.
ATB,
Mark.
Actually this got me thinking, since you're right in that integer division should require a manual trap. This took me into OpenBIOS's bundled version of libgcc and __udivmoddi4.c:
__uint128_t __udivmodti4(__uint128_t num, __uint128_t den, __uint128_t *rem_p) { __uint128_t quot = 0, qbit = 1;
if ( den == 0 ) { __divide_error(); return 0; /* If trap returns... */ }
[It seems you showed __udivmodti4 instead -- but the code is much the same].
That's not libgcc code: it is x86 Linux kernel code or klibc or something like that. Libgcc forces a single-precision divide by zero when the double- precision routine divides by zero, so there can be no problem there.
A quick grep of the source shows that __divide_error() is manually set to the unexpected exception trap handler in start.S:
_GLOBAL(__divide_error): trap_error: mflr r3 LOAD_REG_FUNC(r4, unexpected_excep) mtctr r4 bctr
Heh. So actually all we need to do is provide an empty function for __divide_error() and we magically get the behaviour we want. Revised patch to follow shortly.
Or not have a __divide_error at all, which makes it clearer the intent is to have the double precision divide have the same behaviour as single precision on whatever hardware it runs on. Your patch should work though :-)
Segher
On 12/01/13 14:18, Segher Boessenkool wrote:
[It seems you showed __udivmodti4 instead -- but the code is much the same].
Oops ;)
That's not libgcc code: it is x86 Linux kernel code or klibc or something like that. Libgcc forces a single-precision divide by zero when the double- precision routine divides by zero, so there can be no problem there.
Okay.
A quick grep of the source shows that __divide_error() is manually set to the unexpected exception trap handler in start.S:
_GLOBAL(__divide_error): trap_error: mflr r3 LOAD_REG_FUNC(r4, unexpected_excep) mtctr r4 bctr
Heh. So actually all we need to do is provide an empty function for __divide_error() and we magically get the behaviour we want. Revised patch to follow shortly.
Or not have a __divide_error at all, which makes it clearer the intent is to have the double precision divide have the same behaviour as single precision on whatever hardware it runs on. Your patch should work though :-)
I suspect that it's used by other platforms within OpenBIOS, so I don't really want to remove it as it allows customisation of the behaviour as required.
I really like the last patch, so will commit tomorrow unless I hear any major objections from people on this thread. Thanks for your help on this.
ATB,
Mark.
On Sat, Jan 12, 2013 at 2:18 PM, Segher Boessenkool segher@kernel.crashing.org wrote:
Actually this got me thinking, since you're right in that integer division should require a manual trap. This took me into OpenBIOS's bundled version of libgcc and __udivmoddi4.c:
__uint128_t __udivmodti4(__uint128_t num, __uint128_t den, __uint128_t *rem_p) { __uint128_t quot = 0, qbit = 1;
if ( den == 0 ) { __divide_error(); return 0; /* If trap returns... */ }
[It seems you showed __udivmodti4 instead -- but the code is much the same].
That's not libgcc code: it is x86 Linux kernel code or klibc or something like that. Libgcc forces a single-precision divide by zero when the double- precision routine divides by zero, so there can be no problem there.
Some software archeology follows.
The function __udivmoddi4() was added by me in r4 as arch/sparc32/libgcc/__udivmoddi4.c in 2006. Then I moved the file with r61 to its current location in ./libgcc, with R62 copied this to __udivmodti4.c and changed 64 bit types to 128 bits.
This and files __divdi3.c, __udivdi3.c and __umoddi3.c seem to originate from klibc indeed. With 2f6cd4633e623c29bbcb793eff76c8c08c2c0f9c H. Peter Anvin added the files to klibc/arch/i386/libgcc/ in 2002, they have been moved since to usr/klibc/libgcc/ and reformatted. Before reformatting to kernel style, the files were identical to our version except for a header change.
Some klibc files like these are also used by Syslinux, iPXE and gPXE, but they may actually be newer than OpenBIOS r4. So I think I used klibc directly.
As specified by usr/klibc/LICENSE, klibc is a mix of GPLv2only, BSD and MIT licensed files, the default being MIT unless files originate from BSD or Linux. I think these files are therefore MIT licensed. GPLv2only and BSD would be OK for OpenBIOS too.
Files ashldi3.c and ashrdi3.c are from GCC 2.95.2 libgcc which was GPLv2+ at that time (way before GPLv3 switch). GPLv2+ is also fine for OpenBIOS.
The license situation could be clarified by copying the LICENSE from klibc to OpenBIOS.
A quick grep of the source shows that __divide_error() is manually set to the unexpected exception trap handler in start.S:
_GLOBAL(__divide_error): trap_error: mflr r3 LOAD_REG_FUNC(r4, unexpected_excep) mtctr r4 bctr
Heh. So actually all we need to do is provide an empty function for __divide_error() and we magically get the behaviour we want. Revised patch to follow shortly.
Or not have a __divide_error at all, which makes it clearer the intent is to have the double precision divide have the same behaviour as single precision on whatever hardware it runs on. Your patch should work though :-)
Segher
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
On 07/01/13 18:41, Tarl Neustaedter wrote:
Here is the post I referenced sent by your imposter: http://lists.openbios.org/pipermail/openbios/2011-August/006626.html.
Artyom's post says it returns no value _IFF_ he gets a trap 3e. A trap 3e (on SPARC) is a non-resumable trap; the world has gone to shit, all you can do is poke around in the debris and find out what blew up. In this case, it re-enters forth clobbering anything that might have been in progress.
Ah I see. So for example if on SPARC that was in the middle of a CIF interpret call or similar, then you'd be dropped back to the OpenBoot prompt at this point too? I didn't pick up on that from Artyom's original email.
I think Mark's patch is the correct one if this is indeed what PPC does; I'm suspicious because I see elsewhere that current PPCs do indeed trap on divide by zero. Rather than allowing divide by zero, I'd think it would make more sense to figure out where that is coming from and prevent whatever parameter is causing this from ever being set to values that will cause a trap.
Unfortunately the sad truth is that BootX is written to rely on this (ahem) feature to work if booting from anything that isn't the network:
0 value screenIH 0 value cursorAddr 0 value cursorX 0 value cursorY 0 value cursorW 0 value cursorH 0 value cursorFrames 0 value cursorPixelSize 0 value cursorStage 0 value cursorTime 0 value cursorDelay
...
: slw_spin_init dup FFFF and to cursorH 10 >> drop dup FFFF and to cursorW 10 >> to cursorPixelSize dup FFFF and to cursorY 10 >> d# 1000 swap / to cursorDelay dup FFFF and to cursorX 10 >> to cursorFrames to cursorAddr to screenIH ['] slw_spin to spin ;
And slw_spin_init is invoked from DrawSplashScreen() like this:
if (gBootFileType != kNetworkDeviceType) { SpinInit(0, 0, NULL, 0, 0, 0, 0, 0, 0, 0); }
... which of course when booting from the cd: device immediately traps without the patch. Sigh. On the plus side, the patch only changes the "/" Forth word within OpenBIOS in the hope that anything deliberately dividing by zero to invoke a fatal trap will still stop dead in its tracks.
ATB,
Mark.
On 2013-Jan-7 14:08 , Mark Cave-Ayland wrote:
.
Ah I see. So for example if on SPARC that was in the middle of a CIF interpret call or similar, then you'd be dropped back to the OpenBoot prompt at this point too? I didn't pick up on that from Artyom's original email.
Yup. Either drop to the ok prompt or fully reset the machine, depending on other settings.
Unfortunately the sad truth is that BootX is written to rely on this (ahem) feature to work if booting from anything that isn't the network:
0 value screenIH 0 value cursorAddr 0 value cursorX 0 value cursorY 0 value cursorW 0 value cursorH 0 value cursorFrames 0 value cursorPixelSize 0 value cursorStage 0 value cursorTime 0 value cursorDelay
...
: slw_spin_init dup FFFF and to cursorH 10 >> drop dup FFFF and to cursorW 10 >> to cursorPixelSize dup FFFF and to cursorY 10 >> d# 1000 swap / to cursorDelay dup FFFF and to cursorX 10 >> to cursorFrames to cursorAddr to screenIH ['] slw_spin to spin ;
And slw_spin_init is invoked from DrawSplashScreen() like this:
if (gBootFileType != kNetworkDeviceType) { SpinInit(0, 0, NULL, 0, 0, 0, 0, 0, 0, 0); }
The key error here (as I see it) is:
dup FFFF and to cursorY 10 >> d# 1000 swap / to cursorDelay
Where is that code? It looks like it's the third argument to "slw_spin_init" that is being used as a divisor, and it should be checked for zero.
I think this code (commenting to relate arguments to lines of code) looks like:
: slw_spin_init ( arg7 arg6 arg5 arg4 arg3 arg2 arg1 -- ) dup FFFF and to cursorH 10 >> drop( arg7 arg6 arg5 arg4 arg3 arg2 ) dup FFFF and to cursorW 10 >> to cursorPixelSize ( arg7 arg6 arg5 arg4 arg3 ) dup FFFF and to cursorY 10 >> d# 1000 swap / to cursorDelay ( arg7 arg6 arg5 arg4 ) dup FFFF and to cursorX 10 >> to cursorFrames ( arg7 arg6 arg5 ) to cursorAddr ( arg7 arg6 ) to screenIH ( arg7 ) ['] slw_spin to spin ; ( ) ;
So it looks like this code depends on arg3 being non-zero. I assume the arguments are coming from right-to-left, which is backwards from other c-to-forth implementations I've seen (usually the c call is procedure(arg1, arg2, arg3, ...) ), but that would not seem to fit here, since arg5 appears to require a pointer. Note that not all architectures use 0 == NULL; some use other values, so I wonder if this is a bug originating from some such usage combined with argument order confusion.
The better fix would be to check for zero in slw_spin_init. Or are you saying that both the call to SpinInit() /and/ the slw_spin_init( -- ) forth code are in BootX, code we aren't allowed to touch? If that's the case, indeed, a program which deliberately sets up a divide by zero expecting it to work has to be hacked around. To quote from that PPC 8360 article:
"I've inherited legacy code like this before & I feel your pain. You want to shake your fist at the people who installed such bone-headed behavior, but right now shaking your fist doesn't help you ship product. You need a solution. Good luck."
On 07/01/13 19:37, Tarl Neustaedter wrote:
The key error here (as I see it) is:
dup FFFF and to cursorY 10 >> d# 1000 swap / to cursorDelay
Where is that code? It looks like it's the third argument to "slw_spin_init" that is being used as a divisor, and it should be checked for zero.
...
So it looks like this code depends on arg3 being non-zero. I assume the arguments are coming from right-to-left, which is backwards from other c-to-forth implementations I've seen (usually the c call is procedure(arg1, arg2, arg3, ...) ), but that would not seem to fit here, since arg5 appears to require a pointer. Note that not all architectures use 0 == NULL; some use other values, so I wonder if this is a bug originating from some such usage combined with argument order confusion.
That's an interesting thought about argument order. Of course given that BootX has been supplied on OS X CDs in this state for a long time (and probably isn't even supported now) we are still left with having to work around it :(
The better fix would be to check for zero in slw_spin_init. Or are you saying that both the call to SpinInit() /and/ the slw_spin_init( -- ) forth code are in BootX, code we aren't allowed to touch? If that's the case, indeed, a program which deliberately sets up a divide by zero expecting it to work has to be hacked around. To quote from that PPC 8360 article:
"I've inherited legacy code like this before & I feel your pain. You want to shake your fist at the people who installed such bone-headed behavior, but right now shaking your fist doesn't help you ship product. You need a solution. Good luck."
Well the source for SpinInit() and friends can be found here: http://www.opensource.apple.com/source/BootX/BootX-74.1/bootx.tproj/ci.subpr..., and as you can see slw_spin_init is lovingly hard-coded into BootX in this form. The C snippet from my last email invoking SpinInit() with all zero arguments is also hard-coded into a different file. So it looks like I'll need all the luck I can get... I'm hoping that the RFC patch is the lesser of all evils though ;)
ATB,
Mark.
-
Artyom Tarasenko -
Blue Swirl -
Mark Cave-Ayland -
Programmingkid -
Segher Boessenkool -
Tarl Neustaedter