From eef8719a40b540f6060ed6b32e8cab4b38e308f0 Mon Sep 17 00:00:00 2001 From: Ian Lynagh Date: Thu, 25 Oct 2018 14:19:58 +0100 Subject: [PATCH 1/2] C++: Fix AV Rule 85 We have to be careful to avoid giving alerts to functions that might be correctly defined, but we can't see the definition as it wasn't instantiated. --- cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql | 28 +++++++++++++--- .../4.10 Classes/AV Rule 85/AV Rule 85.cpp | 22 +++++++++++++ .../AV Rule 85/AV Rule 85.expected | 32 ------------------- 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql index 690dd9479d28..3c7902c08eee 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql @@ -28,14 +28,34 @@ predicate implementedAsNegationOf(Operator op1, Operator op2) { c.getTarget() = op2) } +predicate classIsCheckableFor(Class c, string op) { + oppositeOperators(op, _) and + if exists(Class templ | c.isConstructedFrom(templ)) + then // If we are an instantiation, then we can't check `op` if the + // template has it but we don't (because that member wasn't + // instantiated) + exists(Class templ | c.isConstructedFrom(templ) and + (templ.getAMember().hasName(op) implies + exists(Function f | f = c.getAMember() and + f.hasName(op) and + f.isDefined()))) + else any() +} + from Class c, string op, string opp, Operator rator where c.fromSource() and + not c instanceof TemplateClass and oppositeOperators(op, opp) and + classIsCheckableFor(c, op) and + classIsCheckableFor(c, opp) and rator = c.getAMember() and rator.hasName(op) and - not exists(Operator oprator | oprator = c.getAMember() and - oprator.hasName(opp) and - ( implementedAsNegationOf(rator, oprator) - or implementedAsNegationOf(oprator, rator))) + forex(Operator aRator | + aRator = c.getAMember() and aRator.hasName(op) | + not exists(Operator oprator | + oprator = c.getAMember() and + oprator.hasName(opp) and + ( implementedAsNegationOf(aRator, oprator) + or implementedAsNegationOf(oprator, aRator)))) select c, "When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator " + op + " is declared on line " + rator.getLocation().getStartLine().toString() + ", but it is not defined in terms of its opposite operator " + opp + "." diff --git a/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.cpp b/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.cpp index e745e5e904fe..292b8857cb97 100644 --- a/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.cpp +++ b/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.cpp @@ -124,3 +124,25 @@ void f9(void) { b = myClass9 >= myClass9; } +template +class MyClass10 { +public: + int i; + template + bool operator< (const MyClass10 &rhs){ return i < rhs.i; } + template + bool operator>= (const MyClass10 &rhs){ return !(*this < rhs); } + // GOOD + template + bool operator> (const MyClass10 &rhs){ return i < rhs.i; } + template + bool operator<= (const MyClass10 &rhs){ return i >= rhs.i; } + // BAD: neither operator defined in terms of the other +}; +void f10(void) { + bool b; + MyClass10 myClass10; + b = myClass10 < myClass10; + b = myClass10 > myClass10; +} + diff --git a/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected b/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected index 257053ccd577..6012e2f61d94 100644 --- a/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected +++ b/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected @@ -3,39 +3,7 @@ | AV Rule 85.cpp:9:7:9:14 | MyClass2 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 13, but it is not defined in terms of its opposite operator operator<. | | AV Rule 85.cpp:25:7:25:14 | MyClass4 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 32, but it is not defined in terms of its opposite operator operator>. | | AV Rule 85.cpp:25:7:25:14 | MyClass4 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 31, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:37:7:37:14 | MyClass5 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 40, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:37:7:37:14 | MyClass5 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 44, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:37:7:37:14 | MyClass5 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 43, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:37:7:37:14 | MyClass5 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 41, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:49:7:49:14 | MyClass6 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 52, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:49:7:49:14 | MyClass6 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 56, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:49:7:49:14 | MyClass6 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 55, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:49:7:49:14 | MyClass6 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 53, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 66, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 73, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 71, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 68, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 66, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 73, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 71, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:62:7:62:14 | MyClass7 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 68, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 83, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 90, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 88, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 85, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 83, but it is not defined in terms of its opposite operator operator>=. | | AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 90, but it is not defined in terms of its opposite operator operator>. | | AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 88, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 85, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 107, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 114, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 112, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 109, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 107, but it is not defined in terms of its opposite operator operator>=. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 114, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 112, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 109, but it is not defined in terms of its opposite operator operator<. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator< is declared on line 107, but it is not defined in terms of its opposite operator operator>=. | | AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 114, but it is not defined in terms of its opposite operator operator>. | | AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 112, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 109, but it is not defined in terms of its opposite operator operator<. | From 94347aef9bc73810c0d9595cf67b24ed3648268e Mon Sep 17 00:00:00 2001 From: Ian Lynagh Date: Mon, 29 Oct 2018 15:04:30 +0000 Subject: [PATCH 2/2] C++: AV Rule 85: Check templates rather than instantiations --- cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql | 35 +++++++++++-------- .../AV Rule 85/AV Rule 85.expected | 8 ++--- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql index 3c7902c08eee..6ea435afe94c 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 85.ql @@ -19,32 +19,39 @@ predicate oppositeOperators(string op1, string op2) { /* this match is very syntactic: we simply check that op1 is defined as !op2(_, _) */ predicate implementedAsNegationOf(Operator op1, Operator op2) { - exists(Block b, ReturnStmt r, NotExpr n, FunctionCall c | + exists(Block b, ReturnStmt r, NotExpr n, Expr o | b = op1.getBlock() and b.getNumStmt() = 1 and r = b.getStmt(0) and n = r.getExpr() and - c = n.getOperand() and - c.getTarget() = op2) + o = n.getOperand() and + ( + o instanceof LTExpr and op2.hasName("operator<") or + o instanceof LEExpr and op2.hasName("operator<=") or + o instanceof GTExpr and op2.hasName("operator>") or + o instanceof GEExpr and op2.hasName("operator>=") or + o instanceof EQExpr and op2.hasName("operator==") or + o instanceof NEExpr and op2.hasName("operator!=") or + o.(FunctionCall).getTarget() = op2 + ) + ) } predicate classIsCheckableFor(Class c, string op) { oppositeOperators(op, _) and - if exists(Class templ | c.isConstructedFrom(templ)) - then // If we are an instantiation, then we can't check `op` if the - // template has it but we don't (because that member wasn't - // instantiated) - exists(Class templ | c.isConstructedFrom(templ) and - (templ.getAMember().hasName(op) implies - exists(Function f | f = c.getAMember() and - f.hasName(op) and - f.isDefined()))) - else any() + // We check the template, not its instantiations + not c instanceof ClassTemplateInstantiation and + // Member functions of templates are not necessarily instantiated, so + // if the function we want to check exists, then make sure that its + // body also exists + ((c instanceof TemplateClass) + implies + forall(Function f | f = c.getAMember() and f.hasName(op) + | exists(f.getEntryPoint()))) } from Class c, string op, string opp, Operator rator where c.fromSource() and - not c instanceof TemplateClass and oppositeOperators(op, opp) and classIsCheckableFor(c, op) and classIsCheckableFor(c, opp) and diff --git a/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected b/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected index 6012e2f61d94..63e1b23f4024 100644 --- a/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected +++ b/cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 85/AV Rule 85.expected @@ -3,7 +3,7 @@ | AV Rule 85.cpp:9:7:9:14 | MyClass2 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator>= is declared on line 13, but it is not defined in terms of its opposite operator operator<. | | AV Rule 85.cpp:25:7:25:14 | MyClass4 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 32, but it is not defined in terms of its opposite operator operator>. | | AV Rule 85.cpp:25:7:25:14 | MyClass4 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 31, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 90, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 88, but it is not defined in terms of its opposite operator operator<=. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 114, but it is not defined in terms of its opposite operator operator>. | -| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 112, but it is not defined in terms of its opposite operator operator<=. | +| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 90, but it is not defined in terms of its opposite operator operator>. | +| AV Rule 85.cpp:79:7:79:14 | MyClass8 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 88, but it is not defined in terms of its opposite operator operator<=. | +| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator<= is declared on line 114, but it is not defined in terms of its opposite operator operator>. | +| AV Rule 85.cpp:103:7:103:14 | MyClass9 | When two operators are opposites, both should be defined and one should be defined in terms of the other. Operator operator> is declared on line 112, but it is not defined in terms of its opposite operator operator<=. |