From b97ff1a72ffadcc9f59253c304fd56d96512e473 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 30 Apr 2019 15:59:58 +0200 Subject: [PATCH 01/12] C++: Take QualifiedName.qll from Ian's branch This imports `QualifiedName.qll` from 2f74a456290b9e0850b7308582e07f5d68de3a36 and makes minimal changes so it compiles. Original author: Ian Lynagh --- .../code/cpp/internal/QualifiedName.qll | 203 ++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll new file mode 100644 index 000000000000..656c1ec705a4 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -0,0 +1,203 @@ +private import semmle.code.cpp.internal.ResolveClass as ResolveClass + +class Namespace extends @namespace { + string toString() { result = "QualifiedName Namespace" } + + string getName() { namespaces(this, result) } + + string getQualifiedName() { + if namespacembrs(_, this) + then exists(Namespace ns | + namespacembrs(ns, this) and + result = ns.getQualifiedName() + "::" + this.getName()) + else result = this.getName() + } + + Declaration getADeclaration() { + if this.getName() = "" + then result.isTopLevel() and not namespacembrs(_, result) + else namespacembrs(this, result) + } +} + +abstract class Declaration extends @declaration { + string toString() { result = "QualifiedName Declaration" } + + /** Gets the name of this declaration. */ + abstract string getName(); + + // Note: This is not as full featured as the getNamespace in the AST, + // but it covers the cases we need here. + Namespace getNamespace() { + // Top level declaration in a namespace ... + result.getADeclaration() = this + // ... or nested in another structure. + or + exists (Declaration m + | m = this and result = m.getDeclaringType().getNamespace()) + } + + string getQualifiedName() { + // MemberFunction, MemberVariable, MemberType + exists (Declaration m + | m = this and + result = m.getDeclaringType().getQualifiedName() + "::" + m.getName()) + or + exists (EnumConstant c + | c = this and + result = c.getDeclaringEnum().getQualifiedName() + "::" + c.getName()) + or + exists (GlobalOrNamespaceVariable v, string s1, string s2 + | v = this and + s2 = v.getNamespace().getQualifiedName() and + s1 = v.getName() + | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + or + exists (Function f, string s1, string s2 + | f = this and f.isTopLevel() and + s2 = f.getNamespace().getQualifiedName() and + s1 = f.getName() + | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + or + exists (UserType t, string s1, string s2 + | t = this and t.isTopLevel() and + s2 = t.getNamespace().getQualifiedName() and + s1 = t.getName() + | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + } + + predicate isTopLevel() { + not (this.isMember() or + this instanceof FriendDecl or + this instanceof EnumConstant or + this instanceof Parameter or + this instanceof ProxyClass or + this instanceof LocalVariable or + this instanceof TemplateParameter or + this.(UserType).isLocal()) + } + + /** Holds if this declaration is a member of a class/struct/union. */ + predicate isMember() { this.hasDeclaringType() } + + /** Holds if this declaration is a member of a class/struct/union. */ + predicate hasDeclaringType() { + exists(this.getDeclaringType()) + } + + /** + * Gets the class where this member is declared, if it is a member. + * For templates, both the template itself and all instantiations of + * the template are considered to have the same declaring class. + */ + Class getDeclaringType() { + this = result.getAMember() + } +} + +class Variable extends Declaration, @variable { + override string getName() { none() } +} + +class TemplateVariable extends Variable { + TemplateVariable() { is_variable_template(this) } + + Variable getAnInstantiation() { variable_instantiation(result, this) } +} + +class LocalScopeVariable extends Variable, @localscopevariable {} + +class LocalVariable extends LocalScopeVariable, @localvariable { + override string getName() { localvariables(this, _, result) } +} + +class Parameter extends LocalScopeVariable, @parameter { + override string getName() { + exists(int i | params(this, _, i, _) and result = "p#" + i.toString()) + } +} + +class GlobalOrNamespaceVariable extends Variable, @globalvariable { + override string getName() { globalvariables(this, _, result) } +} + +class MemberVariable extends Variable, @membervariable { + MemberVariable() { + this.isMember() + } + + override string getName() { membervariables(this, _, result) } +} + +class EnumConstant extends Declaration, @enumconstant { + override string getName() { enumconstants(this, _, _, _, result,_) } + + UserType getDeclaringEnum() { enumconstants(this, result, _, _, _, _) } +} + +class Function extends Declaration, @function { + override string getName() { functions(this, result, _) } +} + +class TemplateFunction extends Function { + TemplateFunction() { + is_function_template(this) and function_template_argument(this, _, _) + } + + Function getAnInstantiation() { + function_instantiation(result, this) + and not exists(@fun_decl fd | + fun_decls(fd, this, _, _, _) and fun_specialized(fd)) + } +} + +class UserType extends Declaration, @usertype { + override string getName() { usertypes(this, result, _) } + + predicate isLocal() { + enclosingfunction(this, _) + } +} + +class ProxyClass extends UserType { + ProxyClass() { usertypes(this, _, 9) } +} + +class TemplateParameter extends UserType { + TemplateParameter() { usertypes(this, _, 7) or usertypes(this, _, 8) } +} + +class Class extends UserType { + Class() { ResolveClass::isClass(this) } + + Declaration getAMember() { + exists(Declaration d | member(this, _ ,d) | + result = d or + result = d.(TemplateClass).getAnInstantiation() or + result = d.(TemplateFunction).getAnInstantiation() or + result = d.(TemplateVariable).getAnInstantiation()) + } +} + +class TemplateClass extends Class { + TemplateClass() { usertypes(this, _, 6) } + + Class getAnInstantiation() { + class_instantiation(result, this) and + class_template_argument(result, _, _) + } +} + +deprecated class Property extends Declaration { + Property() { none() } + + override string getName() { none() } +} + +class FriendDecl extends Declaration, @frienddecl { + override string getName() { + result = this.getDeclaringClass().getName() + "'s friend" + } + + Class getDeclaringClass() { frienddecls(this, result, _, _) } +} From b51ce87ae893f31daab991140f8bdf46a595c128 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 30 Apr 2019 16:09:41 +0200 Subject: [PATCH 02/12] C++: Autoformat QualifiedName.qll --- .../code/cpp/internal/QualifiedName.qll | 115 ++++++++++-------- 1 file changed, 61 insertions(+), 54 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll index 656c1ec705a4..426e5b77986a 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -7,9 +7,11 @@ class Namespace extends @namespace { string getQualifiedName() { if namespacembrs(_, this) - then exists(Namespace ns | - namespacembrs(ns, this) and - result = ns.getQualifiedName() + "::" + this.getName()) + then + exists(Namespace ns | + namespacembrs(ns, this) and + result = ns.getQualifiedName() + "::" + this.getName() + ) else result = this.getName() } @@ -31,68 +33,81 @@ abstract class Declaration extends @declaration { Namespace getNamespace() { // Top level declaration in a namespace ... result.getADeclaration() = this - // ... or nested in another structure. or - exists (Declaration m - | m = this and result = m.getDeclaringType().getNamespace()) + // ... or nested in another structure. + exists(Declaration m | m = this and result = m.getDeclaringType().getNamespace()) } string getQualifiedName() { // MemberFunction, MemberVariable, MemberType - exists (Declaration m - | m = this and - result = m.getDeclaringType().getQualifiedName() + "::" + m.getName()) + exists(Declaration m | + m = this and + result = m.getDeclaringType().getQualifiedName() + "::" + m.getName() + ) or - exists (EnumConstant c - | c = this and - result = c.getDeclaringEnum().getQualifiedName() + "::" + c.getName()) + exists(EnumConstant c | + c = this and + result = c.getDeclaringEnum().getQualifiedName() + "::" + c.getName() + ) or - exists (GlobalOrNamespaceVariable v, string s1, string s2 - | v = this and + exists(GlobalOrNamespaceVariable v, string s1, string s2 | + v = this and s2 = v.getNamespace().getQualifiedName() and s1 = v.getName() - | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + | + s2 != "" and result = s2 + "::" + s1 + or + s2 = "" and result = s1 + ) or - exists (Function f, string s1, string s2 - | f = this and f.isTopLevel() and + exists(Function f, string s1, string s2 | + f = this and + f.isTopLevel() and s2 = f.getNamespace().getQualifiedName() and s1 = f.getName() - | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + | + s2 != "" and result = s2 + "::" + s1 + or + s2 = "" and result = s1 + ) or - exists (UserType t, string s1, string s2 - | t = this and t.isTopLevel() and + exists(UserType t, string s1, string s2 | + t = this and + t.isTopLevel() and s2 = t.getNamespace().getQualifiedName() and s1 = t.getName() - | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + | + s2 != "" and result = s2 + "::" + s1 + or + s2 = "" and result = s1 + ) } predicate isTopLevel() { - not (this.isMember() or - this instanceof FriendDecl or - this instanceof EnumConstant or - this instanceof Parameter or - this instanceof ProxyClass or - this instanceof LocalVariable or - this instanceof TemplateParameter or - this.(UserType).isLocal()) + not ( + this.isMember() or + this instanceof FriendDecl or + this instanceof EnumConstant or + this instanceof Parameter or + this instanceof ProxyClass or + this instanceof LocalVariable or + this instanceof TemplateParameter or + this.(UserType).isLocal() + ) } /** Holds if this declaration is a member of a class/struct/union. */ predicate isMember() { this.hasDeclaringType() } /** Holds if this declaration is a member of a class/struct/union. */ - predicate hasDeclaringType() { - exists(this.getDeclaringType()) - } + predicate hasDeclaringType() { exists(this.getDeclaringType()) } /** * Gets the class where this member is declared, if it is a member. * For templates, both the template itself and all instantiations of * the template are considered to have the same declaring class. */ - Class getDeclaringType() { - this = result.getAMember() - } + Class getDeclaringType() { this = result.getAMember() } } class Variable extends Declaration, @variable { @@ -105,7 +120,7 @@ class TemplateVariable extends Variable { Variable getAnInstantiation() { variable_instantiation(result, this) } } -class LocalScopeVariable extends Variable, @localscopevariable {} +class LocalScopeVariable extends Variable, @localscopevariable { } class LocalVariable extends LocalScopeVariable, @localvariable { override string getName() { localvariables(this, _, result) } @@ -122,15 +137,13 @@ class GlobalOrNamespaceVariable extends Variable, @globalvariable { } class MemberVariable extends Variable, @membervariable { - MemberVariable() { - this.isMember() - } + MemberVariable() { this.isMember() } override string getName() { membervariables(this, _, result) } } class EnumConstant extends Declaration, @enumconstant { - override string getName() { enumconstants(this, _, _, _, result,_) } + override string getName() { enumconstants(this, _, _, _, result, _) } UserType getDeclaringEnum() { enumconstants(this, result, _, _, _, _) } } @@ -140,23 +153,18 @@ class Function extends Declaration, @function { } class TemplateFunction extends Function { - TemplateFunction() { - is_function_template(this) and function_template_argument(this, _, _) - } + TemplateFunction() { is_function_template(this) and function_template_argument(this, _, _) } Function getAnInstantiation() { - function_instantiation(result, this) - and not exists(@fun_decl fd | - fun_decls(fd, this, _, _, _) and fun_specialized(fd)) + function_instantiation(result, this) and + not exists(@fun_decl fd | fun_decls(fd, this, _, _, _) and fun_specialized(fd)) } } class UserType extends Declaration, @usertype { override string getName() { usertypes(this, result, _) } - predicate isLocal() { - enclosingfunction(this, _) - } + predicate isLocal() { enclosingfunction(this, _) } } class ProxyClass extends UserType { @@ -171,11 +179,12 @@ class Class extends UserType { Class() { ResolveClass::isClass(this) } Declaration getAMember() { - exists(Declaration d | member(this, _ ,d) | + exists(Declaration d | member(this, _, d) | result = d or result = d.(TemplateClass).getAnInstantiation() or result = d.(TemplateFunction).getAnInstantiation() or - result = d.(TemplateVariable).getAnInstantiation()) + result = d.(TemplateVariable).getAnInstantiation() + ) } } @@ -195,9 +204,7 @@ deprecated class Property extends Declaration { } class FriendDecl extends Declaration, @frienddecl { - override string getName() { - result = this.getDeclaringClass().getName() + "'s friend" - } + override string getName() { result = this.getDeclaringClass().getName() + "'s friend" } Class getDeclaringClass() { frienddecls(this, result, _, _) } } From 0a2e28858adc89cba15dbed42467560c6da8d6d2 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 1 May 2019 10:04:12 +0200 Subject: [PATCH 03/12] C++: Rework how qualified names are computed --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 102 +++---- cpp/ql/src/semmle/code/cpp/Enum.qll | 5 - cpp/ql/src/semmle/code/cpp/FriendDecl.qll | 5 - cpp/ql/src/semmle/code/cpp/Function.qll | 2 - cpp/ql/src/semmle/code/cpp/ObjectiveC.qll | 3 - cpp/ql/src/semmle/code/cpp/Parameter.qll | 53 +--- cpp/ql/src/semmle/code/cpp/UserType.qll | 7 +- cpp/ql/src/semmle/code/cpp/Variable.qll | 9 - .../code/cpp/internal/QualifiedName.qll | 253 +++++++++++++----- 9 files changed, 261 insertions(+), 178 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 98a8c733f29d..22f30e1c3773 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -1,6 +1,7 @@ import semmle.code.cpp.Element import semmle.code.cpp.Specifier import semmle.code.cpp.Namespace +private import semmle.code.cpp.internal.QualifiedName as Q /** * A C/C++ declaration: for example, a variable declaration, a type @@ -30,16 +31,7 @@ abstract class Declaration extends Locatable, @declaration { * namespace of the structure. */ Namespace getNamespace() { - // Top level declaration in a namespace ... - result.getADeclaration() = this - - // ... or nested in another structure. - or - exists (Declaration m - | m = this and result = m.getDeclaringType().getNamespace()) - or - exists (EnumConstant c - | c = this and result = c.getDeclaringEnum().getNamespace()) + result = this.(Q::Declaration).getNamespace() or exists (Parameter p | p = this and result = p.getFunction().getNamespace()) @@ -50,40 +42,56 @@ abstract class Declaration extends Locatable, @declaration { /** * Gets the name of the declaration, fully qualified with its - * namespace. For example: "A::B::C::myfcn". + * namespace and declaring type. + * + * For performance, prefer the multi-argument `hasQualifiedName` or + * `hasGlobalName` predicates since they don't construct so many intermediate + * strings. For debugging, the `semmle.code.cpp.Print` module produces more + * detailed output but are also more expensive to compute. + * + * Example: `getQualifiedName() = + * "namespace1::namespace2::TemplateClass1::Class2::memberName"`. */ string getQualifiedName() { - // MemberFunction, MemberVariable, MemberType - exists (Declaration m - | m = this and - not m instanceof EnumConstant and - result = m.getDeclaringType().getQualifiedName() + "::" + m.getName()) - or - exists (EnumConstant c - | c = this and - result = c.getDeclaringEnum().getQualifiedName() + "::" + c.getName()) - or - exists (GlobalOrNamespaceVariable v, string s1, string s2 - | v = this and - s2 = v.getNamespace().getQualifiedName() and - s1 = v.getName() - | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) - or - exists (Function f, string s1, string s2 - | f = this and f.isTopLevel() and - s2 = f.getNamespace().getQualifiedName() and - s1 = f.getName() - | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) - or - exists (UserType t, string s1, string s2 - | t = this and t.isTopLevel() and - s2 = t.getNamespace().getQualifiedName() and - s1 = t.getName() - | (s2 != "" and result = s2 + "::" + s1) or (s2 = "" and result = s1)) + result = this.(Q::Declaration).getQualifiedName() } - predicate hasQualifiedName(string name) { - this.getQualifiedName() = name + /** + * Holds if this declaration has the fully-qualified name `qualifiedName`. + * See `getQualifiedName`. + */ + predicate hasQualifiedName(string qualifiedName) { + this.getQualifiedName() = qualifiedName + } + + /** + * Holds if this declaration has a fully-qualified name with a name-space + * component of `namespaceQualifier`, a declaring type of `typeQualifier`, + * and a base name of `baseName`. Template parameters and arguments are + * stripped from all components. Missing components are `""`. + * + * Example: `hasQualifiedName("namespace1::namespace2", + * "TemplateClass1::Class2", "memberName")`. + * + * Example (the class `std::vector`): `hasQualifiedName("std", "", "vector")` + * or `hasQualifiedName("std", "vector")`. + * + * Example (the `size` member function of class `std::vector`): + * `hasQualifiedName("std", "vector", "size")`. + */ + predicate hasQualifiedName(string namespaceQualifier, string typeQualifier, string baseName) { + this.(Q::Declaration).hasQualifiedName(namespaceQualifier, typeQualifier, baseName) + } + + /** + * Holds if this declaration has a fully-qualified name with a name-space + * component of `namespaceQualifier`, no declaring type, and a base name of + * `baseName`. + * + * See the 3-argument `hasQualifiedName` for more examples. + */ + predicate hasQualifiedName(string namespaceQualifier, string baseName) { + this.hasQualifiedName(namespaceQualifier, "", baseName) } override string toString() { result = this.getName() } @@ -93,22 +101,24 @@ abstract class Declaration extends Locatable, @declaration { * * This name doesn't include a namespace or any argument types, so * for example both functions `::open()` and `::std::ifstream::open(...)` - * have the same name. + * have the same name. The name of a template _class_ includes a string + * representation of its parameters, and the names of its instantiations + * include string representations of their arguments. Template _functions_ + * and their instantiations do not include template parameters or arguments. * - * To get the name including the namespace, use `getQualifiedName` or - * `hasQualifiedName`. + * To get the name including the namespace, use `hasQualifiedName`. * * To test whether this declaration has a particular name in the global * namespace, use `hasGlobalName`. */ - abstract string getName(); + string getName() { result = this.(Q::Declaration).getName() } + /** Holds if this declaration has the given name. */ predicate hasName(string name) { name = this.getName() } /** Holds if this declaration has the given name in the global namespace. */ predicate hasGlobalName(string name) { - hasName(name) - and getNamespace() instanceof GlobalNamespace + this.hasQualifiedName("", "", name) } /** Gets a specifier of this declaration. */ diff --git a/cpp/ql/src/semmle/code/cpp/Enum.qll b/cpp/ql/src/semmle/code/cpp/Enum.qll index b5a06db8879c..a501077cc3ef 100644 --- a/cpp/ql/src/semmle/code/cpp/Enum.qll +++ b/cpp/ql/src/semmle/code/cpp/Enum.qll @@ -100,11 +100,6 @@ class EnumConstant extends Declaration, @enumconstant { result = this.getDeclaringEnum().getDeclaringType() } - /** - * Gets the name of this enumerator. - */ - override string getName() { enumconstants(underlyingElement(this),_,_,_,result,_) } - /** * Gets the value that this enumerator is initialized to, as a * string. This can be a value explicitly given to the enumerator, or an diff --git a/cpp/ql/src/semmle/code/cpp/FriendDecl.qll b/cpp/ql/src/semmle/code/cpp/FriendDecl.qll index 08c52a09cf59..52e57d0199f6 100644 --- a/cpp/ql/src/semmle/code/cpp/FriendDecl.qll +++ b/cpp/ql/src/semmle/code/cpp/FriendDecl.qll @@ -35,11 +35,6 @@ class FriendDecl extends Declaration, @frienddecl { /** Gets the location of this friend declaration. */ override Location getLocation() { frienddecls(underlyingElement(this),_,_,result) } - /** Gets a descriptive string for this friend declaration. */ - override string getName() { - result = this.getDeclaringClass().getName() + "'s friend" - } - /** * Friend declarations do not have specifiers. It makes no difference * whether they are declared in a public, protected or private section of diff --git a/cpp/ql/src/semmle/code/cpp/Function.qll b/cpp/ql/src/semmle/code/cpp/Function.qll index ce78945f5e3c..b0158ffd5318 100644 --- a/cpp/ql/src/semmle/code/cpp/Function.qll +++ b/cpp/ql/src/semmle/code/cpp/Function.qll @@ -17,8 +17,6 @@ private import semmle.code.cpp.internal.ResolveClass * in more detail in `Declaration.qll`. */ class Function extends Declaration, ControlFlowNode, AccessHolder, @function { - override string getName() { functions(underlyingElement(this),result,_) } - /** * DEPRECATED: Use `getIdentityString(Declaration)` from `semmle.code.cpp.Print` instead. * Gets the full signature of this function, including return type, parameter diff --git a/cpp/ql/src/semmle/code/cpp/ObjectiveC.qll b/cpp/ql/src/semmle/code/cpp/ObjectiveC.qll index aa7e9d4ddc5b..3e3c52c35e8c 100644 --- a/cpp/ql/src/semmle/code/cpp/ObjectiveC.qll +++ b/cpp/ql/src/semmle/code/cpp/ObjectiveC.qll @@ -148,9 +148,6 @@ deprecated class FinallyBlock extends Block { deprecated class Property extends Declaration { Property() { none() } - /** Gets the name of this property. */ - override string getName() { none() } - /** * Gets nothing (provided for compatibility with Declaration). * diff --git a/cpp/ql/src/semmle/code/cpp/Parameter.qll b/cpp/ql/src/semmle/code/cpp/Parameter.qll index c6fc33b9355a..5a3c3e59ef2e 100644 --- a/cpp/ql/src/semmle/code/cpp/Parameter.qll +++ b/cpp/ql/src/semmle/code/cpp/Parameter.qll @@ -1,6 +1,7 @@ import semmle.code.cpp.Location import semmle.code.cpp.Declaration private import semmle.code.cpp.internal.ResolveClass +private import semmle.code.cpp.internal.QualifiedName as Q /** * A C/C++ function parameter or catch block parameter. @@ -13,26 +14,6 @@ private import semmle.code.cpp.internal.ResolveClass * have multiple declarations. */ class Parameter extends LocalScopeVariable, @parameter { - - /** - * Gets the canonical name, or names, of this parameter. - * - * The canonical names are the first non-empty category from the - * following list: - * 1. The name given to the parameter at the function's definition or - * (for catch block parameters) at the catch block. - * 2. A name given to the parameter at a function declaration. - * 3. The name "p#i" where i is the index of the parameter. - */ - override string getName() { - exists (VariableDeclarationEntry vde - | vde = getANamedDeclarationEntry() and result = vde.getName() - | vde.isDefinition() or not getANamedDeclarationEntry().isDefinition()) - or - (not exists(getANamedDeclarationEntry()) and - result = "p#" + this.getIndex().toString()) - } - /** * Gets the name of this parameter, including it's type. * @@ -53,27 +34,6 @@ class Parameter extends LocalScopeVariable, @parameter { else result = typeString + nameString)) } - private VariableDeclarationEntry getANamedDeclarationEntry() { - result = getAnEffectiveDeclarationEntry() and result.getName() != "" - } - - /** - * Gets a declaration entry corresponding to this declaration. - * - * This predicate is the same as getADeclarationEntry(), except that for - * parameters of instantiated function templates, gives the declaration - * entry of the prototype instantiation of the parameter (as - * non-prototype instantiations don't have declaration entries of their - * own). - */ - private VariableDeclarationEntry getAnEffectiveDeclarationEntry() { - if getFunction().isConstructedFrom(_) - then exists (Function prototypeInstantiation - | prototypeInstantiation.getParameter(getIndex()) = result.getVariable() and - getFunction().isConstructedFrom(prototypeInstantiation)) - else result = getADeclarationEntry() - } - /** * Gets the name of this parameter in the given block (which should be * the body of a function with which the parameter is associated). @@ -95,7 +55,7 @@ class Parameter extends LocalScopeVariable, @parameter { * In other words, this predicate holds precisely when the result of * `getName()` is not "p#i" (where `i` is the index of the parameter). */ - predicate isNamed() { exists(getANamedDeclarationEntry()) } + predicate isNamed() { exists(this.(Q::Parameter).getANamedDeclarationEntry()) } /** * Gets the function to which this parameter belongs, if it is a function @@ -135,8 +95,13 @@ class Parameter extends LocalScopeVariable, @parameter { * of the declaration locations. */ override Location getLocation() { - exists(VariableDeclarationEntry vde | vde = getAnEffectiveDeclarationEntry() and result = vde.getLocation() | - vde.isDefinition() or not getAnEffectiveDeclarationEntry().isDefinition() + exists(VariableDeclarationEntry vde | + vde = this.(Q::Parameter).getAnEffectiveDeclarationEntry() and + result = vde.getLocation() + | + vde.isDefinition() + or + not this.(Q::Parameter).getAnEffectiveDeclarationEntry().isDefinition() ) } } diff --git a/cpp/ql/src/semmle/code/cpp/UserType.qll b/cpp/ql/src/semmle/code/cpp/UserType.qll index fcecc79770b1..8f4643108b90 100644 --- a/cpp/ql/src/semmle/code/cpp/UserType.qll +++ b/cpp/ql/src/semmle/code/cpp/UserType.qll @@ -9,10 +9,9 @@ private import semmle.code.cpp.internal.ResolveClass * `Enum`, and `TypedefType`. */ class UserType extends Type, Declaration, NameQualifyingElement, AccessHolder, @usertype { - /** - * Gets the name of this type. - */ - override string getName() { usertypes(underlyingElement(this),result,_) } + override string getName() { + result = Declaration.super.getName() + } /** * Gets the simple name of this type, without any template parameters. For example diff --git a/cpp/ql/src/semmle/code/cpp/Variable.qll b/cpp/ql/src/semmle/code/cpp/Variable.qll index c535f742596f..c3f9eaf2bf09 100644 --- a/cpp/ql/src/semmle/code/cpp/Variable.qll +++ b/cpp/ql/src/semmle/code/cpp/Variable.qll @@ -41,9 +41,6 @@ class Variable extends Declaration, @variable { /** Holds if this variable is `volatile`. */ predicate isVolatile() { this.getType().isVolatile() } - /** Gets the name of this variable. */ - override string getName() { none() } - /** Gets the type of this variable. */ Type getType() { none() } @@ -291,8 +288,6 @@ deprecated class StackVariable extends Variable { * A local variable can be declared by a `DeclStmt` or a `ConditionDeclExpr`. */ class LocalVariable extends LocalScopeVariable, @localvariable { - override string getName() { localvariables(underlyingElement(this),_,result) } - override Type getType() { localvariables(underlyingElement(this),unresolveElement(result),_) } override Function getFunction() { @@ -305,8 +300,6 @@ class LocalVariable extends LocalScopeVariable, @localvariable { * A C/C++ variable which has global scope or namespace scope. */ class GlobalOrNamespaceVariable extends Variable, @globalvariable { - override string getName() { globalvariables(underlyingElement(this),_,result) } - override Type getType() { globalvariables(underlyingElement(this),unresolveElement(result),_) } override Element getEnclosingElement() { none() } @@ -354,8 +347,6 @@ class MemberVariable extends Variable, @membervariable { /** Holds if this member is public. */ predicate isPublic() { this.hasSpecifier("public") } - override string getName() { membervariables(underlyingElement(this),_,result) } - override Type getType() { if (strictcount(this.getAType()) = 1) then ( result = this.getAType() diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll index 426e5b77986a..c73a85d8040a 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -1,5 +1,3 @@ -private import semmle.code.cpp.internal.ResolveClass as ResolveClass - class Namespace extends @namespace { string toString() { result = "QualifiedName Namespace" } @@ -26,61 +24,72 @@ abstract class Declaration extends @declaration { string toString() { result = "QualifiedName Declaration" } /** Gets the name of this declaration. */ + cached abstract string getName(); - // Note: This is not as full featured as the getNamespace in the AST, - // but it covers the cases we need here. + string getTypeQualifierWithoutArgs() { + exists(UserType declaringType | + declaringType = this.(EnumConstant).getDeclaringEnum() + or + declaringType = this.getDeclaringType() + | + result = getTypeQualifierForMembersWithoutArgs(declaringType) + ) + } + + string getTypeQualifierWithArgs() { + exists(UserType declaringType | + declaringType = this.(EnumConstant).getDeclaringEnum() + or + declaringType = this.getDeclaringType() + | + result = getTypeQualifierForMembersWithArgs(declaringType) + ) + } + Namespace getNamespace() { // Top level declaration in a namespace ... result.getADeclaration() = this or // ... or nested in another structure. exists(Declaration m | m = this and result = m.getDeclaringType().getNamespace()) + or + exists(EnumConstant c | c = this and result = c.getDeclaringEnum().getNamespace()) + } + + predicate hasQualifiedName(string namespaceQualifier, string typeQualifier, string baseName) { + declarationHasQualifiedName(baseName, typeQualifier, namespaceQualifier, this) } string getQualifiedName() { - // MemberFunction, MemberVariable, MemberType - exists(Declaration m | - m = this and - result = m.getDeclaringType().getQualifiedName() + "::" + m.getName() - ) - or - exists(EnumConstant c | - c = this and - result = c.getDeclaringEnum().getQualifiedName() + "::" + c.getName() - ) - or - exists(GlobalOrNamespaceVariable v, string s1, string s2 | - v = this and - s2 = v.getNamespace().getQualifiedName() and - s1 = v.getName() + exists(string ns, string name | + ns = this.getNamespace().getQualifiedName() and + name = this.getName() and + this.canHaveQualifiedName() | - s2 != "" and result = s2 + "::" + s1 + exists(string t | t = this.getTypeQualifierWithArgs() | + if ns != "" + then result = ns + "::" + t + "::" + name + else result = t + "::" + name + ) or - s2 = "" and result = s1 + not hasTypeQualifier(this) and + if ns != "" + then result = ns + "::" + name + else result = name ) + } + + predicate canHaveQualifiedName() { + this.hasDeclaringType() or - exists(Function f, string s1, string s2 | - f = this and - f.isTopLevel() and - s2 = f.getNamespace().getQualifiedName() and - s1 = f.getName() - | - s2 != "" and result = s2 + "::" + s1 - or - s2 = "" and result = s1 - ) + this instanceof EnumConstant or - exists(UserType t, string s1, string s2 | - t = this and - t.isTopLevel() and - s2 = t.getNamespace().getQualifiedName() and - s1 = t.getName() - | - s2 != "" and result = s2 + "::" + s1 - or - s2 = "" and result = s1 - ) + this instanceof Function + or + this instanceof UserType + or + this instanceof GlobalOrNamespaceVariable } predicate isTopLevel() { @@ -107,11 +116,13 @@ abstract class Declaration extends @declaration { * For templates, both the template itself and all instantiations of * the template are considered to have the same declaring class. */ - Class getDeclaringType() { this = result.getAMember() } + UserType getDeclaringType() { this = result.getAMember() } } class Variable extends Declaration, @variable { override string getName() { none() } + + VariableDeclarationEntry getADeclarationEntry() { result.getDeclaration() = this } } class TemplateVariable extends Variable { @@ -126,9 +137,73 @@ class LocalVariable extends LocalScopeVariable, @localvariable { override string getName() { localvariables(this, _, result) } } +/** + * A particular declaration or definition of a C/C++ variable. + */ +class VariableDeclarationEntry extends @var_decl { + string toString() { result = "QualifiedName DeclarationEntry" } + + Variable getDeclaration() { result = getVariable() } + + /** + * Gets the variable which is being declared or defined. + */ + Variable getVariable() { var_decls(this, result, _, _, _) } + + predicate isDefinition() { var_def(this) } + + string getName() { var_decls(this, _, _, result, _) and result != "" } +} + class Parameter extends LocalScopeVariable, @parameter { + @functionorblock function; + + int index; + + Parameter() { params(this, function, index, _) } + + /** + * Gets the canonical name, or names, of this parameter. + * + * The canonical names are the first non-empty category from the + * following list: + * 1. The name given to the parameter at the function's definition or + * (for catch block parameters) at the catch block. + * 2. A name given to the parameter at a function declaration. + * 3. The name "p#i" where i is the index of the parameter. + */ override string getName() { - exists(int i | params(this, _, i, _) and result = "p#" + i.toString()) + exists(VariableDeclarationEntry vde | + vde = getANamedDeclarationEntry() and result = vde.getName() + | + vde.isDefinition() or not getANamedDeclarationEntry().isDefinition() + ) + or + not exists(getANamedDeclarationEntry()) and + result = "p#" + index.toString() + } + + VariableDeclarationEntry getANamedDeclarationEntry() { + result = getAnEffectiveDeclarationEntry() and exists(result.getName()) + } + + /** + * Gets a declaration entry corresponding to this declaration. + * + * This predicate is the same as getADeclarationEntry(), except that for + * parameters of instantiated function templates, gives the declaration + * entry of the prototype instantiation of the parameter (as + * non-prototype instantiations don't have declaration entries of their + * own). + */ + VariableDeclarationEntry getAnEffectiveDeclarationEntry() { + if function.(Function).isConstructedFrom(_) + then + exists(Function prototypeInstantiation | + prototypeInstantiation.getParameter(index) = result.getVariable() and + function.(Function).isConstructedFrom(prototypeInstantiation) + ) + else result = getADeclarationEntry() } } @@ -146,10 +221,15 @@ class EnumConstant extends Declaration, @enumconstant { override string getName() { enumconstants(this, _, _, _, result, _) } UserType getDeclaringEnum() { enumconstants(this, result, _, _, _, _) } + // Unlike the usual `EnumConstant`, this one doesn't have a `getDeclaringType()`. } class Function extends Declaration, @function { override string getName() { functions(this, result, _) } + + predicate isConstructedFrom(Function f) { function_instantiation(this, f) } + + Parameter getParameter(int n) { params(result, this, n, _) } } class TemplateFunction extends Function { @@ -162,22 +242,11 @@ class TemplateFunction extends Function { } class UserType extends Declaration, @usertype { - override string getName() { usertypes(this, result, _) } + override string getName() { result = getUserTypeNameWithArgs(this) } predicate isLocal() { enclosingfunction(this, _) } -} - -class ProxyClass extends UserType { - ProxyClass() { usertypes(this, _, 9) } -} - -class TemplateParameter extends UserType { - TemplateParameter() { usertypes(this, _, 7) or usertypes(this, _, 8) } -} - -class Class extends UserType { - Class() { ResolveClass::isClass(this) } + // Gets a member of this class, if it's a class. Declaration getAMember() { exists(Declaration d | member(this, _, d) | result = d or @@ -188,10 +257,18 @@ class Class extends UserType { } } -class TemplateClass extends Class { +class ProxyClass extends UserType { + ProxyClass() { usertypes(this, _, 9) } +} + +class TemplateParameter extends UserType { + TemplateParameter() { usertypes(this, _, 7) or usertypes(this, _, 8) } +} + +class TemplateClass extends UserType { TemplateClass() { usertypes(this, _, 6) } - Class getAnInstantiation() { + UserType getAnInstantiation() { class_instantiation(result, this) and class_template_argument(result, _, _) } @@ -204,7 +281,63 @@ deprecated class Property extends Declaration { } class FriendDecl extends Declaration, @frienddecl { - override string getName() { result = this.getDeclaringClass().getName() + "'s friend" } + override string getName() { + result = getUserTypeNameWithArgs(this.getDeclaringClass()) + "'s friend" + } + + UserType getDeclaringClass() { frienddecls(this, result, _, _) } +} + +private string getUserTypeNameWithArgs(UserType t) { usertypes(t, result, _) } + +private string getUserTypeNameWithoutArgs(UserType t) { + result = getUserTypeNameWithArgs(t).splitAt("<", 0) +} + +private predicate hasTypeQualifier(Declaration d) { + d instanceof EnumConstant + or + d.hasDeclaringType() +} - Class getDeclaringClass() { frienddecls(this, result, _, _) } +private string getTypeQualifierForMembersWithArgs(UserType t) { + result = t.getTypeQualifierWithArgs() + "::" + getUserTypeNameWithArgs(t) + or + not hasTypeQualifier(t) and + result = getUserTypeNameWithArgs(t) +} + +private string getTypeQualifierForMembersWithoutArgs(UserType t) { + result = t.getTypeQualifierWithoutArgs() + "::" + getUserTypeNameWithoutArgs(t) + or + not hasTypeQualifier(t) and + result = getUserTypeNameWithoutArgs(t) +} + +// The order of parameters on this predicate is chosen to match the most common +// use case: finding a declaration that has a specific name. The declaration +// comes last because it's the output. +cached +private predicate declarationHasQualifiedName( + string baseName, string typeQualifier, string namespaceQualifier, Declaration d +) { + namespaceQualifier = d.getNamespace().getQualifiedName() and + ( + if hasTypeQualifier(d) + then typeQualifier = d.getTypeQualifierWithoutArgs() + else typeQualifier = "" + ) and + ( + baseName = getUserTypeNameWithoutArgs(d) + or + // If a declaration isn't a `UserType`, there are two ways it can still + // contain `<`: + // 1. If it's `operator<` or `operator<<`. + // 2. If it's a conversion operator like `operator TemplateClass`. + // Perhaps these names ought to be fixed up, but we don't do that + // currently. + not d instanceof UserType and + baseName = d.getName() + ) and + d.canHaveQualifiedName() } From 64a87a863c1dcf834dee1180a1f918a84c15b32d Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 3 May 2019 08:44:41 +0200 Subject: [PATCH 04/12] C++: Remove uses of getQualifiedName This removes all uses of `Declaration.getQualifiedName` that I think can be removed without changing any behaviour. The following uses in the LGTM default suite remain: * `cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql` (in `select`). * `cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowDispatch.qll` (needs template args). * `cpp/ql/src/semmle/code/cpp/security/FunctionWithWrappers.qll` (used for alert messages). --- cpp/ql/src/Critical/InitialisationNotRun.ql | 2 +- cpp/ql/src/Critical/SizeCheck.ql | 4 ++-- cpp/ql/src/Critical/SizeCheck2.ql | 4 ++-- cpp/ql/src/DefaultOptions.qll | 2 +- .../ReturnCstrOfLocalStdString.ql | 6 +++--- cpp/ql/src/Metrics/Files/FNumberOfTests.ql | 7 ++----- cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql | 7 ++----- .../CWE/CWE-676/PotentiallyDangerousFunction.ql | 4 ++-- cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql | 6 +++--- cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 14 -------------- .../src/semmle/code/cpp/controlflow/Nullness.qll | 8 ++++---- .../code/cpp/models/implementations/Printf.qll | 6 +++--- .../src/semmle/code/cpp/security/BufferWrite.qll | 2 +- .../semmle/code/cpp/security/CommandExecution.qll | 8 ++++---- cpp/ql/src/semmle/code/cpp/security/FileWrite.qll | 11 ++--------- .../src/semmle/code/cpp/security/OutputWrite.qll | 4 ++-- 16 files changed, 34 insertions(+), 61 deletions(-) diff --git a/cpp/ql/src/Critical/InitialisationNotRun.ql b/cpp/ql/src/Critical/InitialisationNotRun.ql index dff313d4c42c..6295543084b8 100644 --- a/cpp/ql/src/Critical/InitialisationNotRun.ql +++ b/cpp/ql/src/Critical/InitialisationNotRun.ql @@ -20,7 +20,7 @@ predicate global(GlobalVariable v) { } predicate mainCalled(Function f) { - f.getQualifiedName() = "main" + f.hasGlobalName("main") or exists(Function caller | mainCalled(caller) and allCalls(caller, f)) } diff --git a/cpp/ql/src/Critical/SizeCheck.ql b/cpp/ql/src/Critical/SizeCheck.ql index 8b90be0d0714..da841b73c9bc 100644 --- a/cpp/ql/src/Critical/SizeCheck.ql +++ b/cpp/ql/src/Critical/SizeCheck.ql @@ -17,12 +17,12 @@ import cpp class Allocation extends FunctionCall { Allocation() { exists(string name | - this.getTarget().hasQualifiedName(name) and + this.getTarget().hasGlobalName(name) and (name = "malloc" or name = "calloc" or name = "realloc") ) } - string getName() { result = this.getTarget().getQualifiedName() } + private string getName() { this.getTarget().hasGlobalName(result) } int getSize() { this.getName() = "malloc" and diff --git a/cpp/ql/src/Critical/SizeCheck2.ql b/cpp/ql/src/Critical/SizeCheck2.ql index 5c58a03dcf25..3cb5d1d28b00 100644 --- a/cpp/ql/src/Critical/SizeCheck2.ql +++ b/cpp/ql/src/Critical/SizeCheck2.ql @@ -17,12 +17,12 @@ import cpp class Allocation extends FunctionCall { Allocation() { exists(string name | - this.getTarget().hasQualifiedName(name) and + this.getTarget().hasGlobalName(name) and (name = "malloc" or name = "calloc" or name = "realloc") ) } - string getName() { result = this.getTarget().getQualifiedName() } + private string getName() { this.getTarget().hasGlobalName(result) } int getSize() { this.getName() = "malloc" and diff --git a/cpp/ql/src/DefaultOptions.qll b/cpp/ql/src/DefaultOptions.qll index 0e48d9a6771a..1ab26a01d683 100644 --- a/cpp/ql/src/DefaultOptions.qll +++ b/cpp/ql/src/DefaultOptions.qll @@ -61,7 +61,7 @@ class Options extends string */ predicate exits(Function f) { f.getAnAttribute().hasName("noreturn") or - exists(string name | f.getQualifiedName() = name | + exists(string name | f.hasGlobalName(name) | name = "exit" or name = "_exit" or name = "abort" or diff --git a/cpp/ql/src/Likely Bugs/Memory Management/ReturnCstrOfLocalStdString.ql b/cpp/ql/src/Likely Bugs/Memory Management/ReturnCstrOfLocalStdString.ql index f2b189e4ea82..ea198567c53c 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/ReturnCstrOfLocalStdString.ql +++ b/cpp/ql/src/Likely Bugs/Memory Management/ReturnCstrOfLocalStdString.ql @@ -58,11 +58,11 @@ predicate refToStdString(Expr e, ConstructorCall source) { * will also become invalid. */ predicate flowFunction(Function fcn, int argIndex) { - (fcn.getQualifiedName() = "_JNIEnv::NewStringUTF" and argIndex = 0) + (fcn.hasQualifiedName("", "_JNIEnv", "NewStringUTF") and argIndex = 0) or - (fcn.getQualifiedName() = "art::JNI::NewStringUTF" and argIndex = 1) + (fcn.hasQualifiedName("art", "JNI", "NewStringUTF") and argIndex = 1) or - (fcn.getQualifiedName() = "art::CheckJNI::NewStringUTF" and argIndex = 1) + (fcn.hasQualifiedName("art", "CheckJNI", "NewStringUTF") and argIndex = 1) // Add other functions that behave like NewStringUTF here. } diff --git a/cpp/ql/src/Metrics/Files/FNumberOfTests.ql b/cpp/ql/src/Metrics/Files/FNumberOfTests.ql index f5e45897970c..5ed8230bd991 100644 --- a/cpp/ql/src/Metrics/Files/FNumberOfTests.ql +++ b/cpp/ql/src/Metrics/Files/FNumberOfTests.ql @@ -13,13 +13,10 @@ import cpp Expr getTest() { // cppunit tests; https://freedesktop.org/wiki/Software/cppunit/ - exists(Function f | result.(FunctionCall).getTarget() = f - and f.getNamespace().getName() = "CppUnit" - and f.getName() = "addTest") + result.(FunctionCall).getTarget().hasQualifiedName("CppUnit", _, "addTest") or // boost tests; http://www.boost.org/ - exists(Function f | result.(FunctionCall).getTarget() = f - and f.getQualifiedName() = "boost::unit_test::make_test_case") + result.(FunctionCall).getTarget().hasQualifiedName("boost::unit_test", "make_test_case") } from File f, int n diff --git a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql index 3546b2bdbc22..972efb703126 100644 --- a/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql +++ b/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql @@ -23,7 +23,7 @@ import semmle.code.cpp.security.TaintTracking */ class FileFunction extends FunctionWithWrappers { FileFunction() { - exists(string nme | this.getQualifiedName() = nme | + exists(string nme | this.hasGlobalName(nme) | nme = "fopen" or nme = "_fopen" or nme = "_wfopen" or @@ -32,10 +32,7 @@ class FileFunction extends FunctionWithWrappers { nme = "_wopen" or // create file function on windows - nme.matches("CreateFile%") or - - // Objective C standard library - nme.matches("NSFileHandle%::+fileHandleFor%AtPath:") + nme.matches("CreateFile%") ) or ( diff --git a/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql b/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql index d53dd0c682cc..271d7e190231 100644 --- a/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql +++ b/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql @@ -12,7 +12,7 @@ import cpp predicate potentiallyDangerousFunction(Function f, string message) { - exists(string name | name = f.getQualifiedName() | + exists(string name | f.hasGlobalName(name) | ( name = "gmtime" or name = "localtime" or @@ -21,7 +21,7 @@ predicate potentiallyDangerousFunction(Function f, string message) { ) and message = "Call to " + name + " is potentially dangerous" ) or ( - f.getQualifiedName() = "gets" and + f.hasGlobalName("gets") and message = "gets does not guard against buffer overflow" ) } diff --git a/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql b/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql index 1fa2ab8d1769..99c8ab4bfe76 100644 --- a/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql +++ b/cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql @@ -21,7 +21,7 @@ predicate acquireExpr(Expr acquire, string kind) { exists(FunctionCall fc, Function f, string name | fc = acquire and f = fc.getTarget() and - name = f.getQualifiedName() and + f.hasGlobalName(name) and ( ( name = "fopen" and @@ -47,7 +47,7 @@ predicate releaseExpr(Expr release, Expr resource, string kind) { exists(FunctionCall fc, Function f, string name | fc = release and f = fc.getTarget() and - name = f.getQualifiedName() and + f.hasGlobalName(name) and ( ( name = "fclose" and @@ -252,7 +252,7 @@ pragma[noopt] predicate badRelease(Resource r, Expr acquire, Function functionCa ) } -Class qtObject() { result.getABaseClass*().getQualifiedName() = "QObject" } +Class qtObject() { result.getABaseClass*().hasGlobalName("QObject") } PointerType qtObjectReference() { result.getBaseType() = qtObject() } Constructor qtParentConstructor() { exists(Parameter p | diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll index 96c3c46646fb..bdb31c2ae2c3 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll @@ -74,20 +74,6 @@ class UserDefinedFormattingFunction extends FormattingFunction { override int getFormatParameterIndex() { callsVariadicFormatter(this, result) } } -/** - * The Objective C method `stringWithFormat:`. - */ -class NsstringStringWithFormat extends FormattingFunction { - NsstringStringWithFormat() { - getQualifiedName().matches("NSString%::+stringWithFormat:") or - getQualifiedName().matches("NSString%::+localizedStringWithFormat:") - } - - override int getFormatParameterIndex() { - result = 0 - } -} - /** * A call to one of the formatting functions. */ diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll b/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll index af2429fecb32..e398e182d4de 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll @@ -260,13 +260,13 @@ predicate callMayReturnNull(Call call) */ predicate mayReturnNull(Function f) { - f.getQualifiedName() = "malloc" + f.hasGlobalName("malloc") or - f.getQualifiedName() = "calloc" + f.hasGlobalName("calloc") or -// f.getQualifiedName() = "strchr" +// f.hasGlobalName("strchr") // or -// f.getQualifiedName() = "strstr" +// f.hasGlobalName("strstr") // or exists(ReturnStmt ret | nullValue(ret.getExpr()) and diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll index ef696a526f79..1ed4af8a7522 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll @@ -69,8 +69,8 @@ class Sprintf extends FormattingFunction { or hasGlobalName("__builtin___sprintf_chk") and result = 3 or - getQualifiedName() != "g_strdup_printf" and - getQualifiedName() != "__builtin___sprintf_chk" and + getName() != "g_strdup_printf" and + getName() != "__builtin___sprintf_chk" and result = 1 } override int getOutputParameterIndex() { @@ -127,7 +127,7 @@ class Snprintf extends FormattingFunction { override int getFirstFormatArgumentIndex() { exists(string name | - name = getQualifiedName() + hasGlobalName(name) and ( name = "__builtin___snprintf_chk" and result = 5 diff --git a/cpp/ql/src/semmle/code/cpp/security/BufferWrite.qll b/cpp/ql/src/semmle/code/cpp/security/BufferWrite.qll index bed73b5a49e3..ca65fe4f4d57 100644 --- a/cpp/ql/src/semmle/code/cpp/security/BufferWrite.qll +++ b/cpp/ql/src/semmle/code/cpp/security/BufferWrite.qll @@ -532,7 +532,7 @@ private int path_max() { class RealpathBW extends BufferWriteCall { RealpathBW() { exists(path_max()) and // Ignore realpath() calls if PATH_MAX cannot be determined - getTarget().getQualifiedName() = "realpath" // realpath(path, resolved_path); + getTarget().hasGlobalName("realpath") // realpath(path, resolved_path); } override Type getBufferType() diff --git a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll index d80486b32e3d..72323f8b4727 100644 --- a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll +++ b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll @@ -85,8 +85,8 @@ class VarargsExecFunctionCall extends FunctionCall { * all the other ones start with the command. */ private int getCommandIdx() { if ( - getTarget().getQualifiedName().matches("\\_spawn%") - or getTarget().getQualifiedName().matches("\\_wspawn%")) + getTarget().getName().matches("\\_spawn%") + or getTarget().getName().matches("\\_wspawn%")) then result = 1 else result = 0 } @@ -137,8 +137,8 @@ class ArrayExecFunctionCall extends FunctionCall { * all the other ones start with the command. */ private int getCommandIdx() { if ( - getTarget().getQualifiedName().matches("\\_spawn%") - or getTarget().getQualifiedName().matches("\\_wspawn%")) + getTarget().getName().matches("\\_spawn%") + or getTarget().getName().matches("\\_wspawn%")) then result = 1 else result = 0 } diff --git a/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll b/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll index 85ca02aee563..9a2070fe1112 100644 --- a/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll +++ b/cpp/ql/src/semmle/code/cpp/security/FileWrite.qll @@ -154,7 +154,7 @@ private predicate fileStreamChain(ChainedOutputCall out, Expr source, Expr dest) */ private predicate fileWrite(Call write, Expr source, Expr dest) { exists(Function f, int s, int d | f = write.getTarget() and source = write.getArgument(s) and dest = write.getArgument(d) | - exists(string name | name = f.getQualifiedName() | + exists(string name | f.hasGlobalName(name) | // named functions name = "fwrite" and s = 0 and d = 3 or ( @@ -165,14 +165,7 @@ private predicate fileWrite(Call write, Expr source, Expr dest) { name = "putc" or name = "putwc" or name = "putw" - ) and s = 0 and d = 1 or - name.matches("NSFileManager%::-createFileAtPath:contents:attributes:") and s = 1 and d = 0 or - ( - // methods that write into the receiver - dest = write.getQualifier() and - source = write.getArgument(0) and - name.matches("NSFileHandle%::-writeData:") - ) + ) and s = 0 and d = 1 ) or ( // fprintf s >= f.(Fprintf).getFormatParameterIndex() and diff --git a/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll b/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll index f1a98c62be62..47ed3f4f45e9 100644 --- a/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll +++ b/cpp/ql/src/semmle/code/cpp/security/OutputWrite.qll @@ -66,8 +66,8 @@ private predicate outputWrite(Expr write, Expr source) { ) or ( // puts, putchar ( - f.getQualifiedName() = "puts" or - f.getQualifiedName() = "putchar" + f.hasGlobalName("puts") or + f.hasGlobalName("putchar") ) and arg = 0 ) or exists(Call wrappedCall, Expr wrappedSource | // wrapped output call (recursive case) From 5e789901dfde45c6b8917ec9fe488c27603b1122 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 3 May 2019 10:20:48 +0200 Subject: [PATCH 05/12] C++: Remove all uses of hasQualifiedName/1 --- .../src/Critical/DescriptorMayNotBeClosed.ql | 2 +- cpp/ql/src/Critical/DescriptorNeverClosed.ql | 2 +- cpp/ql/src/Critical/GlobalUseBeforeInit.ql | 2 +- cpp/ql/src/Critical/MemoryMayNotBeFreed.ql | 4 +- cpp/ql/src/Critical/OverflowCalculated.ql | 12 +-- cpp/ql/src/Critical/OverflowDestination.ql | 2 +- cpp/ql/src/Critical/OverflowStatic.ql | 16 ++-- cpp/ql/src/Critical/UseAfterFree.ql | 2 +- cpp/ql/src/DefaultOptions.qll | 8 +- cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql | 4 +- .../CWE/CWE-121/UnterminatedVarargsCall.ql | 6 +- .../CWE/CWE-131/NoSpaceForZeroTerminator.ql | 4 +- cpp/ql/src/semmle/code/cpp/Print.qll | 2 +- cpp/ql/src/semmle/code/cpp/commons/Alloc.qll | 6 +- cpp/ql/src/semmle/code/cpp/commons/File.qll | 28 +++--- .../code/cpp/commons/StringAnalysis.qll | 12 +-- .../code/cpp/controlflow/Dereferenced.qll | 2 +- .../semmle/code/cpp/controlflow/Nullness.qll | 4 +- .../code/cpp/models/implementations/Inet.qll | 20 ++-- .../cpp/models/implementations/Strftime.qll | 2 +- .../src/semmle/code/cpp/pointsto/PointsTo.qll | 2 +- .../code/cpp/security/CommandExecution.qll | 96 +++++++++---------- 22 files changed, 119 insertions(+), 119 deletions(-) diff --git a/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql b/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql index 0bb2c3e52347..9c93e066119c 100644 --- a/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql +++ b/cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql @@ -13,7 +13,7 @@ import semmle.code.cpp.pointsto.PointsTo import Negativity predicate closeCall(FunctionCall fc, Variable v) { - fc.getTarget().hasQualifiedName("close") and v.getAnAccess() = fc.getArgument(0) + fc.getTarget().hasGlobalName("close") and v.getAnAccess() = fc.getArgument(0) or exists(FunctionCall midcall, Function mid, int arg | fc.getArgument(arg) = v.getAnAccess() and diff --git a/cpp/ql/src/Critical/DescriptorNeverClosed.ql b/cpp/ql/src/Critical/DescriptorNeverClosed.ql index 9bef46878ffc..b52af1bf2a3d 100644 --- a/cpp/ql/src/Critical/DescriptorNeverClosed.ql +++ b/cpp/ql/src/Critical/DescriptorNeverClosed.ql @@ -13,7 +13,7 @@ import semmle.code.cpp.pointsto.PointsTo predicate closed(Expr e) { exists(FunctionCall fc | - fc.getTarget().hasQualifiedName("close") and + fc.getTarget().hasGlobalName("close") and fc.getArgument(0) = e ) } diff --git a/cpp/ql/src/Critical/GlobalUseBeforeInit.ql b/cpp/ql/src/Critical/GlobalUseBeforeInit.ql index 34ec6f8c768e..5483a0f2fbe8 100644 --- a/cpp/ql/src/Critical/GlobalUseBeforeInit.ql +++ b/cpp/ql/src/Critical/GlobalUseBeforeInit.ql @@ -30,7 +30,7 @@ predicate useFunc(GlobalVariable v, Function f) { } predicate uninitialisedBefore(GlobalVariable v, Function f) { - f.hasQualifiedName("main") + f.hasGlobalName("main") or exists(Call call, Function g | uninitialisedBefore(v, g) and diff --git a/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql b/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql index 378b62ff287e..181c57a7e55c 100644 --- a/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql +++ b/cpp/ql/src/Critical/MemoryMayNotBeFreed.ql @@ -55,7 +55,7 @@ predicate allocCallOrIndirect(Expr e) { * can cause memory leaks. */ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode verified) { - reallocCall.getTarget().hasQualifiedName("realloc") and + reallocCall.getTarget().hasGlobalName("realloc") and reallocCall.getArgument(0) = v.getAnAccess() and ( exists(Variable newV, ControlFlowNode node | @@ -82,7 +82,7 @@ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode predicate freeCallOrIndirect(ControlFlowNode n, Variable v) { // direct free call freeCall(n, v.getAnAccess()) and - not n.(FunctionCall).getTarget().hasQualifiedName("realloc") + not n.(FunctionCall).getTarget().hasGlobalName("realloc") or // verified realloc call verifiedRealloc(_, v, n) diff --git a/cpp/ql/src/Critical/OverflowCalculated.ql b/cpp/ql/src/Critical/OverflowCalculated.ql index 5f7c132a3518..36ee0140cf7e 100644 --- a/cpp/ql/src/Critical/OverflowCalculated.ql +++ b/cpp/ql/src/Critical/OverflowCalculated.ql @@ -14,8 +14,8 @@ import cpp class MallocCall extends FunctionCall { MallocCall() { - this.getTarget().hasQualifiedName("malloc") or - this.getTarget().hasQualifiedName("std::malloc") + this.getTarget().hasGlobalName("malloc") or + this.getTarget().hasQualifiedName("std", "malloc") } Expr getAllocatedSize() { @@ -36,12 +36,12 @@ predicate spaceProblem(FunctionCall append, string msg) { malloc.getAllocatedSize() = add and buffer.getAnAccess() = strlen.getStringExpr() and ( - insert.getTarget().hasQualifiedName("strcpy") or - insert.getTarget().hasQualifiedName("strncpy") + insert.getTarget().hasGlobalName("strcpy") or + insert.getTarget().hasGlobalName("strncpy") ) and ( - append.getTarget().hasQualifiedName("strcat") or - append.getTarget().hasQualifiedName("strncat") + append.getTarget().hasGlobalName("strcat") or + append.getTarget().hasGlobalName("strncat") ) and malloc.getASuccessor+() = insert and insert.getArgument(1) = buffer.getAnAccess() and diff --git a/cpp/ql/src/Critical/OverflowDestination.ql b/cpp/ql/src/Critical/OverflowDestination.ql index 09136c729097..c72fe16e9842 100644 --- a/cpp/ql/src/Critical/OverflowDestination.ql +++ b/cpp/ql/src/Critical/OverflowDestination.ql @@ -25,7 +25,7 @@ import semmle.code.cpp.security.TaintTracking predicate sourceSized(FunctionCall fc, Expr src) { exists(string name | (name = "strncpy" or name = "strncat" or name = "memcpy" or name = "memmove") and - fc.getTarget().hasQualifiedName(name) + fc.getTarget().hasGlobalName(name) ) and exists(Expr dest, Expr size, Variable v | fc.getArgument(0) = dest and diff --git a/cpp/ql/src/Critical/OverflowStatic.ql b/cpp/ql/src/Critical/OverflowStatic.ql index cccfb93b959c..a0917c470244 100644 --- a/cpp/ql/src/Critical/OverflowStatic.ql +++ b/cpp/ql/src/Critical/OverflowStatic.ql @@ -59,21 +59,21 @@ predicate overflowOffsetInLoop(BufferAccess bufaccess, string msg) { } predicate bufferAndSizeFunction(Function f, int buf, int size) { - f.hasQualifiedName("read") and buf = 1 and size = 2 + f.hasGlobalName("read") and buf = 1 and size = 2 or - f.hasQualifiedName("fgets") and buf = 0 and size = 1 + f.hasGlobalName("fgets") and buf = 0 and size = 1 or - f.hasQualifiedName("strncpy") and buf = 0 and size = 2 + f.hasGlobalName("strncpy") and buf = 0 and size = 2 or - f.hasQualifiedName("strncat") and buf = 0 and size = 2 + f.hasGlobalName("strncat") and buf = 0 and size = 2 or - f.hasQualifiedName("memcpy") and buf = 0 and size = 2 + f.hasGlobalName("memcpy") and buf = 0 and size = 2 or - f.hasQualifiedName("memmove") and buf = 0 and size = 2 + f.hasGlobalName("memmove") and buf = 0 and size = 2 or - f.hasQualifiedName("snprintf") and buf = 0 and size = 1 + f.hasGlobalName("snprintf") and buf = 0 and size = 1 or - f.hasQualifiedName("vsnprintf") and buf = 0 and size = 1 + f.hasGlobalName("vsnprintf") and buf = 0 and size = 1 } class CallWithBufferSize extends FunctionCall { diff --git a/cpp/ql/src/Critical/UseAfterFree.ql b/cpp/ql/src/Critical/UseAfterFree.ql index 16e165d11bab..db78c206ea1a 100644 --- a/cpp/ql/src/Critical/UseAfterFree.ql +++ b/cpp/ql/src/Critical/UseAfterFree.ql @@ -16,7 +16,7 @@ import semmle.code.cpp.controlflow.LocalScopeVariableReachability predicate isFreeExpr(Expr e, LocalScopeVariable v) { exists(VariableAccess va | va.getTarget() = v | exists(FunctionCall fc | fc = e | - fc.getTarget().hasQualifiedName("free") and + fc.getTarget().hasGlobalName("free") and va = fc.getArgument(0) ) or diff --git a/cpp/ql/src/DefaultOptions.qll b/cpp/ql/src/DefaultOptions.qll index 1ab26a01d683..02759e27d421 100644 --- a/cpp/ql/src/DefaultOptions.qll +++ b/cpp/ql/src/DefaultOptions.qll @@ -32,7 +32,7 @@ class Options extends string */ predicate overrideReturnsNull(Call call) { // Used in CVS: - call.(FunctionCall).getTarget().hasQualifiedName("Xstrdup") + call.(FunctionCall).getTarget().hasGlobalName("Xstrdup") or CustomOptions::overrideReturnsNull(call) // old Options.qll } @@ -46,7 +46,7 @@ class Options extends string */ predicate returnsNull(Call call) { // Used in CVS: - call.(FunctionCall).getTarget().hasQualifiedName("Xstrdup") and + call.(FunctionCall).getTarget().hasGlobalName("Xstrdup") and nullValue(call.getArgument(0)) or CustomOptions::returnsNull(call) // old Options.qll @@ -92,7 +92,7 @@ class Options extends string * By default holds only for `fgets`. */ predicate alwaysCheckReturnValue(Function f) { - f.hasQualifiedName("fgets") or + f.hasGlobalName("fgets") or CustomOptions::alwaysCheckReturnValue(f) // old Options.qll } @@ -108,7 +108,7 @@ class Options extends string fc.isInMacroExpansion() or // common way of sleeping using select: - (fc.getTarget().hasQualifiedName("select") and + (fc.getTarget().hasGlobalName("select") and fc.getArgument(0).getValue() = "0") or CustomOptions::okToIgnoreReturnValue(fc) // old Options.qll diff --git a/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql b/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql index 217886ca8c39..f297171a9d1c 100644 --- a/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql +++ b/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql @@ -16,8 +16,8 @@ import semmle.code.cpp.security.TaintTracking /** A call that prints its arguments to `stdout`. */ class PrintStdoutCall extends FunctionCall { PrintStdoutCall() { - getTarget().hasQualifiedName("puts") or - getTarget().hasQualifiedName("printf") + getTarget().hasGlobalName("puts") or + getTarget().hasGlobalName("printf") } } diff --git a/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql b/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql index 30872663b005..cfd70334a313 100644 --- a/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql +++ b/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql @@ -73,9 +73,9 @@ class VarargsFunction extends Function { } predicate isWhitelisted() { - this.hasQualifiedName("open") or - this.hasQualifiedName("fcntl") or - this.hasQualifiedName("ptrace") + this.hasGlobalName("open") or + this.hasGlobalName("fcntl") or + this.hasGlobalName("ptrace") } } diff --git a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql index 1f15e305690d..d3813e6bcdd1 100644 --- a/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql +++ b/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql @@ -18,8 +18,8 @@ import cpp class MallocCall extends FunctionCall { MallocCall() { - this.getTarget().hasQualifiedName("malloc") or - this.getTarget().hasQualifiedName("std::malloc") + this.getTarget().hasGlobalName("malloc") or + this.getTarget().hasQualifiedName("std", "malloc") } Expr getAllocatedSize() { diff --git a/cpp/ql/src/semmle/code/cpp/Print.qll b/cpp/ql/src/semmle/code/cpp/Print.qll index a63161be3519..38c3eac414a5 100644 --- a/cpp/ql/src/semmle/code/cpp/Print.qll +++ b/cpp/ql/src/semmle/code/cpp/Print.qll @@ -40,7 +40,7 @@ private abstract class DumpDeclaration extends Declaration { * Gets a string that uniquely identifies this declaration, suitable for use when debugging queries. Only holds for * functions, user-defined types, global and namespace-scope variables, and member variables. * - * This operation is very expensive, and should not be used in production queries. Consider using `hasName()` or + * This operation is very expensive, and should not be used in production queries. Consider using * `hasQualifiedName()` for identifying known declarations in production queries. */ string getIdentityString() { diff --git a/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll b/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll index 4e6651668fae..ecc82ba81473 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Alloc.qll @@ -6,7 +6,7 @@ import cpp predicate allocationFunction(Function f) { exists(string name | - f.hasQualifiedName(name) and + f.hasGlobalName(name) and ( name = "malloc" or name = "calloc" or @@ -61,7 +61,7 @@ predicate allocationCall(FunctionCall fc) allocationFunction(fc.getTarget()) and ( // realloc(ptr, 0) only frees the pointer - fc.getTarget().hasQualifiedName("realloc") implies + fc.getTarget().hasGlobalName("realloc") implies not fc.getArgument(1).getValue() = "0" ) } @@ -72,7 +72,7 @@ predicate allocationCall(FunctionCall fc) predicate freeFunction(Function f, int argNum) { exists(string name | - f.hasQualifiedName(name) and + f.hasGlobalName(name) and ( (name = "free" and argNum = 0) or (name = "realloc" and argNum = 0) or diff --git a/cpp/ql/src/semmle/code/cpp/commons/File.qll b/cpp/ql/src/semmle/code/cpp/commons/File.qll index 602eccb33b05..fd9d1f46f37d 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/File.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/File.qll @@ -6,16 +6,16 @@ import cpp predicate fopenCall(FunctionCall fc) { exists(Function f | f = fc.getTarget() | - f.hasQualifiedName("fopen") or - f.hasQualifiedName("open") or - f.hasQualifiedName("_open") or - f.hasQualifiedName("_wopen") or - f.hasQualifiedName("CreateFile") or - f.hasQualifiedName("CreateFileA") or - f.hasQualifiedName("CreateFileW") or - f.hasQualifiedName("CreateFileTransacted") or - f.hasQualifiedName("CreateFileTransactedA") or - f.hasQualifiedName("CreateFileTransactedW") + f.hasGlobalName("fopen") or + f.hasGlobalName("open") or + f.hasGlobalName("_open") or + f.hasGlobalName("_wopen") or + f.hasGlobalName("CreateFile") or + f.hasGlobalName("CreateFileA") or + f.hasGlobalName("CreateFileW") or + f.hasGlobalName("CreateFileTransacted") or + f.hasGlobalName("CreateFileTransactedA") or + f.hasGlobalName("CreateFileTransactedW") ) } @@ -26,16 +26,16 @@ predicate fcloseCall(FunctionCall fc, Expr closed) { exists(Function f | f = fc.getTarget() | ( - f.hasQualifiedName("fclose") and + f.hasGlobalName("fclose") and closed = fc.getArgument(0) ) or ( - f.hasQualifiedName("close") and + f.hasGlobalName("close") and closed = fc.getArgument(0) ) or ( - f.hasQualifiedName("_close") and + f.hasGlobalName("_close") and closed = fc.getArgument(0) ) or ( - f.hasQualifiedName("CloseHandle") and + f.hasGlobalName("CloseHandle") and closed = fc.getArgument(0) ) ) diff --git a/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll b/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll index f61bac3963f6..555ea507bba7 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/StringAnalysis.qll @@ -56,12 +56,12 @@ class AnalysedString extends Expr */ class StrlenCall extends FunctionCall { StrlenCall() { - this.getTarget().hasQualifiedName("strlen") or - this.getTarget().hasQualifiedName("wcslen") or - this.getTarget().hasQualifiedName("_mbslen") or - this.getTarget().hasQualifiedName("_mbslen_l") or - this.getTarget().hasQualifiedName("_mbstrlen") or - this.getTarget().hasQualifiedName("_mbstrlen_l") + this.getTarget().hasGlobalName("strlen") or + this.getTarget().hasGlobalName("wcslen") or + this.getTarget().hasGlobalName("_mbslen") or + this.getTarget().hasGlobalName("_mbslen_l") or + this.getTarget().hasGlobalName("_mbstrlen") or + this.getTarget().hasGlobalName("_mbstrlen_l") } /** diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll b/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll index 69ffc314d374..54c08fc2d7f8 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/Dereferenced.qll @@ -7,7 +7,7 @@ import Nullness predicate callDereferences(FunctionCall fc, int i) { exists(string name | - fc.getTarget().hasQualifiedName(name) and + fc.getTarget().hasGlobalName(name) and ( (name = "bcopy" and i in [0..1]) or (name = "memcpy" and i in [0..1]) or diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll b/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll index e398e182d4de..9b35fda199f9 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/Nullness.qll @@ -47,7 +47,7 @@ predicate nullCheckExpr(Expr checkExpr, Variable var) or exists(FunctionCall fc, AnalysedExpr child | expr = fc and - fc.getTarget().hasQualifiedName("__builtin_expect") and + fc.getTarget().hasGlobalName("__builtin_expect") and fc.getArgument(0) = child and nullCheckExpr(child, v)) ) } @@ -87,7 +87,7 @@ predicate validCheckExpr(Expr checkExpr, Variable var) or exists(FunctionCall fc, AnalysedExpr child | expr = fc and - fc.getTarget().hasQualifiedName("__builtin_expect") and + fc.getTarget().hasGlobalName("__builtin_expect") and fc.getArgument(0) = child and validCheckExpr(child, v)) ) } diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll index 3d47e5c88166..655bec40dd1e 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Inet.qll @@ -3,7 +3,7 @@ import semmle.code.cpp.models.interfaces.ArrayFunction class InetNtoa extends TaintFunction { InetNtoa() { - hasQualifiedName("inet_ntoa") + hasGlobalName("inet_ntoa") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -14,7 +14,7 @@ class InetNtoa extends TaintFunction { class InetAton extends TaintFunction, ArrayFunction { InetAton() { - hasQualifiedName("inet_aton") + hasGlobalName("inet_aton") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -42,7 +42,7 @@ class InetAton extends TaintFunction, ArrayFunction { class InetAddr extends TaintFunction, ArrayFunction { InetAddr() { - hasQualifiedName("inet_addr") + hasGlobalName("inet_addr") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -61,7 +61,7 @@ class InetAddr extends TaintFunction, ArrayFunction { class InetNetwork extends TaintFunction, ArrayFunction { InetNetwork() { - hasQualifiedName("inet_network") + hasGlobalName("inet_network") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -80,7 +80,7 @@ class InetNetwork extends TaintFunction, ArrayFunction { class InetMakeaddr extends TaintFunction { InetMakeaddr() { - hasQualifiedName("inet_makeaddr") + hasGlobalName("inet_makeaddr") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -94,7 +94,7 @@ class InetMakeaddr extends TaintFunction { class InetLnaof extends TaintFunction { InetLnaof() { - hasQualifiedName("inet_lnaof") + hasGlobalName("inet_lnaof") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -105,7 +105,7 @@ class InetLnaof extends TaintFunction { class InetNetof extends TaintFunction { InetNetof() { - hasQualifiedName("inet_netof") + hasGlobalName("inet_netof") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -116,7 +116,7 @@ class InetNetof extends TaintFunction { class InetPton extends TaintFunction, ArrayFunction { InetPton() { - hasQualifiedName("inet_pton") + hasGlobalName("inet_pton") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -146,7 +146,7 @@ class InetPton extends TaintFunction, ArrayFunction { class Gethostbyname extends TaintFunction, ArrayFunction { Gethostbyname() { - hasQualifiedName("gethostbyname") + hasGlobalName("gethostbyname") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { @@ -165,7 +165,7 @@ class Gethostbyname extends TaintFunction, ArrayFunction { class Gethostbyaddr extends TaintFunction, ArrayFunction { Gethostbyaddr() { - hasQualifiedName("gethostbyaddr") + hasGlobalName("gethostbyaddr") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll index da64cec94db5..bde5b4461bc9 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Strftime.qll @@ -3,7 +3,7 @@ import semmle.code.cpp.models.interfaces.ArrayFunction class Strftime extends TaintFunction, ArrayFunction { Strftime() { - hasQualifiedName("strftime") + hasGlobalName("strftime") } override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) { diff --git a/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll b/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll index 6f1ab3758262..1baa9141cd7c 100644 --- a/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll +++ b/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll @@ -588,7 +588,7 @@ predicate allocateDescriptorCall(FunctionCall fc) { exists(string name | name = "socket" and - fc.getTarget().hasQualifiedName(name)) + fc.getTarget().hasGlobalName(name)) } /** diff --git a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll index 72323f8b4727..4a8d95f84bdb 100644 --- a/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll +++ b/cpp/ql/src/semmle/code/cpp/security/CommandExecution.qll @@ -9,13 +9,13 @@ import semmle.code.cpp.security.FunctionWithWrappers */ class SystemFunction extends FunctionWithWrappers { SystemFunction() { - hasQualifiedName("system") - or hasQualifiedName("popen") + hasGlobalName("system") + or hasGlobalName("popen") // Windows variants - or hasQualifiedName("_popen") - or hasQualifiedName("_wpopen") - or hasQualifiedName("_wsystem") + or hasGlobalName("_popen") + or hasGlobalName("_wpopen") + or hasGlobalName("_wsystem") } override predicate interestingArg(int arg) { @@ -31,36 +31,36 @@ class SystemFunction extends FunctionWithWrappers { */ class VarargsExecFunctionCall extends FunctionCall { VarargsExecFunctionCall() { - getTarget().hasQualifiedName("execl") - or getTarget().hasQualifiedName("execle") - or getTarget().hasQualifiedName("execlp") + getTarget().hasGlobalName("execl") + or getTarget().hasGlobalName("execle") + or getTarget().hasGlobalName("execlp") // Windows - or getTarget().hasQualifiedName("_execl") - or getTarget().hasQualifiedName("_execle") - or getTarget().hasQualifiedName("_execlp") - or getTarget().hasQualifiedName("_execlpe") - or getTarget().hasQualifiedName("_spawnl") - or getTarget().hasQualifiedName("_spawnle") - or getTarget().hasQualifiedName("_spawnlp") - or getTarget().hasQualifiedName("_spawnlpe") - or getTarget().hasQualifiedName("_wexecl") - or getTarget().hasQualifiedName("_wexecle") - or getTarget().hasQualifiedName("_wexeclp") - or getTarget().hasQualifiedName("_wexeclpe") - or getTarget().hasQualifiedName("_wspawnl") - or getTarget().hasQualifiedName("_wspawnle") - or getTarget().hasQualifiedName("_wspawnlp") - or getTarget().hasQualifiedName("_wspawnlpe") + or getTarget().hasGlobalName("_execl") + or getTarget().hasGlobalName("_execle") + or getTarget().hasGlobalName("_execlp") + or getTarget().hasGlobalName("_execlpe") + or getTarget().hasGlobalName("_spawnl") + or getTarget().hasGlobalName("_spawnle") + or getTarget().hasGlobalName("_spawnlp") + or getTarget().hasGlobalName("_spawnlpe") + or getTarget().hasGlobalName("_wexecl") + or getTarget().hasGlobalName("_wexecle") + or getTarget().hasGlobalName("_wexeclp") + or getTarget().hasGlobalName("_wexeclpe") + or getTarget().hasGlobalName("_wspawnl") + or getTarget().hasGlobalName("_wspawnle") + or getTarget().hasGlobalName("_wspawnlp") + or getTarget().hasGlobalName("_wspawnlpe") } /** Whether the last argument to the function is an environment pointer */ predicate hasEnvironmentArgument() { - getTarget().hasQualifiedName("execle") - or getTarget().hasQualifiedName("_execle") - or getTarget().hasQualifiedName("_execlpe") - or getTarget().hasQualifiedName("_wexecle") - or getTarget().hasQualifiedName("_wexeclpe") + getTarget().hasGlobalName("execle") + or getTarget().hasGlobalName("_execle") + or getTarget().hasGlobalName("_execlpe") + or getTarget().hasGlobalName("_wexecle") + or getTarget().hasGlobalName("_wexeclpe") } /** The arguments passed to the command. The 0th such argument is conventionally @@ -100,27 +100,27 @@ class VarargsExecFunctionCall extends FunctionCall { */ class ArrayExecFunctionCall extends FunctionCall { ArrayExecFunctionCall() { - getTarget().hasQualifiedName("execv") - or getTarget().hasQualifiedName("execvp") - or getTarget().hasQualifiedName("execvpe") + getTarget().hasGlobalName("execv") + or getTarget().hasGlobalName("execvp") + or getTarget().hasGlobalName("execvpe") // Windows variants - or getTarget().hasQualifiedName("_execv") - or getTarget().hasQualifiedName("_execve") - or getTarget().hasQualifiedName("_execvp") - or getTarget().hasQualifiedName("_execvpe") - or getTarget().hasQualifiedName("_spawnv") - or getTarget().hasQualifiedName("_spawnve") - or getTarget().hasQualifiedName("_spawnvp") - or getTarget().hasQualifiedName("_spawnvpe") - or getTarget().hasQualifiedName("_wexecv") - or getTarget().hasQualifiedName("_wexecve") - or getTarget().hasQualifiedName("_wexecvp") - or getTarget().hasQualifiedName("_wexecvpe") - or getTarget().hasQualifiedName("_wspawnv") - or getTarget().hasQualifiedName("_wspawnve") - or getTarget().hasQualifiedName("_wspawnvp") - or getTarget().hasQualifiedName("_wspawnvpe") + or getTarget().hasGlobalName("_execv") + or getTarget().hasGlobalName("_execve") + or getTarget().hasGlobalName("_execvp") + or getTarget().hasGlobalName("_execvpe") + or getTarget().hasGlobalName("_spawnv") + or getTarget().hasGlobalName("_spawnve") + or getTarget().hasGlobalName("_spawnvp") + or getTarget().hasGlobalName("_spawnvpe") + or getTarget().hasGlobalName("_wexecv") + or getTarget().hasGlobalName("_wexecve") + or getTarget().hasGlobalName("_wexecvp") + or getTarget().hasGlobalName("_wexecvpe") + or getTarget().hasGlobalName("_wspawnv") + or getTarget().hasGlobalName("_wspawnve") + or getTarget().hasGlobalName("_wspawnvp") + or getTarget().hasGlobalName("_wspawnvpe") } /** The argument with the array of command arguments */ From 6d954fe53ebfb6a1cae430be656615e8fa1a2286 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 3 May 2019 10:10:36 +0200 Subject: [PATCH 06/12] C++: Deprecate hasQualifiedName/1 This predicate handles templates differently from the other overloads with the same name, so it's likely to cause confusion. --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 22f30e1c3773..40e43f1ff60c 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -57,9 +57,14 @@ abstract class Declaration extends Locatable, @declaration { } /** + * DEPRECATED: Prefer `hasGlobalName` or the 2-argument or 3-argument + * `hasQualifiedName` predicates. To get the exact same results as this + * predicate in all edge cases, use `getQualifiedName()`. + * * Holds if this declaration has the fully-qualified name `qualifiedName`. * See `getQualifiedName`. */ + deprecated predicate hasQualifiedName(string qualifiedName) { this.getQualifiedName() = qualifiedName } From b98daae077593ef8105b6bf2ecbb4e38460dd790 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 3 May 2019 13:22:23 +0200 Subject: [PATCH 07/12] C++: Remove `deprecated` from hasQualifiedName/1 The predicate is still deprecated, but we can't mark it as such until the queries in our internal repo have migrated away from it. --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 40e43f1ff60c..7d8423cfb18b 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -64,7 +64,6 @@ abstract class Declaration extends Locatable, @declaration { * Holds if this declaration has the fully-qualified name `qualifiedName`. * See `getQualifiedName`. */ - deprecated predicate hasQualifiedName(string qualifiedName) { this.getQualifiedName() = qualifiedName } From 98657ebea70b70b6dfabaf00f38c7352461d3ff8 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 6 May 2019 10:14:30 +0200 Subject: [PATCH 08/12] C++: Change note for hasGlobalName --- change-notes/1.21/analysis-cpp.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/change-notes/1.21/analysis-cpp.md b/change-notes/1.21/analysis-cpp.md index 882b1bba7cae..7b63786dbb0a 100644 --- a/change-notes/1.21/analysis-cpp.md +++ b/change-notes/1.21/analysis-cpp.md @@ -30,6 +30,8 @@ | `()`-declared function called with too many arguments (`cpp/futile-params`) | Improved coverage | Query has been generalized to find all cases where the number of arguments exceedes the number of parameters of the function, provided the function is also properly declared/defined elsewhere. | ## 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. - 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. From 662d55fd72434847f58013b1d5c738d05900e940 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 6 May 2019 10:58:05 +0200 Subject: [PATCH 09/12] C++: Add tests for qualified names --- .../qualified_names/qualifiedNames.cpp | 61 +++++++++++++++++++ .../qualified_names/qualifiedNames.expected | 54 ++++++++++++++++ .../qualified_names/qualifiedNames.ql | 13 ++++ .../qualified_names/unnamed.expected | 19 ++++++ .../identifiers/qualified_names/unnamed.ql | 13 ++++ 5 files changed, 160 insertions(+) create mode 100644 cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp create mode 100644 cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected create mode 100644 cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.ql create mode 100644 cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.expected create mode 100644 cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.ql diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp new file mode 100644 index 000000000000..f340bc20b350 --- /dev/null +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.cpp @@ -0,0 +1,61 @@ + + +void overloadedFunction(int); +void overloadedFunction(void *); + +enum Enum { enumConstant }; +enum class EnumClass { classEnumConstant }; + +class C { + void privateFunction(); +public: + void publicFunction(); + + enum Enum { enumConstant }; + enum class EnumClass { classEnumConstant }; + + class Nested { + friend class FriendOfNested; + Nested(); + + enum Enum { enumConstant }; + }; +}; + +int main() { + class LocalClass { + int localClassField; + }; + return 0; +} + +namespace ns { + int f(); + class C; +} + +typedef struct { + int structField; +} typedefStructName, *ptypedefStructName; + +typedef C typedefC; + +namespace templates { + template + struct TemplateClass { + T x; + + template + const T &getMember(Ignored ignored) const { return x; } + }; + + template + const T &getMember(TemplateClass &tc, Ignored ignored) { + return tc.getMember(ignored); + } + + long use() { + TemplateClass tc = { 22 }; + return getMember(tc, typedefC()); + } +} diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected new file mode 100644 index 000000000000..c9807e801ea6 --- /dev/null +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.expected @@ -0,0 +1,54 @@ +| file://:0:0:0:0 | __va_list_tag | __va_list_tag | | | __va_list_tag | __va_list_tag | +| file://:0:0:0:0 | fp_offset | __va_list_tag::fp_offset | | __va_list_tag | fp_offset | (not global) | +| file://:0:0:0:0 | gp_offset | __va_list_tag::gp_offset | | __va_list_tag | gp_offset | (not global) | +| file://:0:0:0:0 | operator= | __va_list_tag::operator= | | __va_list_tag | operator= | (not global) | +| file://:0:0:0:0 | operator= | __va_list_tag::operator= | | __va_list_tag | operator= | (not global) | +| file://:0:0:0:0 | overflow_arg_area | __va_list_tag::overflow_arg_area | | __va_list_tag | overflow_arg_area | (not global) | +| file://:0:0:0:0 | reg_save_area | __va_list_tag::reg_save_area | | __va_list_tag | reg_save_area | (not global) | +| qualifiedNames.cpp:3:6:3:23 | overloadedFunction | overloadedFunction | | | overloadedFunction | overloadedFunction | +| qualifiedNames.cpp:4:6:4:23 | overloadedFunction | overloadedFunction | | | overloadedFunction | overloadedFunction | +| qualifiedNames.cpp:6:6:6:9 | Enum | Enum | | | Enum | Enum | +| qualifiedNames.cpp:6:13:6:24 | enumConstant | Enum::enumConstant | | Enum | enumConstant | (not global) | +| qualifiedNames.cpp:7:12:7:20 | EnumClass | EnumClass | | | EnumClass | EnumClass | +| qualifiedNames.cpp:7:24:7:40 | classEnumConstant | EnumClass::classEnumConstant | | EnumClass | classEnumConstant | (not global) | +| qualifiedNames.cpp:9:7:9:7 | C | C | | | C | C | +| qualifiedNames.cpp:9:7:9:7 | operator= | C::operator= | | C | operator= | (not global) | +| qualifiedNames.cpp:9:7:9:7 | operator= | C::operator= | | C | operator= | (not global) | +| qualifiedNames.cpp:10:8:10:22 | privateFunction | C::privateFunction | | C | privateFunction | (not global) | +| qualifiedNames.cpp:12:8:12:21 | publicFunction | C::publicFunction | | C | publicFunction | (not global) | +| qualifiedNames.cpp:14:8:14:11 | Enum | C::Enum | | C | Enum | (not global) | +| qualifiedNames.cpp:14:15:14:26 | enumConstant | C::Enum::enumConstant | | C::Enum | enumConstant | (not global) | +| qualifiedNames.cpp:15:14:15:22 | EnumClass | C::EnumClass | | C | EnumClass | (not global) | +| qualifiedNames.cpp:15:26:15:42 | classEnumConstant | C::EnumClass::classEnumConstant | | C::EnumClass | classEnumConstant | (not global) | +| qualifiedNames.cpp:17:9:17:9 | Nested | C::Nested::Nested | | C::Nested | Nested | (not global) | +| qualifiedNames.cpp:17:9:17:9 | Nested | C::Nested::Nested | | C::Nested | Nested | (not global) | +| qualifiedNames.cpp:17:9:17:9 | operator= | C::Nested::operator= | | C::Nested | operator= | (not global) | +| qualifiedNames.cpp:17:9:17:9 | operator= | C::Nested::operator= | | C::Nested | operator= | (not global) | +| qualifiedNames.cpp:17:9:17:14 | Nested | C::Nested | | C | Nested | (not global) | +| qualifiedNames.cpp:18:18:18:31 | FriendOfNested | FriendOfNested | | | FriendOfNested | FriendOfNested | +| qualifiedNames.cpp:19:5:19:10 | Nested | C::Nested::Nested | | C::Nested | Nested | (not global) | +| qualifiedNames.cpp:21:10:21:13 | Enum | C::Nested::Enum | | C::Nested | Enum | (not global) | +| qualifiedNames.cpp:21:17:21:28 | enumConstant | C::Nested::Enum::enumConstant | | C::Nested::Enum | enumConstant | (not global) | +| qualifiedNames.cpp:25:5:25:8 | main | main | | | main | main | +| qualifiedNames.cpp:33:7:33:7 | f | ns::f | ns | | f | (not global) | +| qualifiedNames.cpp:34:9:34:9 | C | ns::C | ns | | C | (not global) | +| qualifiedNames.cpp:37:16:37:16 | operator= | typedefStructName::operator= | | typedefStructName | operator= | (not global) | +| qualifiedNames.cpp:37:16:37:16 | operator= | typedefStructName::operator= | | typedefStructName | operator= | (not global) | +| qualifiedNames.cpp:37:16:37:32 | typedefStructName | typedefStructName | | | typedefStructName | typedefStructName | +| qualifiedNames.cpp:38:7:38:17 | structField | typedefStructName::structField | | typedefStructName | structField | (not global) | +| qualifiedNames.cpp:39:3:39:19 | typedefStructName | typedefStructName | | | typedefStructName | typedefStructName | +| qualifiedNames.cpp:39:23:39:40 | ptypedefStructName | ptypedefStructName | | | ptypedefStructName | ptypedefStructName | +| qualifiedNames.cpp:41:11:41:18 | typedefC | typedefC | | | typedefC | typedefC | +| qualifiedNames.cpp:45:10:45:10 | operator= | templates::TemplateClass::operator= | templates | TemplateClass | operator= | (not global) | +| qualifiedNames.cpp:45:10:45:10 | operator= | templates::TemplateClass::operator= | templates | TemplateClass | operator= | (not global) | +| qualifiedNames.cpp:45:10:45:22 | TemplateClass | templates::TemplateClass | templates | | TemplateClass | (not global) | +| qualifiedNames.cpp:45:10:45:22 | TemplateClass | templates::TemplateClass | templates | | TemplateClass | (not global) | +| qualifiedNames.cpp:45:10:45:22 | TemplateClass | templates::TemplateClass | templates | | TemplateClass | (not global) | +| qualifiedNames.cpp:46:7:46:7 | x | templates::TemplateClass::x | templates | TemplateClass | x | (not global) | +| qualifiedNames.cpp:46:7:46:7 | x | templates::TemplateClass::x | templates | TemplateClass | x | (not global) | +| qualifiedNames.cpp:49:14:49:14 | getMember | templates::TemplateClass::getMember | templates | TemplateClass | getMember | (not global) | +| qualifiedNames.cpp:49:14:49:22 | getMember | templates::TemplateClass::getMember | templates | TemplateClass | getMember | (not global) | +| qualifiedNames.cpp:49:14:49:22 | getMember | templates::TemplateClass::getMember | templates | TemplateClass | getMember | (not global) | +| 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) | diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.ql b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.ql new file mode 100644 index 000000000000..76b392f4d83e --- /dev/null +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/qualifiedNames.ql @@ -0,0 +1,13 @@ +import cpp + +from + Declaration d, string namespaceQualifier, string typeQualifier, string baseName, string globalName +where + d.hasQualifiedName(namespaceQualifier, typeQualifier, baseName) and + ( + d.hasGlobalName(globalName) + or + not d.hasGlobalName(_) and + globalName = "(not global)" + ) +select d, d.getQualifiedName(), namespaceQualifier, typeQualifier, baseName, globalName diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.expected b/cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.expected new file mode 100644 index 000000000000..04c877da4ca4 --- /dev/null +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.expected @@ -0,0 +1,19 @@ +| qualifiedNames.cpp:3:25:3:27 | p#0 | +| qualifiedNames.cpp:4:25:4:28 | p#0 | +| qualifiedNames.cpp:18:18:18:31 | Nested's friend | +| qualifiedNames.cpp:26:9:26:9 | operator= | +| qualifiedNames.cpp:26:9:26:9 | operator= | +| qualifiedNames.cpp:26:9:26:18 | LocalClass | +| qualifiedNames.cpp:27:9:27:23 | localClassField | +| qualifiedNames.cpp:44:21:44:21 | T | +| qualifiedNames.cpp:48:23:48:29 | Ignored | +| qualifiedNames.cpp:49:32:49:38 | ignored | +| qualifiedNames.cpp:49:32:49:38 | ignored | +| qualifiedNames.cpp:49:32:49:38 | ignored | +| qualifiedNames.cpp:52:21:52:21 | T | +| qualifiedNames.cpp:52:33:52:39 | Ignored | +| qualifiedNames.cpp:53:40:53:41 | tc | +| qualifiedNames.cpp:53:40:53:41 | tc | +| qualifiedNames.cpp:53:52:53:58 | ignored | +| qualifiedNames.cpp:53:52:53:58 | ignored | +| qualifiedNames.cpp:58:34:58:35 | tc | diff --git a/cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.ql b/cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.ql new file mode 100644 index 000000000000..866035f0ebb6 --- /dev/null +++ b/cpp/ql/test/library-tests/identifiers/qualified_names/unnamed.ql @@ -0,0 +1,13 @@ +// This query lists the declarations that don't have a qualified name + +import cpp + +from Declaration d +where + ( + not exists(d.getQualifiedName()) + or + not d.hasQualifiedName(_, _, _) + ) and + exists(d.getFile().getRelativePath()) +select d From 56e88cbac01e278aab1e7928a37e010ffe779a7c Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 6 May 2019 11:24:28 +0200 Subject: [PATCH 10/12] C++: Use underlyingElement for QualifiedName calls Since the types in `QualifiedName.qll` are raw db types, callers need to use `underlyingElement` and `unresolveElement` as appropriate. This has no effect right now but will be needed when we switch the AST type hierarchy to `newtype`s. --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 9 +++++---- cpp/ql/src/semmle/code/cpp/Parameter.qll | 8 +++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 7d8423cfb18b..5912d7ff281a 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -31,7 +31,7 @@ abstract class Declaration extends Locatable, @declaration { * namespace of the structure. */ Namespace getNamespace() { - result = this.(Q::Declaration).getNamespace() + result = underlyingElement(this).(Q::Declaration).getNamespace() or exists (Parameter p | p = this and result = p.getFunction().getNamespace()) @@ -53,7 +53,7 @@ abstract class Declaration extends Locatable, @declaration { * "namespace1::namespace2::TemplateClass1::Class2::memberName"`. */ string getQualifiedName() { - result = this.(Q::Declaration).getQualifiedName() + result = underlyingElement(this).(Q::Declaration).getQualifiedName() } /** @@ -84,7 +84,8 @@ abstract class Declaration extends Locatable, @declaration { * `hasQualifiedName("std", "vector", "size")`. */ predicate hasQualifiedName(string namespaceQualifier, string typeQualifier, string baseName) { - this.(Q::Declaration).hasQualifiedName(namespaceQualifier, typeQualifier, baseName) + underlyingElement(this).(Q::Declaration) + .hasQualifiedName(namespaceQualifier, typeQualifier, baseName) } /** @@ -115,7 +116,7 @@ abstract class Declaration extends Locatable, @declaration { * To test whether this declaration has a particular name in the global * namespace, use `hasGlobalName`. */ - string getName() { result = this.(Q::Declaration).getName() } + string getName() { result = underlyingElement(this).(Q::Declaration).getName() } /** Holds if this declaration has the given name. */ predicate hasName(string name) { name = this.getName() } diff --git a/cpp/ql/src/semmle/code/cpp/Parameter.qll b/cpp/ql/src/semmle/code/cpp/Parameter.qll index 5a3c3e59ef2e..14f0ee462c56 100644 --- a/cpp/ql/src/semmle/code/cpp/Parameter.qll +++ b/cpp/ql/src/semmle/code/cpp/Parameter.qll @@ -55,7 +55,9 @@ class Parameter extends LocalScopeVariable, @parameter { * In other words, this predicate holds precisely when the result of * `getName()` is not "p#i" (where `i` is the index of the parameter). */ - predicate isNamed() { exists(this.(Q::Parameter).getANamedDeclarationEntry()) } + predicate isNamed() { + exists(underlyingElement(this).(Q::Parameter).getANamedDeclarationEntry()) + } /** * Gets the function to which this parameter belongs, if it is a function @@ -96,12 +98,12 @@ class Parameter extends LocalScopeVariable, @parameter { */ override Location getLocation() { exists(VariableDeclarationEntry vde | - vde = this.(Q::Parameter).getAnEffectiveDeclarationEntry() and + vde = underlyingElement(this).(Q::Parameter).getAnEffectiveDeclarationEntry() and result = vde.getLocation() | vde.isDefinition() or - not this.(Q::Parameter).getAnEffectiveDeclarationEntry().isDefinition() + not underlyingElement(this).(Q::Parameter).getAnEffectiveDeclarationEntry().isDefinition() ) } } From b52015a5843c427eaa99e94eca1429d287923e10 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 6 May 2019 11:28:56 +0200 Subject: [PATCH 11/12] C++: QLDoc for QualifiedName.qll --- .../code/cpp/internal/QualifiedName.qll | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll index c73a85d8040a..737ff23f985e 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -1,3 +1,16 @@ +/** + * INTERNAL: Do not use. Provides classes and predicates for getting names of + * declarations, especially qualified names. Import this library `private` and + * qualified. + * + * This file contains classes that mirror the standard AST classes for C++, but + * these classes are only concerned with naming. The other difference is that + * these classes don't use the `ResolveClass.qll` mechanisms like + * `unresolveElement` because these classes should eventually be part of the + * implementation of `ResolveClass.qll`, allowing it to match up classes when + * their qualified names and parameters match. + */ + class Namespace extends @namespace { string toString() { result = "QualifiedName Namespace" } @@ -274,12 +287,6 @@ class TemplateClass extends UserType { } } -deprecated class Property extends Declaration { - Property() { none() } - - override string getName() { none() } -} - class FriendDecl extends Declaration, @frienddecl { override string getName() { result = getUserTypeNameWithArgs(this.getDeclaringClass()) + "'s friend" From d820fc9cd28e7ef37fc276328e26770d82e6a66b Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 15 May 2019 14:55:26 +0200 Subject: [PATCH 12/12] C++: Address review comments about the comments --- cpp/ql/src/semmle/code/cpp/Declaration.qll | 2 +- cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Declaration.qll b/cpp/ql/src/semmle/code/cpp/Declaration.qll index 5912d7ff281a..c9ac6a498c0f 100644 --- a/cpp/ql/src/semmle/code/cpp/Declaration.qll +++ b/cpp/ql/src/semmle/code/cpp/Declaration.qll @@ -93,7 +93,7 @@ abstract class Declaration extends Locatable, @declaration { * component of `namespaceQualifier`, no declaring type, and a base name of * `baseName`. * - * See the 3-argument `hasQualifiedName` for more examples. + * See the 3-argument `hasQualifiedName` for examples. */ predicate hasQualifiedName(string namespaceQualifier, string baseName) { this.hasQualifiedName(namespaceQualifier, "", baseName) diff --git a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll index 737ff23f985e..9f2ca38a5524 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/QualifiedName.qll @@ -230,11 +230,14 @@ class MemberVariable extends Variable, @membervariable { override string getName() { membervariables(this, _, result) } } +// Unlike the usual `EnumConstant`, this one doesn't have a +// `getDeclaringType()`. This simplifies the recursive computation of type +// qualifier names since it can assume that any declaration with a +// `getDeclaringType()` should use that type in its type qualifier name. class EnumConstant extends Declaration, @enumconstant { override string getName() { enumconstants(this, _, _, _, result, _) } UserType getDeclaringEnum() { enumconstants(this, result, _, _, _, _) } - // Unlike the usual `EnumConstant`, this one doesn't have a `getDeclaringType()`. } class Function extends Declaration, @function {