On Fri, Aug 28, 2015 at 01:58:56PM -0400, Lennart Sorensen wrote:
> On Fri, Aug 28, 2015 at 08:58:22AM +0100, Jo Shields wrote:
> > Okay. At this point, I've tried testing on a different box I was
> > generously offered access to, and it works as well as you indicate -
> > so my suspicion is there is Something Weird(tm) on partch.debian.org.
>
> That's good, I think.
>
> > I intend to upload the next upstream bugfix version of Mono early next
> > week, with a katamari if your patches & the patches in my ppc32 branch
> > - - I don't know if the buildd will lean closer to the "fine" or "on
> > crack" side of things, but I don't intend to remove powerpc from the
> > arch list with this upload.
>
> Hopefully the buildd decides to behave.
>
> > Would you be able to file GH pull requests with your fixes? They're
> > good stuff (and in some cases generally useful for BE) so it'd help to
> > get those merged upstream for all users
>
> Assuming GH means github, then I have no idea how one would do that.
> I only ever just pull things from github.
>
> Certainly getting the fixes in would be great, but I don't much care
> who puts them in. I suspect the ppc64 would like to add those missing
> instructions too.
>
> I am still curious about the last test suite failures (the hanging one
> and the other one I didn't look at yet, and the two that went away when
> the value was made smaller).
here is a patch that actually fixes the AddHours exception problem.
Turns out the problem is that the calculation is done as a double, then
converted to a long (64 bit int) which is then checked against range.
Unfortunately 9E100 is too big for a 64bit int, while 9E10 is not, and
overflow on signed values is undefined in C, and hence probably also in
C#. So the fix is to keep the result as a double, and do the range check
as a double and then if it is valid, convert to a long before using it.
diff --git a/mscorlib/system/datetime.cs b/mscorlib/system/datetime.cs
index c6f9af1..74ea154 100644
--- a/mscorlib/system/datetime.cs
+++ b/mscorlib/system/datetime.cs
@@ -348,10 +348,10 @@ namespace System {
// Returns the DateTime resulting from adding a fractional number of
// time units to this DateTime.
private DateTime Add(double value, int scale) {
- long millis = (long)(value * scale + (value >= 0? 0.5: -0.5));
- if (millis <= -MaxMillis || millis >= MaxMillis)
+ double millis = value * scale + (value >= 0? 0.5: -0.5);
+ if (millis <= (double)-MaxMillis || millis >= (double)MaxMillis)
throw new ArgumentOutOfRangeException("value",
Environment.GetResourceString("ArgumentOutOfRange_AddValue"));
- return AddTicks(millis * TicksPerMillisecond);
+ return AddTicks((long)millis * TicksPerMillisecond);
}
// Returns the DateTime resulting from adding a fractional number of
With that patch, both 9E10 and the original 9E100 give the correct exception.
It is entirely possible that on x86 the overflow behaved differently and
hence the exception still triggered. On powerpc a small test program
give a result of -1 as the long when the input was 9E100.
--
Len Sorensen
