Skip to content

C++: more conservative resolveClass #202

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
merged 3 commits into from
Sep 19, 2018
Merged

C++: more conservative resolveClass #202

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
b633ee1
C++: Add more tests of resolveClass
jbj Sep 14, 2018
d7f442b
C++: Force unique resolveClass results
jbj Sep 14, 2018
a7d8971
C++: Exclude non-toplevel items from resolveClass
jbj Sep 14, 2018
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
26 changes: 16 additions & 10 deletions cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,31 @@
import semmle.code.cpp.Type

/** Holds if `d` is a complete class named `name`. */
pragma[noinline]
private string getTopLevelClassName(@usertype c) {
isClass(c) and
usertypes(c, result, _) and
not namespacembrs(_, c) and // not in a namespace
not member(_, _, c) and // not in some structure
not class_instantiation(c, _) // not a template instantiation
}

/** Holds if `d` is a unique complete class named `name`. */
pragma[noinline]
private predicate existsCompleteWithName(string name, @usertype d) {
isClass(d) and
is_complete(d) and
usertypes(d, name, _)
name = getTopLevelClassName(d) and
strictcount(@usertype other | is_complete(other) and getTopLevelClassName(other) = name) = 1
}

/** Holds if `c` is an incomplete class named `name`. */
pragma[noinline]
private predicate existsIncompleteWithName(string name, @usertype c) {
isClass(c) and
not is_complete(c) and
usertypes(c, name, _)
name = getTopLevelClassName(c)
}

/**
* Holds if `c` is an imcomplete class, and there exists a complete class `d`
* Holds if `c` is an imcomplete class, and there exists a unique complete class `d`
* with the same name.
*/
private predicate hasCompleteTwin(@usertype c, @usertype d) {
Expand All @@ -30,10 +38,8 @@ private predicate hasCompleteTwin(@usertype c, @usertype d) {
import Cached
cached private module Cached {
/**
* If `c` is incomplete, and there exists a complete class with the same name,
* then the result is that complete class. Otherwise, the result is `c`. If
* multiple complete classes have the same name, this predicate may have
* multiple results.
* If `c` is incomplete, and there exists a unique complete class with the same name,
* then the result is that complete class. Otherwise, the result is `c`.
*/
cached @usertype resolveClass(@usertype c) {
hasCompleteTwin(c, result)
Expand Down
6 changes: 6 additions & 0 deletions cpp/ql/test/library-tests/structs/compatible_cpp/b1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,9 @@ class Damson {
int damson_x;
void foo();
};

namespace unrelated {
class AppleCompatible {
long apple_x;
};
}
3 changes: 3 additions & 0 deletions cpp/ql/test/library-tests/structs/compatible_cpp/compatible.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
| b1.cpp:23:7:23:12 | Damson | 5 members | 2 locations | 1 | foo |
| b1.cpp:23:7:23:12 | Damson | 5 members | 2 locations | 2 | operator= |
| b1.cpp:23:7:23:12 | Damson | 5 members | 2 locations | 3 | operator= |
| b1.cpp:29:9:29:23 | AppleCompatible | 3 members | 1 locations | 0 | apple_x |
| b1.cpp:29:9:29:23 | AppleCompatible | 3 members | 1 locations | 1 | operator= |
| b1.cpp:29:9:29:23 | AppleCompatible | 3 members | 1 locations | 2 | operator= |
| b2.cpp:2:7:2:21 | AppleCompatible | 3 members | 2 locations | 0 | apple_x |
| b2.cpp:2:7:2:21 | AppleCompatible | 3 members | 2 locations | 1 | operator= |
| b2.cpp:2:7:2:21 | AppleCompatible | 3 members | 2 locations | 2 | operator= |
Expand Down
1 change: 1 addition & 0 deletions cpp/ql/test/library-tests/structs/compatible_cpp/compatible_types.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
| b1.cpp:11:7:11:22 | BananaCompatible | 0 | file://:0:0:0:0 | int | 1 types |
| b1.cpp:16:7:16:12 | Cherry | 0 | file://:0:0:0:0 | int | 1 types |
| b1.cpp:23:7:23:12 | Damson | 0 | file://:0:0:0:0 | int | 1 types |
| b1.cpp:29:9:29:23 | AppleCompatible | 0 | file://:0:0:0:0 | long | 1 types |
| b2.cpp:2:7:2:21 | AppleCompatible | 0 | file://:0:0:0:0 | int | 1 types |
| b2.cpp:9:7:9:22 | BananaCompatible | 0 | file://:0:0:0:0 | int | 1 types |
| b2.cpp:14:7:14:12 | Cherry | 0 | file://:0:0:0:0 | short | 1 types |
Expand Down
4 changes: 4 additions & 0 deletions cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
| a.h:5:8:5:13 | cheese | y.cpp:4:8:4:10 | Foo | 3 |
| x.cpp:3:6:3:10 | bar_x | a.h:4:8:4:10 | Bar | 3 |
| x.cpp:19:6:19:10 | foo_x | y.cpp:4:8:4:10 | Foo | 3 |
| x.cpp:23:5:23:17 | templateField | x.cpp:6:10:6:12 | Foo | 3 |
| x.cpp:23:5:23:17 | templateField | x.cpp:12:9:12:11 | Foo | 3 |
| x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template<Foo *> | 0 |
28 changes: 28 additions & 0 deletions cpp/ql/test/library-tests/structs/incomplete_definition/x.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,31 @@
#include "a.h"

Bar *bar_x;

namespace unrelated {
struct Foo {
short val;
};
}

struct ContainsAnotherFoo {
class Foo {
long val;
};
};

// The type of `foo_x` should not refer to any of the above classes, none of
// which are named `Foo` in the global scope.
Foo *foo_x;

template<typename T>
class Template {
T templateField;
};

Template<Foo *> *template_foo;

// Instantiation of the template with unrelated classes named `Foo` should not
// get mixed up with the instantiation above.
template class Template<unrelated::Foo *>;
template class Template<ContainsAnotherFoo::Foo *>;
9 changes: 9 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/c1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include "header.h"

struct MultipleDefsButSameHeader {
int i;
};

struct OneDefInDifferentFile {
int i;
};
6 changes: 6 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/c2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "header.h"

struct MultipleDefsButSameHeader {
char char1;
char char2;
};
5 changes: 5 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/decls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace foo {
class C;

C *x;
}
29 changes: 29 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/defs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
namespace foo {
class C {
};
}

namespace bar {
class C {
};
}

class DefinedAndDeclared {
};

// Despite this declaration being present, the variable below is associated
// with the definition above rather than this declaration.
class DefinedAndDeclared;

DefinedAndDeclared *definedAndDeclared;

#include "header.h"

// Because there are multiple definitions of `MultipleDefsButSameHeader`, the
// type of this variable will refer to the declaration in `header.h` rather
// than any of the definitions.
MultipleDefsButSameHeader *mdbsh;

// Because there is only one definition of `OneDefInDifferentFile`, the type of
// this variable will refer to that definition.
OneDefInDifferentFile *odidf;
3 changes: 3 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
struct MultipleDefsButSameHeader;

struct OneDefInDifferentFile;
4 changes: 4 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
| decls.cpp:4:6:4:6 | x | decls.cpp:2:9:2:9 | C |
| defs.cpp:18:21:18:38 | definedAndDeclared | defs.cpp:11:7:11:24 | DefinedAndDeclared |
| defs.cpp:25:28:25:32 | mdbsh | header.h:1:8:1:32 | MultipleDefsButSameHeader |
| defs.cpp:29:24:29:28 | odidf | c1.cpp:7:8:7:28 | OneDefInDifferentFile |
5 changes: 5 additions & 0 deletions cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import cpp

from Variable x
where exists(x.getFile().getRelativePath())
select x, x.getType().(PointerType).getBaseType()
19 changes: 0 additions & 19 deletions .../library-tests/templates/isfromtemplateinstantiation/isfromtemplateinstantiation.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,32 +116,13 @@
| isfromtemplateinstantiation.cpp:134:29:134:29 | Outer<int>::operator=(const Outer<int> &) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:134:29:134:29 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:134:29:134:29 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<T>::Inner<long>::operator=(Inner<long> &&) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<T>::Inner<long>::operator=(Inner<long> &&) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<T>::Inner<long>::operator=(const Inner<long> &) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<T>::Inner<long>::operator=(const Inner<long> &) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<int>::Inner<long>::operator=(Inner<long> &&) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<int>::Inner<long>::operator=(Inner<long> &&) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<int>::Inner<long>::operator=(const Inner<long> &) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer<int>::Inner<long>::operator=(const Inner<long> &) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<U> | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:135:31:135:35 | definition of Inner<U> | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:136:7:136:7 | x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:136:7:136:7 | x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:136:7:136:7 | x | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| isfromtemplateinstantiation.cpp:137:7:137:7 | y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:137:7:137:7 | y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<int> |
| isfromtemplateinstantiation.cpp:137:7:137:7 | y | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> |
| load.cpp:13:7:13:7 | basic_text_iprimitive<std_istream_mockup>::basic_text_iprimitive(basic_text_iprimitive<std_istream_mockup> &&) | load.cpp:13:7:13:27 | basic_text_iprimitive<std_istream_mockup> |
| load.cpp:13:7:13:7 | basic_text_iprimitive<std_istream_mockup>::basic_text_iprimitive(const basic_text_iprimitive<std_istream_mockup> &) | load.cpp:13:7:13:27 | basic_text_iprimitive<std_istream_mockup> |
Expand Down
11 changes: 6 additions & 5 deletions ...library-tests/templates/isfromtemplateinstantiation/isfromuninstantiatedtemplate.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ isFromUninstantiatedTemplate
| file://:0:0:0:0 | 0 | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass<T> |
| file://:0:0:0:0 | (int)... | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass<T> |
| file://:0:0:0:0 | (int)... | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass<T> |
| file://:0:0:0:0 | Inner<U> | file://:0:0:0:0 | Inner<U> |
| file://:0:0:0:0 | declaration of 1st parameter | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<T> |
| file://:0:0:0:0 | declaration of 1st parameter | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer<T> |
| file://:0:0:0:0 | initializer for MyClassEnumConst | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass<T> |
Expand Down Expand Up @@ -433,15 +434,15 @@ isFromUninstantiatedTemplate
| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | I | T | DeclarationEntry | |
| isfromtemplateinstantiation.cpp:135:31:135:31 | operator= | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:135:31:135:31 | operator= | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<U> | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<U> | | T | Declaration | |
| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner<long> | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | | T | Definition | |
| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | I | T | Definition | |
| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | I | T | Definition | |
| isfromtemplateinstantiation.cpp:136:7:136:7 | x | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:136:7:136:7 | x | | T | Declaration | |
| isfromtemplateinstantiation.cpp:136:7:136:7 | x | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | | T | Definition | |
| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | I | T | Definition | |
| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | I | T | Definition | |
| isfromtemplateinstantiation.cpp:137:7:137:7 | y | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:137:7:137:7 | y | | T | Declaration | |
| isfromtemplateinstantiation.cpp:137:7:137:7 | y | I | T | Declaration | |
| isfromtemplateinstantiation.cpp:144:28:144:56 | incomplete_never_instantiated<T> | | T | Declaration | |
| isfromtemplateinstantiation.cpp:146:28:146:45 | never_instantiated<T> | | T | Declaration | |
Expand Down