moonsharp-devs / moonsharp

An interpreter for the Lua language, written entirely in C# for the .NET, Mono, Xamarin and Unity3D platforms, including handy remote debugger facilities.
http://www.moonsharp.org
Other
1.41k stars 213 forks source link

math.randomseed() called with very large numbers causes an overflowexception #230

Closed anotak closed 4 years ago

anotak commented 6 years ago

try with anything greater than int.MaxValue

here's the relevant line https://github.com/xanathar/moonsharp/blob/master/src/MoonSharp.Interpreter/CoreLib/MathModule.cs#L288

this 9 year old stackoverflow post might be of interest https://stackoverflow.com/questions/1148278/bug-in-system-random-constructor

throws


Exception: Negating the minimum value of a twos complement number is invalid.
Exception:    at System.Math.AbsHelper(Int32 value)
   at System.Random..ctor(Int32 Seed)
   at MoonSharp.Interpreter.CoreLib.MathModule.randomseed(ScriptExecutionContext executionContext, CallbackArguments args) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\CoreLib\MathModule.cs:line 287
   at MoonSharp.Interpreter.CallbackFunction.Invoke(ScriptExecutionContext executionContext, IList`1 args, Boolean isMethodCall) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\DataTypes\CallbackFunction.cs:line 58
   at MoonSharp.Interpreter.Execution.VM.Processor.Internal_ExecCall(Int32 argsCount, Int32 instructionPtr, CallbackFunction handler, CallbackFunction continuation, Boolean thisCall, String debugText, DynValue unwindHandler) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\Execution\VM\Processor\Processor_InstructionLoop.cs:line 720
   at MoonSharp.Interpreter.Execution.VM.Processor.Processing_Loop(Int32 instructionPtr) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\Execution\VM\Processor\Processor_InstructionLoop.cs:line 115
   at MoonSharp.Interpreter.Execution.VM.Processor.Call(DynValue function, DynValue[] args) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\Execution\VM\Processor\Processor.cs:line 67
   at MoonSharp.Interpreter.Script.Call(DynValue function, DynValue[] args) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\Script.cs:line 483
   at MoonSharp.Interpreter.Script.DoFile(String filename, Table globalContext, String codeFriendlyName) in Z:\git\my\moonsharp\src\MoonSharp.Interpreter\Script.cs:line 362```
anotak commented 6 years ago

so apparently this isn't a problem on newer .NET runtimes, just, i'm stuck with 3.5 for this particular project

LimpingNinja commented 4 years ago

@anotak Interesting, does this happen often on your side? The description seems to imply that this is a very rare occurrence limited to 3.5. I'm wondering how impactful this is.

anotak commented 4 years ago

when a user writes a lua script that calls math.randomseed with a large double, after the conversion you end up with the exception.

LimpingNinja commented 4 years ago

Gotcha, and the way that lua math.randomseed() works is via conversion to an integer, in this case the limitation is int32. So this should only be related to 3.5, though right? Want to make sure any changes are specific here.

LimpingNinja commented 4 years ago

@anotak We are looking at this, but currently I don't think we will be pushing a fix for this because of the NET35 specificity. You could override this and implement the checks on your side for the time being, I will reopen this if we decide to go that route.