tldr; saw a Windows Calculator bug on reddit. Since calc.exe was open-sourced I thought I’d try to find the bug and fix it. Cloned the code, recreated the bug, and found a minimal fix.
I was goofing-off browsing Reddit, as you do on a Sunday. Scrolling past playing puppies and bad programmer humor, one particular post caught my eye.
It was about a bug in calc.exe. “Well that looks like a curious bug, I wonder what could cause it”, I though to myself. The large number of weeks certainly makes it look like some under-/overflow issue or some off-by-one error, you know, the typical causes. But it could always be some flipped bit by some high-energy ray from some friendly cosmic neighbor.
So being curious about the cause, I did what you do in these cases: try it on your machine in giddy anticipation of posting “works for me”. Actually, the exact case in the post “July 31st – December 31st” did actually give the correct result of “5 months” on my machine. But testing around a bit I found that on my machine “July 31st – December 30th” actually causes the bug. Showing the not quite so correct value of “5 months, 613566756 weeks, 3 days”.
I was not done slacking-off and I remembered “Oh, Isn’t calculator one of those things that Microsoft open-sourced?”. It is indeed. This bug couldn’t be this complex, so I thought I’d try and see if I could locate it. Downloading the source was easy enough and adding the required UWP workload to Visual Studio went off without a hitch too.
Navigating code-bases you’re not familiar with is something you get used to after a while. Especially when you like to contribute to open-source projects when you find bugs. But not even being familiar with XAML or WinRT certainly doesn’t make it trivial.
I opened the solution file and looked into the “Calculator” project, searching for any file that seemed related. Found the DateCalculator.xaml
and traced the relevant sounding DateDiff_FromDate
to DateCalculatorViewModel.cpp
and finally DateCalculator.cpp
.
While setting a breakpoint and looking at some variables I saw that the value of the final DateDifference
was already wrong. So it was not just an error in the conversion to the string, but the actual calculation that was bugged.
The actual calculation looks something like this in simplified pseudo-code:
DateDifference calculate_difference(start_date, end_date) {
uint[] diff_types = [year, month, week, day]
uint[] typical_days_in_type = [365, 31, 7, 1]
uint[] calculated_difference = [0, 0, 0, 0]
date temp_pivot_date
date pivot_date = start_date
uint days_diff = calculate_days_difference(start_date, end_date)
for(type in differenceTypes) {
temp_pivot_date = pivot_date
uint current_guess = days_diff /typicalDaysInType[type]
if(current_guess !=0)
pivot_date = advance_date_by(pivot_date, type, current_guess)
int diff_remaining
bool best_guess_hit = false
do{
diff_remaining = calculate_days_difference(pivot_date, end_date)
if(diff_remaining < 0) {
// pivotDate has gone over the end date; start from the beginning of this unit
current_guess = current_guess - 1
pivot_date = temp_pivot_date
pivot_date = advance_date_by(pivot_date, type, current_guess)
best_guess_hit = true
} else if(diff_remaining > 0) {
// pivot_date is still below the end date
if(best_guess_hit)
break;
current_guess = current_guess + 1
pivot_date = advance_date_by(pivot_date, type, 1)
}
} while(diff_remaining!=0)
temp_pivot_date = advance_date_by(temp_pivot_date, type, current_guess)
pivot_date = temp_pivot_date
calculated_difference[type] = current_guess
days_diff = calculate_days_difference(pivot_date, end_date)
}
calculcated_difference[day] = days_diff
return calculcated_difference
}
Looked good to me. Didn’t see an issue in the logic. It’s basically:
- from the start day step in years towards the end, count years
- from the last year before the end step in months towards the end, count months
- from the last month before the end step in weeks towards the end, count weeks
- set the remaining days from the week before the end
The issue is actually the assumption that running date = advance_date_by(date, month, somenumber)
date = advance_date_by(date, month, 1)
one after the other is equal todate = advance_date_by(date, month, somenumber + 1)
which it typically is. But the question is: “If you are at the 31st of a month and you increment to a month that only has 30 days, where do you land?”.
The answer for Windows.Globalization.Calendar.AddMonths(Int32) is apparently “on the 30th”.
This means specifically:
“July 31st + 4 Months = November 30th” and
“November 30th + 1 Month = December 30th”.
However
“July 31st + 5 Months = December 31st”.
So the operation AddMonths
is neither distributive (with AddMonth being the multiply) nor commutative or associative. As you would intuitively assume of an “addition” operation. Isn’t it fun working with time and calendars?
Anyway, the reason why this seemingly off-by-one value results in such a huge number is as you might suspect that then days_diff
being an unsigned type. It makes the -1 days into a huge number which then gets passed onto the following loop iteration with weeks. Which then tries to correct current_guess
downwards. But it tries to correct it once again by decrementing an unsigned variable.
Well this was fun way to spend a part of my weekend. I created a Pull Request on Github with a minimal “fix”. I put fix in quotes because it then results in the following result:
Which I guess is technically correct if you concede that “July 31st + 4 Months = November 30th”. But the complete result does not really mesh with a humans intuition of date-difference. But it’s less wrong in any case.