From 40e035617749dd45751f006d9431ac8300c9ae1e Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Wed, 22 Jun 2022 07:26:15 +0200 Subject: [PATCH 1/5] C++: Test that we can go from a `DeclarationEntry` to a `Declaration` and back --- .../declarationEntry/roundTrip.expected | 37 +++++++++++++++++++ .../declarationEntry/roundTrip.ql | 6 +++ 2 files changed, 43 insertions(+) create mode 100644 cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected create mode 100644 cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected new file mode 100644 index 00000000000..cd2e30b42a0 --- /dev/null +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected @@ -0,0 +1,37 @@ +| declarationEntry.c:2:6:2:20 | declaration of myFirstFunction | declarationEntry.c:2:6:2:20 | myFirstFunction | yes | +| declarationEntry.c:4:6:4:21 | definition of mySecondFunction | declarationEntry.c:4:6:4:21 | mySecondFunction | yes | +| declarationEntry.c:8:6:8:20 | definition of myThirdFunction | declarationEntry.c:8:6:8:20 | myThirdFunction | yes | +| declarationEntry.c:13:2:13:2 | declaration of myFourthFunction | declarationEntry.c:13:2:13:2 | myFourthFunction | yes | +| declarationEntry.c:13:2:13:2 | declaration of myFourthFunction | declarationEntry.c:17:6:17:21 | myFourthFunction | yes | +| declarationEntry.c:14:2:14:2 | declaration of myFifthFunction | declarationEntry.c:14:2:14:2 | myFifthFunction | yes | +| declarationEntry.c:17:6:17:21 | declaration of myFourthFunction | declarationEntry.c:13:2:13:2 | myFourthFunction | yes | +| declarationEntry.c:17:6:17:21 | declaration of myFourthFunction | declarationEntry.c:17:6:17:21 | myFourthFunction | yes | +| declarationEntry.cpp:3:12:3:21 | declaration of myVariable | declarationEntry.cpp:5:5:5:14 | myVariable | yes | +| declarationEntry.cpp:5:5:5:14 | definition of myVariable | declarationEntry.cpp:5:5:5:14 | myVariable | yes | +| declarationEntry.cpp:9:6:9:15 | declaration of myFunction | declarationEntry.cpp:11:6:11:15 | myFunction | yes | +| declarationEntry.cpp:9:21:9:31 | declaration of myParameter | declarationEntry.cpp:11:21:11:31 | myParameter | yes | +| declarationEntry.cpp:11:6:11:15 | definition of myFunction | declarationEntry.cpp:11:6:11:15 | myFunction | yes | +| declarationEntry.cpp:11:21:11:31 | definition of myParameter | declarationEntry.cpp:11:21:11:31 | myParameter | yes | +| declarationEntry.cpp:18:6:18:11 | declaration of myEnum | declarationEntry.cpp:20:6:20:11 | myEnum | yes | +| declarationEntry.cpp:20:6:20:11 | definition of myEnum | declarationEntry.cpp:20:6:20:11 | myEnum | yes | +| declarationEntry.cpp:27:20:27:20 | definition of T | declarationEntry.cpp:27:20:27:20 | T | yes | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | declarationEntry.cpp:28:7:28:7 | operator= | yes | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | declarationEntry.cpp:28:7:28:7 | operator= | yes | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | declarationEntry.cpp:28:7:28:7 | operator= | yes | +| declarationEntry.cpp:28:7:28:7 | declaration of operator= | declarationEntry.cpp:28:7:28:7 | operator= | yes | +| declarationEntry.cpp:28:7:28:21 | definition of myTemplateClass | declarationEntry.cpp:28:7:28:21 | myTemplateClass | yes | +| declarationEntry.cpp:31:4:31:19 | definition of myMemberVariable | declarationEntry.cpp:31:4:31:19 | myMemberVariable | yes | +| declarationEntry.cpp:31:4:31:19 | definition of myMemberVariable | declarationEntry.cpp:31:4:31:19 | myMemberVariable | yes | +| declarationEntry.cpp:31:4:31:19 | definition of myMemberVariable | declarationEntry.cpp:31:4:31:19 | myMemberVariable | yes | +| declarationEntry.cpp:34:22:34:28 | definition of mtc_int | declarationEntry.cpp:34:22:34:28 | mtc_int | yes | +| declarationEntry.cpp:35:24:35:32 | definition of mtc_short | declarationEntry.cpp:35:24:35:32 | mtc_short | yes | +| file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | +| file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | +| file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | +| file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | +| file://:0:0:0:0 | definition of fp_offset | file://:0:0:0:0 | fp_offset | yes | +| file://:0:0:0:0 | definition of gp_offset | file://:0:0:0:0 | gp_offset | yes | +| file://:0:0:0:0 | definition of overflow_arg_area | file://:0:0:0:0 | overflow_arg_area | yes | +| file://:0:0:0:0 | definition of reg_save_area | file://:0:0:0:0 | reg_save_area | yes | +| macro.c:2:1:2:3 | declaration of foo | macro.c:2:1:2:3 | foo | yes | +| macro.c:4:5:4:8 | definition of main | macro.c:4:5:4:8 | main | yes | diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql new file mode 100644 index 00000000000..57c87fc053d --- /dev/null +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql @@ -0,0 +1,6 @@ +import cpp + +from DeclarationEntry de, Declaration d, string canRoundTrip +where d = de.getDeclaration() and + if d.getADeclarationEntry() = de then canRoundTrip = "yes" else canRoundTrip = "no" +select de, d, canRoundTrip From a8833a0c70980cf5f2b8ab23e8a8d95af1e1b53e Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Wed, 22 Jun 2022 07:35:12 +0200 Subject: [PATCH 2/5] C++: Test showing going from a forward class declaration to a class but not back --- .../declarationEntry/declarationEntry.cpp | 8 ++++++++ .../declarationEntry/declarationEntry.expected | 6 ++++++ .../declarationEntry/declarationEntry/fde.expected | 2 ++ .../declarationEntry/forwardDeclaration.cpp | 3 +++ .../declarationEntry/declarationEntry/roundTrip.expected | 8 ++++++++ 5 files changed, 27 insertions(+) create mode 100644 cpp/ql/test/library-tests/declarationEntry/declarationEntry/forwardDeclaration.cpp diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.cpp b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.cpp index 3bbfc05ec65..45732eb6ca8 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.cpp +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.cpp @@ -33,3 +33,11 @@ public: myTemplateClass mtc_int; myTemplateClass mtc_short; + +// Class (UserType) + +class myClass +{ +public: + int myMemberVariable; +}; diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected index 76f564d1f7b..0645e923643 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected @@ -27,5 +27,11 @@ | declarationEntry.cpp:31:4:31:19 | myMemberVariable | declarationEntry.cpp:31:4:31:19 | definition of myMemberVariable | 1 | 1 | | declarationEntry.cpp:34:22:34:28 | mtc_int | declarationEntry.cpp:34:22:34:28 | definition of mtc_int | 1 | 1 | | declarationEntry.cpp:35:24:35:32 | mtc_short | declarationEntry.cpp:35:24:35:32 | definition of mtc_short | 1 | 1 | +| declarationEntry.cpp:39:7:39:7 | operator= | declarationEntry.cpp:39:7:39:7 | declaration of operator= | 1 | 1 | +| declarationEntry.cpp:39:7:39:7 | operator= | declarationEntry.cpp:39:7:39:7 | declaration of operator= | 1 | 1 | +| declarationEntry.cpp:39:7:39:13 | myClass | declarationEntry.cpp:39:7:39:13 | definition of myClass | 1 | 1 | +| declarationEntry.cpp:39:7:39:13 | myClass | forwardDeclaration.cpp:1:7:1:13 | declaration of myClass | 0 | 1 | +| declarationEntry.cpp:42:6:42:21 | myMemberVariable | declarationEntry.cpp:42:6:42:21 | definition of myMemberVariable | 1 | 1 | +| forwardDeclaration.cpp:3:10:3:19 | myClassPtr | forwardDeclaration.cpp:3:10:3:19 | definition of myClassPtr | 1 | 1 | | macro.c:2:1:2:3 | foo | macro.c:2:1:2:3 | declaration of foo | 1 | 1 | | macro.c:4:5:4:8 | main | macro.c:4:5:4:8 | definition of main | 1 | 1 | diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected index 9a544200cae..71c81c7ac82 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/fde.expected @@ -10,5 +10,7 @@ | declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | | declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | | declarationEntry.cpp:28:7:28:7 | declaration of operator= | | 0 | | +| declarationEntry.cpp:39:7:39:7 | declaration of operator= | | 0 | | +| declarationEntry.cpp:39:7:39:7 | declaration of operator= | | 0 | | | macro.c:2:1:2:3 | declaration of foo | | 2 | c_linkage, static | | macro.c:4:5:4:8 | definition of main | | 1 | c_linkage | diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/forwardDeclaration.cpp b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/forwardDeclaration.cpp new file mode 100644 index 00000000000..1e5e0c15ce0 --- /dev/null +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/forwardDeclaration.cpp @@ -0,0 +1,3 @@ +class myClass; + +myClass *myClassPtr; diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected index cd2e30b42a0..5066ff021bf 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected @@ -25,6 +25,12 @@ | declarationEntry.cpp:31:4:31:19 | definition of myMemberVariable | declarationEntry.cpp:31:4:31:19 | myMemberVariable | yes | | declarationEntry.cpp:34:22:34:28 | definition of mtc_int | declarationEntry.cpp:34:22:34:28 | mtc_int | yes | | declarationEntry.cpp:35:24:35:32 | definition of mtc_short | declarationEntry.cpp:35:24:35:32 | mtc_short | yes | +| declarationEntry.cpp:39:7:39:7 | declaration of operator= | declarationEntry.cpp:39:7:39:7 | operator= | yes | +| declarationEntry.cpp:39:7:39:7 | declaration of operator= | declarationEntry.cpp:39:7:39:7 | operator= | yes | +| declarationEntry.cpp:39:7:39:13 | definition of myClass | declarationEntry.cpp:39:7:39:13 | myClass | yes | +| declarationEntry.cpp:42:6:42:21 | definition of myMemberVariable | declarationEntry.cpp:42:6:42:21 | myMemberVariable | yes | +| file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | +| file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | | file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | | file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | | file://:0:0:0:0 | declaration of 1st parameter | file://:0:0:0:0 | (unnamed parameter 0) | yes | @@ -33,5 +39,7 @@ | file://:0:0:0:0 | definition of gp_offset | file://:0:0:0:0 | gp_offset | yes | | file://:0:0:0:0 | definition of overflow_arg_area | file://:0:0:0:0 | overflow_arg_area | yes | | file://:0:0:0:0 | definition of reg_save_area | file://:0:0:0:0 | reg_save_area | yes | +| forwardDeclaration.cpp:1:7:1:13 | declaration of myClass | declarationEntry.cpp:39:7:39:13 | myClass | no | +| forwardDeclaration.cpp:3:10:3:19 | definition of myClassPtr | forwardDeclaration.cpp:3:10:3:19 | myClassPtr | yes | | macro.c:2:1:2:3 | declaration of foo | macro.c:2:1:2:3 | foo | yes | | macro.c:4:5:4:8 | definition of main | macro.c:4:5:4:8 | main | yes | From 880c785efe4cf940668cfe6fd86510a412b8de1c Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Wed, 22 Jun 2022 07:43:50 +0200 Subject: [PATCH 3/5] C++: Ensure we can round trip between (forward) class declarations This was already possible when the forward class declaration and the class definition occurred in the same scope. However, there is a common C++ usage pattern in which this is not the case (when only a pointer to the class is needed). In this latter scenario we could not round trip between the (forward) `DeclarationEntry` and the `Declaration`. Effectively this changes the code to: ``` if exists(TypeDeclarationEntry e | e.getType() = this) then result.getType() = this else ... ``` We use `type_decls` instead to stay close to the original code. --- cpp/ql/lib/semmle/code/cpp/UserType.qll | 4 ++-- .../declarationEntry/declarationEntry.expected | 2 +- .../declarationEntry/declarationEntry/roundTrip.expected | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/UserType.qll b/cpp/ql/lib/semmle/code/cpp/UserType.qll index 13697722190..9d85f555ec1 100644 --- a/cpp/ql/lib/semmle/code/cpp/UserType.qll +++ b/cpp/ql/lib/semmle/code/cpp/UserType.qll @@ -48,8 +48,8 @@ class UserType extends Type, Declaration, NameQualifyingElement, AccessHolder, @ } override TypeDeclarationEntry getADeclarationEntry() { - if type_decls(_, underlyingElement(this), _) - then type_decls(unresolveElement(result), underlyingElement(this), _) + if type_decls(_, unresolveElement(this), _) + then type_decls(underlyingElement(result), unresolveElement(this), _) else exists(Class t | this.(Class).isConstructedFrom(t) and result = t.getADeclarationEntry()) } diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected index 0645e923643..19c55430e1c 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/declarationEntry.expected @@ -30,7 +30,7 @@ | declarationEntry.cpp:39:7:39:7 | operator= | declarationEntry.cpp:39:7:39:7 | declaration of operator= | 1 | 1 | | declarationEntry.cpp:39:7:39:7 | operator= | declarationEntry.cpp:39:7:39:7 | declaration of operator= | 1 | 1 | | declarationEntry.cpp:39:7:39:13 | myClass | declarationEntry.cpp:39:7:39:13 | definition of myClass | 1 | 1 | -| declarationEntry.cpp:39:7:39:13 | myClass | forwardDeclaration.cpp:1:7:1:13 | declaration of myClass | 0 | 1 | +| declarationEntry.cpp:39:7:39:13 | myClass | forwardDeclaration.cpp:1:7:1:13 | declaration of myClass | 1 | 1 | | declarationEntry.cpp:42:6:42:21 | myMemberVariable | declarationEntry.cpp:42:6:42:21 | definition of myMemberVariable | 1 | 1 | | forwardDeclaration.cpp:3:10:3:19 | myClassPtr | forwardDeclaration.cpp:3:10:3:19 | definition of myClassPtr | 1 | 1 | | macro.c:2:1:2:3 | foo | macro.c:2:1:2:3 | declaration of foo | 1 | 1 | diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected index 5066ff021bf..e0ea52ab027 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.expected @@ -39,7 +39,7 @@ | file://:0:0:0:0 | definition of gp_offset | file://:0:0:0:0 | gp_offset | yes | | file://:0:0:0:0 | definition of overflow_arg_area | file://:0:0:0:0 | overflow_arg_area | yes | | file://:0:0:0:0 | definition of reg_save_area | file://:0:0:0:0 | reg_save_area | yes | -| forwardDeclaration.cpp:1:7:1:13 | declaration of myClass | declarationEntry.cpp:39:7:39:13 | myClass | no | +| forwardDeclaration.cpp:1:7:1:13 | declaration of myClass | declarationEntry.cpp:39:7:39:13 | myClass | yes | | forwardDeclaration.cpp:3:10:3:19 | definition of myClassPtr | forwardDeclaration.cpp:3:10:3:19 | myClassPtr | yes | | macro.c:2:1:2:3 | declaration of foo | macro.c:2:1:2:3 | foo | yes | | macro.c:4:5:4:8 | definition of main | macro.c:4:5:4:8 | main | yes | From 4a78c9b06d16de929e8c222ad06f9203b7ff696a Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Wed, 22 Jun 2022 08:07:55 +0200 Subject: [PATCH 4/5] C++: Add change note --- .../change-notes/2022-06-22-class-declaration-entry-fix.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2022-06-22-class-declaration-entry-fix.md diff --git a/cpp/ql/lib/change-notes/2022-06-22-class-declaration-entry-fix.md b/cpp/ql/lib/change-notes/2022-06-22-class-declaration-entry-fix.md new file mode 100644 index 00000000000..fb301705e79 --- /dev/null +++ b/cpp/ql/lib/change-notes/2022-06-22-class-declaration-entry-fix.md @@ -0,0 +1,4 @@ +--- +category: fix +--- +* `UserType.getADeclarationEntry()` now yields all forward declarations when the user type is a `class`, `struct`, or `union`. From b1dd8da587798aa16f8ab4883a45d22468bba418 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Wed, 22 Jun 2022 12:59:49 +0200 Subject: [PATCH 5/5] C++: Fix query formatting --- .../declarationEntry/declarationEntry/roundTrip.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql index 57c87fc053d..24bbc4b7922 100644 --- a/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql +++ b/cpp/ql/test/library-tests/declarationEntry/declarationEntry/roundTrip.ql @@ -1,6 +1,7 @@ import cpp from DeclarationEntry de, Declaration d, string canRoundTrip -where d = de.getDeclaration() and +where + d = de.getDeclaration() and if d.getADeclarationEntry() = de then canRoundTrip = "yes" else canRoundTrip = "no" select de, d, canRoundTrip