Skip to content

Commit 27edb07

Browse files
committed
Adding new div 4 check and false positive test cases
1 parent 7604938 commit 27edb07

File tree

2 files changed

+62
-10
lines changed

2 files changed

+62
-10
lines changed

cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -595,14 +595,23 @@ class LeapYearGuardCondition extends GuardCondition {
595595
|
596596
// Cannonical case:
597597
// form: `(year % 4 == 0) && (year % 100 != 0 || year % 400 == 0)`
598-
// or : `!(year % 4 == 0) && (year % 100 != 0 || year % 400 == 0)`
598+
// or : `!((year % 4 == 0) && (year % 100 != 0 || year % 400 == 0))`
599+
// Also accepting `(year & 3 == 0) && (year % 100 != 0 || year % 400 == 0)`
599600
this = andExpr and
600601
andExpr.hasOperands(div4Check, orExpr) and
601602
orExpr.hasOperands(div100Check, div400Check) and
602-
exists(RemExpr e |
603-
div4Check.comparesEq(e, 0, true, gv) and
604-
e.getRightOperand().getValue().toInt() = 4 and
605-
yearSinkDiv4 = e.getLeftOperand()
603+
(
604+
exists(RemExpr e |
605+
div4Check.comparesEq(e, 0, true, gv) and
606+
e.getRightOperand().getValue().toInt() = 4 and
607+
yearSinkDiv4 = e.getLeftOperand()
608+
)
609+
or
610+
exists(BitwiseAndExpr e |
611+
div4Check.comparesEq(e, 0, true, gv) and
612+
e.getRightOperand().getValue().toInt() = 3 and
613+
yearSinkDiv4 = e.getLeftOperand()
614+
)
606615
) and
607616
exists(RemExpr e |
608617
div100Check.comparesEq(e, 0, false, gv) and
@@ -616,14 +625,23 @@ class LeapYearGuardCondition extends GuardCondition {
616625
)
617626
or
618627
// Inverted logic case:
619-
// `year % 400 == 0 || (year % 100 != 0 && year % 4 == 0)`
628+
// `year % 4 != 0 || (year % 100 == 0 && year % 400 != 0)`
629+
// or `year & 3 != 0 || (year % 100 == 0 && year % 400 != 0)`
620630
this = orExpr and
621631
orExpr.hasOperands(div4Check, andExpr) and
622632
andExpr.hasOperands(div100Check, div400Check) and
623-
exists(RemExpr e |
624-
div4Check.comparesEq(e, 0, false, gv) and
625-
e.getRightOperand().getValue().toInt() = 4 and
626-
yearSinkDiv4 = e.getLeftOperand()
633+
(
634+
exists(RemExpr e |
635+
div4Check.comparesEq(e, 0, false, gv) and
636+
e.getRightOperand().getValue().toInt() = 4 and
637+
yearSinkDiv4 = e.getLeftOperand()
638+
)
639+
or
640+
exists(BitwiseAndExpr e |
641+
div4Check.comparesEq(e, 0, false, gv) and
642+
e.getRightOperand().getValue().toInt() = 3 and
643+
yearSinkDiv4 = e.getLeftOperand()
644+
)
627645
) and
628646
exists(RemExpr e |
629647
div100Check.comparesEq(e, 0, true, gv) and

cpp/ql/test/query-tests/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification/test.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,5 +1569,39 @@ void modification_before_conversion_with_leap_check2(tm timeinfo){
15691569
bool b = year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
15701570
}
15711571

1572+
void odd_leap_year_check1(tm timeinfo){
1573+
timeinfo.tm_year += 1;
1574+
1575+
1576+
// Using an odd sytle of checking divisible by 4 presumably as an optimization trick
1577+
if(((timeinfo.tm_year & 3) == 0) && (timeinfo.tm_year % 100 != 0 || timeinfo.tm_year % 400 == 0))
1578+
{
1579+
// do something
1580+
}
1581+
}
1582+
1583+
void odd_leap_year_check2(tm timeinfo){
1584+
timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification]
1585+
1586+
// Using an odd sytle of checking divisible by 4 presumably as an optimization trick
1587+
// but also check unrelated conditions on the year as an optimization to rule out irrelevant years
1588+
// for gregorian leap years
1589+
if(timeinfo.tm_mon == 2 && ((timeinfo.tm_year & 3) == 0) && (timeinfo.tm_year <= 1582 || timeinfo.tm_year % 100 != 0 || timeinfo.tm_year % 400 == 0))
1590+
{
1591+
// do something
1592+
}
1593+
}
1594+
1595+
void odd_leap_year_check3(tm timeinfo){
1596+
timeinfo.tm_year += 1; // $ SPURIOUS: Alert[cpp/microsoft/public/leap-year/unchecked-after-arithmetic-year-modification]
1597+
1598+
// Using an odd sytle of checking divisible by 4 presumably as an optimization trick
1599+
// but also check unrelated conditions on the year as an optimization to rule out irrelevant years
1600+
// for gregorian leap years
1601+
if(timeinfo.tm_mon == 2 && (timeinfo.tm_year % 4) == 0 && (timeinfo.tm_year <= 1582 || timeinfo.tm_year % 100 != 0 || timeinfo.tm_year % 400 == 0))
1602+
{
1603+
// do something
1604+
}
1605+
}
15721606

15731607

0 commit comments

Comments
 (0)