Issue 1078 - Std.random(0) causes a division by zero in windows CPP target - haxe

[issuesbot]

[Google Issue #1078 : https://code.google.com/p/haxe/issues/detail?id=1078] by AtlasS...@gmail.com, at 22/07/2012, 00:38:17 What steps will reproduce the problem?

1. Call Std.random() with a zero parameter

What is the expected output? What do you see instead? I expect a return value of zero, but I get an error instead.

Please complete the following class with minimal code reproducing the problem :

class Test {
    static function main() {
        var x = Std.random(0);
        }
    }

Debugging in Visual Studio allows me to see the error "0xC0000094: Integer division by zero." in StdLibs.cpp. The relevant function is this: int __hxcpp_irand(int inMax)

{
    unsigned int lo = rand() & 0xfff;
    unsigned int mid = rand() & 0xfff;
    unsigned int hi = rand() & 0xff;
    return (lo | (mid<<12) | (hi<<24) ) % inMax;
    }

Clearly, it would be a good idea to check the input value. Maybe something like this:

int __hxcpp_irand(int inMax)

{
    if (inMax == 0)
    {
        return 0;
        }
    unsigned int lo = rand() & 0xfff;
    unsigned int mid = rand() & 0xfff;
    unsigned int hi = rand() & 0xff;
    return (lo | (mid<<12) | (hi<<24) ) % inMax;
    }

I marked this as urgent because it's a simple fix and rally shouldn't be causing an error, but I'm not sure what the convention is.

[issuesbot]

[comment from gameh...@gmail.com, published at 23/07/2012, 05:58:03] Since random returns a value 0 <= x < inMax, random(0) is an error (no valid x satisfies this equation), so maybe it should be:

if (inMax == 0) CriticalError("Invalid input");

which could be added in debug mode.

[issuesbot]

[comment from ncanna...@gmail.com, published at 23/07/2012, 10:24:44] I agree this is an error in theory, but this is not consistent with other platforms, and in practice from my own experience it introduces hard-to-reproduce bugs in games mechanisms.

Example :

var max = Std.random(100); var min = Std.random(max);

There's a 1/100 probability to trigger the bug, which is hard to reproduce.

So for neko as well I ended up ignoring <= 0 values (simply returning 0 in that case).

[issuesbot]

[comment from gameh...@gmail.com, published at 25/07/2012, 02:00:23] Well in this case, I think crashing is a good thing to do - exactly to prevent those hard-to-find bugs. Left as is, all your ranges are going to be bigger than zero - except the 0-0 range, which will need to be treated as a special case later, or perhaps case hard-to-find bugs.. Do you want max==min ? this is either desirable, in which case you want min = Std.random(max+1) or is undesirable, in which case you want: max = Std.random(99)+1 Either way, I think it should be explicitly addressed in the random call.

[issuesbot]

[comment from AtlasS...@gmail.com, published at 25/07/2012, 02:19:29] I must have misread the docs on this one. At any rate, it would certainly be nice to check and provide a meaningful error, rather than crashing in such an obscure way.

[issuesbot]

[comment from ncanna...@gmail.com, published at 25/07/2012, 02:42:13] @ hugh : well, it depends if you are crashing while testing your game or if it's the player which is crashing. In the later case your only debug info you'll get is "the game is crashing sometimes" :) That's what I call hard to reproduce.

I'm definitely for input valid validation and defensive programming in general, but I don't think that applying it to Std.random is a good idea, especially since other platforms don't have the issue. I'm talking from my own experience using a lot of randoms in our games, and really you don't want to spend time ensuring that all of them will never get a 0 input, given that 0 is always a valid output for it.

[issuesbot]

[comment from gameh...@gmail.com, published at 25/07/2012, 05:02:43] Yes, as I said, It would be best to throw an error, and then the user will report "What is this random-number out of range error I get every now and then?" Alternatively, Nicolas could put an exception in the documentation and a unit test - this would make me fix the problem according to spec. I guess the output for negative numbers should also be specified.

[issuesbot]

[comment from ncanna...@gmail.com, published at 25/07/2012, 09:30:07] Even in that case, unless you have a full exception stack, you don't want to have to check every Std.random call for possible invalid input. I agree with exceptions when there is no valid result to return given the input, but in that case returning 0 actually makes sense.

I'll specify Std.random(0) == 0 and Std.random(-1) == 0 as well in unit tests :)

[issuesbot]

[comment from simon.kr...@simn.de, published at 14/08/2012, 15:49:38] This has been fixed.