Skip to content

C++: Always recognize pointers as iterators #7159

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.

Sign up for GitHub

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

Merged
geoffw0 merged 3 commits into from
Nov 18, 2021
Merged

C++: Always recognize pointers as iterators #7159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6dc6a78
C++: Add a 'IteratorByPointer' class so pointers are always iterators.
MathiasVP Nov 17, 2021
36585a7
C++: Accept test changes.
MathiasVP Nov 17, 2021
c998370
C++: Accept more test changes.
MathiasVP Nov 17, 2021
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
10 changes: 10 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/models/implementations/Iterator.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ private class IteratorByTraits extends Iterator {
IteratorByTraits() { exists(IteratorTraits it | it.getIteratorType() = this) }
}

/**
* The C++ standard includes an `std::iterator_traits` specialization for pointer types. When
* this specialization is included in the database, a pointer type `T*` will be an instance
* of the `IteratorByTraits` class. However, if the `T*` specialization is not in the database,
* we need to explicitly include them with this class.
*/
private class IteratorByPointer extends Iterator instanceof PointerType {
IteratorByPointer() { not this instanceof IteratorByTraits }
}

@geoffw0 geoffw0 Nov 17, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM.

Do you have any idea what the downside would be if we made something an Iterator when it shouldn't be? e.g. if we made every IntegralType an Iterator, would we get some kind of incorrect flow through integer parameters?

@MathiasVP MathiasVP Nov 17, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think such a change would mean that we get flow in cases like:

void f(int n) {
  n = source();
}

void test() {
  int n = 0;
  f(n);
  sink(n);
}

I'm not sure if we've set up everything such that this was the only thing necessary to get flow in such cases, but it would certainly be a likely outcome of such a change.


/**
* A type which has the typedefs expected for an iterator.
*/
Expand Down
3 changes: 3 additions & 0 deletions cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
| example.c:15:37:15:37 | b | example.c:15:37:15:37 | b |
| example.c:15:37:15:37 | b | example.c:19:6:19:6 | b |
| example.c:15:44:15:46 | pos | example.c:24:24:24:26 | pos |
| example.c:15:44:15:46 | pos | example.c:28:23:28:25 | pos |
Expand Down Expand Up @@ -70,8 +71,10 @@
| test.cpp:391:11:391:13 | tmp | test.cpp:391:10:391:13 | & ... |
| test.cpp:391:17:391:23 | source1 | test.cpp:391:10:391:13 | ref arg & ... |
| test.cpp:391:17:391:23 | source1 | test.cpp:391:16:391:23 | & ... |
| test.cpp:480:67:480:67 | s | test.cpp:480:67:480:67 | s |
| test.cpp:480:67:480:67 | s | test.cpp:481:21:481:21 | s |
| test.cpp:480:67:480:67 | s | test.cpp:482:20:482:20 | s |
| test.cpp:481:21:481:21 | s [post update] | test.cpp:480:67:480:67 | s |
| test.cpp:481:21:481:21 | s [post update] | test.cpp:482:20:482:20 | s |
| test.cpp:481:24:481:30 | ref arg content | test.cpp:482:23:482:29 | content |
| test.cpp:482:23:482:29 | content | test.cpp:483:9:483:17 | p_content |
212 changes: 212 additions & 0 deletions cpp/ql/test/library-tests/dataflow/fields/path-flow.expected
View file
Open in desktop

Large diffs are not rendered by default.

76 changes: 76 additions & 0 deletions cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion cpp/ql/test/library-tests/types/__wchar_t/wchar_t.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
| file://:0:0:0:0 | __wchar_t * | PointerType | Wchar_t, WideCharType |
| file://:0:0:0:0 | __wchar_t * | IteratorByPointer, PointerType | Wchar_t, WideCharType |
| file://:0:0:0:0 | const __wchar_t | SpecifiedType | Wchar_t, WideCharType |
| file://:0:0:0:0 | wchar_t | Wchar_t, WideCharType | |
2 changes: 1 addition & 1 deletion cpp/ql/test/library-tests/types/wchar_t_typedef/wchar_t.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
| file://:0:0:0:0 | wchar_t | Wchar_t, WideCharType | |
| file://:0:0:0:0 | wchar_t * | PointerType | CTypedefType, Wchar_t |
| file://:0:0:0:0 | wchar_t * | IteratorByPointer, PointerType | CTypedefType, Wchar_t |
| ms.c:2:24:2:30 | wchar_t | CTypedefType, Wchar_t | |
8 changes: 4 additions & 4 deletions cpp/ql/test/library-tests/variables/variables/types.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
| char8_t | Char8Type | | | | |
| char16_t | Char16Type | | | | |
| char32_t | Char32Type | | | | |
| char * | CharPointerType | | char | | |
| char * | CharPointerType, IteratorByPointer | | char | | |
| char *[3] | ArrayType | char * | char * | | |
| char *[32] | ArrayType | char * | char * | | |
| char *[] | ArrayType | char * | char * | | |
Expand All @@ -48,7 +48,7 @@
| const address | SpecifiedType | | address | | |
| const address & | LValueReferenceType | | const address | | |
| const char | SpecifiedType | | char | | |
| const char * | PointerType | | const char | | |
| const char * | IteratorByPointer, PointerType | | const char | | |
| const char *[3] | ArrayType | const char * | const char * | | |
| const char *[] | ArrayType | const char * | const char * | | |
| const char[5] | ArrayType | const char | const char | | |
Expand All @@ -65,7 +65,7 @@
| float | FloatType | | | | |
| float[3] | ArrayType | float | float | | |
| int | IntType, MicrosoftInt32Type | | | | |
| int * | IntPointerType | | int | | |
| int * | IntPointerType, IteratorByPointer | | int | | |
| int[4] | ArrayType | int | int | | |
| int[8] | ArrayType | int | int | | |
| int[10] | ArrayType | int | int | | |
Expand All @@ -90,5 +90,5 @@
| unsigned long long | LongLongType | | | | unsigned integral |
| unsigned short | ShortType | | | | unsigned integral |
| void | VoidType | | | | |
| void * | VoidPointerType | | void | | |
| void * | IteratorByPointer, VoidPointerType | | void | | |
| wchar_t | Wchar_t, WideCharType | | | | |
90 changes: 90 additions & 0 deletions ...sts/Likely Bugs/Conversion/CastArrayPointerArithmetic/CastArrayPointerArithmetic.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,137 @@
edges
| test.cpp:26:29:26:29 | b | test.cpp:26:29:26:29 | b |
| test.cpp:26:29:26:29 | b | test.cpp:27:2:27:2 | b |
| test.cpp:30:34:30:34 | b | test.cpp:31:2:31:2 | b |
| test.cpp:34:31:34:31 | b | test.cpp:34:31:34:31 | b |
| test.cpp:34:31:34:31 | b | test.cpp:35:2:35:2 | b |
| test.cpp:38:35:38:35 | d | test.cpp:38:35:38:35 | d |
| test.cpp:38:35:38:35 | d | test.cpp:39:2:39:2 | d |
| test.cpp:42:40:42:40 | d | test.cpp:43:2:43:2 | d |
| test.cpp:46:37:46:37 | d | test.cpp:46:37:46:37 | d |
| test.cpp:46:37:46:37 | d | test.cpp:47:2:47:2 | d |
| test.cpp:50:31:50:31 | b | test.cpp:51:11:51:11 | b |
| test.cpp:57:19:57:19 | d | test.cpp:26:29:26:29 | b |
| test.cpp:57:19:57:19 | d | test.cpp:57:19:57:19 | ref arg d |
| test.cpp:57:19:57:19 | ref arg d | test.cpp:58:25:58:25 | d |
| test.cpp:57:19:57:19 | ref arg d | test.cpp:59:21:59:21 | d |
| test.cpp:57:19:57:19 | ref arg d | test.cpp:61:22:61:22 | d |
| test.cpp:57:19:57:19 | ref arg d | test.cpp:62:28:62:28 | d |
| test.cpp:57:19:57:19 | ref arg d | test.cpp:63:24:63:24 | d |
| test.cpp:57:19:57:19 | ref arg d | test.cpp:95:21:95:21 | d |
| test.cpp:58:25:58:25 | d | test.cpp:30:34:30:34 | b |
| test.cpp:59:21:59:21 | d | test.cpp:34:31:34:31 | b |
| test.cpp:59:21:59:21 | d | test.cpp:59:21:59:21 | ref arg d |
| test.cpp:59:21:59:21 | ref arg d | test.cpp:61:22:61:22 | d |
| test.cpp:59:21:59:21 | ref arg d | test.cpp:62:28:62:28 | d |
| test.cpp:59:21:59:21 | ref arg d | test.cpp:63:24:63:24 | d |
| test.cpp:59:21:59:21 | ref arg d | test.cpp:95:21:95:21 | d |
| test.cpp:61:22:61:22 | d | test.cpp:38:35:38:35 | d |
| test.cpp:61:22:61:22 | d | test.cpp:61:22:61:22 | ref arg d |
| test.cpp:61:22:61:22 | ref arg d | test.cpp:62:28:62:28 | d |
| test.cpp:61:22:61:22 | ref arg d | test.cpp:63:24:63:24 | d |
| test.cpp:61:22:61:22 | ref arg d | test.cpp:95:21:95:21 | d |
| test.cpp:62:28:62:28 | d | test.cpp:42:40:42:40 | d |
| test.cpp:63:24:63:24 | d | test.cpp:46:37:46:37 | d |
| test.cpp:63:24:63:24 | d | test.cpp:63:24:63:24 | ref arg d |
| test.cpp:63:24:63:24 | ref arg d | test.cpp:95:21:95:21 | d |
| test.cpp:74:19:74:21 | dss | test.cpp:26:29:26:29 | b |
| test.cpp:74:19:74:21 | dss | test.cpp:74:19:74:21 | ref arg dss |
| test.cpp:74:19:74:21 | ref arg dss | test.cpp:75:25:75:27 | dss |
| test.cpp:74:19:74:21 | ref arg dss | test.cpp:76:21:76:23 | dss |
| test.cpp:74:19:74:21 | ref arg dss | test.cpp:96:21:96:23 | dss |
| test.cpp:75:25:75:27 | dss | test.cpp:30:34:30:34 | b |
| test.cpp:76:21:76:23 | dss | test.cpp:34:31:34:31 | b |
| test.cpp:76:21:76:23 | dss | test.cpp:76:21:76:23 | ref arg dss |
| test.cpp:76:21:76:23 | ref arg dss | test.cpp:96:21:96:23 | dss |
| test.cpp:86:19:86:20 | d2 | test.cpp:26:29:26:29 | b |
| test.cpp:86:19:86:20 | d2 | test.cpp:86:19:86:20 | ref arg d2 |
| test.cpp:86:19:86:20 | ref arg d2 | test.cpp:87:25:87:26 | d2 |
| test.cpp:86:19:86:20 | ref arg d2 | test.cpp:88:21:88:22 | d2 |
| test.cpp:86:19:86:20 | ref arg d2 | test.cpp:90:22:90:23 | d2 |
| test.cpp:86:19:86:20 | ref arg d2 | test.cpp:91:28:91:29 | d2 |
| test.cpp:86:19:86:20 | ref arg d2 | test.cpp:92:24:92:25 | d2 |
| test.cpp:87:25:87:26 | d2 | test.cpp:30:34:30:34 | b |
| test.cpp:88:21:88:22 | d2 | test.cpp:34:31:34:31 | b |
| test.cpp:88:21:88:22 | d2 | test.cpp:88:21:88:22 | ref arg d2 |
| test.cpp:88:21:88:22 | ref arg d2 | test.cpp:90:22:90:23 | d2 |
| test.cpp:88:21:88:22 | ref arg d2 | test.cpp:91:28:91:29 | d2 |
| test.cpp:88:21:88:22 | ref arg d2 | test.cpp:92:24:92:25 | d2 |
| test.cpp:90:22:90:23 | d2 | test.cpp:38:35:38:35 | d |
| test.cpp:90:22:90:23 | d2 | test.cpp:90:22:90:23 | ref arg d2 |
| test.cpp:90:22:90:23 | ref arg d2 | test.cpp:91:28:91:29 | d2 |
| test.cpp:90:22:90:23 | ref arg d2 | test.cpp:92:24:92:25 | d2 |
| test.cpp:91:28:91:29 | d2 | test.cpp:42:40:42:40 | d |
| test.cpp:92:24:92:25 | d2 | test.cpp:46:37:46:37 | d |
| test.cpp:95:21:95:21 | d | test.cpp:50:31:50:31 | b |
| test.cpp:96:21:96:23 | dss | test.cpp:50:31:50:31 | b |
nodes
| test.cpp:26:29:26:29 | b | semmle.label | b |
| test.cpp:26:29:26:29 | b | semmle.label | b |
| test.cpp:27:2:27:2 | b | semmle.label | b |
| test.cpp:30:34:30:34 | b | semmle.label | b |
| test.cpp:31:2:31:2 | b | semmle.label | b |
| test.cpp:34:31:34:31 | b | semmle.label | b |
| test.cpp:34:31:34:31 | b | semmle.label | b |
| test.cpp:35:2:35:2 | b | semmle.label | b |
| test.cpp:38:35:38:35 | d | semmle.label | d |
| test.cpp:38:35:38:35 | d | semmle.label | d |
| test.cpp:39:2:39:2 | d | semmle.label | d |
| test.cpp:42:40:42:40 | d | semmle.label | d |
| test.cpp:43:2:43:2 | d | semmle.label | d |
| test.cpp:46:37:46:37 | d | semmle.label | d |
| test.cpp:46:37:46:37 | d | semmle.label | d |
| test.cpp:47:2:47:2 | d | semmle.label | d |
| test.cpp:50:31:50:31 | b | semmle.label | b |
| test.cpp:51:11:51:11 | b | semmle.label | b |
| test.cpp:57:19:57:19 | d | semmle.label | d |
| test.cpp:57:19:57:19 | ref arg d | semmle.label | ref arg d |
| test.cpp:58:25:58:25 | d | semmle.label | d |
| test.cpp:59:21:59:21 | d | semmle.label | d |
| test.cpp:59:21:59:21 | ref arg d | semmle.label | ref arg d |
| test.cpp:61:22:61:22 | d | semmle.label | d |
| test.cpp:61:22:61:22 | ref arg d | semmle.label | ref arg d |
| test.cpp:62:28:62:28 | d | semmle.label | d |
| test.cpp:63:24:63:24 | d | semmle.label | d |
| test.cpp:63:24:63:24 | ref arg d | semmle.label | ref arg d |
| test.cpp:74:19:74:21 | dss | semmle.label | dss |
| test.cpp:74:19:74:21 | ref arg dss | semmle.label | ref arg dss |
| test.cpp:75:25:75:27 | dss | semmle.label | dss |
| test.cpp:76:21:76:23 | dss | semmle.label | dss |
| test.cpp:76:21:76:23 | ref arg dss | semmle.label | ref arg dss |
| test.cpp:86:19:86:20 | d2 | semmle.label | d2 |
| test.cpp:86:19:86:20 | ref arg d2 | semmle.label | ref arg d2 |
| test.cpp:87:25:87:26 | d2 | semmle.label | d2 |
| test.cpp:88:21:88:22 | d2 | semmle.label | d2 |
| test.cpp:88:21:88:22 | ref arg d2 | semmle.label | ref arg d2 |
| test.cpp:90:22:90:23 | d2 | semmle.label | d2 |
| test.cpp:90:22:90:23 | ref arg d2 | semmle.label | ref arg d2 |
| test.cpp:91:28:91:29 | d2 | semmle.label | d2 |
| test.cpp:92:24:92:25 | d2 | semmle.label | d2 |
| test.cpp:95:21:95:21 | d | semmle.label | d |
| test.cpp:96:21:96:23 | dss | semmle.label | dss |
subpaths
| test.cpp:57:19:57:19 | d | test.cpp:26:29:26:29 | b | test.cpp:26:29:26:29 | b | test.cpp:57:19:57:19 | ref arg d |
| test.cpp:59:21:59:21 | d | test.cpp:34:31:34:31 | b | test.cpp:34:31:34:31 | b | test.cpp:59:21:59:21 | ref arg d |
| test.cpp:61:22:61:22 | d | test.cpp:38:35:38:35 | d | test.cpp:38:35:38:35 | d | test.cpp:61:22:61:22 | ref arg d |
| test.cpp:63:24:63:24 | d | test.cpp:46:37:46:37 | d | test.cpp:46:37:46:37 | d | test.cpp:63:24:63:24 | ref arg d |
| test.cpp:74:19:74:21 | dss | test.cpp:26:29:26:29 | b | test.cpp:26:29:26:29 | b | test.cpp:74:19:74:21 | ref arg dss |
| test.cpp:76:21:76:23 | dss | test.cpp:34:31:34:31 | b | test.cpp:34:31:34:31 | b | test.cpp:76:21:76:23 | ref arg dss |
| test.cpp:86:19:86:20 | d2 | test.cpp:26:29:26:29 | b | test.cpp:26:29:26:29 | b | test.cpp:86:19:86:20 | ref arg d2 |
| test.cpp:88:21:88:22 | d2 | test.cpp:34:31:34:31 | b | test.cpp:34:31:34:31 | b | test.cpp:88:21:88:22 | ref arg d2 |
| test.cpp:90:22:90:23 | d2 | test.cpp:38:35:38:35 | d | test.cpp:38:35:38:35 | d | test.cpp:90:22:90:23 | ref arg d2 |
#select
| test.cpp:27:2:27:2 | b | test.cpp:57:19:57:19 | d | test.cpp:27:2:27:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:57:19:57:19 | d | here |
| test.cpp:27:2:27:2 | b | test.cpp:74:19:74:21 | dss | test.cpp:27:2:27:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:74:19:74:21 | dss | here |
| test.cpp:27:2:27:2 | b | test.cpp:86:19:86:20 | d2 | test.cpp:27:2:27:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:86:19:86:20 | d2 | here |
| test.cpp:31:2:31:2 | b | test.cpp:57:19:57:19 | d | test.cpp:31:2:31:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:57:19:57:19 | d | here |
| test.cpp:31:2:31:2 | b | test.cpp:58:25:58:25 | d | test.cpp:31:2:31:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:58:25:58:25 | d | here |
| test.cpp:31:2:31:2 | b | test.cpp:74:19:74:21 | dss | test.cpp:31:2:31:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:74:19:74:21 | dss | here |
| test.cpp:31:2:31:2 | b | test.cpp:75:25:75:27 | dss | test.cpp:31:2:31:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:75:25:75:27 | dss | here |
| test.cpp:31:2:31:2 | b | test.cpp:86:19:86:20 | d2 | test.cpp:31:2:31:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:86:19:86:20 | d2 | here |
| test.cpp:31:2:31:2 | b | test.cpp:87:25:87:26 | d2 | test.cpp:31:2:31:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:87:25:87:26 | d2 | here |
| test.cpp:35:2:35:2 | b | test.cpp:57:19:57:19 | d | test.cpp:35:2:35:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:57:19:57:19 | d | here |
| test.cpp:35:2:35:2 | b | test.cpp:59:21:59:21 | d | test.cpp:35:2:35:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:59:21:59:21 | d | here |
| test.cpp:35:2:35:2 | b | test.cpp:74:19:74:21 | dss | test.cpp:35:2:35:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:74:19:74:21 | dss | here |
| test.cpp:35:2:35:2 | b | test.cpp:76:21:76:23 | dss | test.cpp:35:2:35:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:76:21:76:23 | dss | here |
| test.cpp:35:2:35:2 | b | test.cpp:86:19:86:20 | d2 | test.cpp:35:2:35:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:86:19:86:20 | d2 | here |
| test.cpp:35:2:35:2 | b | test.cpp:88:21:88:22 | d2 | test.cpp:35:2:35:2 | b | Pointer arithmetic here may be done with the wrong type because of the cast $@. | test.cpp:88:21:88:22 | d2 | here |