diff --git a/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql b/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql index 4660e69b68..20b6e5e59e 100644 --- a/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql +++ b/c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql @@ -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() diff --git a/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql b/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql index 821b79c8e4..2fc334ba50 100644 --- a/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql +++ b/c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql @@ -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() diff --git a/c/misra/src/rules/RULE-9-4/RepeatedInitializationOfAggregateObjectElement.ql b/c/misra/src/rules/RULE-9-4/RepeatedInitializationOfAggregateObjectElement.ql index 4f72d6720b..dfe3fd8fff 100644 --- a/c/misra/src/rules/RULE-9-4/RepeatedInitializationOfAggregateObjectElement.ql +++ b/c/misra/src/rules/RULE-9-4/RepeatedInitializationOfAggregateObjectElement.ql @@ -62,25 +62,15 @@ 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) ) } @@ -88,7 +78,15 @@ predicate hasMultipleInitializerExprsForSameIndexInternal( * 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 + ) } /** diff --git a/change_notes/2024-10-24-compatible-types.md b/change_notes/2024-10-24-compatible-types.md new file mode 100644 index 0000000000..05afbd64d9 --- /dev/null +++ b/change_notes/2024-10-24-compatible-types.md @@ -0,0 +1,2 @@ + - `DCL40-C` - `IncompatibleFunctionDeclarations.ql`: + - Reduce false positives by identifying compatible integer arithmetic types (e.g. "signed int" and "int"). \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Compatible.qll b/cpp/common/src/codingstandards/cpp/Compatible.qll index 12a53965fe..0f6e2108ff 100644 --- a/cpp/common/src/codingstandards/cpp/Compatible.qll +++ b/cpp/common/src/codingstandards/cpp/Compatible.qll @@ -1,5 +1,7 @@ import cpp +pragma[noinline] +pragma[nomagic] predicate typesCompatible(Type t1, Type t2) { t1 = t2 or @@ -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) @@ -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) diff --git a/cpp/common/src/codingstandards/cpp/rules/notdistinctidentifier/NotDistinctIdentifier.qll b/cpp/common/src/codingstandards/cpp/rules/notdistinctidentifier/NotDistinctIdentifier.qll index 093b804e0f..102c53428b 100644 --- a/cpp/common/src/codingstandards/cpp/rules/notdistinctidentifier/NotDistinctIdentifier.qll +++ b/cpp/common/src/codingstandards/cpp/rules/notdistinctidentifier/NotDistinctIdentifier.qll @@ -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 ) { @@ -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 $@"