Skip to content

Commit 4ad4b19

Browse files
jbjjbj
authored
Merge pull request #189 from geoffw0/wrongtypedef
CPP: Permit more typedefs in WrongTypeFormatArguments.ql
2 parents 09aa04b + d975c09 commit 4ad4b19

File tree

11 files changed

+115
-122
lines changed

11 files changed

+115
-122
lines changed

‎change-notes/1.19/analysis-cpp.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
| **Query** | **Expected impact** | **Change** |
1414
|----------------------------|------------------------|------------------------------------------------------------------|
1515
| Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. |
16-
16+
| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed. |
1717

1818
## Changes to QL libraries
1919

‎cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql‎

Lines changed: 48 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -66,30 +66,15 @@ predicate formatOtherArgType(FormattingFunctionCall ffc, int pos, Type expected,
6666
class ExpectedType extends Type
6767
{
6868
ExpectedType() {
69-
formatArgType(_, _, this, _, _) or
70-
formatOtherArgType(_, _, this, _, _) or
71-
exists(ExpectedType t |
72-
this = t.(PointerType).getBaseType()
69+
exists(Type t |
70+
(
71+
formatArgType(_, _, t, _, _) or
72+
formatOtherArgType(_, _, t, _, _)
73+
) and this = t.getUnspecifiedType()
7374
)
7475
}
7576
}
7677

77-
/**
78-
* Gets an 'interesting' type that can be reached from `t` by removing
79-
* typedefs and specifiers. Note that this does not always mean removing
80-
* all typedefs and specifiers as `Type.getUnspecifiedType()` would, for
81-
* example if the interesting type is itself a typedef.
82-
*/
83-
ExpectedType getAnUnderlyingExpectedType(Type t) {
84-
(
85-
result = t
86-
) or (
87-
result = getAnUnderlyingExpectedType(t.(TypedefType).getBaseType())
88-
) or (
89-
result = getAnUnderlyingExpectedType(t.(SpecifiedType).getBaseType())
90-
)
91-
}
92-
9378
/**
9479
* Holds if it is safe to display a value of type `actual` when `printf`
9580
* expects a value of type `expected`.
@@ -100,59 +85,48 @@ ExpectedType getAnUnderlyingExpectedType(Type t) {
10085
* are converted to `double`.
10186
*/
10287
predicate trivialConversion(ExpectedType expected, Type actual) {
103-
formatArgType(_, _, expected, _, actual) and
104-
105-
exists(Type actualU |
106-
actualU = actual.getUnspecifiedType() and
88+
exists(Type exp, Type act |
89+
formatArgType(_, _, exp, _, act) and
90+
expected = exp.getUnspecifiedType() and
91+
actual = act.getUnspecifiedType()
92+
) and (
10793
(
108-
(
109-
// allow a pointer type to be displayed with `%p`
110-
expected instanceof VoidPointerType and actualU instanceof PointerType
111-
) or (
112-
// allow a function pointer type to be displayed with `%p`
113-
expected instanceof VoidPointerType and actualU instanceof FunctionPointerType and expected.getSize() = actual.getSize()
114-
) or (
115-
// allow an `enum` type to be displayed with `%i`, `%c` etc
116-
expected instanceof IntegralType and actualU instanceof Enum
117-
) or (
118-
// allow any `char *` type to be displayed with `%s`
119-
expected instanceof CharPointerType and actualU instanceof CharPointerType
120-
) or (
121-
// allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
122-
// with `%ws`
123-
expected.(PointerType).getBaseType().hasName("wchar_t") and
124-
exists(Wchar_t t |
125-
actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
126-
)
127-
) or (
128-
// allow an `int` (or anything promoted to `int`) to be displayed with `%c`
129-
expected instanceof CharType and actualU instanceof IntType
130-
) or (
131-
// allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
132-
expected instanceof Wchar_t and actualU instanceof IntType
133-
) or (
134-
expected instanceof UnsignedCharType and actualU instanceof IntType
135-
) or (
136-
// allow the underlying type of a `size_t` (e.g. `unsigned long`) for
137-
// `%zu`, since this is the type of a `sizeof` expression
138-
expected instanceof Size_t and
139-
actual.getUnspecifiedType() = expected.getUnspecifiedType()
140-
) or (
141-
// allow the underlying type of a `ssize_t` (e.g. `long`) for `%zd`
142-
expected instanceof Ssize_t and
143-
actual.getUnspecifiedType() = expected.getUnspecifiedType()
144-
) or (
145-
// allow any integral type of the same size
146-
// (this permits signedness changes)
147-
expected.(IntegralType).getSize() = actualU.(IntegralType).getSize()
148-
) or (
149-
// allow a pointer to any integral type of the same size
150-
// (this permits signedness changes)
151-
expected.(PointerType).getBaseType().(IntegralType).getSize() = actualU.(PointerType).getBaseType().(IntegralType).getSize()
152-
) or (
153-
// allow expected, or a typedef or specified version of expected
154-
expected = getAnUnderlyingExpectedType(actual)
94+
// allow a pointer type to be displayed with `%p`
95+
expected instanceof VoidPointerType and actual instanceof PointerType
96+
) or (
97+
// allow a function pointer type to be displayed with `%p`
98+
expected instanceof VoidPointerType and actual instanceof FunctionPointerType and expected.getSize() = actual.getSize()
99+
) or (
100+
// allow an `enum` type to be displayed with `%i`, `%c` etc
101+
expected instanceof IntegralType and actual instanceof Enum
102+
) or (
103+
// allow any `char *` type to be displayed with `%s`
104+
expected instanceof CharPointerType and actual instanceof CharPointerType
105+
) or (
106+
// allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
107+
// with `%ws`
108+
expected.(PointerType).getBaseType().hasName("wchar_t") and
109+
exists(Wchar_t t |
110+
actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
155111
)
112+
) or (
113+
// allow an `int` (or anything promoted to `int`) to be displayed with `%c`
114+
expected instanceof CharType and actual instanceof IntType
115+
) or (
116+
// allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
117+
expected instanceof Wchar_t and actual instanceof IntType
118+
) or (
119+
expected instanceof UnsignedCharType and actual instanceof IntType
120+
) or (
121+
// allow any integral type of the same size
122+
// (this permits signedness changes)
123+
expected.(IntegralType).getSize() = actual.(IntegralType).getSize()
124+
) or (
125+
// allow a pointer to any integral type of the same size
126+
// (this permits signedness changes)
127+
expected.(PointerType).getBaseType().(IntegralType).getSize() = actual.(PointerType).getBaseType().(IntegralType).getSize()
128+
) or (
129+
expected = actual
156130
)
157131
)
158132
}
@@ -164,16 +138,16 @@ int sizeof_IntType() {
164138
exists(IntType it | result = it.getSize())
165139
}
166140

167-
from FormattingFunctionCall ffc, int n, Expr arg, ExpectedType expected, Type actual
141+
from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual
168142
where (
169143
(
170144
formatArgType(ffc, n, expected, arg, actual) and
171-
not trivialConversion(expected, actual)
145+
not trivialConversion(expected.getUnspecifiedType(), actual.getUnspecifiedType())
172146
)
173147
or
174148
(
175149
formatOtherArgType(ffc, n, expected, arg, actual) and
176-
not actual.getUnderlyingType().(IntegralType).getSize() = sizeof_IntType()
150+
not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
177151
)
178152
)
179153
and not arg.isAffectedByMacro()

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,6 @@
1111
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1212
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1313
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
14-
| printf1.h:68:19:68:21 | sst | This argument should be of type 'size_t' but is of type 'long' |
15-
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
16-
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
17-
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
18-
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
19-
| printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
20-
| printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
21-
| printf1.h:83:23:83:35 | ... - ... | This argument should be of type 'size_t' but is of type 'long' |
2214
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
2315
| real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
2416
| real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,23 +65,23 @@ void g()
6565
printf("%zu", c_st); // ok
6666
printf("%zu", C_ST); // ok
6767
printf("%zu", sizeof(ul)); // ok
68-
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
69-
70-
printf("%zd", ul); // not ok
71-
printf("%zd", st); // not ok
72-
printf("%zd", ST); // not ok
73-
printf("%zd", c_st); // not ok
74-
printf("%zd", C_ST); // not ok
75-
printf("%zd", sizeof(ul)); // not ok
68+
printf("%zu", sst); // not ok [NOT DETECTED]
69+
70+
printf("%zd", ul); // not ok [NOT DETECTED]
71+
printf("%zd", st); // not ok [NOT DETECTED]
72+
printf("%zd", ST); // not ok [NOT DETECTED]
73+
printf("%zd", c_st); // not ok [NOT DETECTED]
74+
printf("%zd", C_ST); // not ok [NOT DETECTED]
75+
printf("%zd", sizeof(ul)); // not ok [NOT DETECTED]
7676
printf("%zd", sst); // ok
7777
{
7878
char *ptr_a, *ptr_b;
7979
char buf[100];
8080

8181
printf("%tu", ptr_a - ptr_b); // ok
8282
printf("%td", ptr_a - ptr_b); // ok
83-
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
84-
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
83+
printf("%zu", ptr_a - ptr_b); // ok (dubious)
84+
printf("%zd", ptr_a - ptr_b); // ok (dubious)
8585
}
8686
}
8787

@@ -92,3 +92,12 @@ void h(int i, struct some_type *j, int k)
9292
// going on.
9393
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
9494
}
95+
96+
typedef long ptrdiff_t;
97+
98+
void fun1(unsigned char* a, unsigned char* b) {
99+
ptrdiff_t pdt;
100+
101+
printf("%td\n", pdt); // GOOD
102+
printf("%td\n", a-b); // GOOD
103+
}

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,6 @@
1111
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1212
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1313
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
14-
| printf1.h:68:19:68:21 | sst | This argument should be of type 'size_t' but is of type 'long' |
15-
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
16-
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
17-
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
18-
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
19-
| printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
20-
| printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
21-
| printf1.h:83:23:83:35 | ... - ... | This argument should be of type 'size_t' but is of type 'long' |
2214
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
2315
| real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
2416
| real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,23 +65,23 @@ void g()
6565
printf("%zu", c_st); // ok
6666
printf("%zu", C_ST); // ok
6767
printf("%zu", sizeof(ul)); // ok
68-
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
69-
70-
printf("%zd", ul); // not ok
71-
printf("%zd", st); // not ok
72-
printf("%zd", ST); // not ok
73-
printf("%zd", c_st); // not ok
74-
printf("%zd", C_ST); // not ok
75-
printf("%zd", sizeof(ul)); // not ok
68+
printf("%zu", sst); // not ok [NOT DETECTED]
69+
70+
printf("%zd", ul); // not ok [NOT DETECTED]
71+
printf("%zd", st); // not ok [NOT DETECTED]
72+
printf("%zd", ST); // not ok [NOT DETECTED]
73+
printf("%zd", c_st); // not ok [NOT DETECTED]
74+
printf("%zd", C_ST); // not ok [NOT DETECTED]
75+
printf("%zd", sizeof(ul)); // not ok [NOT DETECTED]
7676
printf("%zd", sst); // ok
7777
{
7878
char *ptr_a, *ptr_b;
7979
char buf[100];
8080

8181
printf("%tu", ptr_a - ptr_b); // ok
8282
printf("%td", ptr_a - ptr_b); // ok
83-
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
84-
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
83+
printf("%zu", ptr_a - ptr_b); // ok (dubious)
84+
printf("%zd", ptr_a - ptr_b); // ok (dubious)
8585
}
8686
}
8787

@@ -92,3 +92,12 @@ void h(int i, struct some_type *j, int k)
9292
// going on.
9393
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
9494
}
95+
96+
typedef long ptrdiff_t;
97+
98+
void fun1(unsigned char* a, unsigned char* b) {
99+
ptrdiff_t pdt;
100+
101+
printf("%td\n", pdt); // GOOD
102+
printf("%td\n", a-b); // GOOD
103+
}

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1212
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1313
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
14-
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
1514
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
1615
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
1716
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ void g()
6565
printf("%zu", c_st); // ok
6666
printf("%zu", C_ST); // ok
6767
printf("%zu", sizeof(ul)); // ok
68-
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
68+
printf("%zu", sst); // not ok [NOT DETECTED]
6969

70-
printf("%zd", ul); // not ok
70+
printf("%zd", ul); // not ok [NOT DETECTED]
7171
printf("%zd", st); // not ok
7272
printf("%zd", ST); // not ok
7373
printf("%zd", c_st); // not ok
@@ -80,8 +80,8 @@ void g()
8080

8181
printf("%tu", ptr_a - ptr_b); // ok
8282
printf("%td", ptr_a - ptr_b); // ok
83-
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
84-
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
83+
printf("%zu", ptr_a - ptr_b); // ok (dubious)
84+
printf("%zd", ptr_a - ptr_b); // ok (dubious) [FALSE POSITIVE]
8585
}
8686
}
8787

@@ -92,3 +92,12 @@ void h(int i, struct some_type *j, int k)
9292
// going on.
9393
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
9494
}
95+
96+
typedef long long ptrdiff_t;
97+
98+
void fun1(unsigned char* a, unsigned char* b) {
99+
ptrdiff_t pdt;
100+
101+
printf("%td\n", pdt); // GOOD
102+
printf("%td\n", a-b); // GOOD
103+
}

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/real_world.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ void someFunction()
4343
WCHAR filename[MAX_LONGPATH];
4444
int linenum;
4545

46-
msg_out("Source file: %S @ %d\n", filename, linenum); // GOOD
46+
msg_out("Source file: %S @ %d\n", filename, linenum); // GOOD [FALSE POSITIVE]
4747
}
4848

4949
// --------------------------------------------------------------

‎cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1212
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1313
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
14-
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
1514
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
1615
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
1716
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
1817
| printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
1918
| printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
2019
| printf1.h:84:23:84:35 | ... - ... | This argument should be of type 'ssize_t' but is of type 'long long' |
20+
| real_world.h:46:36:46:43 | filename | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
2121
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
2222
| real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
2323
| real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |

0 commit comments

Comments
 (0)