Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge from main to 2.37.0 #782

Merged
merged 6 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 19 additions & 17 deletions c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,32 @@

import cpp
import codingstandards.c.cert
import codingstandards.cpp.Compatible
import ExternalIdentifiers

//checks if they are incompatible based on return type, number of parameters and parameter types
predicate checkMatchingFunction(FunctionDeclarationEntry d, FunctionDeclarationEntry d2) {
not d.getType() = d2.getType()
or
not d.getNumberOfParameters() = d2.getNumberOfParameters()
or
exists(ParameterDeclarationEntry p, ParameterDeclarationEntry p2, int i |
d.getParameterDeclarationEntry(i) = p and
d2.getParameterDeclarationEntry(i) = p2 and
not p.getType() = p2.getType()
)
}

from ExternalIdentifiers d, FunctionDeclarationEntry f1, FunctionDeclarationEntry f2
where
not isExcluded(f1, Declarations2Package::incompatibleFunctionDeclarationsQuery()) and
not isExcluded(f2, Declarations2Package::incompatibleFunctionDeclarationsQuery()) and
f1 = d.getADeclarationEntry() and
f2 = d.getADeclarationEntry() and
not f1 = f2 and
f1.getLocation().getStartLine() >= f2.getLocation().getStartLine() and
f1.getDeclaration() = d and
f2.getDeclaration() = d and
f1.getName() = f2.getName() and
checkMatchingFunction(f1, f2)
(
//return type check
not typesCompatible(f1.getType(), f2.getType())
or
//parameter type check
parameterTypesIncompatible(f1, f2)
or
not f1.getNumberOfParameters() = f2.getNumberOfParameters()
) and
// Apply ordering on start line, trying to avoid the optimiser applying this join too early
// in the pipeline
exists(int f1Line, int f2Line |
f1.getLocation().hasLocationInfo(_, f1Line, _, _, _) and
f2.getLocation().hasLocationInfo(_, f2Line, _, _, _) and
f1Line >= f2Line
)
select f1, "The object $@ is not compatible with re-declaration $@", f1, f1.getName(), f2,
f2.getName()
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,19 @@ predicate sameSource(VaAccess e1, VaAccess e2) {
)
}

/**
* Extracted to avoid poor magic join ordering on the `isExcluded` predicate.
*/
predicate query(VaAccess va_acc, VaArgArg va_arg, FunctionCall fc) {
sameSource(va_acc, va_arg) and
fc = preceedsFC(va_acc) and
fc.getTarget().calls*(va_arg.getEnclosingFunction())
}

from VaAccess va_acc, VaArgArg va_arg, FunctionCall fc
where
not isExcluded(va_acc,
Contracts7Package::doNotCallVaArgOnAVaListThatHasAnIndeterminateValueQuery()) and
sameSource(va_acc, va_arg) and
fc = preceedsFC(va_acc) and
fc.getTarget().calls*(va_arg.getEnclosingFunction())
query(va_acc, va_arg, fc)
select va_acc, "The value of " + va_acc.toString() + " is indeterminate after the $@.", fc,
fc.toString()
Original file line number Diff line number Diff line change
Expand Up @@ -62,33 +62,31 @@ int getMaxDepth(ArrayAggregateLiteral al) {

// internal recursive predicate for `hasMultipleInitializerExprsForSameIndex`
predicate hasMultipleInitializerExprsForSameIndexInternal(
ArrayAggregateLiteral al1, ArrayAggregateLiteral al2, Expr out_al1_expr, Expr out_al2_expr
ArrayAggregateLiteral root, Expr e1, Expr e2
) {
exists(int shared_index, Expr al1_expr, Expr al2_expr |
// an `Expr` initializing an element of the same index in both `al1` and `al2`
shared_index = [0 .. al1.getArraySize() - 1] and
al1_expr = al1.getAnElementExpr(shared_index) and
al2_expr = al2.getAnElementExpr(shared_index) and
// but not the same `Expr`
not al1_expr = al2_expr and
(
// case A - the children are not aggregate literals
// holds if `al1` and `al2` both hold for .getElement[sharedIndex]
not al1_expr instanceof ArrayAggregateLiteral and
out_al1_expr = al1_expr and
out_al2_expr = al2_expr
or
// case B - `al1` and `al2` both have an aggregate literal child at the same index, so recurse
hasMultipleInitializerExprsForSameIndexInternal(al1_expr, al2_expr, out_al1_expr, out_al2_expr)
)
root = e1 and root = e2
or
exists(ArrayAggregateLiteral parent1, ArrayAggregateLiteral parent2, int shared_index |
hasMultipleInitializerExprsForSameIndexInternal(root, parent1, parent2) and
shared_index = [0 .. parent1.getArraySize() - 1] and
e1 = parent1.getAnElementExpr(shared_index) and
e2 = parent2.getAnElementExpr(shared_index)
)
}

/**
* Holds if `expr1` and `expr2` both initialize the same array element of `root`.
*/
predicate hasMultipleInitializerExprsForSameIndex(ArrayAggregateLiteral root, Expr expr1, Expr expr2) {
hasMultipleInitializerExprsForSameIndexInternal(root, root, expr1, expr2)
hasMultipleInitializerExprsForSameIndexInternal(root, expr1, expr2) and
not root = expr1 and
not root = expr2 and
not expr1 = expr2 and
(
not expr1 instanceof ArrayAggregateLiteral
or
not expr2 instanceof ArrayAggregateLiteral
)
}

/**
Expand Down
2 changes: 2 additions & 0 deletions change_notes/2024-10-24-compatible-types.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `DCL40-C` - `IncompatibleFunctionDeclarations.ql`:
- Reduce false positives by identifying compatible integer arithmetic types (e.g. "signed int" and "int").
4 changes: 4 additions & 0 deletions cpp/common/src/codingstandards/cpp/Compatible.qll
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import cpp

pragma[noinline]
pragma[nomagic]
predicate typesCompatible(Type t1, Type t2) {
t1 = t2
or
Expand All @@ -8,6 +10,7 @@ predicate typesCompatible(Type t1, Type t2) {
}

predicate parameterTypesIncompatible(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
f1.getDeclaration() = f2.getDeclaration() and
exists(ParameterDeclarationEntry p1, ParameterDeclarationEntry p2, int i |
p1 = f1.getParameterDeclarationEntry(i) and
p2 = f2.getParameterDeclarationEntry(i)
Expand All @@ -17,6 +20,7 @@ predicate parameterTypesIncompatible(FunctionDeclarationEntry f1, FunctionDeclar
}

predicate parameterNamesIncompatible(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
f1.getDeclaration() = f2.getDeclaration() and
exists(ParameterDeclarationEntry p1, ParameterDeclarationEntry p2, int i |
p1 = f1.getParameterDeclarationEntry(i) and
p2 = f2.getParameterDeclarationEntry(i)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ abstract class NotDistinctIdentifierSharedQuery extends Query { }

Query getQuery() { result instanceof NotDistinctIdentifierSharedQuery }

bindingset[d, d2]
pragma[inline_late]
private predicate after(ExternalIdentifiers d, ExternalIdentifiers d2) {
exists(int dStartLine, int d2StartLine |
d.getLocation().hasLocationInfo(_, dStartLine, _, _, _) and
d2.getLocation().hasLocationInfo(_, d2StartLine, _, _, _) and
dStartLine >= d2StartLine
)
}

query predicate problems(
ExternalIdentifiers d, string message, ExternalIdentifiers d2, string nameplaceholder
) {
Expand All @@ -20,10 +30,10 @@ query predicate problems(
d.getName().length() >= 31 and
d2.getName().length() >= 31 and
not d = d2 and
d.getLocation().getStartLine() >= d2.getLocation().getStartLine() and
d.getSignificantName() = d2.getSignificantName() and
not d.getName() = d2.getName() and
nameplaceholder = d2.getName() and
after(d, d2) and
message =
"External identifer " + d.getName() +
" is nondistinct in characters at or over 31 limit, compared to $@"
Expand Down
Loading