Skip to content

Commit 097e687

Browse files
author
Nikita Kraiouchkine
committed
Fix EXP37-C false-negative, refactor, and update test
1 parent 39af2b3 commit 097e687

File tree

4 files changed

+122
-94
lines changed

4 files changed

+122
-94
lines changed

‎c/cert/src/rules/EXP37-C/DoNotCallFunctionsWithIncompatibleArguments.ql

Lines changed: 6 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -13,102 +13,15 @@
1313

1414
import cpp
1515
import codingstandards.c.cert
16+
import codingstandards.cpp.MistypedFunctionArguments
1617

17-
pragma[inline]
18-
private predicate arithTypesMatch(Type arg, Type parm) {
19-
arg = parm
20-
or
21-
arg.getSize() = parm.getSize() and
22-
(
23-
arg instanceof IntegralOrEnumType and
24-
parm instanceof IntegralOrEnumType
25-
or
26-
arg instanceof FloatingPointType and
27-
parm instanceof FloatingPointType
28-
)
29-
}
30-
31-
pragma[inline]
32-
private predicate nestedPointerArgTypeMayBeUsed(Type arg, Type parm) {
33-
// arithmetic types
34-
arithTypesMatch(arg, parm)
35-
or
36-
// conversion to/from pointers to void is allowed
37-
arg instanceof VoidType
38-
or
39-
parm instanceof VoidType
40-
}
41-
42-
pragma[inline]
43-
private predicate pointerArgTypeMayBeUsed(Type arg, Type parm) {
44-
nestedPointerArgTypeMayBeUsed(arg, parm)
45-
or
46-
// nested pointers
47-
nestedPointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
48-
parm.(PointerType).getBaseType().getUnspecifiedType())
49-
or
50-
nestedPointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
51-
parm.(PointerType).getBaseType().getUnspecifiedType())
52-
}
53-
54-
pragma[inline]
55-
private predicate argTypeMayBeUsed(Type arg, Type parm) {
56-
// arithmetic types
57-
arithTypesMatch(arg, parm)
58-
or
59-
// pointers to compatible types
60-
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
61-
parm.(PointerType).getBaseType().getUnspecifiedType())
62-
or
63-
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
64-
parm.(PointerType).getBaseType().getUnspecifiedType())
65-
or
66-
// C11 arrays
67-
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
68-
parm.(ArrayType).getBaseType().getUnspecifiedType())
69-
or
70-
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
71-
parm.(ArrayType).getBaseType().getUnspecifiedType())
72-
}
73-
74-
// This predicate holds whenever expression `arg` may be used to initialize
75-
// function parameter `parm` without need for run-time conversion.
76-
pragma[inline]
77-
private predicate argMayBeUsed(Expr arg, Parameter parm) {
78-
argTypeMayBeUsed(arg.getFullyConverted().getUnspecifiedType(), parm.getUnspecifiedType())
79-
}
80-
81-
// True if function was ()-declared, but not (void)-declared or K&R-defined
82-
private predicate hasZeroParamDecl(Function f) {
83-
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
84-
not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
85-
)
86-
}
87-
88-
// True if this file (or header) was compiled as a C file
89-
private predicate isCompiledAsC(File f) {
90-
f.compiledAsC()
91-
or
92-
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
93-
}
94-
95-
predicate mistypedFunctionArguments(FunctionCall fc, Function f, Parameter p) {
96-
f = fc.getTarget() and
97-
p = f.getAParameter() and
98-
hasZeroParamDecl(f) and
99-
isCompiledAsC(f.getFile()) and
100-
not f.isVarargs() and
101-
not f instanceof BuiltInFunction and
102-
p.getIndex() < fc.getNumberOfArguments() and
103-
// Parameter p and its corresponding call argument must have mismatched types
104-
not argMayBeUsed(fc.getArgument(p.getIndex()), p)
105-
}
106-
107-
// The implementation of this query was taken from the following standard library query:
108-
// codeql/cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.ql
10918
from FunctionCall fc, Function f, Parameter p
11019
where
11120
not isExcluded(fc, ExpressionsPackage::doNotCallFunctionsWithIncompatibleArgumentsQuery()) and
112-
mistypedFunctionArguments(fc, f, p)
21+
(
22+
mistypedFunctionArguments(fc, f, p)
23+
or
24+
complexArgumentPassedToRealParameter(fc, f, p)
25+
)
11326
select fc, "Argument $@ in call to $@ is incompatible with parameter $@.",
11427
fc.getArgument(p.getIndex()) as arg, arg.toString(), f, f.toString(), p, p.getTypedName()

‎c/cert/test/rules/EXP37-C/DoNotCallFunctionsWithIncompatibleArguments.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1+
| test.c:83:12:83:16 | call to atan2 | Argument $@ in call to $@ is incompatible with parameter $@. | test.c:83:18:83:18 | c | c | file:///Users/kraiouchkine/codeql-coding-standards/c/common/test/includes/standard-library/math.h:155:13:155:17 | atan2 | atan2 | file:///Users/kraiouchkine/codeql-coding-standards/c/common/test/includes/standard-library/math.h:155:19:155:24 | (unnamed parameter 0) | double (unnamed parameter 0) |
12
| test.c:93:3:93:12 | call to test_func1 | Argument $@ in call to $@ is incompatible with parameter $@. | test.c:93:14:93:15 | p1 | p1 | test2.c:1:6:1:15 | test_func1 | test_func1 | test2.c:1:23:1:24 | p1 | short p1 |
23
| test.c:94:3:94:12 | call to test_func1 | Argument $@ in call to $@ is incompatible with parameter $@. | test.c:94:14:94:15 | p2 | p2 | test2.c:1:6:1:15 | test_func1 | test_func1 | test2.c:1:23:1:24 | p1 | short p1 |

‎c/cert/test/rules/EXP37-C/test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,6 @@ void test_incompatible_arguments(int p1, short p2) {
9494
test_func1(p2); // NON_COMPLIANT
9595
((int (*)(short))test_func1)(p2); // COMPLIANT
9696
test_func2(0); // COMPLIANT
97-
test_func3(0); // NON_COMPLIANT
97+
test_func3(0); // NON_COMPLIANT[FALSE_NEGATIVE]
9898
test_func3(0, 1); // NON_COMPLIANT[FALSE_NEGATIVE]
9999
}

‎cpp/common/src/codingstandards/cpp/MistypedFunctionArguments.qll

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/**
2+
* Provides the implementation of the MistypedFunctionArguments query. The
3+
* query is implemented as a library, so that we can avoid producing
4+
* duplicate results in other similar queries.
5+
*
6+
* The implementation of this library is based on the following file from the standard library:
7+
* codeql/cpp/ql/src/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.qll
8+
*/
9+
10+
import cpp
11+
12+
pragma[inline]
13+
private predicate arithTypesMatch(Type arg, Type parm) {
14+
arg = parm
15+
or
16+
arg.getSize() = parm.getSize() and
17+
(
18+
arg instanceof IntegralOrEnumType and
19+
parm instanceof IntegralOrEnumType
20+
or
21+
arg instanceof FloatingPointType and
22+
parm instanceof FloatingPointType
23+
)
24+
}
25+
26+
pragma[inline]
27+
private predicate nestedPointerArgTypeMayBeUsed(Type arg, Type parm) {
28+
// arithmetic types
29+
arithTypesMatch(arg, parm)
30+
or
31+
// conversion to/from pointers to void is allowed
32+
arg instanceof VoidType
33+
or
34+
parm instanceof VoidType
35+
}
36+
37+
pragma[inline]
38+
private predicate pointerArgTypeMayBeUsed(Type arg, Type parm) {
39+
nestedPointerArgTypeMayBeUsed(arg, parm)
40+
or
41+
// nested pointers
42+
nestedPointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
43+
parm.(PointerType).getBaseType().getUnspecifiedType())
44+
or
45+
nestedPointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
46+
parm.(PointerType).getBaseType().getUnspecifiedType())
47+
}
48+
49+
pragma[inline]
50+
private predicate argTypeMayBeUsed(Type arg, Type parm) {
51+
// arithmetic types
52+
arithTypesMatch(arg, parm)
53+
or
54+
// pointers to compatible types
55+
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
56+
parm.(PointerType).getBaseType().getUnspecifiedType())
57+
or
58+
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
59+
parm.(PointerType).getBaseType().getUnspecifiedType())
60+
or
61+
// C11 arrays
62+
pointerArgTypeMayBeUsed(arg.(PointerType).getBaseType().getUnspecifiedType(),
63+
parm.(ArrayType).getBaseType().getUnspecifiedType())
64+
or
65+
pointerArgTypeMayBeUsed(arg.(ArrayType).getBaseType().getUnspecifiedType(),
66+
parm.(ArrayType).getBaseType().getUnspecifiedType())
67+
}
68+
69+
// This predicate holds whenever expression `arg` may be used to initialize
70+
// function parameter `parm` without need for run-time conversion.
71+
pragma[inline]
72+
private predicate argMayBeUsed(Expr arg, Parameter parm) {
73+
argTypeMayBeUsed(arg.getFullyConverted().getUnspecifiedType(), parm.getUnspecifiedType())
74+
}
75+
76+
// True if function was ()-declared, but not (void)-declared or K&R-defined
77+
private predicate hasZeroParamDecl(Function f) {
78+
exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
79+
not fde.hasVoidParamList() and fde.getNumberOfParameters() = 0 and not fde.isDefinition()
80+
)
81+
}
82+
83+
// True if this file (or header) was compiled as a C file
84+
private predicate isCompiledAsC(File f) {
85+
f.compiledAsC()
86+
or
87+
exists(File src | isCompiledAsC(src) | src.getAnIncludedFile() = f)
88+
}
89+
90+
private predicate isTypeInComplexDomain(FloatingPointType type) {
91+
type.getUnderlyingType().(FloatingPointType).getDomain() instanceof ComplexDomain
92+
}
93+
94+
predicate mistypedFunctionArguments(FunctionCall fc, Function f, Parameter p) {
95+
f = fc.getTarget() and
96+
p = f.getAParameter() and
97+
hasZeroParamDecl(f) and
98+
isCompiledAsC(f.getFile()) and
99+
not f.isVarargs() and
100+
not f instanceof BuiltInFunction and
101+
p.getIndex() < fc.getNumberOfArguments() and
102+
// Parameter p and its corresponding call argument must have mismatched types
103+
not argMayBeUsed(fc.getArgument(p.getIndex()), p)
104+
}
105+
106+
predicate complexArgumentPassedToRealParameter(FunctionCall fc, Function f, Parameter p) {
107+
f = fc.getTarget() and
108+
p = f.getAParameter() and
109+
// Some implementations implicitly convert complex floating point values by
110+
// extracting the real part of the complex number (in-place or via a creal() call).
111+
// This predicate includes those results unless the value is explicitly converted.
112+
not isTypeInComplexDomain(p.getType()) and
113+
isTypeInComplexDomain(fc.getArgument(p.getIndex()).getExplicitlyConverted().getType())
114+
}

0 commit comments

Comments
 (0)