From 4bb65fddf75cd155e227856e1300dd6473b6aab6 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 29 May 2019 15:07:38 +0200 Subject: [PATCH 1/3] C++: Test that hasQualifiedName/3 ignores `inline` --- .../qualified_names/qualifiedNames.cpp | 23 +++++++++++++++++++ .../qualified_names/qualifiedNames.expected | 7 ++++++ 2 files changed, 30 insertions(+) diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp index f340bc20b350..1a935b2c278e 100644 --- a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp @@ -59,3 +59,26 @@ namespace templates { return getMember(tc, typedefC()); } } + +namespace std { + inline namespace cpp17 { + void functionInTwoNamespaces(); // BUG: should also show up in `std` + class classInTwoNameSpaces { // BUG: should also show up in `std` + }; + inline namespace implementation { + namespace ns { + void functionInFourNamespaces(); // BUG: should also show up the outer namespaces + } + } + } +} + +// This code demonstrates that `functionInFourNamespaces` is indeed visible in +// four name spaces. +using void_fptr = void(*)(); +void_fptr ptrs[] = { + std::ns::functionInFourNamespaces, + std::cpp17::ns::functionInFourNamespaces, + std::implementation::ns::functionInFourNamespaces, + std::cpp17::implementation::ns::functionInFourNamespaces, +}; diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected index c9807e801ea6..30ea23fc92a8 100644 --- a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected @@ -52,3 +52,10 @@ | qualifiedNames.cpp:53:12:53:12 | getMember | templates::getMember | templates | | getMember | (not global) | | qualifiedNames.cpp:53:12:53:20 | getMember | templates::getMember | templates | | getMember | (not global) | | qualifiedNames.cpp:57:8:57:10 | use | templates::use | templates | | use | (not global) | +| qualifiedNames.cpp:65:10:65:32 | functionInTwoNamespaces | std::cpp17::functionInTwoNamespaces | std::cpp17 | | functionInTwoNamespaces | (not global) | +| qualifiedNames.cpp:66:11:66:11 | operator= | std::cpp17::classInTwoNameSpaces::operator= | std::cpp17 | classInTwoNameSpaces | operator= | (not global) | +| qualifiedNames.cpp:66:11:66:11 | operator= | std::cpp17::classInTwoNameSpaces::operator= | std::cpp17 | classInTwoNameSpaces | operator= | (not global) | +| qualifiedNames.cpp:66:11:66:30 | classInTwoNameSpaces | std::cpp17::classInTwoNameSpaces | std::cpp17 | | classInTwoNameSpaces | (not global) | +| qualifiedNames.cpp:70:14:70:37 | functionInFourNamespaces | std::cpp17::implementation::ns::functionInFourNamespaces | std::cpp17::implementation::ns | | functionInFourNamespaces | (not global) | +| qualifiedNames.cpp:78:7:78:15 | void_fptr | void_fptr | | | void_fptr | void_fptr | +| qualifiedNames.cpp:79:11:79:14 | ptrs | ptrs | | | ptrs | ptrs | From df4c57648cbb9dbb5a37f226671e78f2c9a48278 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 29 May 2019 15:18:37 +0200 Subject: [PATCH 2/3] C++: Support inline namespaces in hasQualifiedName --- change-notes/1.21/analysis-cpp.md | 2 +- .../semmle/code/cpp/internal/QualifiedName.qll | 18 +++++++++++++++++- .../qualified_names/qualifiedNames.cpp | 6 +++--- .../qualified_names/qualifiedNames.expected | 7 +++++++ 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/change-notes/1.21/analysis-cpp.md b/change-notes/1.21/analysis-cpp.md index 202599735a8b..fe856ee64e24 100644 --- a/change-notes/1.21/analysis-cpp.md +++ b/change-notes/1.21/analysis-cpp.md @@ -31,7 +31,7 @@ ## Changes to QL libraries - The predicate `Declaration.hasGlobalName` now only holds for declarations that are not nested in a class. For example, it no longer holds for a member function `MyClass::myFunction` or a constructor `MyClass::MyClass`, whereas previously it would classify those two declarations as global names. -- In class `Declaration`, predicates `getQualifiedName/0` and `hasQualifiedName/1` are no longer recommended for finding functions by name. Instead, use `hasGlobalName/1` and the new `hasQualifiedName/2` and `hasQualifiedName/3` predicates. This improves performance and makes it more reliable to identify names involving templates. +- In class `Declaration`, predicates `getQualifiedName/0` and `hasQualifiedName/1` are no longer recommended for finding functions by name. Instead, use `hasGlobalName/1` and the new `hasQualifiedName/2` and `hasQualifiedName/3` predicates. This improves performance and makes it more reliable to identify names involving templates and inline namespaces. - Additional support for definition by reference has been added to the `semmle.code.cpp.dataflow.TaintTracking` library. - The taint tracking library now includes taint-specific edges for functions modeled in `semmle.code.cpp.models.interfaces.DataFlow`. - The taint tracking library adds flow through library functions that are modeled in `semmle.code.cpp.models.interfaces.Taint`. Queries can add subclasses of `TaintFunction` to specify additional flow. diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll index 9f2ca38a5524..d341c6f3b2b1 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -26,6 +26,22 @@ class Namespace extends @namespace { else result = this.getName() } + string getQualifierForMembers() { + if namespacembrs(_, this) + then + exists(Namespace ns | + namespacembrs(ns, this) + | + result = ns.getQualifierForMembers() + "::" + this.getName() + or + // If this is an inline namespace, its members are also visible in any + // namespace where the members of the parent are visible. + namespace_inline(this) and + result = ns.getQualifierForMembers() + ) + else result = this.getName() + } + Declaration getADeclaration() { if this.getName() = "" then result.isTopLevel() and not namespacembrs(_, result) @@ -331,7 +347,7 @@ cached private predicate declarationHasQualifiedName( string baseName, string typeQualifier, string namespaceQualifier, Declaration d ) { - namespaceQualifier = d.getNamespace().getQualifiedName() and + namespaceQualifier = d.getNamespace().getQualifierForMembers() and ( if hasTypeQualifier(d) then typeQualifier = d.getTypeQualifierWithoutArgs() diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp index 1a935b2c278e..3304e1f6a42d 100644 --- a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp @@ -62,12 +62,12 @@ namespace templates { namespace std { inline namespace cpp17 { - void functionInTwoNamespaces(); // BUG: should also show up in `std` - class classInTwoNameSpaces { // BUG: should also show up in `std` + void functionInTwoNamespaces(); + class classInTwoNameSpaces { }; inline namespace implementation { namespace ns { - void functionInFourNamespaces(); // BUG: should also show up the outer namespaces + void functionInFourNamespaces(); } } } diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected index 30ea23fc92a8..c98c493af257 100644 --- a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected @@ -52,10 +52,17 @@ | qualifiedNames.cpp:53:12:53:12 | getMember | templates::getMember | templates | | getMember | (not global) | | qualifiedNames.cpp:53:12:53:20 | getMember | templates::getMember | templates | | getMember | (not global) | | qualifiedNames.cpp:57:8:57:10 | use | templates::use | templates | | use | (not global) | +| qualifiedNames.cpp:65:10:65:32 | functionInTwoNamespaces | std::cpp17::functionInTwoNamespaces | std | | functionInTwoNamespaces | (not global) | | qualifiedNames.cpp:65:10:65:32 | functionInTwoNamespaces | std::cpp17::functionInTwoNamespaces | std::cpp17 | | functionInTwoNamespaces | (not global) | +| qualifiedNames.cpp:66:11:66:11 | operator= | std::cpp17::classInTwoNameSpaces::operator= | std | classInTwoNameSpaces | operator= | (not global) | +| qualifiedNames.cpp:66:11:66:11 | operator= | std::cpp17::classInTwoNameSpaces::operator= | std | classInTwoNameSpaces | operator= | (not global) | | qualifiedNames.cpp:66:11:66:11 | operator= | std::cpp17::classInTwoNameSpaces::operator= | std::cpp17 | classInTwoNameSpaces | operator= | (not global) | | qualifiedNames.cpp:66:11:66:11 | operator= | std::cpp17::classInTwoNameSpaces::operator= | std::cpp17 | classInTwoNameSpaces | operator= | (not global) | +| qualifiedNames.cpp:66:11:66:30 | classInTwoNameSpaces | std::cpp17::classInTwoNameSpaces | std | | classInTwoNameSpaces | (not global) | | qualifiedNames.cpp:66:11:66:30 | classInTwoNameSpaces | std::cpp17::classInTwoNameSpaces | std::cpp17 | | classInTwoNameSpaces | (not global) | | qualifiedNames.cpp:70:14:70:37 | functionInFourNamespaces | std::cpp17::implementation::ns::functionInFourNamespaces | std::cpp17::implementation::ns | | functionInFourNamespaces | (not global) | +| qualifiedNames.cpp:70:14:70:37 | functionInFourNamespaces | std::cpp17::implementation::ns::functionInFourNamespaces | std::cpp17::ns | | functionInFourNamespaces | (not global) | +| qualifiedNames.cpp:70:14:70:37 | functionInFourNamespaces | std::cpp17::implementation::ns::functionInFourNamespaces | std::implementation::ns | | functionInFourNamespaces | (not global) | +| qualifiedNames.cpp:70:14:70:37 | functionInFourNamespaces | std::cpp17::implementation::ns::functionInFourNamespaces | std::ns | | functionInFourNamespaces | (not global) | | qualifiedNames.cpp:78:7:78:15 | void_fptr | void_fptr | | | void_fptr | void_fptr | | qualifiedNames.cpp:79:11:79:14 | ptrs | ptrs | | | ptrs | ptrs | From 2b424bfb817e09122eafb04d5ecbe61f3c42f6f6 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Thu, 30 May 2019 10:06:35 +0200 Subject: [PATCH 3/3] C++: Clarify getAQualifierForMembers --- .../semmle/code/cpp/internal/QualifiedName.qll | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll index d341c6f3b2b1..e4f4771b52e4 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -26,18 +26,28 @@ class Namespace extends @namespace { else result = this.getName() } - string getQualifierForMembers() { + /** + * Gets a namespace qualifier, like `"namespace1::namespace2"`, through which + * the members of this namespace can be named. When `inline namespace` is + * used, this predicate may have multiple results. + * + * This predicate does not take namespace aliases into account. Unlike inline + * namespaces, specialization of templates cannot happen through an alias. + * Aliases are also local to the compilation unit, while inline namespaces + * affect the whole program. + */ + string getAQualifierForMembers() { if namespacembrs(_, this) then exists(Namespace ns | namespacembrs(ns, this) | - result = ns.getQualifierForMembers() + "::" + this.getName() + result = ns.getAQualifierForMembers() + "::" + this.getName() or // If this is an inline namespace, its members are also visible in any // namespace where the members of the parent are visible. namespace_inline(this) and - result = ns.getQualifierForMembers() + result = ns.getAQualifierForMembers() ) else result = this.getName() } @@ -347,7 +357,7 @@ cached private predicate declarationHasQualifiedName( string baseName, string typeQualifier, string namespaceQualifier, Declaration d ) { - namespaceQualifier = d.getNamespace().getQualifierForMembers() and + namespaceQualifier = d.getNamespace().getAQualifierForMembers() and ( if hasTypeQualifier(d) then typeQualifier = d.getTypeQualifierWithoutArgs()