hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-24 04:36:35 +01:00
Code Issues Packages Projects Releases Wiki Activity

C++: Autoformat Architecture + Best Practices

Browse Source
This commit is contained in:
Jonas Jensen
2019-04-11 14:27:07 +02:00
parent 75ab311c3a
commit 6049c2ccfd
42 changed files with 762 additions and 518 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
cpp/ql/src/Architecture/FeatureEnvy.ql
View File

@@ -10,11 +10,12 @@
* statistical
* non-attributable
*/
import cpp
predicate functionUsesVariable(Function source, Variable v, File target) {
v.getAnAccess().getEnclosingFunction() = source and
not (v.(LocalScopeVariable).getFunction() = source) and
not v.(LocalScopeVariable).getFunction() = source and
v.getFile() = target
}
@@ -25,15 +26,15 @@ predicate functionUsesFunction(Function source, Function f, File target) {
predicate dependencyCount(Function source, File target, int res) {
res = strictcount(Declaration d |
functionUsesVariable(source, d, target) or
functionUsesFunction(source, d, target)
)
functionUsesVariable(source, d, target) or
functionUsesFunction(source, d, target)
)
}
predicate selfDependencyCountOrZero(Function source, int res) {
exists(File target
| target = source.getFile() and onlyInFile(source, target)
| res = max(int i | dependencyCount(source, target, i) or i = 0))
exists(File target | target = source.getFile() and onlyInFile(source, target) |
res = max(int i | dependencyCount(source, target, i) or i = 0)
)
}
predicate dependsHighlyOn(Function source, File target, int res) {
@@ -41,7 +42,7 @@ predicate dependsHighlyOn(Function source, File target, int res) {
target.fromSource() and
exists(int selfCount |
selfDependencyCountOrZero(source, selfCount) and
res > 2*selfCount and
res > 2 * selfCount and
res > 4
)
}
@@ -52,14 +53,18 @@ predicate onlyInFile(Function f, File file) {
}
from Function f, File other, int selfCount, int depCount, string selfDeps
where dependsHighlyOn(f, other, depCount) and
selfDependencyCountOrZero(f, selfCount) and
not exists(File yetAnother | dependsHighlyOn(f, yetAnother, _) and yetAnother != other) and
not other instanceof HeaderFile and
not f instanceof MemberFunction
and if selfCount = 0 then selfDeps = "0 dependencies"
else if selfCount = 1 then selfDeps = "only 1 dependency"
else selfDeps = "only " + selfCount.toString() + " dependencies"
select f, "Function " + f.getName() + " could be moved to file $@" +
" since it has " + depCount.toString() + " dependencies to that file, but " +
selfDeps + " to its own file.", other, other.getBaseName()
where
dependsHighlyOn(f, other, depCount) and
selfDependencyCountOrZero(f, selfCount) and
not exists(File yetAnother | dependsHighlyOn(f, yetAnother, _) and yetAnother != other) and
not other instanceof HeaderFile and
not f instanceof MemberFunction and
if selfCount = 0
then selfDeps = "0 dependencies"
else
if selfCount = 1
then selfDeps = "only 1 dependency"
else selfDeps = "only " + selfCount.toString() + " dependencies"
select f,
"Function " + f.getName() + " could be moved to file $@" + " since it has " + depCount.toString() +
" dependencies to that file, but " + selfDeps + " to its own file.", other, other.getBaseName()

1
cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.ql
View File

@@ -9,6 +9,7 @@
* @result_ondemand succeed 48
* @tags maintainability
*/
import cpp
from Class s

7
cpp/ql/src/Architecture/General Class-Level Information/HubClasses.ql
View File

@@ -7,11 +7,10 @@
* @treemap.warnOn highValues
* @tags maintainability
*/
import cpp
from Class c
where c.fromSource()
select c as Class,
c.getMetrics().getAfferentCoupling() as AfferentCoupling,
c.getMetrics().getEfferentSourceCoupling() as EfferentCoupling
order by AfferentCoupling desc
select c as Class, c.getMetrics().getAfferentCoupling() as AfferentCoupling,
c.getMetrics().getEfferentSourceCoupling() as EfferentCoupling order by AfferentCoupling desc

6
cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.ql
View File

@@ -9,6 +9,7 @@
* @result_ondemand succeed 48
* @tags maintainability
*/
import cpp
/** does source class c have inheritance depth d? */
@@ -18,6 +19,5 @@ predicate hasInheritanceDepth(Class c, int d) {
from int depth
where hasInheritanceDepth(_, depth)
select depth as InheritanceDepth,
count(Class c | hasInheritanceDepth(c, depth)) as NumberOfClasses
order by InheritanceDepth
select depth as InheritanceDepth, count(Class c | hasInheritanceDepth(c, depth)) as NumberOfClasses
order by InheritanceDepth

1
cpp/ql/src/Architecture/General Namespace-Level Information/CyclicNamespaces.ql
View File

@@ -7,6 +7,7 @@
* @tags maintainability
* modularity
*/
import cpp
from MetricNamespace a, MetricNamespace b

8
cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.ql
View File

@@ -6,10 +6,12 @@
* @tags maintainability
* modularity
*/
import cpp
from Class c
where c.fromSource()
and c.isTopLevel()
and c.getParentScope() instanceof GlobalNamespace
where
c.fromSource() and
c.isTopLevel() and
c.getParentScope() instanceof GlobalNamespace
select c, "This class is not declared in any namespace"

1
cpp/ql/src/Architecture/General Namespace-Level Information/NamespaceDependencies.ql
View File

@@ -7,6 +7,7 @@
* @tags maintainability
* modularity
*/
import cpp
from MetricNamespace a, MetricNamespace b

74
cpp/ql/src/Architecture/General Top-Level Information/GeneralStatistics.ql
View File

@@ -5,31 +5,57 @@
* @id cpp/architecture/general-statistics
* @tags maintainability
*/
import cpp
from string l, string n
where (l = "Number of Namespaces" and
n = count(Namespace p | p.fromSource()).toString())
or (l = "Number of Files" and
n = count(File f | f.fromSource()).toString())
or (l = "Number of Header Files" and
n = count(HeaderFile f | f.fromSource()).toString())
or (l = "Number of C Files" and
n = count(CFile f | f.fromSource()).toString())
or (l = "Number of C++ Files" and
n = count(CppFile f | f.fromSource()).toString())
or (l = "Number of Classes" and
n = count(Class c | c.fromSource() and not c instanceof Struct).toString())
or (l = "Number of Structs" and
n = count(Struct s | s.fromSource()and not s instanceof Union).toString())
or (l = "Number of Unions" and
n = count(Union u | u.fromSource()).toString())
or (l = "Number of Functions" and
n = count(Function f | f.fromSource()).toString())
or (l = "Number of Lines Of Code" and
n = sum(File f, int toSum | (f.fromSource()) and (toSum = f.getMetrics().getNumberOfLinesOfCode()) | toSum).toString())
or (l = "Self-Containedness" and
n = (100 * sum(Class c, int toSum | (c.fromSource()) and (toSum = c.getMetrics().getEfferentSourceCoupling()) | toSum)
/ sum(Class c, int toSum | (c.fromSource()) and (toSum = c.getMetrics().getEfferentCoupling()) | toSum)).toString()
+ "%")
where
l = "Number of Namespaces" and
n = count(Namespace p | p.fromSource()).toString()
or
l = "Number of Files" and
n = count(File f | f.fromSource()).toString()
or
l = "Number of Header Files" and
n = count(HeaderFile f | f.fromSource()).toString()
or
l = "Number of C Files" and
n = count(CFile f | f.fromSource()).toString()
or
l = "Number of C++ Files" and
n = count(CppFile f | f.fromSource()).toString()
or
l = "Number of Classes" and
n = count(Class c | c.fromSource() and not c instanceof Struct).toString()
or
l = "Number of Structs" and
n = count(Struct s | s.fromSource() and not s instanceof Union).toString()
or
l = "Number of Unions" and
n = count(Union u | u.fromSource()).toString()
or
l = "Number of Functions" and
n = count(Function f | f.fromSource()).toString()
or
l = "Number of Lines Of Code" and
n = sum(File f, int toSum |
f.fromSource() and toSum = f.getMetrics().getNumberOfLinesOfCode()
|
toSum
).toString()
or
l = "Self-Containedness" and
n = (
100 *
sum(Class c, int toSum |
c.fromSource() and toSum = c.getMetrics().getEfferentSourceCoupling()
|
toSum
) /
sum(Class c, int toSum |
c.fromSource() and toSum = c.getMetrics().getEfferentCoupling()
|
toSum
)
).toString() + "%"
select l as Title, n as Value

24
cpp/ql/src/Architecture/InappropriateIntimacy.ql
View File

@@ -10,6 +10,7 @@
* statistical
* non-attributable
*/
import cpp
predicate remoteVarAccess(File source, File target, VariableAccess va) {
@@ -48,16 +49,19 @@ predicate highDependencyCount(File source, File target, int res) {
variableDependencyCount(source, target, varCount) and
functionDependencyCount(source, target, funCount) and
res = varCount + funCount and
res > 20)
res > 20
)
}
from File a, File b, int ca, int cb
where highDependencyCount(a, b, ca) and
highDependencyCount(b, a, cb) and
ca >= cb and
a != b and
not a instanceof HeaderFile and
not b instanceof HeaderFile and
b.getShortName().trim().length() > 0
select a, "File is too closely tied to $@ (" + ca.toString() + " dependencies one way and " + cb.toString() + " the other).",
b, b.getBaseName()
where
highDependencyCount(a, b, ca) and
highDependencyCount(b, a, cb) and
ca >= cb and
a != b and
not a instanceof HeaderFile and
not b instanceof HeaderFile and
b.getShortName().trim().length() > 0
select a,
"File is too closely tied to $@ (" + ca.toString() + " dependencies one way and " + cb.toString() +
" the other).", b, b.getBaseName()

8
cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyDependencies.ql
View File

@@ -11,10 +11,12 @@
* statistical
* non-attributable
*/
import cpp
from Class t, int n
where t.fromSource() and
n = t.getMetrics().getEfferentSourceCoupling() and
n > 10
where
t.fromSource() and
n = t.getMetrics().getEfferentSourceCoupling() and
n > 10
select t as Class, "This class has too many dependencies (" + n.toString() + ")"

122
cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyFields.ql
View File

@@ -9,41 +9,51 @@
* statistical
* non-attributable
*/
import cpp
string kindstr(Class c)
{
string kindstr(Class c) {
exists(int kind | usertypes(unresolveElement(c), _, kind) |
(kind = 1 and result = "Struct") or
(kind = 2 and result = "Class") or
(kind = 6 and result = "Template class")
kind = 1 and result = "Struct"
or
kind = 2 and result = "Class"
or
kind = 6 and result = "Template class"
)
}
predicate vdeInfo(VariableDeclarationEntry vde, Class c, File f, int line)
{
predicate vdeInfo(VariableDeclarationEntry vde, Class c, File f, int line) {
c = vde.getVariable().getDeclaringType() and
f = vde.getLocation().getFile() and
line = vde.getLocation().getStartLine()
}
predicate previousVde(VariableDeclarationEntry previous, VariableDeclarationEntry vde)
{
predicate previousVde(VariableDeclarationEntry previous, VariableDeclarationEntry vde) {
exists(Class c, File f, int line | vdeInfo(vde, c, f, line) |
vdeInfo(previous, c, f, line - 3) or
vdeInfo(previous, c, f, line - 2) or
vdeInfo(previous, c, f, line - 1) or
(vdeInfo(previous, c, f, line) and exists(int prevCol, int vdeCol |
prevCol = previous.getLocation().getStartColumn() and vdeCol = vde.getLocation().getStartColumn() |
prevCol < vdeCol or (prevCol = vdeCol and previous.getName() < vde.getName())
))
vdeInfo(previous, c, f, line - 3)
or
vdeInfo(previous, c, f, line - 2)
or
vdeInfo(previous, c, f, line - 1)
or
vdeInfo(previous, c, f, line) and
exists(int prevCol, int vdeCol |
prevCol = previous.getLocation().getStartColumn() and
vdeCol = vde.getLocation().getStartColumn()
|
prevCol < vdeCol
or
prevCol = vdeCol and previous.getName() < vde.getName()
)
)
}
predicate masterVde(VariableDeclarationEntry master, VariableDeclarationEntry vde)
{
(not previousVde(_, vde) and master = vde) or
exists(VariableDeclarationEntry previous | previousVde(previous, vde) and masterVde(master, previous))
predicate masterVde(VariableDeclarationEntry master, VariableDeclarationEntry vde) {
not previousVde(_, vde) and master = vde
or
exists(VariableDeclarationEntry previous |
previousVde(previous, vde) and masterVde(master, previous)
)
}
class VariableDeclarationGroup extends ElementBase {
@@ -51,9 +61,8 @@ class VariableDeclarationGroup extends ElementBase {
this instanceof VariableDeclarationEntry and
not previousVde(_, this)
}
Class getClass() {
vdeInfo(this, result, _, _)
}
Class getClass() { vdeInfo(this, result, _, _) }
// pragma[noopt] since otherwise the two locationInfo relations get join-ordered
// after each other
@@ -63,7 +72,9 @@ class VariableDeclarationGroup extends ElementBase {
masterVde(this, last) and
this instanceof VariableDeclarationGroup and
not previousVde(last, _) and
exists(VariableDeclarationEntry vde | vde=this and vde instanceof VariableDeclarationEntry and vde.getLocation() = lstart) and
exists(VariableDeclarationEntry vde |
vde = this and vde instanceof VariableDeclarationEntry and vde.getLocation() = lstart
) and
last.getLocation() = lend and
lstart.hasLocationInfo(path, startline, startcol, _, _) and
lend.hasLocationInfo(path, _, _, endline, endcol)
@@ -71,15 +82,16 @@ class VariableDeclarationGroup extends ElementBase {
}
string describeGroup() {
if previousVde(this, _) then
result = "group of "
+ strictcount(string name
| exists(VariableDeclarationEntry vde
| masterVde(this, vde) and
name = vde.getName()))
+ " fields here"
else
result = "declaration of " + this.(VariableDeclarationEntry).getVariable().getName()
if previousVde(this, _)
then
result = "group of " +
strictcount(string name |
exists(VariableDeclarationEntry vde |
masterVde(this, vde) and
name = vde.getName()
)
) + " fields here"
else result = "declaration of " + this.(VariableDeclarationEntry).getVariable().getName()
}
}
@@ -89,26 +101,32 @@ class ExtClass extends Class {
}
predicate hasLocationInfo(string path, int startline, int startcol, int endline, int endcol) {
if hasOneVariableGroup() then
exists(VariableDeclarationGroup vdg | vdg.getClass() = this | vdg.hasLocationInfo(path, startline, startcol, endline, endcol))
else
getLocation().hasLocationInfo(path, startline, startcol, endline, endcol)
if hasOneVariableGroup()
then
exists(VariableDeclarationGroup vdg | vdg.getClass() = this |
vdg.hasLocationInfo(path, startline, startcol, endline, endcol)
)
else getLocation().hasLocationInfo(path, startline, startcol, endline, endcol)
}
}
from ExtClass c, int n, VariableDeclarationGroup vdg, string suffix
where n = strictcount(string fieldName
| exists(Field f
| f.getDeclaringType() = c and
fieldName = f.getName() and
// IBOutlet's are a way of building GUIs
// automatically out of ObjC properties.
// We don't want to count those for the
// purposes of this query.
not (f.getType().getAnAttribute().hasName("iboutlet")))) and
n > 15 and
not c.isConstructedFrom(_) and
c = vdg.getClass() and
if c.hasOneVariableGroup() then suffix = "" else suffix = " - see $@"
select c, kindstr(c) + " " + c.getName() + " has " + n + " fields; we suggest refactoring to 15 fields or fewer" + suffix + ".",
vdg, vdg.describeGroup()
where
n = strictcount(string fieldName |
exists(Field f |
f.getDeclaringType() = c and
fieldName = f.getName() and
// IBOutlet's are a way of building GUIs
// automatically out of ObjC properties.
// We don't want to count those for the
// purposes of this query.
not f.getType().getAnAttribute().hasName("iboutlet")
)
) and
n > 15 and
not c.isConstructedFrom(_) and
c = vdg.getClass() and
if c.hasOneVariableGroup() then suffix = "" else suffix = " - see $@"
select c,
kindstr(c) + " " + c.getName() + " has " + n +
" fields; we suggest refactoring to 15 fields or fewer" + suffix + ".", vdg, vdg.describeGroup()

10
cpp/ql/src/Architecture/Refactoring Opportunities/ComplexFunctions.ql
View File

@@ -8,11 +8,13 @@
* statistical
* non-attributable
*/
import cpp
from Function f, int n
where f.fromSource() and
n = f.getMetrics().getNumberOfCalls() and
n > 99 and
not f.isMultiplyDefined()
where
f.fromSource() and
n = f.getMetrics().getNumberOfCalls() and
n > 99 and
not f.isMultiplyDefined()
select f as Function, "This function makes too many calls (" + n.toString() + ")"

6
cpp/ql/src/Architecture/Refactoring Opportunities/CyclomaticComplexity.ql
View File

@@ -8,9 +8,11 @@
* statistical
* non-attributable
*/
import cpp
from Function f, int complexity
where complexity = f.getMetrics().getCyclomaticComplexity()
and complexity > 250
where
complexity = f.getMetrics().getCyclomaticComplexity() and
complexity > 250
select f, "Function has high cyclomatic complexity: " + complexity.toString()

11
cpp/ql/src/Architecture/Refactoring Opportunities/FunctionsWithManyParameters.ql
View File

@@ -9,10 +9,13 @@
* statistical
* non-attributable
*/
import cpp
from Function f
where f.fromSource() and
f.getMetrics().getNumberOfParameters() > 15
select f, "This function has too many parameters ("
+ f.getMetrics().getNumberOfParameters().toString() + ")"
where
f.fromSource() and
f.getMetrics().getNumberOfParameters() > 15
select f,
"This function has too many parameters (" + f.getMetrics().getNumberOfParameters().toString() +
")"

24
cpp/ql/src/Best Practices/BlockWithTooManyStatements.ql
View File

@@ -9,19 +9,27 @@
* readability
* maintainability
*/
import cpp
class ComplexStmt extends Stmt {
ComplexStmt() {
exists(Block body | body = this.(Loop ).getStmt() or
body = this.(SwitchStmt).getStmt()
| strictcount(body.getAStmt+()) > 6)
and not exists (this.getGeneratingMacro())
exists(Block body |
body = this.(Loop).getStmt() or
body = this.(SwitchStmt).getStmt()
|
strictcount(body.getAStmt+()) > 6
) and
not exists(this.getGeneratingMacro())
}
}
from Block b, int n, ComplexStmt complexStmt
where n = strictcount(ComplexStmt s | s = b.getAStmt()) and n > 3
and complexStmt = b.getAStmt()
select b, "Block with too many statements (" + n.toString() + " complex statements in the block). Complex statements at: $@", complexStmt, complexStmt.toString()
where
n = strictcount(ComplexStmt s | s = b.getAStmt()) and
n > 3 and
complexStmt = b.getAStmt()
select b,
"Block with too many statements (" + n.toString() +
" complex statements in the block). Complex statements at: $@", complexStmt,
complexStmt.toString()

18
cpp/ql/src/Best Practices/ComplexCondition.ql
View File

@@ -11,23 +11,23 @@
* statistical
* non-attributable
*/
import cpp
predicate logicalOp(string op) {
op = "&&" or op = "||"
}
predicate logicalOp(string op) { op = "&&" or op = "||" }
predicate nontrivialLogicalOperator(Operation e) {
exists(string op |
op = e.getOperator() and
logicalOp(op) and
not (op = e.getParent().(Operation).getOperator())
)
and not e.isInMacroExpansion()
not op = e.getParent().(Operation).getOperator()
) and
not e.isInMacroExpansion()
}
from Expr e, int operators
where not (e.getParent() instanceof Expr)
and operators = count(Operation op | op.getParent*() = e and nontrivialLogicalOperator(op))
and operators > 5
where
not e.getParent() instanceof Expr and
operators = count(Operation op | op.getParent*() = e and nontrivialLogicalOperator(op)) and
operators > 5
select e, "Complex condition: too many logical operations in this expression."

7
cpp/ql/src/Best Practices/Exceptions/AccidentalRethrow.ql
View File

@@ -13,10 +13,11 @@
import cpp
predicate isInCatch(Expr e) {
e.getEnclosingStmt().getParent*() instanceof CatchBlock or // Lexically enclosing catch blocks will cause there to be a current exception,
e.getEnclosingStmt().getParent*() instanceof CatchBlock // Lexically enclosing catch blocks will cause there to be a current exception,
or
exists(Function f | f = e.getEnclosingFunction() |
isInCatch(f.getACallToThisFunction()) or // as will dynamically enclosing catch blocks.
f.getName().toLowerCase().matches("%exception%") // We assume that rethrows are intended when the function is called *exception*.
isInCatch(f.getACallToThisFunction()) or // as will dynamically enclosing catch blocks.
f.getName().toLowerCase().matches("%exception%") // We assume that rethrows are intended when the function is called *exception*.
)
}

3
cpp/ql/src/Best Practices/Exceptions/CatchingByValue.ql
View File

@@ -14,4 +14,5 @@ import cpp
from CatchBlock cb, Class caughtType
where caughtType = cb.getParameter().getType().getUnderlyingType().getUnspecifiedType()
select cb, "This should catch a " + caughtType.getName() + " by (const) reference rather than by value."
select cb,
"This should catch a " + caughtType.getName() + " by (const) reference rather than by value."

30
cpp/ql/src/Best Practices/Exceptions/LeakyCatch.ql
View File

@@ -18,31 +18,33 @@ predicate doesRethrow(Function f) {
not e.getEnclosingStmt().getParent*() instanceof CatchBlock
)
or
exists(FunctionCall fc | fc.getEnclosingFunction() = f |
doesRethrow(fc.getTarget())
)
exists(FunctionCall fc | fc.getEnclosingFunction() = f | doesRethrow(fc.getTarget()))
}
predicate deletesException(Expr expr, Parameter exception) {
expr.getEnclosingBlock().getParent*().(CatchBlock).getParameter() = exception and (
expr.getEnclosingBlock().getParent*().(CatchBlock).getParameter() = exception and
(
exists(FunctionCall fc | fc = expr |
// Calling a delete function on the exception will free it (MFC's CException has a Delete function).
(fc.getQualifier() = exception.getAnAccess() and fc.getTarget().getName().toLowerCase().matches("%delete%")) or
fc.getQualifier() = exception.getAnAccess() and
fc.getTarget().getName().toLowerCase().matches("%delete%")
or
// Passing the exception to a function might free it.
(fc.getAnArgument() = exception.getAnAccess()) or
fc.getAnArgument() = exception.getAnAccess()
or
// Calling a function which rethrows the current exception might cause the exception to be freed.
doesRethrow(fc.getTarget())
) or
// Calling operator delete on the exception will free it.
exists(DeleteExpr d | d = expr |
d.getExpr() = exception.getAnAccess()
)
or
// Calling operator delete on the exception will free it.
exists(DeleteExpr d | d = expr | d.getExpr() = exception.getAnAccess())
)
}
from CatchBlock cb
where cb.getParameter().getType().getUnderlyingType() instanceof PointerType
and not exists(Expr e | e.getEnclosingBlock().getParent*() = cb |
deletesException(e, cb.getParameter())
)
where
cb.getParameter().getType().getUnderlyingType() instanceof PointerType and
not exists(Expr e | e.getEnclosingBlock().getParent*() = cb |
deletesException(e, cb.getParameter())
)
select cb, "This catch block does not free the caught exception, thereby leaking memory."

7
cpp/ql/src/Best Practices/Exceptions/ThrowingPointers.ql
View File

@@ -13,8 +13,9 @@
import cpp
from ThrowExpr throw, NewExpr new, Type t
where new.getParent() = throw
where
new.getParent() = throw and
// Microsoft MFC's CException hierarchy should be thrown (and caught) as pointers
and t = new.getAllocatedType()
and not t.getUnderlyingType().(Class).getABaseClass*().hasName("CException")
t = new.getAllocatedType() and
not t.getUnderlyingType().(Class).getABaseClass*().hasName("CException")
select throw, "This should throw a " + t.toString() + " rather than a pointer to one."

14
cpp/ql/src/Best Practices/Hiding/DeclarationHidesVariable.ql
View File

@@ -8,14 +8,14 @@
* @tags maintainability
* readability
*/
import cpp
import cpp
import Best_Practices.Hiding.Shadowing
from LocalVariable lv1, LocalVariable lv2
where shadowing(lv1, lv2) and
not lv1.getParentScope().(Block).isInMacroExpansion() and
not lv2.getParentScope().(Block).isInMacroExpansion()
select lv1, "Variable " + lv1.getName() +
" hides another variable of the same name (on $@).",
lv2, "line " + lv2.getLocation().getStartLine().toString()
where
shadowing(lv1, lv2) and
not lv1.getParentScope().(Block).isInMacroExpansion() and
not lv2.getParentScope().(Block).isInMacroExpansion()
select lv1, "Variable " + lv1.getName() + " hides another variable of the same name (on $@).", lv2,
"line " + lv2.getLocation().getStartLine().toString()

6
cpp/ql/src/Best Practices/Hiding/LocalVariableHidesGlobalVariable.ql
View File

@@ -8,6 +8,7 @@
* @tags maintainability
* readability
*/
import cpp
class LocalVariableOrParameter extends VariableDeclarationEntry {
@@ -31,6 +32,7 @@ class LocalVariableOrParameter extends VariableDeclarationEntry {
}
from LocalVariableOrParameter lv, GlobalVariable gv
where lv.getName() = gv.getName() and
lv.getFile() = gv.getFile()
where
lv.getName() = gv.getName() and
lv.getFile() = gv.getFile()
select lv, lv.type() + gv.getName() + " hides $@ with the same name.", gv, "a global variable"

33
cpp/ql/src/Best Practices/Hiding/Shadowing.qll
View File

@@ -1,20 +1,19 @@
import cpp
predicate ancestorScope(Element b1, Element b2) {
b1.getParentScope+() = b2
}
predicate ancestorScope(Element b1, Element b2) { b1.getParentScope+() = b2 }
pragma[noopt]
predicate localVariablesSameNameInNestedScopes(LocalVariable lv1, LocalVariable lv2) {
exists(Element b1, Element b2
| b1 = lv1.getParentScope() and
not b1 instanceof Namespace and
lv1 instanceof LocalVariable and
ancestorScope(b1, b2) and
not b2 instanceof Namespace and
b2 = lv2.getParentScope() and
lv2 instanceof LocalVariable and
lv1.getName() = lv2.getName())
exists(Element b1, Element b2 |
b1 = lv1.getParentScope() and
not b1 instanceof Namespace and
lv1 instanceof LocalVariable and
ancestorScope(b1, b2) and
not b2 instanceof Namespace and
b2 = lv2.getParentScope() and
lv2 instanceof LocalVariable and
lv1.getName() = lv2.getName()
)
}
predicate shadowing(LocalVariable lv1, LocalVariable lv2) {
@@ -23,11 +22,11 @@ predicate shadowing(LocalVariable lv1, LocalVariable lv2) {
l1 = lv1.getLocation() and
l2 = lv2.getLocation() and
(
// variables declared later in parent scope are not shadowed
l2.getEndLine() < l1.getStartLine()
or (l2.getEndLine() = l1.getStartLine() and
l2.getEndColumn() <= l1.getStartColumn())
// variables declared later in parent scope are not shadowed
l2.getEndLine() < l1.getStartLine()
or
l2.getEndLine() = l1.getStartLine() and
l2.getEndColumn() <= l1.getStartColumn()
)
)
}

41
cpp/ql/src/Best Practices/Likely Errors/EmptyBlock.ql
View File

@@ -11,6 +11,7 @@
* @tags reliability
* readability
*/
import cpp
predicate emptyBlock(ControlStructure s, Block b) {
@@ -35,7 +36,8 @@ class AffectedFile extends File {
*/
class BlockOrNonChild extends Element {
BlockOrNonChild() {
( this instanceof Block
(
this instanceof Block
or
this instanceof Comment
or
@@ -49,26 +51,24 @@ class BlockOrNonChild extends Element {
private int getNonContiguousStartRankIn(AffectedFile file) {
// When using `rank` with `order by`, the ranks may not be contiguous.
this = rank[result](BlockOrNonChild boc, int startLine, int startCol |
boc.getLocation()
.hasLocationInfo(file.getAbsolutePath(), startLine, startCol, _, _)
| boc
order by startLine, startCol
)
boc.getLocation().hasLocationInfo(file.getAbsolutePath(), startLine, startCol, _, _)
|
boc order by startLine, startCol
)
}
int getStartRankIn(AffectedFile file) {
this.getNonContiguousStartRankIn(file) = rank[result](int rnk |
exists(BlockOrNonChild boc | boc.getNonContiguousStartRankIn(file) = rnk)
)
exists(BlockOrNonChild boc | boc.getNonContiguousStartRankIn(file) = rnk)
)
}
int getNonContiguousEndRankIn(AffectedFile file) {
this = rank[result](BlockOrNonChild boc, int endLine, int endCol |
boc.getLocation()
.hasLocationInfo(file.getAbsolutePath(), _, _, endLine, endCol)
| boc
order by endLine, endCol
)
boc.getLocation().hasLocationInfo(file.getAbsolutePath(), _, _, endLine, endCol)
|
boc order by endLine, endCol
)
}
}
@@ -78,10 +78,10 @@ class BlockOrNonChild extends Element {
predicate emptyBlockContainsNonchild(Block b) {
emptyBlock(_, b) and
exists(BlockOrNonChild c, AffectedFile file |
c.(BlockOrNonChild).getStartRankIn(file) =
1 + b.(BlockOrNonChild).getStartRankIn(file) and
c.(BlockOrNonChild).getNonContiguousEndRankIn(file) <
b.(BlockOrNonChild).getNonContiguousEndRankIn(file)
c.(BlockOrNonChild).getStartRankIn(file) = 1 + b.(BlockOrNonChild).getStartRankIn(file) and
c.(BlockOrNonChild).getNonContiguousEndRankIn(file) < b
.(BlockOrNonChild)
.getNonContiguousEndRankIn(file)
)
}
@@ -105,7 +105,8 @@ predicate lineComment(Block b) {
}
from ControlStructure s, Block eb
where emptyBlock(s, eb)
and not emptyBlockContainsNonchild(eb)
and not lineComment(eb)
where
emptyBlock(s, eb) and
not emptyBlockContainsNonchild(eb) and
not lineComment(eb)
select eb, "Empty block without comment"

3
cpp/ql/src/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql
View File

@@ -39,4 +39,5 @@ where
beforeArrayAccess(v, access, altcheck) and
altcheck.getLeftOperand() = v.getAnAccess()
)
select access, "This use of offset '" + v.getName() + "' should follow the $@.", rangecheck, "range check"
select access, "This use of offset '" + v.getName() + "' should follow the $@.", rangecheck,
"range check"

17
cpp/ql/src/Best Practices/Likely Errors/Slicing.ql
View File

@@ -9,16 +9,19 @@
* correctness
* types
*/
import cpp
//see http://www.cs.ualberta.ca/~hoover/Courses/201/201-New-Notes/lectures/section/slice.htm
//Does not find anything in rivers (unfortunately)
from AssignExpr e, Class lhsType, Class rhsType
where e.getLValue().getType() = lhsType
and e.getRValue().getType() = rhsType
and rhsType.getABaseClass+() = lhsType
and exists(Declaration m | rhsType.getAMember() = m and
not m.(VirtualFunction).isPure()) //add additional checks for concrete members in in-between supertypes
select e, "This assignment expression slices from type $@ to $@",
rhsType, rhsType.getName(),
where
e.getLValue().getType() = lhsType and
e.getRValue().getType() = rhsType and
rhsType.getABaseClass+() = lhsType and
exists(Declaration m |
rhsType.getAMember() = m and
not m.(VirtualFunction).isPure()
) //add additional checks for concrete members in in-between supertypes
select e, "This assignment expression slices from type $@ to $@", rhsType, rhsType.getName(),
lhsType, lhsType.getName()

361
cpp/ql/src/Best Practices/Magic Constants/MagicConstants.qll
View File

@@ -2,62 +2,208 @@ import cpp
import semmle.code.cpp.AutogeneratedFile
/*
*
* Counting nontrivial literal occurrences
*
*/
predicate trivialPositiveIntValue(string s) {
s="0" or s="1" or s="2" or s="3" or s="4" or s="5" or s="6" or s="7" or s="8" or
s="9" or s="10" or s="11" or s="12" or s="13" or s="14" or s="15" or s="16" or s="17" or
s="18" or s="19" or s="20"
or
s="16" or s="24" or s="32" or s="64" or s="128" or s="256" or s="512" or s="1024" or
s="2048" or s="4096" or s="16384" or s="32768" or s="65536" or
s="1048576" or s="2147483648" or s="4294967296"
or
s="15" or s="31" or s="63" or s="127" or s="255" or s="511" or s="1023" or
s="2047" or s="4095" or s="16383" or s="32767" or s="65535" or
s="1048577" or s="2147483647" or s="4294967295"
or
s = "0x00000001" or s = "0x00000002" or s = "0x00000004" or s = "0x00000008" or s = "0x00000010" or s = "0x00000020" or s = "0x00000040" or s = "0x00000080" or s = "0x00000100" or s = "0x00000200" or s = "0x00000400" or s = "0x00000800" or s = "0x00001000" or s = "0x00002000" or s = "0x00004000" or s = "0x00008000" or s = "0x00010000" or s = "0x00020000" or s = "0x00040000" or s = "0x00080000" or s = "0x00100000" or s = "0x00200000" or s = "0x00400000" or s = "0x00800000" or s = "0x01000000" or s = "0x02000000" or s = "0x04000000" or s = "0x08000000" or s = "0x10000000" or s = "0x20000000" or s = "0x40000000" or s = "0x80000000" or
s = "0x00000001" or s = "0x00000003" or s = "0x00000007" or s = "0x0000000f" or s = "0x0000001f" or s = "0x0000003f" or s = "0x0000007f" or s = "0x000000ff" or s = "0x000001ff" or s = "0x000003ff" or s = "0x000007ff" or s = "0x00000fff" or s = "0x00001fff" or s = "0x00003fff" or s = "0x00007fff" or s = "0x0000ffff" or s = "0x0001ffff" or s = "0x0003ffff" or s = "0x0007ffff" or s = "0x000fffff" or s = "0x001fffff" or s = "0x003fffff" or s = "0x007fffff" or s = "0x00ffffff" or s = "0x01ffffff" or s = "0x03ffffff" or s = "0x07ffffff" or s = "0x0fffffff" or s = "0x1fffffff" or s = "0x3fffffff" or s = "0x7fffffff" or s = "0xffffffff" or
s = "0x0001" or s = "0x0002" or s = "0x0004" or s = "0x0008" or s = "0x0010" or s = "0x0020" or s = "0x0040" or s = "0x0080" or s = "0x0100" or s = "0x0200" or s = "0x0400" or s = "0x0800" or s = "0x1000" or s = "0x2000" or s = "0x4000" or s = "0x8000" or
s = "0x0001" or s = "0x0003" or s = "0x0007" or s = "0x000f" or s = "0x001f" or s = "0x003f" or s = "0x007f" or s = "0x00ff" or s = "0x01ff" or s = "0x03ff" or s = "0x07ff" or s = "0x0fff" or s = "0x1fff" or s = "0x3fff" or s = "0x7fff" or s = "0xffff" or
s = "0x01" or s = "0x02" or s = "0x04" or s = "0x08" or s = "0x10" or s = "0x20" or s = "0x40" or s = "0x80" or
s = "0x01" or s = "0x03" or s = "0x07" or s = "0x0f" or s = "0x1f" or s = "0x3f" or s = "0x7f" or s = "0xff" or
s = "0x00"
or
s = "10" or s = "100" or s = "1000" or s = "10000" or s = "100000" or s = "1000000" or s = "10000000" or s = "100000000" or s = "1000000000"
s = "0" or
s = "1" or
s = "2" or
s = "3" or
s = "4" or
s = "5" or
s = "6" or
s = "7" or
s = "8" or
s = "9" or
s = "10" or
s = "11" or
s = "12" or
s = "13" or
s = "14" or
s = "15" or
s = "16" or
s = "17" or
s = "18" or
s = "19" or
s = "20" or
s = "16" or
s = "24" or
s = "32" or
s = "64" or
s = "128" or
s = "256" or
s = "512" or
s = "1024" or
s = "2048" or
s = "4096" or
s = "16384" or
s = "32768" or
s = "65536" or
s = "1048576" or
s = "2147483648" or
s = "4294967296" or
s = "15" or
s = "31" or
s = "63" or
s = "127" or
s = "255" or
s = "511" or
s = "1023" or
s = "2047" or
s = "4095" or
s = "16383" or
s = "32767" or
s = "65535" or
s = "1048577" or
s = "2147483647" or
s = "4294967295" or
s = "0x00000001" or
s = "0x00000002" or
s = "0x00000004" or
s = "0x00000008" or
s = "0x00000010" or
s = "0x00000020" or
s = "0x00000040" or
s = "0x00000080" or
s = "0x00000100" or
s = "0x00000200" or
s = "0x00000400" or
s = "0x00000800" or
s = "0x00001000" or
s = "0x00002000" or
s = "0x00004000" or
s = "0x00008000" or
s = "0x00010000" or
s = "0x00020000" or
s = "0x00040000" or
s = "0x00080000" or
s = "0x00100000" or
s = "0x00200000" or
s = "0x00400000" or
s = "0x00800000" or
s = "0x01000000" or
s = "0x02000000" or
s = "0x04000000" or
s = "0x08000000" or
s = "0x10000000" or
s = "0x20000000" or
s = "0x40000000" or
s = "0x80000000" or
s = "0x00000001" or
s = "0x00000003" or
s = "0x00000007" or
s = "0x0000000f" or
s = "0x0000001f" or
s = "0x0000003f" or
s = "0x0000007f" or
s = "0x000000ff" or
s = "0x000001ff" or
s = "0x000003ff" or
s = "0x000007ff" or
s = "0x00000fff" or
s = "0x00001fff" or
s = "0x00003fff" or
s = "0x00007fff" or
s = "0x0000ffff" or
s = "0x0001ffff" or
s = "0x0003ffff" or
s = "0x0007ffff" or
s = "0x000fffff" or
s = "0x001fffff" or
s = "0x003fffff" or
s = "0x007fffff" or
s = "0x00ffffff" or
s = "0x01ffffff" or
s = "0x03ffffff" or
s = "0x07ffffff" or
s = "0x0fffffff" or
s = "0x1fffffff" or
s = "0x3fffffff" or
s = "0x7fffffff" or
s = "0xffffffff" or
s = "0x0001" or
s = "0x0002" or
s = "0x0004" or
s = "0x0008" or
s = "0x0010" or
s = "0x0020" or
s = "0x0040" or
s = "0x0080" or
s = "0x0100" or
s = "0x0200" or
s = "0x0400" or
s = "0x0800" or
s = "0x1000" or
s = "0x2000" or
s = "0x4000" or
s = "0x8000" or
s = "0x0001" or
s = "0x0003" or
s = "0x0007" or
s = "0x000f" or
s = "0x001f" or
s = "0x003f" or
s = "0x007f" or
s = "0x00ff" or
s = "0x01ff" or
s = "0x03ff" or
s = "0x07ff" or
s = "0x0fff" or
s = "0x1fff" or
s = "0x3fff" or
s = "0x7fff" or
s = "0xffff" or
s = "0x01" or
s = "0x02" or
s = "0x04" or
s = "0x08" or
s = "0x10" or
s = "0x20" or
s = "0x40" or
s = "0x80" or
s = "0x01" or
s = "0x03" or
s = "0x07" or
s = "0x0f" or
s = "0x1f" or
s = "0x3f" or
s = "0x7f" or
s = "0xff" or
s = "0x00" or
s = "10" or
s = "100" or
s = "1000" or
s = "10000" or
s = "100000" or
s = "1000000" or
s = "10000000" or
s = "100000000" or
s = "1000000000"
}
predicate trivialIntValue(string s) {
trivialPositiveIntValue(s) or
trivialPositiveIntValue(s)
or
exists(string pos | trivialPositiveIntValue(pos) and s = "-" + pos)
}
predicate trivialLongValue(string s) {
exists(string v | trivialIntValue(v) and s = v + "L")
}
predicate trivialLongValue(string s) { exists(string v | trivialIntValue(v) and s = v + "L") }
predicate intTrivial(Literal lit) {
exists(string v | trivialIntValue(v) and v = lit.getValue())
}
predicate intTrivial(Literal lit) { exists(string v | trivialIntValue(v) and v = lit.getValue()) }
predicate longTrivial(Literal lit) {
exists(string v | trivialLongValue(v) and v = lit.getValue())
}
predicate longTrivial(Literal lit) { exists(string v | trivialLongValue(v) and v = lit.getValue()) }
predicate powerOfTen(float f) {
f = 10 or f = 100 or f = 1000 or f = 10000 or f = 100000 or f = 1000000 or f = 10000000 or f = 100000000 or f = 1000000000
f = 10 or
f = 100 or
f = 1000 or
f = 10000 or
f = 100000 or
f = 1000000 or
f = 10000000 or
f = 100000000 or
f = 1000000000
}
predicate floatTrivial(Literal lit) {
@@ -69,14 +215,9 @@ predicate floatTrivial(Literal lit) {
)
}
predicate charLiteral(Literal lit) {
lit instanceof CharLiteral
}
predicate charLiteral(Literal lit) { lit instanceof CharLiteral }
Type literalType(Literal literal) {
result = literal.getType()
}
Type literalType(Literal literal) { result = literal.getType() }
predicate stringType(DerivedType t) {
t.getBaseType() instanceof CharType
@@ -88,13 +229,9 @@ predicate stringType(DerivedType t) {
)
}
predicate numberType(Type t) {
t instanceof FloatingPointType or t instanceof IntegralType
}
predicate numberType(Type t) { t instanceof FloatingPointType or t instanceof IntegralType }
predicate stringLiteral(Literal literal) {
literal instanceof StringLiteral
}
predicate stringLiteral(Literal literal) { literal instanceof StringLiteral }
predicate stringTrivial(Literal lit) {
stringLiteral(lit) and
@@ -117,9 +254,7 @@ predicate joiningStringTrivial(Literal lit) {
lit.getValue().length() < 16
}
predicate small(Literal lit) {
lit.getValue().length() <= 1
}
predicate small(Literal lit) { lit.getValue().length() <= 1 }
predicate trivial(Literal lit) {
charLiteral(lit) or
@@ -133,15 +268,15 @@ predicate trivial(Literal lit) {
private predicate isReferenceTo(Variable ref, Variable to) {
exists(VariableAccess a |
ref.getInitializer().getExpr().getConversion().(ReferenceToExpr).getExpr() = a and a.getTarget() = to)
ref.getInitializer().getExpr().getConversion().(ReferenceToExpr).getExpr() = a and
a.getTarget() = to
)
}
private predicate variableNotModifiedAfterInitializer(Variable v) {
not exists(VariableAccess a | a.getTarget() = v and a.isModified()) and
not exists(AddressOfExpr e | e.getAddressable() = v) and
forall(Variable v2 |
isReferenceTo(v2, v) |
variableNotModifiedAfterInitializer(v2))
forall(Variable v2 | isReferenceTo(v2, v) | variableNotModifiedAfterInitializer(v2))
}
predicate literalIsConstantInitializer(Literal literal, Variable f) {
@@ -155,20 +290,20 @@ predicate literalIsEnumInitializer(Literal literal) {
}
predicate literalInArrayInitializer(Literal literal) {
exists(AggregateLiteral arrayInit |
arrayInitializerChild(arrayInit, literal)
)
exists(AggregateLiteral arrayInit | arrayInitializerChild(arrayInit, literal))
}
predicate arrayInitializerChild(AggregateLiteral parent, Expr e) {
e = parent
or
exists (Expr mid | arrayInitializerChild(parent, mid) and e.getParent() = mid)
exists(Expr mid | arrayInitializerChild(parent, mid) and e.getParent() = mid)
}
// i.e. not a constant folded expression
predicate literallyLiteral(Literal lit) {
lit.getValueText().regexpMatch(".*\".*|\\s*+[-+]?+\\s*+(0[xob][0-9a-fA-F]|[0-9])[0-9a-fA-F,._]*+([eE][-+]?+[0-9,._]*+)?+\\s*+[a-zA-Z]*+\\s*+")
lit
.getValueText()
.regexpMatch(".*\".*|\\s*+[-+]?+\\s*+(0[xob][0-9a-fA-F]|[0-9])[0-9a-fA-F,._]*+([eE][-+]?+[0-9,._]*+)?+\\s*+[a-zA-Z]*+\\s*+")
}
predicate nonTrivialValue(string value, Literal literal) {
@@ -183,15 +318,11 @@ predicate nonTrivialValue(string value, Literal literal) {
predicate valueOccurrenceCount(string value, int n) {
n = strictcount(Location loc |
exists(Literal lit |
lit.getLocation() = loc |
nonTrivialValue(value, lit)
exists(Literal lit | lit.getLocation() = loc | nonTrivialValue(value, lit)) and
// Exclude generated files (they do not have the same maintainability
// concerns as ordinary source files)
not loc.getFile() instanceof AutogeneratedFile
) and
// Exclude generated files (they do not have the same maintainability
// concerns as ordinary source files)
not loc.getFile() instanceof AutogeneratedFile
) and
n > 20
}
@@ -201,29 +332,25 @@ predicate occurenceCount(Literal lit, string value, int n) {
nonTrivialValue(_, lit)
}
/*
*
* Literals repeated frequently
*
*/
predicate check(Literal lit, string value, int n, File f) {
// Check that the literal is nontrivial
not trivial(lit) and
// Check that it is repeated a number of times
occurenceCount(lit, value, n) and n > 20 and
occurenceCount(lit, value, n) and
n > 20 and
f = lit.getFile() and
// Exclude generated files
// Exclude generated files
not f instanceof AutogeneratedFile
}
predicate checkWithFileCount(string value, int overallCount, int fileCount, File f) {
fileCount = strictcount(Location loc |
exists(Literal lit |
lit.getLocation() = loc |
check(lit, value, overallCount, f)))
exists(Literal lit | lit.getLocation() = loc | check(lit, value, overallCount, f))
)
}
predicate start(Literal lit, int startLine) {
@@ -239,50 +366,51 @@ predicate firstOccurrence(Literal lit, string value, int n) {
check(lit2, value, n, f) and
start(lit, start1) and
start(lit2, start2) and
start2 < start1)
start2 < start1
)
)
}
predicate magicConstant(Literal e, string msg) {
exists(string value, int n | firstOccurrence(e, value, n)
and msg = "Magic constant: literal '" + value + "' is repeated " + n.toString() + " times and should be encapsulated in a constant.")
exists(string value, int n |
firstOccurrence(e, value, n) and
msg = "Magic constant: literal '" + value + "' is repeated " + n.toString() +
" times and should be encapsulated in a constant."
)
}
/*
*
* Literals where there is a defined constant with the same value
*
*/
predicate relevantVariable(Variable f, string value) {
exists(Literal lit | not trivial(lit) and value = lit.getValue() and literalIsConstantInitializer(lit, f))
exists(Literal lit |
not trivial(lit) and value = lit.getValue() and literalIsConstantInitializer(lit, f)
)
}
predicate relevantCallable(Function f, string value) {
exists(Literal lit | not trivial(lit) and value = lit.getValue() and lit.getEnclosingFunction() = f)
exists(Literal lit |
not trivial(lit) and value = lit.getValue() and lit.getEnclosingFunction() = f
)
}
predicate isVisible(Variable field, Function fromCallable) {
exists(string value |
//public fields
(
exists(string value |
//public fields
relevantVariable(field, value) and
field.(MemberVariable).isPublic() and
relevantCallable(fromCallable, value)
)
or
//in same class
(
or
//in same class
relevantVariable(field, value) and
exists(Type t |
t = field.getDeclaringType() and
t = fromCallable.getDeclaringType())
and relevantCallable(fromCallable, value)
)
or
//in subclass and not private
(
t = fromCallable.getDeclaringType()
) and
relevantCallable(fromCallable, value)
or
//in subclass and not private
relevantVariable(field, value) and
not field.(MemberVariable).isPrivate() and
exists(Class sup, Class sub |
@@ -292,7 +420,6 @@ predicate isVisible(Variable field, Function fromCallable) {
) and
relevantCallable(fromCallable, value)
)
)
}
predicate canUseFieldInsteadOfLiteral(Variable constField, Literal magicLiteral) {
@@ -303,25 +430,25 @@ predicate canUseFieldInsteadOfLiteral(Variable constField, Literal magicLiteral)
exists(string value |
value = initLiteral.getValue() and
magicLiteral.getValue() = value
)
and constField.getType() = magicLiteral.getType()
and not literalIsConstantInitializer(magicLiteral, _)
and exists(Function c |
) and
constField.getType() = magicLiteral.getType() and
not literalIsConstantInitializer(magicLiteral, _) and
exists(Function c |
c = magicLiteral.getEnclosingFunction() and
(
(
constField.isTopLevel() and (not constField.isStatic() or constField.getFile() = c.getFile())
)
constField.isTopLevel() and
(not constField.isStatic() or constField.getFile() = c.getFile())
or
isVisible(constField,c)
isVisible(constField, c)
)
)
)
}
predicate literalInsteadOfConstant(Literal magicLiteral, string message, Variable constField, string linkText) {
predicate literalInsteadOfConstant(
Literal magicLiteral, string message, Variable constField, string linkText
) {
canUseFieldInsteadOfLiteral(constField, magicLiteral) and
message =
"Literal value '" + magicLiteral.getValue() + "' used instead of constant $@." and
message = "Literal value '" + magicLiteral.getValue() + "' used instead of constant $@." and
linkText = constField.getName()
}

4
cpp/ql/src/Best Practices/Magic Constants/MagicConstantsNumbers.ql
View File

@@ -9,12 +9,14 @@
* statistical
* non-attributable
*/
import cpp
import MagicConstants
pragma[noopt]
predicate selection(Element e, string msg) {
magicConstant(e, msg) and exists(Literal l, Type t | l=e and t = l.getType() and numberType(t) and l instanceof Literal)
magicConstant(e, msg) and
exists(Literal l, Type t | l = e and t = l.getType() and numberType(t) and l instanceof Literal)
}
from Literal e, string msg

5
cpp/ql/src/Best Practices/Magic Constants/MagicConstantsString.ql
View File

@@ -9,13 +9,12 @@
* statistical
* non-attributable
*/
import cpp
import MagicConstants
pragma[noopt]
predicate selection(Element e, string msg) {
magicConstant(e, msg) and stringLiteral(e)
}
predicate selection(Element e, string msg) { magicConstant(e, msg) and stringLiteral(e) }
from Literal e, string msg
where selection(e, msg)

6
cpp/ql/src/Best Practices/Magic Constants/MagicNumbersUseConstant.ql
View File

@@ -7,10 +7,12 @@
* @precision low
* @tags maintainability
*/
import cpp
import MagicConstants
from Literal magicLiteral, string message, Variable constant, string linkText
where numberType(magicLiteral.getType())
and literalInsteadOfConstant(magicLiteral, message, constant, linkText)
where
numberType(magicLiteral.getType()) and
literalInsteadOfConstant(magicLiteral, message, constant, linkText)
select magicLiteral, message, constant, linkText

6
cpp/ql/src/Best Practices/Magic Constants/MagicStringsUseConstant.ql
View File

@@ -7,10 +7,12 @@
* @precision low
* @tags maintainability
*/
import cpp
import MagicConstants
from Literal magicLiteral, string message, Variable constant, string linkText
where stringLiteral(magicLiteral)
and literalInsteadOfConstant(magicLiteral, message, constant, linkText)
where
stringLiteral(magicLiteral) and
literalInsteadOfConstant(magicLiteral, message, constant, linkText)
select magicLiteral, message, constant, linkText

11
cpp/ql/src/Best Practices/NVI.ql
View File

@@ -8,13 +8,14 @@
* @precision low
* @tags maintainability
*/
import cpp
//see http://www.gotw.ca/publications/mill18.htm
from MemberFunction f
where f.hasSpecifier("public") and
f.hasSpecifier("virtual") and
f.getFile().fromSource() and
not (f instanceof Destructor)
where
f.hasSpecifier("public") and
f.hasSpecifier("virtual") and
f.getFile().fromSource() and
not f instanceof Destructor
select f, "Avoid having public virtual methods (NVI idiom)"

17
cpp/ql/src/Best Practices/NVIHub.ql
View File

@@ -10,16 +10,17 @@
* @precision low
* @tags maintainability
*/
import cpp
//see http://www.gotw.ca/publications/mill18.htm
from MemberFunction f, int hubIndex, Class fclass
where f.hasSpecifier("public") and
f.hasSpecifier("virtual") and
f.getFile().fromSource() and
not (f instanceof Destructor) and
fclass = f.getDeclaringType() and
hubIndex = fclass.getMetrics().getAfferentCoupling() * fclass.getMetrics().getEfferentCoupling() and
hubIndex > 100
where
f.hasSpecifier("public") and
f.hasSpecifier("virtual") and
f.getFile().fromSource() and
not f instanceof Destructor and
fclass = f.getDeclaringType() and
hubIndex = fclass.getMetrics().getAfferentCoupling() * fclass.getMetrics().getEfferentCoupling() and
hubIndex > 100
select f, "Avoid having public virtual methods (NVI idiom)"

21
cpp/ql/src/Best Practices/RuleOfThree.ql
View File

@@ -8,19 +8,24 @@
* @problem.severity warning
* @tags reliability
*/
import cpp
class BigThree extends MemberFunction {
BigThree() {
this instanceof Destructor
or this instanceof CopyConstructor
or this instanceof CopyAssignmentOperator
this instanceof Destructor or
this instanceof CopyConstructor or
this instanceof CopyAssignmentOperator
}
}
from Class c, BigThree b
where b.getDeclaringType() = c and
not (c.hasDestructor() and
c.getAMemberFunction() instanceof CopyConstructor and
c.getAMemberFunction() instanceof CopyAssignmentOperator)
select c, "Class defines a destructor, copy constructor, or copy assignment operator, but not all three."
where
b.getDeclaringType() = c and
not (
c.hasDestructor() and
c.getAMemberFunction() instanceof CopyConstructor and
c.getAMemberFunction() instanceof CopyAssignmentOperator
)
select c,
"Class defines a destructor, copy constructor, or copy assignment operator, but not all three."

55
cpp/ql/src/Best Practices/RuleOfTwo.ql
View File

@@ -11,6 +11,7 @@
* readability
* language-features
*/
import cpp
// This query enforces the Rule of Two, which is a conservative variation of
@@ -25,41 +26,39 @@ import cpp
// auto-generated. This can lead to memory safety issues. It's a separate issue
// whether one is callable while the other is not callable; that is an API
// design question and carries has no safety risk.
predicate generatedCopyAssignment(CopyConstructor cc, string msg) {
cc.getDeclaringType().hasImplicitCopyAssignmentOperator() and
msg = "No matching copy assignment operator in class " +
cc.getDeclaringType().getName() +
". It is good practice to match a copy constructor with a " +
"copy assignment operator."
msg = "No matching copy assignment operator in class " + cc.getDeclaringType().getName() +
". It is good practice to match a copy constructor with a " + "copy assignment operator."
}
predicate generatedCopyConstructor(CopyAssignmentOperator ca, string msg) {
ca.getDeclaringType().hasImplicitCopyConstructor() and
msg = "No matching copy constructor in class " +
ca.getDeclaringType().getName() +
". It is good practice to match a copy assignment operator with a " +
"copy constructor."
msg = "No matching copy constructor in class " + ca.getDeclaringType().getName() +
". It is good practice to match a copy assignment operator with a " + "copy constructor."
}
from MemberFunction f, string msg
where (generatedCopyAssignment(f, msg) or
generatedCopyConstructor(f, msg))
// Ignore template instantiations to prevent an explosion of alerts
and not f.getDeclaringType().isConstructedFrom(_)
// Ignore private members since a private constructor or assignment operator
// is a common idiom that simulates suppressing the default-generated
// members. It would be better to use C++11's "delete" facility or use
// appropriate Boost helper classes, but it is too common to report as a
// violation.
and not f.isPrivate()
// If it is truly user-defined then it must have a body. This leaves out
// C++11 members that use `= delete` or `= default`.
and exists(f.getBlock())
// In rare cases, the extractor pretends that an auto-generated copy
// constructor has a block that is one character long and is located on top
// of the first character of the class name. Checking for
// `isCompilerGenerated` will remove those results.
and not f.isCompilerGenerated()
and not f.isDeleted()
where
(
generatedCopyAssignment(f, msg) or
generatedCopyConstructor(f, msg)
) and
// Ignore template instantiations to prevent an explosion of alerts
not f.getDeclaringType().isConstructedFrom(_) and
// Ignore private members since a private constructor or assignment operator
// is a common idiom that simulates suppressing the default-generated
// members. It would be better to use C++11's "delete" facility or use
// appropriate Boost helper classes, but it is too common to report as a
// violation.
not f.isPrivate() and
// If it is truly user-defined then it must have a body. This leaves out
// C++11 members that use `= delete` or `= default`.
exists(f.getBlock()) and
// In rare cases, the extractor pretends that an auto-generated copy
// constructor has a block that is one character long and is located on top
// of the first character of the class name. Checking for
// `isCompilerGenerated` will remove those results.
not f.isCompilerGenerated() and
not f.isDeleted()
select f, msg

10
cpp/ql/src/Best Practices/SloppyGlobal.ql
View File

@@ -7,9 +7,13 @@
* @id cpp/short-global-name
* @tags maintainability
*/
import cpp
from GlobalVariable gv
where gv.getName().length() <= 3
and not gv.isStatic()
select gv, "Poor global variable name '" + gv.getName() + "'. Prefer longer, descriptive names for globals (eg. kMyGlobalConstant, not foo)."
where
gv.getName().length() <= 3 and
not gv.isStatic()
select gv,
"Poor global variable name '" + gv.getName() +
"'. Prefer longer, descriptive names for globals (eg. kMyGlobalConstant, not foo)."

35
cpp/ql/src/Best Practices/SwitchLongCase.ql
View File

@@ -8,14 +8,12 @@
* @tags maintainability
* readability
*/
import cpp
predicate switchCaseStartLine(SwitchCase sc, int start) {
sc.getLocation().getStartLine() = start
}
predicate switchStmtEndLine(SwitchStmt s, int start) {
s.getLocation().getEndLine() = start
}
predicate switchCaseStartLine(SwitchCase sc, int start) { sc.getLocation().getStartLine() = start }
predicate switchStmtEndLine(SwitchStmt s, int start) { s.getLocation().getEndLine() = start }
predicate switchCaseLength(SwitchCase sc, int length) {
exists(SwitchCase next, int l1, int l2 |
@@ -25,21 +23,20 @@ predicate switchCaseLength(SwitchCase sc, int length) {
length = l1 - l2 - 1
)
or
(
not exists(sc.getNextSwitchCase()) and
exists(int l1, int l2 |
switchStmtEndLine(sc.getSwitchStmt(), l1) and
switchCaseStartLine(sc, l2) and
length = l1 - l2 - 1
)
not exists(sc.getNextSwitchCase()) and
exists(int l1, int l2 |
switchStmtEndLine(sc.getSwitchStmt(), l1) and
switchCaseStartLine(sc, l2) and
length = l1 - l2 - 1
)
}
predicate tooLong(SwitchCase sc) {
exists(int n | switchCaseLength(sc, n) and n > 30)
}
predicate tooLong(SwitchCase sc) { exists(int n | switchCaseLength(sc, n) and n > 30) }
from SwitchStmt switch, SwitchCase sc, int lines
where sc = switch.getASwitchCase() and tooLong(sc)
and switchCaseLength(sc, lines)
select switch, "Switch has at least one case that is too long: $@", sc, sc.getExpr().toString() + " (" + lines.toString() + " lines)"
where
sc = switch.getASwitchCase() and
tooLong(sc) and
switchCaseLength(sc, lines)
select switch, "Switch has at least one case that is too long: $@", sc,
sc.getExpr().toString() + " (" + lines.toString() + " lines)"

16
cpp/ql/src/Best Practices/Unused Entities/UnusedIncludes.ql
View File

@@ -18,10 +18,12 @@ File sourceFile() {
}
from Include include, File source, File unneeded
where include.getFile() = source
and source = sourceFile()
and unneeded = include.getIncludedFile()
and not unneeded.getAnIncludedFile*() = source.getMetrics().getAFileDependency()
and unneeded.fromSource()
and not unneeded.getBaseName().matches("%Debug.h")
select include, "Redundant include, this file does not require $@.", unneeded, unneeded.getAbsolutePath()
where
include.getFile() = source and
source = sourceFile() and
unneeded = include.getIncludedFile() and
not unneeded.getAnIncludedFile*() = source.getMetrics().getAFileDependency() and
unneeded.fromSource() and
not unneeded.getBaseName().matches("%Debug.h")
select include, "Redundant include, this file does not require $@.", unneeded,
unneeded.getAbsolutePath()

30
cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql
View File

@@ -10,22 +10,24 @@
* useless-code
* external/cwe/cwe-563
*/
import cpp
/**
* A type that contains a template parameter type
* (doesn't count pointers or references).
*
*
* These types may have a constructor / destructor when they are
* instantiated, that is not visible in their template form.
*
* Such types include template parameters, classes with a member variable
* of template parameter type, and classes that derive from other such
* classes.
* classes.
*/
class TemplateDependentType extends Type {
TemplateDependentType() {
this instanceof TemplateParameter or
this instanceof TemplateParameter
or
exists(TemplateDependentType t |
this.refersToDirectly(t) and
not this instanceof PointerType and
@@ -38,20 +40,22 @@ class TemplateDependentType extends Type {
* A variable whose declaration has, or may have, side effects.
*/
predicate declarationHasSideEffects(Variable v) {
exists (Class c | c = v.getType().getUnderlyingType().getUnspecifiedType() |
exists(Class c | c = v.getType().getUnderlyingType().getUnspecifiedType() |
c.hasConstructor() or
c.hasDestructor()
) or
)
or
v.getType() instanceof TemplateDependentType // may have a constructor/destructor
}
from LocalVariable v, Function f
where f = v.getFunction()
and not exists(v.getAnAccess())
and not v.isConst() // workaround for folded constants
and not exists(DeclStmt ds | ds.getADeclaration() = v and ds.isInMacroExpansion()) // variable declared in a macro expansion
and not declarationHasSideEffects(v)
and not exists(AsmStmt s | f = s.getEnclosingFunction())
and not v.getAnAttribute().getName() = "unused"
and not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr likely used `v`
where
f = v.getFunction() and
not exists(v.getAnAccess()) and
not v.isConst() and // workaround for folded constants
not exists(DeclStmt ds | ds.getADeclaration() = v and ds.isInMacroExpansion()) and // variable declared in a macro expansion
not declarationHasSideEffects(v) and
not exists(AsmStmt s | f = s.getEnclosingFunction()) and
not v.getAnAttribute().getName() = "unused" and
not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr likely used `v`
select v, "Variable " + v.getName() + " is not used"

122
cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql
View File

@@ -2,7 +2,6 @@
* @name Unused static function
* @description A static function that is never called or accessed may be an
* indication that the code is incomplete or has a typo.
* @description Static functions that are never called or accessed.
* @kind problem
* @problem.severity recommendation
* @precision high
@@ -11,82 +10,93 @@
* useless-code
* external/cwe/cwe-561
*/
import cpp
predicate immediatelyReachableFunction(Function f) {
not f.isStatic()
or exists(BlockExpr be | be.getFunction() = f)
or f instanceof MemberFunction
or f instanceof TemplateFunction
or f.getFile() instanceof HeaderFile
or f.getAnAttribute().hasName("constructor")
or f.getAnAttribute().hasName("destructor")
or f.getAnAttribute().hasName("used")
or f.getAnAttribute().hasName("unused")
not f.isStatic() or
exists(BlockExpr be | be.getFunction() = f) or
f instanceof MemberFunction or
f instanceof TemplateFunction or
f.getFile() instanceof HeaderFile or
f.getAnAttribute().hasName("constructor") or
f.getAnAttribute().hasName("destructor") or
f.getAnAttribute().hasName("used") or
f.getAnAttribute().hasName("unused")
}
predicate immediatelyReachableVariable(Variable v) {
(v.isTopLevel() and not v.isStatic())
or exists(v.getDeclaringType())
or v.getFile() instanceof HeaderFile
or v.getAnAttribute().hasName("used")
or v.getAnAttribute().hasName("unused")
v.isTopLevel() and not v.isStatic()
or
exists(v.getDeclaringType())
or
v.getFile() instanceof HeaderFile
or
v.getAnAttribute().hasName("used")
or
v.getAnAttribute().hasName("unused")
}
class ImmediatelyReachableThing extends Thing {
ImmediatelyReachableThing() {
immediatelyReachableFunction(this) or
immediatelyReachableVariable(this)
}
ImmediatelyReachableThing() {
immediatelyReachableFunction(this) or
immediatelyReachableVariable(this)
}
}
predicate reachableThing(Thing t) {
t instanceof ImmediatelyReachableThing or
exists(Thing mid | reachableThing(mid) and mid.callsOrAccesses() = t)
t instanceof ImmediatelyReachableThing
or
exists(Thing mid | reachableThing(mid) and mid.callsOrAccesses() = t)
}
class Thing extends Locatable {
Thing() {
this instanceof Function or
this instanceof Variable
}
Thing() {
this instanceof Function or
this instanceof Variable
}
string getName() {
result = this.(Function).getName() or
result = this.(Variable).getName()
}
string getName() {
result = this.(Function).getName() or
result = this.(Variable).getName()
}
Thing callsOrAccesses() {
this.(Function).calls((Function)result) or
this.(Function).accesses((Function)result) or
this.(Function).accesses((Variable)result) or
(exists(Access a | this.(Variable).getInitializer().getExpr().getAChild*() = a
| result = a.getTarget()))
}
Thing callsOrAccesses() {
this.(Function).calls(result.(Function))
or
this.(Function).accesses(result.(Function))
or
this.(Function).accesses(result.(Variable))
or
exists(Access a | this.(Variable).getInitializer().getExpr().getAChild*() = a |
result = a.getTarget()
)
}
}
class FunctionToRemove extends Function {
FunctionToRemove() {
this.hasDefinition()
and not reachableThing(this)
}
FunctionToRemove() {
this.hasDefinition() and
not reachableThing(this)
}
Thing getOther() {
result.callsOrAccesses+() = this
and this != result
// We will already be reporting the enclosing function of a
// local variable, so don't also report the variable
and not result instanceof LocalVariable
}
Thing getOther() {
result.callsOrAccesses+() = this and
this != result and
// We will already be reporting the enclosing function of a
// local variable, so don't also report the variable
not result instanceof LocalVariable
}
}
from FunctionToRemove f, string clarification, Thing other
where if exists(f.getOther())
then (clarification = " ($@ must be removed at the same time)"
and other = f.getOther())
else (clarification = "" and other = f)
select f,
"Static function " + f.getName() + " is unreachable" + clarification,
other,
other.getName()
where
if exists(f.getOther())
then (
clarification = " ($@ must be removed at the same time)" and
other = f.getOther()
) else (
clarification = "" and other = f
)
select f, "Static function " + f.getName() + " is unreachable" + clarification, other,
other.getName()

18
cpp/ql/src/Best Practices/Unused Entities/UnusedStaticVariables.ql
View File

@@ -10,20 +10,22 @@
* useless-code
* external/cwe/cwe-563
*/
import cpp
predicate declarationHasSideEffects(Variable v) {
exists (Class c | c = v.getType().getUnderlyingType().getUnspecifiedType() |
exists(Class c | c = v.getType().getUnderlyingType().getUnspecifiedType() |
c.hasConstructor() or c.hasDestructor()
)
}
from Variable v
where v.isStatic()
and v.hasDefinition()
and not exists(VariableAccess a | a.getTarget() = v)
and not v instanceof MemberVariable
and not declarationHasSideEffects(v)
and not v.getAnAttribute().hasName("used")
and not v.getAnAttribute().hasName("unused")
where
v.isStatic() and
v.hasDefinition() and
not exists(VariableAccess a | a.getTarget() = v) and
not v instanceof MemberVariable and
not declarationHasSideEffects(v) and
not v.getAnAttribute().hasName("used") and
not v.getAnAttribute().hasName("unused")
select v, "Static variable " + v.getName() + " is never read"

38
cpp/ql/src/Best Practices/UseOfGoto.ql
View File

@@ -14,27 +14,29 @@
import cpp
class JumpTarget extends Stmt {
JumpTarget() {
exists(GotoStmt g | g.getTarget() = this)
}
JumpTarget() { exists(GotoStmt g | g.getTarget() = this) }
FunctionDeclarationEntry getFDE() {
result.getBlock() = this.getParentStmt+()
}
FunctionDeclarationEntry getFDE() { result.getBlock() = this.getParentStmt+() }
predicate isForward() {
exists(GotoStmt g | g.getTarget() = this | g.getLocation().getEndLine() < this.getLocation().getStartLine())
}
predicate isForward() {
exists(GotoStmt g | g.getTarget() = this |
g.getLocation().getEndLine() < this.getLocation().getStartLine()
)
}
predicate isBackward() {
exists(GotoStmt g | g.getTarget() = this | this.getLocation().getEndLine() < g.getLocation().getStartLine())
}
predicate isBackward() {
exists(GotoStmt g | g.getTarget() = this |
this.getLocation().getEndLine() < g.getLocation().getStartLine()
)
}
}
from FunctionDeclarationEntry fde, int nforward, int nbackward
where nforward = strictcount(JumpTarget t | t.getFDE() = fde and t.isForward())
and nbackward = strictcount(JumpTarget t | t.getFDE() = fde and t.isBackward())
and nforward != 1
and nbackward != 1
select fde, "Multiple forward and backward goto statements may make function "
+ fde.getName() + " hard to understand."
where
nforward = strictcount(JumpTarget t | t.getFDE() = fde and t.isForward()) and
nbackward = strictcount(JumpTarget t | t.getFDE() = fde and t.isBackward()) and
nforward != 1 and
nbackward != 1
select fde,
"Multiple forward and backward goto statements may make function " + fde.getName() +
" hard to understand."