mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2024-11-25 00:50:22 +00:00
AK+Kernel: Eliminate UB (signed overflow) from days_since_epoch
This commit is contained in:
parent
7a69219a35
commit
3334cf675a
Notes:
sideshowbarker
2024-07-17 22:09:47 +09:00
Author: https://github.com/BenWiederhake Commit: https://github.com/SerenityOS/serenity/commit/3334cf675a Pull-request: https://github.com/SerenityOS/serenity/pull/16760 Reviewed-by: https://github.com/nico ✅ Reviewed-by: https://github.com/trflynn89 ✅
3 changed files with 20 additions and 16 deletions
15
AK/Time.h
15
AK/Time.h
|
@ -70,9 +70,9 @@ namespace Detail {
|
|||
// Integer division rounding towards negative infinity.
|
||||
// TODO: This feels like there should be an easier way to do this.
|
||||
template<int divisor>
|
||||
constexpr int floor_div_by(int dividend)
|
||||
constexpr i64 floor_div_by(i64 dividend)
|
||||
{
|
||||
static_assert(divisor >= 1);
|
||||
static_assert(divisor > 1);
|
||||
int is_negative = dividend < 0;
|
||||
return (dividend + is_negative) / divisor - is_negative;
|
||||
}
|
||||
|
@ -80,13 +80,13 @@ constexpr int floor_div_by(int dividend)
|
|||
// Counts how many integers n are in the interval [begin, end) with n % positive_mod == 0.
|
||||
// NOTE: "end" is not considered to be part of the range, hence "[begin, end)".
|
||||
template<int positive_mod>
|
||||
constexpr int mod_zeros_in_range(int begin, int end)
|
||||
constexpr i64 mod_zeros_in_range(i64 begin, i64 end)
|
||||
{
|
||||
return floor_div_by<positive_mod>(end - 1) - floor_div_by<positive_mod>(begin - 1);
|
||||
}
|
||||
}
|
||||
|
||||
constexpr int years_to_days_since_epoch(int year)
|
||||
constexpr i64 years_to_days_since_epoch(int year)
|
||||
{
|
||||
int begin_year, end_year, leap_sign;
|
||||
if (year < 1970) {
|
||||
|
@ -98,17 +98,18 @@ constexpr int years_to_days_since_epoch(int year)
|
|||
end_year = year;
|
||||
leap_sign = +1;
|
||||
}
|
||||
i64 year_i64 = year;
|
||||
// This duplicates the logic of 'is_leap_year', with the advantage of not needing any loops.
|
||||
// Given that the definition of leap years is not expected to change, this should be a good trade-off.
|
||||
int days = 365 * (year - 1970);
|
||||
int extra_leap_days = 0;
|
||||
i64 days = 365 * (year_i64 - 1970);
|
||||
i64 extra_leap_days = 0;
|
||||
extra_leap_days += Detail::mod_zeros_in_range<4>(begin_year, end_year);
|
||||
extra_leap_days -= Detail::mod_zeros_in_range<100>(begin_year, end_year);
|
||||
extra_leap_days += Detail::mod_zeros_in_range<400>(begin_year, end_year);
|
||||
return days + extra_leap_days * leap_sign;
|
||||
}
|
||||
|
||||
constexpr int days_since_epoch(int year, int month, int day)
|
||||
constexpr i64 days_since_epoch(int year, int month, int day)
|
||||
{
|
||||
return years_to_days_since_epoch(year) + day_of_year(year, month, day);
|
||||
}
|
||||
|
|
|
@ -112,8 +112,8 @@ time_t now()
|
|||
|
||||
dmesgln("RTC: {} Year: {}, month: {}, day: {}, hour: {}, minute: {}, second: {}", (did_read_rtc_successfully ? "" : "(failed to read)"), year, month, day, hour, minute, second);
|
||||
|
||||
time_t days_since_epoch = years_to_days_since_epoch(year) + day_of_year(year, month, day);
|
||||
return ((days_since_epoch * 24 + hour) * 60 + minute) * 60 + second;
|
||||
time_t days = days_since_epoch(year, month, day);
|
||||
return ((days * 24 + hour) * 60 + minute) * 60 + second;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -277,12 +277,12 @@ TEST_CASE(is_negative)
|
|||
|
||||
struct YearAndDays {
|
||||
int year;
|
||||
int days;
|
||||
i64 days;
|
||||
};
|
||||
|
||||
TEST_CASE(years_to_days_since_epoch_points)
|
||||
{
|
||||
Array<YearAndDays, 22> test_data = { {
|
||||
Array<YearAndDays, 24> test_data = { {
|
||||
{ 1969, -365 },
|
||||
{ 1970, 0 },
|
||||
{ 1971, 365 },
|
||||
|
@ -306,11 +306,14 @@ TEST_CASE(years_to_days_since_epoch_points)
|
|||
{ 5881474, 2147444740 },
|
||||
// Very important year: https://github.com/SerenityOS/serenity/pull/16760#issuecomment-1369054745
|
||||
{ -999999, -365961662 },
|
||||
// The following two values haven't been verified by any other algorithm, but are very close to "year * 365.2425", and prove that there is no UB due to signed overflow:
|
||||
{ 2147483647, 784351576412 },
|
||||
{ -2147483648, -784353015833 },
|
||||
} };
|
||||
for (auto entry : test_data) {
|
||||
int year = entry.year;
|
||||
int expected_days = entry.days;
|
||||
int actual_days = years_to_days_since_epoch(year);
|
||||
i64 expected_days = entry.days;
|
||||
i64 actual_days = years_to_days_since_epoch(year);
|
||||
EXPECT_EQ(actual_days, expected_days);
|
||||
}
|
||||
}
|
||||
|
@ -319,7 +322,7 @@ BENCHMARK_CASE(years_to_days_since_epoch_benchmark)
|
|||
{
|
||||
// This benchmark takes consistently "0ms" on Linux, and "0ms" on Serenity.
|
||||
for (size_t i = 0; i < 100; ++i) {
|
||||
int actual_days = years_to_days_since_epoch(-5877640);
|
||||
i64 actual_days = years_to_days_since_epoch(-5877640);
|
||||
(void)actual_days;
|
||||
EXPECT_EQ(actual_days, -2147483456);
|
||||
}
|
||||
|
@ -467,8 +470,8 @@ TEST_CASE(years_to_days_since_epoch_span)
|
|||
// clang-format on
|
||||
for (size_t offset = 0; offset < test_data.size(); ++offset) {
|
||||
int year = offset + test_data_start_year;
|
||||
int expected_days = test_data[offset];
|
||||
int actual_days = years_to_days_since_epoch(year);
|
||||
i64 expected_days = test_data[offset];
|
||||
i64 actual_days = years_to_days_since_epoch(year);
|
||||
EXPECT_EQ(actual_days, expected_days);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue