From b634b59b9cc54dd9e75dd46224cff9e9c652082d Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 30 Mar 2020 18:52:12 +0100 Subject: [PATCH 01/13] C++: Merge the two allocators tests. --- .../allocators/allocators.expected | 46 +++++++++---------- .../library-tests/allocators/allocators.ql | 10 ++-- .../allocators/placement.expected | 2 - .../library-tests/allocators/placement.ql | 4 -- 4 files changed, 29 insertions(+), 33 deletions(-) delete mode 100644 cpp/ql/test/library-tests/allocators/placement.expected delete mode 100644 cpp/ql/test/library-tests/allocators/placement.ql diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index ee67a413692e..db0775ab6618 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -1,28 +1,28 @@ newExprs -| allocators.cpp:49:3:49:9 | new | int | operator new(unsigned long) -> void * | 4 | 4 | | -| allocators.cpp:50:3:50:15 | new | int | operator new(size_t, float) -> void * | 4 | 4 | | -| allocators.cpp:51:3:51:11 | new | int | operator new(unsigned long) -> void * | 4 | 4 | | -| allocators.cpp:52:3:52:14 | new | String | operator new(unsigned long) -> void * | 8 | 8 | | -| allocators.cpp:53:3:53:27 | new | String | operator new(size_t, float) -> void * | 8 | 8 | | -| allocators.cpp:54:3:54:17 | new | Overaligned | operator new(unsigned long, align_val_t) -> void * | 256 | 128 | aligned | -| allocators.cpp:55:3:55:25 | new | Overaligned | operator new(size_t, align_val_t, float) -> void * | 256 | 128 | aligned | -| allocators.cpp:107:3:107:18 | new | FailedInit | FailedInit::operator new(size_t) -> void * | 1 | 1 | | -| allocators.cpp:109:3:109:35 | new | FailedInitOveraligned | FailedInitOveraligned::operator new(size_t, align_val_t, float) -> void * | 128 | 128 | aligned | -| allocators.cpp:129:3:129:21 | new | int | operator new(size_t, void *) -> void * | 4 | 4 | | -| allocators.cpp:135:3:135:26 | new | int | operator new(size_t, const nothrow_t &) -> void * | 4 | 4 | | +| allocators.cpp:49:3:49:9 | new | int | operator new(unsigned long) -> void * | 4 | 4 | | | +| allocators.cpp:50:3:50:15 | new | int | operator new(size_t, float) -> void * | 4 | 4 | | | +| allocators.cpp:51:3:51:11 | new | int | operator new(unsigned long) -> void * | 4 | 4 | | | +| allocators.cpp:52:3:52:14 | new | String | operator new(unsigned long) -> void * | 8 | 8 | | | +| allocators.cpp:53:3:53:27 | new | String | operator new(size_t, float) -> void * | 8 | 8 | | | +| allocators.cpp:54:3:54:17 | new | Overaligned | operator new(unsigned long, align_val_t) -> void * | 256 | 128 | aligned | | +| allocators.cpp:55:3:55:25 | new | Overaligned | operator new(size_t, align_val_t, float) -> void * | 256 | 128 | aligned | | +| allocators.cpp:107:3:107:18 | new | FailedInit | FailedInit::operator new(size_t) -> void * | 1 | 1 | | | +| allocators.cpp:109:3:109:35 | new | FailedInitOveraligned | FailedInitOveraligned::operator new(size_t, align_val_t, float) -> void * | 128 | 128 | aligned | | +| allocators.cpp:129:3:129:21 | new | int | operator new(size_t, void *) -> void * | 4 | 4 | | & ... | +| allocators.cpp:135:3:135:26 | new | int | operator new(size_t, const nothrow_t &) -> void * | 4 | 4 | | | newArrayExprs -| allocators.cpp:68:3:68:12 | new[] | int[] | int | operator new[](unsigned long) -> void * | 4 | 4 | | n | -| allocators.cpp:69:3:69:18 | new[] | int[] | int | operator new[](size_t, float) -> void * | 4 | 4 | | n | -| allocators.cpp:70:3:70:15 | new[] | String[] | String | operator new[](unsigned long) -> void * | 8 | 8 | | n | -| allocators.cpp:71:3:71:20 | new[] | Overaligned[] | Overaligned | operator new[](unsigned long, align_val_t) -> void * | 256 | 128 | aligned | n | -| allocators.cpp:72:3:72:16 | new[] | String[10] | String | operator new[](unsigned long) -> void * | 8 | 8 | | | -| allocators.cpp:108:3:108:19 | new[] | FailedInit[] | FailedInit | FailedInit::operator new[](size_t) -> void * | 1 | 1 | | n | -| allocators.cpp:110:3:110:37 | new[] | FailedInitOveraligned[10] | FailedInitOveraligned | FailedInitOveraligned::operator new[](size_t, align_val_t, float) -> void * | 128 | 128 | aligned | | -| allocators.cpp:132:3:132:17 | new[] | int[1] | int | operator new[](size_t, void *) -> void * | 4 | 4 | | | -| allocators.cpp:136:3:136:26 | new[] | int[2] | int | operator new[](size_t, const nothrow_t &) -> void * | 4 | 4 | | | -| allocators.cpp:142:13:142:27 | new[] | char[][10] | char[10] | operator new[](unsigned long) -> void * | 10 | 1 | | x | -| allocators.cpp:143:13:143:28 | new[] | char[20][20] | char[20] | operator new[](unsigned long) -> void * | 20 | 1 | | | -| allocators.cpp:144:13:144:31 | new[] | char[][30][30] | char[30][30] | operator new[](unsigned long) -> void * | 900 | 1 | | x | +| allocators.cpp:68:3:68:12 | new[] | int[] | int | operator new[](unsigned long) -> void * | 4 | 4 | | n | | +| allocators.cpp:69:3:69:18 | new[] | int[] | int | operator new[](size_t, float) -> void * | 4 | 4 | | n | | +| allocators.cpp:70:3:70:15 | new[] | String[] | String | operator new[](unsigned long) -> void * | 8 | 8 | | n | | +| allocators.cpp:71:3:71:20 | new[] | Overaligned[] | Overaligned | operator new[](unsigned long, align_val_t) -> void * | 256 | 128 | aligned | n | | +| allocators.cpp:72:3:72:16 | new[] | String[10] | String | operator new[](unsigned long) -> void * | 8 | 8 | | | | +| allocators.cpp:108:3:108:19 | new[] | FailedInit[] | FailedInit | FailedInit::operator new[](size_t) -> void * | 1 | 1 | | n | | +| allocators.cpp:110:3:110:37 | new[] | FailedInitOveraligned[10] | FailedInitOveraligned | FailedInitOveraligned::operator new[](size_t, align_val_t, float) -> void * | 128 | 128 | aligned | | | +| allocators.cpp:132:3:132:17 | new[] | int[1] | int | operator new[](size_t, void *) -> void * | 4 | 4 | | | buf | +| allocators.cpp:136:3:136:26 | new[] | int[2] | int | operator new[](size_t, const nothrow_t &) -> void * | 4 | 4 | | | | +| allocators.cpp:142:13:142:27 | new[] | char[][10] | char[10] | operator new[](unsigned long) -> void * | 10 | 1 | | x | | +| allocators.cpp:143:13:143:28 | new[] | char[20][20] | char[20] | operator new[](unsigned long) -> void * | 20 | 1 | | | | +| allocators.cpp:144:13:144:31 | new[] | char[][30][30] | char[30][30] | operator new[](unsigned long) -> void * | 900 | 1 | | x | | newExprDeallocators | allocators.cpp:52:3:52:14 | new | String | operator delete(void *, unsigned long) -> void | 8 | 8 | sized | | allocators.cpp:53:3:53:27 | new | String | operator delete(void *, float) -> void | 8 | 8 | | diff --git a/cpp/ql/test/library-tests/allocators/allocators.ql b/cpp/ql/test/library-tests/allocators/allocators.ql index 37603f77a3ef..89abd415cb1c 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.ql +++ b/cpp/ql/test/library-tests/allocators/allocators.ql @@ -1,6 +1,6 @@ import default -query predicate newExprs(NewExpr expr, string type, string sig, int size, int alignment, string form) { +query predicate newExprs(NewExpr expr, string type, string sig, int size, int alignment, string form, string placement) { exists(Function allocator, Type allocatedType | expr.getAllocator() = allocator and sig = allocator.getFullSignature() and @@ -8,13 +8,14 @@ query predicate newExprs(NewExpr expr, string type, string sig, int size, int al type = allocatedType.toString() and size = allocatedType.getSize() and alignment = allocatedType.getAlignment() and - if expr.hasAlignedAllocation() then form = "aligned" else form = "" + if expr.hasAlignedAllocation() then form = "aligned" else form = "" and + if exists(expr.getPlacementPointer()) then placement = expr.getPlacementPointer().toString() else placement = "" ) } query predicate newArrayExprs( NewArrayExpr expr, string t1, string t2, string sig, int size, int alignment, string form, - string extents + string extents, string placement ) { exists(Function allocator, Type arrayType, Type elementType | expr.getAllocator() = allocator and @@ -26,7 +27,8 @@ query predicate newArrayExprs( size = elementType.getSize() and alignment = elementType.getAlignment() and (if expr.hasAlignedAllocation() then form = "aligned" else form = "") and - extents = concat(Expr e | e = expr.getExtent() | e.toString(), ", ") + extents = concat(Expr e | e = expr.getExtent() | e.toString(), ", ") and + if exists(expr.getPlacementPointer()) then placement = expr.getPlacementPointer().toString() else placement = "" ) } diff --git a/cpp/ql/test/library-tests/allocators/placement.expected b/cpp/ql/test/library-tests/allocators/placement.expected deleted file mode 100644 index 5755e349eb30..000000000000 --- a/cpp/ql/test/library-tests/allocators/placement.expected +++ /dev/null @@ -1,2 +0,0 @@ -| allocators.cpp:129:3:129:21 | new | allocators.cpp:129:7:129:13 | & ... | -| allocators.cpp:132:3:132:17 | new[] | allocators.cpp:132:7:132:9 | buf | diff --git a/cpp/ql/test/library-tests/allocators/placement.ql b/cpp/ql/test/library-tests/allocators/placement.ql deleted file mode 100644 index a632c3c5afe1..000000000000 --- a/cpp/ql/test/library-tests/allocators/placement.ql +++ /dev/null @@ -1,4 +0,0 @@ -import cpp - -from NewOrNewArrayExpr new -select new, new.getPlacementPointer() as placement From 0cb7d4c82df33e0948a33c7e8f6fbbc531073054 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 30 Mar 2020 20:28:21 +0100 Subject: [PATCH 02/13] C++: Add an explicit test of AllocationFunction and AllocationExpr. --- .../allocators/allocators.expected | 25 +++++++++++++ .../library-tests/allocators/allocators.ql | 36 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index db0775ab6618..44847688e1fe 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -46,3 +46,28 @@ deleteArrayExprs | allocators.cpp:81:3:81:45 | delete[] | Overaligned | operator delete[](void *, unsigned long, align_val_t) -> void | 256 | 128 | sized aligned | | allocators.cpp:82:3:82:49 | delete[] | PolymorphicBase | operator delete[](void *, unsigned long) -> void | 8 | 8 | sized | | allocators.cpp:83:3:83:23 | delete[] | int | operator delete[](void *, unsigned long) -> void | 4 | 4 | sized | +allocationFunctions +allocationExprs +| allocators.cpp:49:3:49:9 | new | getSizeBytes = 4, requiresDealloc | +| allocators.cpp:50:3:50:15 | new | getSizeBytes = 4, requiresDealloc | +| allocators.cpp:51:3:51:11 | new | getSizeBytes = 4, requiresDealloc | +| allocators.cpp:52:3:52:14 | new | getSizeBytes = 8, requiresDealloc | +| allocators.cpp:53:3:53:27 | new | getSizeBytes = 8, requiresDealloc | +| allocators.cpp:54:3:54:17 | new | getSizeBytes = 256, requiresDealloc | +| allocators.cpp:55:3:55:25 | new | getSizeBytes = 256, requiresDealloc | +| allocators.cpp:68:3:68:12 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc | +| allocators.cpp:69:3:69:18 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc | +| allocators.cpp:70:3:70:15 | new[] | getSizeExpr = n, getSizeMult = 8, requiresDealloc | +| allocators.cpp:71:3:71:20 | new[] | getSizeExpr = n, getSizeMult = 256, requiresDealloc | +| allocators.cpp:72:3:72:16 | new[] | getSizeBytes = 80, requiresDealloc | +| allocators.cpp:107:3:107:18 | new | getSizeBytes = 1, requiresDealloc | +| allocators.cpp:108:3:108:19 | new[] | getSizeExpr = n, getSizeMult = 1, requiresDealloc | +| allocators.cpp:109:3:109:35 | new | getSizeBytes = 128, requiresDealloc | +| allocators.cpp:110:3:110:37 | new[] | getSizeBytes = 1280, requiresDealloc | +| allocators.cpp:129:3:129:21 | new | getSizeBytes = 4 | +| allocators.cpp:132:3:132:17 | new[] | getSizeBytes = 4 | +| allocators.cpp:135:3:135:26 | new | getSizeBytes = 4, requiresDealloc | +| allocators.cpp:136:3:136:26 | new[] | getSizeBytes = 8, requiresDealloc | +| allocators.cpp:142:13:142:27 | new[] | getSizeExpr = x, getSizeMult = 10, requiresDealloc | +| allocators.cpp:143:13:143:28 | new[] | getSizeBytes = 400, requiresDealloc | +| allocators.cpp:144:13:144:31 | new[] | getSizeExpr = x, getSizeMult = 900, requiresDealloc | diff --git a/cpp/ql/test/library-tests/allocators/allocators.ql b/cpp/ql/test/library-tests/allocators/allocators.ql index 89abd415cb1c..e8a0d1b90c3e 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.ql +++ b/cpp/ql/test/library-tests/allocators/allocators.ql @@ -103,3 +103,39 @@ query predicate deleteArrayExprs( ) ) } + +string describeAllocationFunction(AllocationFunction f) { + result = "getSizeArg = " + f.getSizeArg().toString() + or + result = "getSizeMult = " + f.getSizeMult().toString() + or + result = "getReallocPtrArg = " + f.getReallocPtrArg().toString() + or + ( + f.requiresDealloc() and + result = "requiresDealloc" + ) +} + +query predicate allocationFunctions(AllocationFunction f, string descr) { + descr = concat(describeAllocationFunction(f), ", ") +} + +string describeAllocationExpr(AllocationExpr e) { + result = "getSizeExpr = " + e.getSizeExpr().toString() + or + result = "getSizeMult = " + e.getSizeMult().toString() + or + result = "getSizeBytes = " + e.getSizeBytes().toString() + or + result = "getReallocPtr = " + e.getReallocPtr().toString() + or + ( + e.requiresDealloc() and + result = "requiresDealloc" + ) +} + +query predicate allocationExprs(AllocationExpr e, string descr) { + descr = concat(describeAllocationExpr(e), ", ") +} From aa49b35d2c34c5ee0299ec16d849fe4b9b7e171c Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 09:57:02 +0100 Subject: [PATCH 03/13] C++: Add an explicit test of DeallocationFunction and DeallocationExpr as well. --- .../library-tests/allocators/allocators.expected | 14 ++++++++++++++ .../test/library-tests/allocators/allocators.ql | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index 44847688e1fe..355660b6d8cd 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -71,3 +71,17 @@ allocationExprs | allocators.cpp:142:13:142:27 | new[] | getSizeExpr = x, getSizeMult = 10, requiresDealloc | | allocators.cpp:143:13:143:28 | new[] | getSizeBytes = 400, requiresDealloc | | allocators.cpp:144:13:144:31 | new[] | getSizeExpr = x, getSizeMult = 900, requiresDealloc | +deallocationFunctions +deallocationExprs +| allocators.cpp:59:3:59:35 | delete | getFreedExpr = 0 | +| allocators.cpp:60:3:60:38 | delete | getFreedExpr = 0 | +| allocators.cpp:61:3:61:44 | delete | getFreedExpr = 0 | +| allocators.cpp:62:3:62:43 | delete | getFreedExpr = 0 | +| allocators.cpp:63:3:63:47 | delete | getFreedExpr = 0 | +| allocators.cpp:64:3:64:44 | delete | getFreedExpr = 0 | +| allocators.cpp:78:3:78:37 | delete[] | getFreedExpr = 0 | +| allocators.cpp:79:3:79:40 | delete[] | getFreedExpr = 0 | +| allocators.cpp:80:3:80:46 | delete[] | getFreedExpr = 0 | +| allocators.cpp:81:3:81:45 | delete[] | getFreedExpr = 0 | +| allocators.cpp:82:3:82:49 | delete[] | getFreedExpr = 0 | +| allocators.cpp:83:3:83:23 | delete[] | getFreedExpr = call to GetPointer | diff --git a/cpp/ql/test/library-tests/allocators/allocators.ql b/cpp/ql/test/library-tests/allocators/allocators.ql index e8a0d1b90c3e..a50fe4be3d4b 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.ql +++ b/cpp/ql/test/library-tests/allocators/allocators.ql @@ -139,3 +139,19 @@ string describeAllocationExpr(AllocationExpr e) { query predicate allocationExprs(AllocationExpr e, string descr) { descr = concat(describeAllocationExpr(e), ", ") } + +string describeDeallocationFunction(DeallocationFunction f) { + result = "getFreedArg = " + f.getFreedArg().toString() +} + +query predicate deallocationFunctions(DeallocationFunction f, string descr) { + descr = concat(describeDeallocationFunction(f), ", ") +} + +string describeDeallocationExpr(DeallocationExpr e) { + result = "getFreedExpr = " + e.getFreedExpr().toString() +} + +query predicate deallocationExprs(DeallocationExpr e, string descr) { + descr = concat(describeDeallocationExpr(e), ", ") +} From ef68bd6bf4b208de16a39d146ef93ce13de1673d Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 11:01:29 +0100 Subject: [PATCH 04/13] C++: Add a test of direct calls to operator new / operator dedelete. --- cpp/ql/test/library-tests/allocators/allocators.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cpp/ql/test/library-tests/allocators/allocators.cpp b/cpp/ql/test/library-tests/allocators/allocators.cpp index 6571245f5fc3..92241b9c9e3f 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.cpp +++ b/cpp/ql/test/library-tests/allocators/allocators.cpp @@ -143,3 +143,9 @@ void multidimensionalNew(int x, int y) { auto p2 = new char[20][20]; auto p3 = new char[x][30][30]; } + +void directOperatorCall() { + void *ptr; + ptr = operator new(sizeof(int)); + operator delete(ptr); +} From 259f714d91c96ec79a5115669bd4deb266df0a48 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Mon, 30 Mar 2020 18:11:07 +0100 Subject: [PATCH 05/13] C++: Model operator new and operator new[]. --- cpp/ql/src/semmle/code/cpp/exprs/Expr.qll | 13 +++----- .../cpp/models/implementations/Allocation.qll | 33 +++++++++++++++++++ .../allocators/allocators.expected | 21 ++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll index bb6aa86b4e73..97371919b028 100644 --- a/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll +++ b/cpp/ql/src/semmle/code/cpp/exprs/Expr.qll @@ -2,6 +2,7 @@ import semmle.code.cpp.Element private import semmle.code.cpp.Enclosing private import semmle.code.cpp.internal.ResolveClass private import semmle.code.cpp.internal.AddressConstantExpression +private import semmle.code.cpp.models.implementations.Allocation /** * A C/C++ expression. @@ -804,8 +805,10 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr { * call the constructor of `T` but will not allocate memory. */ Expr getPlacementPointer() { - isStandardPlacementNewAllocator(this.getAllocator()) and - result = this.getAllocatorCall().getArgument(1) + result = + this + .getAllocatorCall() + .getArgument(this.getAllocator().(OperatorNewAllocationFunction).getPlacementArgument()) } } @@ -1194,12 +1197,6 @@ private predicate convparents(Expr child, int idx, Element parent) { ) } -private predicate isStandardPlacementNewAllocator(Function operatorNew) { - operatorNew.getName().matches("operator new%") and - operatorNew.getNumberOfParameters() = 2 and - operatorNew.getParameter(1).getType() instanceof VoidPointerType -} - // Pulled out for performance. See QL-796. private predicate hasNoConversions(Expr e) { not e.hasConversion() } diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll index e2f34fe03ccf..81b4a6c8ea61 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll @@ -215,6 +215,39 @@ class SizelessAllocationFunction extends AllocationFunction { } } +/** + * An `operator new` or `operator new[]` function that may be associated with a `new` or + * `new[]` expression. Note that `new` and `new[]` are not function calls, but these + * functions may also be called directly. + */ +class OperatorNewAllocationFunction extends AllocationFunction { + OperatorNewAllocationFunction() { + exists(string name | + hasGlobalOrStdName(name) and + ( + // operator new(bytes, ...) + name = "operator new" or + // operator new[](bytes, ...) + name = "operator new[]" + ) + ) + } + + override int getSizeArg() { result = 0 } + + override predicate requiresDealloc() { not exists(getPlacementArgument()) } + + /** + * Gets the position of the placement pointer if this is a placement + * `operator new` function. + */ + int getPlacementArgument() { + getNumberOfParameters() = 2 and + getParameter(1).getType() instanceof VoidPointerType and + result = 1 + } +} + /** * An allocation expression that is a function call, such as call to `malloc`. */ diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index 355660b6d8cd..9c598181f045 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -47,15 +47,31 @@ deleteArrayExprs | allocators.cpp:82:3:82:49 | delete[] | PolymorphicBase | operator delete[](void *, unsigned long) -> void | 8 | 8 | sized | | allocators.cpp:83:3:83:23 | delete[] | int | operator delete[](void *, unsigned long) -> void | 4 | 4 | sized | allocationFunctions +| allocators.cpp:7:7:7:18 | operator new | getSizeArg = 0, requiresDealloc | +| allocators.cpp:8:7:8:20 | operator new[] | getSizeArg = 0, requiresDealloc | +| allocators.cpp:9:7:9:18 | operator new | getSizeArg = 0, requiresDealloc | +| allocators.cpp:10:7:10:20 | operator new[] | getSizeArg = 0, requiresDealloc | +| allocators.cpp:121:7:121:18 | operator new | getSizeArg = 0 | +| allocators.cpp:122:7:122:20 | operator new[] | getSizeArg = 0 | +| allocators.cpp:123:7:123:18 | operator new | getSizeArg = 0, requiresDealloc | +| allocators.cpp:124:7:124:20 | operator new[] | getSizeArg = 0, requiresDealloc | +| file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc | +| file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc | +| file://:0:0:0:0 | operator new[] | getSizeArg = 0, requiresDealloc | +| file://:0:0:0:0 | operator new[] | getSizeArg = 0, requiresDealloc | allocationExprs | allocators.cpp:49:3:49:9 | new | getSizeBytes = 4, requiresDealloc | +| allocators.cpp:50:3:50:15 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:50:3:50:15 | new | getSizeBytes = 4, requiresDealloc | | allocators.cpp:51:3:51:11 | new | getSizeBytes = 4, requiresDealloc | | allocators.cpp:52:3:52:14 | new | getSizeBytes = 8, requiresDealloc | +| allocators.cpp:53:3:53:27 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:53:3:53:27 | new | getSizeBytes = 8, requiresDealloc | | allocators.cpp:54:3:54:17 | new | getSizeBytes = 256, requiresDealloc | +| allocators.cpp:55:3:55:25 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:55:3:55:25 | new | getSizeBytes = 256, requiresDealloc | | allocators.cpp:68:3:68:12 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc | +| allocators.cpp:69:3:69:18 | call to operator new[] | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:69:3:69:18 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc | | allocators.cpp:70:3:70:15 | new[] | getSizeExpr = n, getSizeMult = 8, requiresDealloc | | allocators.cpp:71:3:71:20 | new[] | getSizeExpr = n, getSizeMult = 256, requiresDealloc | @@ -64,13 +80,18 @@ allocationExprs | allocators.cpp:108:3:108:19 | new[] | getSizeExpr = n, getSizeMult = 1, requiresDealloc | | allocators.cpp:109:3:109:35 | new | getSizeBytes = 128, requiresDealloc | | allocators.cpp:110:3:110:37 | new[] | getSizeBytes = 1280, requiresDealloc | +| allocators.cpp:129:3:129:21 | call to operator new | getSizeExpr = , getSizeMult = 1 | | allocators.cpp:129:3:129:21 | new | getSizeBytes = 4 | +| allocators.cpp:132:3:132:17 | call to operator new[] | getSizeExpr = , getSizeMult = 1 | | allocators.cpp:132:3:132:17 | new[] | getSizeBytes = 4 | +| allocators.cpp:135:3:135:26 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:135:3:135:26 | new | getSizeBytes = 4, requiresDealloc | +| allocators.cpp:136:3:136:26 | call to operator new[] | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:136:3:136:26 | new[] | getSizeBytes = 8, requiresDealloc | | allocators.cpp:142:13:142:27 | new[] | getSizeExpr = x, getSizeMult = 10, requiresDealloc | | allocators.cpp:143:13:143:28 | new[] | getSizeBytes = 400, requiresDealloc | | allocators.cpp:144:13:144:31 | new[] | getSizeExpr = x, getSizeMult = 900, requiresDealloc | +| allocators.cpp:149:8:149:19 | call to operator new | getSizeBytes = 4, getSizeExpr = sizeof(int), getSizeMult = 1, requiresDealloc | deallocationFunctions deallocationExprs | allocators.cpp:59:3:59:35 | delete | getFreedExpr = 0 | From 254c877d0a77253fcd0228528dd9b53fb27addad Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 10:32:22 +0100 Subject: [PATCH 06/13] C++: Deduplicate AllocationExprs. --- .../semmle/code/cpp/models/implementations/Allocation.qll | 3 +++ cpp/ql/test/library-tests/allocators/allocators.expected | 8 -------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll index 81b4a6c8ea61..9461213f56c6 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll @@ -261,6 +261,9 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall { exists(target.getReallocPtrArg()) and getArgument(target.getSizeArg()).getValue().toInt() = 0 ) + and + // these are modelled directly (and more accurately), avoid duplication + not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this) } override Expr getSizeExpr() { result = getArgument(target.getSizeArg()) } diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index 9c598181f045..405b69e30efb 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -61,17 +61,13 @@ allocationFunctions | file://:0:0:0:0 | operator new[] | getSizeArg = 0, requiresDealloc | allocationExprs | allocators.cpp:49:3:49:9 | new | getSizeBytes = 4, requiresDealloc | -| allocators.cpp:50:3:50:15 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:50:3:50:15 | new | getSizeBytes = 4, requiresDealloc | | allocators.cpp:51:3:51:11 | new | getSizeBytes = 4, requiresDealloc | | allocators.cpp:52:3:52:14 | new | getSizeBytes = 8, requiresDealloc | -| allocators.cpp:53:3:53:27 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:53:3:53:27 | new | getSizeBytes = 8, requiresDealloc | | allocators.cpp:54:3:54:17 | new | getSizeBytes = 256, requiresDealloc | -| allocators.cpp:55:3:55:25 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:55:3:55:25 | new | getSizeBytes = 256, requiresDealloc | | allocators.cpp:68:3:68:12 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc | -| allocators.cpp:69:3:69:18 | call to operator new[] | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:69:3:69:18 | new[] | getSizeExpr = n, getSizeMult = 4, requiresDealloc | | allocators.cpp:70:3:70:15 | new[] | getSizeExpr = n, getSizeMult = 8, requiresDealloc | | allocators.cpp:71:3:71:20 | new[] | getSizeExpr = n, getSizeMult = 256, requiresDealloc | @@ -80,13 +76,9 @@ allocationExprs | allocators.cpp:108:3:108:19 | new[] | getSizeExpr = n, getSizeMult = 1, requiresDealloc | | allocators.cpp:109:3:109:35 | new | getSizeBytes = 128, requiresDealloc | | allocators.cpp:110:3:110:37 | new[] | getSizeBytes = 1280, requiresDealloc | -| allocators.cpp:129:3:129:21 | call to operator new | getSizeExpr = , getSizeMult = 1 | | allocators.cpp:129:3:129:21 | new | getSizeBytes = 4 | -| allocators.cpp:132:3:132:17 | call to operator new[] | getSizeExpr = , getSizeMult = 1 | | allocators.cpp:132:3:132:17 | new[] | getSizeBytes = 4 | -| allocators.cpp:135:3:135:26 | call to operator new | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:135:3:135:26 | new | getSizeBytes = 4, requiresDealloc | -| allocators.cpp:136:3:136:26 | call to operator new[] | getSizeExpr = , getSizeMult = 1, requiresDealloc | | allocators.cpp:136:3:136:26 | new[] | getSizeBytes = 8, requiresDealloc | | allocators.cpp:142:13:142:27 | new[] | getSizeExpr = x, getSizeMult = 10, requiresDealloc | | allocators.cpp:143:13:143:28 | new[] | getSizeBytes = 400, requiresDealloc | From 3b12d1adfd3dcfeb02aa1792e99c9db028d9a333 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 09:46:21 +0100 Subject: [PATCH 07/13] C++: Test getPlacementArgument(). --- cpp/ql/test/library-tests/allocators/allocators.expected | 4 ++-- cpp/ql/test/library-tests/allocators/allocators.ql | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index 405b69e30efb..ce6a893af7f0 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -51,8 +51,8 @@ allocationFunctions | allocators.cpp:8:7:8:20 | operator new[] | getSizeArg = 0, requiresDealloc | | allocators.cpp:9:7:9:18 | operator new | getSizeArg = 0, requiresDealloc | | allocators.cpp:10:7:10:20 | operator new[] | getSizeArg = 0, requiresDealloc | -| allocators.cpp:121:7:121:18 | operator new | getSizeArg = 0 | -| allocators.cpp:122:7:122:20 | operator new[] | getSizeArg = 0 | +| allocators.cpp:121:7:121:18 | operator new | getPlacementArgument = 1, getSizeArg = 0 | +| allocators.cpp:122:7:122:20 | operator new[] | getPlacementArgument = 1, getSizeArg = 0 | | allocators.cpp:123:7:123:18 | operator new | getSizeArg = 0, requiresDealloc | | allocators.cpp:124:7:124:20 | operator new[] | getSizeArg = 0, requiresDealloc | | file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc | diff --git a/cpp/ql/test/library-tests/allocators/allocators.ql b/cpp/ql/test/library-tests/allocators/allocators.ql index a50fe4be3d4b..5993bf6d3905 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.ql +++ b/cpp/ql/test/library-tests/allocators/allocators.ql @@ -1,4 +1,5 @@ import default +import semmle.code.cpp.models.implementations.Allocation query predicate newExprs(NewExpr expr, string type, string sig, int size, int alignment, string form, string placement) { exists(Function allocator, Type allocatedType | @@ -115,6 +116,8 @@ string describeAllocationFunction(AllocationFunction f) { f.requiresDealloc() and result = "requiresDealloc" ) + or + result = "getPlacementArgument = " + f.(OperatorNewAllocationFunction).getPlacementArgument().toString() } query predicate allocationFunctions(AllocationFunction f, string descr) { From 18e60fabaf45bde94c8ca78c6f547a03080e0e4a Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 10:19:50 +0100 Subject: [PATCH 08/13] C++: Model operator delete and operator delete[]. --- .../models/implementations/Deallocation.qll | 21 +++++++++++++++++++ .../allocators/allocators.expected | 10 +++++++++ 2 files changed, 31 insertions(+) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll index 83309855e189..c3b4cdd65840 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll @@ -79,6 +79,27 @@ class StandardDeallocationFunction extends DeallocationFunction { override int getFreedArg() { result = freedArg } } +/** + * An `operator delete` or `operator delete[]` function that may be associated + * with a `delete` or `delete[]` expression. Note that `delete` and `delete[]` + * are not function calls, but these functions may also be called directly. + */ +class OperatorDeleteDeallocationFunction extends DeallocationFunction { + OperatorDeleteDeallocationFunction() { + exists(string name | + hasGlobalOrStdName(name) and + ( + // operator delete(pointer, ...) + name = "operator delete" or + // operator delete[](pointer, ...) + name = "operator delete[]" + ) + ) + } + + override int getFreedArg() { result = 0 } +} + /** * An deallocation expression that is a function call, such as call to `free`. */ diff --git a/cpp/ql/test/library-tests/allocators/allocators.expected b/cpp/ql/test/library-tests/allocators/allocators.expected index ce6a893af7f0..2f9b4413e8cc 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.expected +++ b/cpp/ql/test/library-tests/allocators/allocators.expected @@ -85,6 +85,15 @@ allocationExprs | allocators.cpp:144:13:144:31 | new[] | getSizeExpr = x, getSizeMult = 900, requiresDealloc | | allocators.cpp:149:8:149:19 | call to operator new | getSizeBytes = 4, getSizeExpr = sizeof(int), getSizeMult = 1, requiresDealloc | deallocationFunctions +| allocators.cpp:11:6:11:20 | operator delete | getFreedArg = 0 | +| allocators.cpp:12:6:12:22 | operator delete[] | getFreedArg = 0 | +| allocators.cpp:13:6:13:20 | operator delete | getFreedArg = 0 | +| allocators.cpp:14:6:14:22 | operator delete[] | getFreedArg = 0 | +| file://:0:0:0:0 | operator delete | getFreedArg = 0 | +| file://:0:0:0:0 | operator delete | getFreedArg = 0 | +| file://:0:0:0:0 | operator delete | getFreedArg = 0 | +| file://:0:0:0:0 | operator delete[] | getFreedArg = 0 | +| file://:0:0:0:0 | operator delete[] | getFreedArg = 0 | deallocationExprs | allocators.cpp:59:3:59:35 | delete | getFreedExpr = 0 | | allocators.cpp:60:3:60:38 | delete | getFreedExpr = 0 | @@ -98,3 +107,4 @@ deallocationExprs | allocators.cpp:81:3:81:45 | delete[] | getFreedExpr = 0 | | allocators.cpp:82:3:82:49 | delete[] | getFreedExpr = 0 | | allocators.cpp:83:3:83:23 | delete[] | getFreedExpr = call to GetPointer | +| allocators.cpp:150:2:150:16 | call to operator delete | getFreedExpr = ptr | From a75e249112b1d6500b0d9d9c77c1b73699ac122e Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 10:36:22 +0100 Subject: [PATCH 09/13] C++: Autoformat test. --- .../library-tests/allocators/allocators.ql | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/cpp/ql/test/library-tests/allocators/allocators.ql b/cpp/ql/test/library-tests/allocators/allocators.ql index 5993bf6d3905..e45667fbf018 100644 --- a/cpp/ql/test/library-tests/allocators/allocators.ql +++ b/cpp/ql/test/library-tests/allocators/allocators.ql @@ -1,7 +1,9 @@ import default import semmle.code.cpp.models.implementations.Allocation -query predicate newExprs(NewExpr expr, string type, string sig, int size, int alignment, string form, string placement) { +query predicate newExprs( + NewExpr expr, string type, string sig, int size, int alignment, string form, string placement +) { exists(Function allocator, Type allocatedType | expr.getAllocator() = allocator and sig = allocator.getFullSignature() and @@ -9,8 +11,10 @@ query predicate newExprs(NewExpr expr, string type, string sig, int size, int al type = allocatedType.toString() and size = allocatedType.getSize() and alignment = allocatedType.getAlignment() and - if expr.hasAlignedAllocation() then form = "aligned" else form = "" and - if exists(expr.getPlacementPointer()) then placement = expr.getPlacementPointer().toString() else placement = "" + (if expr.hasAlignedAllocation() then form = "aligned" else form = "") and + if exists(expr.getPlacementPointer()) + then placement = expr.getPlacementPointer().toString() + else placement = "" ) } @@ -29,7 +33,9 @@ query predicate newArrayExprs( alignment = elementType.getAlignment() and (if expr.hasAlignedAllocation() then form = "aligned" else form = "") and extents = concat(Expr e | e = expr.getExtent() | e.toString(), ", ") and - if exists(expr.getPlacementPointer()) then placement = expr.getPlacementPointer().toString() else placement = "" + if exists(expr.getPlacementPointer()) + then placement = expr.getPlacementPointer().toString() + else placement = "" ) } @@ -106,55 +112,52 @@ query predicate deleteArrayExprs( } string describeAllocationFunction(AllocationFunction f) { - result = "getSizeArg = " + f.getSizeArg().toString() - or - result = "getSizeMult = " + f.getSizeMult().toString() - or - result = "getReallocPtrArg = " + f.getReallocPtrArg().toString() - or - ( - f.requiresDealloc() and - result = "requiresDealloc" - ) - or - result = "getPlacementArgument = " + f.(OperatorNewAllocationFunction).getPlacementArgument().toString() + result = "getSizeArg = " + f.getSizeArg().toString() + or + result = "getSizeMult = " + f.getSizeMult().toString() + or + result = "getReallocPtrArg = " + f.getReallocPtrArg().toString() + or + f.requiresDealloc() and + result = "requiresDealloc" + or + result = + "getPlacementArgument = " + f.(OperatorNewAllocationFunction).getPlacementArgument().toString() } query predicate allocationFunctions(AllocationFunction f, string descr) { - descr = concat(describeAllocationFunction(f), ", ") + descr = concat(describeAllocationFunction(f), ", ") } string describeAllocationExpr(AllocationExpr e) { - result = "getSizeExpr = " + e.getSizeExpr().toString() - or - result = "getSizeMult = " + e.getSizeMult().toString() - or - result = "getSizeBytes = " + e.getSizeBytes().toString() - or - result = "getReallocPtr = " + e.getReallocPtr().toString() - or - ( - e.requiresDealloc() and - result = "requiresDealloc" - ) + result = "getSizeExpr = " + e.getSizeExpr().toString() + or + result = "getSizeMult = " + e.getSizeMult().toString() + or + result = "getSizeBytes = " + e.getSizeBytes().toString() + or + result = "getReallocPtr = " + e.getReallocPtr().toString() + or + e.requiresDealloc() and + result = "requiresDealloc" } query predicate allocationExprs(AllocationExpr e, string descr) { - descr = concat(describeAllocationExpr(e), ", ") + descr = concat(describeAllocationExpr(e), ", ") } string describeDeallocationFunction(DeallocationFunction f) { - result = "getFreedArg = " + f.getFreedArg().toString() + result = "getFreedArg = " + f.getFreedArg().toString() } query predicate deallocationFunctions(DeallocationFunction f, string descr) { - descr = concat(describeDeallocationFunction(f), ", ") + descr = concat(describeDeallocationFunction(f), ", ") } string describeDeallocationExpr(DeallocationExpr e) { - result = "getFreedExpr = " + e.getFreedExpr().toString() + result = "getFreedExpr = " + e.getFreedExpr().toString() } query predicate deallocationExprs(DeallocationExpr e, string descr) { - descr = concat(describeDeallocationExpr(e), ", ") + descr = concat(describeDeallocationExpr(e), ", ") } From aa13257c1bb371db9d2366e2afba2ef65919920a Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 14:37:54 +0100 Subject: [PATCH 10/13] C++: Correct QLDoc. --- .../src/semmle/code/cpp/models/implementations/Allocation.qll | 4 ++-- .../semmle/code/cpp/models/implementations/Deallocation.qll | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll index 9461213f56c6..1454f64f5098 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll @@ -216,8 +216,8 @@ class SizelessAllocationFunction extends AllocationFunction { } /** - * An `operator new` or `operator new[]` function that may be associated with a `new` or - * `new[]` expression. Note that `new` and `new[]` are not function calls, but these + * An `operator new` or `operator new[]` function that may be associated with `new` or + * `new[]` expressions. Note that `new` and `new[]` are not function calls, but these * functions may also be called directly. */ class OperatorNewAllocationFunction extends AllocationFunction { diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll index c3b4cdd65840..35aa5c2227a5 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll @@ -81,7 +81,7 @@ class StandardDeallocationFunction extends DeallocationFunction { /** * An `operator delete` or `operator delete[]` function that may be associated - * with a `delete` or `delete[]` expression. Note that `delete` and `delete[]` + * with `delete` or `delete[]` expressions. Note that `delete` and `delete[]` * are not function calls, but these functions may also be called directly. */ class OperatorDeleteDeallocationFunction extends DeallocationFunction { From f430cf9d189763c01bc69b07ebfb437b7597effb Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 31 Mar 2020 18:10:41 +0100 Subject: [PATCH 11/13] C++: Use hasGlobalName. --- .../src/semmle/code/cpp/models/implementations/Allocation.qll | 2 +- .../src/semmle/code/cpp/models/implementations/Deallocation.qll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll index 1454f64f5098..ad26086998ac 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll @@ -223,7 +223,7 @@ class SizelessAllocationFunction extends AllocationFunction { class OperatorNewAllocationFunction extends AllocationFunction { OperatorNewAllocationFunction() { exists(string name | - hasGlobalOrStdName(name) and + hasGlobalName(name) and ( // operator new(bytes, ...) name = "operator new" or diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll index 35aa5c2227a5..f94800f657dc 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll @@ -87,7 +87,7 @@ class StandardDeallocationFunction extends DeallocationFunction { class OperatorDeleteDeallocationFunction extends DeallocationFunction { OperatorDeleteDeallocationFunction() { exists(string name | - hasGlobalOrStdName(name) and + hasGlobalName(name) and ( // operator delete(pointer, ...) name = "operator delete" or From 119d4a40a0462f2d2760e707c331f21355119906 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 1 Apr 2020 14:29:28 +0100 Subject: [PATCH 12/13] C++: Fix unintended consequence in IR. --- .../ir/implementation/raw/internal/TranslatedCall.qll | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll index fb7dc1738128..f2d098afbcc9 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll @@ -343,6 +343,11 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects { override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType type) { expr.getTarget() instanceof AllocationFunction and + not exists(NewOrNewArrayExpr newExpr | + // we synthesize allocator calls for `new` and `new[]`, so don't add instructions to + // the existing allocator call when it exists. + newExpr.getAllocatorCall() = expr + ) and opcode instanceof Opcode::InitializeDynamicAllocation and tag = OnlyInstructionTag() and type = getUnknownType() @@ -358,6 +363,11 @@ class TranslatedSideEffects extends TranslatedElement, TTranslatedSideEffects { tag = OnlyInstructionTag() and kind = gotoEdge() and expr.getTarget() instanceof AllocationFunction and + not exists(NewOrNewArrayExpr newExpr | + // we synthesize allocator calls for `new` and `new[]`, so don't add instructions to + // the existing allocator call when it exists. + newExpr.getAllocatorCall() = expr + ) and if exists(getChild(0)) then result = getChild(0).getFirstInstruction() else result = getParent().getChildSuccessor(this) From ead5feb921388305acf20e3621478c0e8c859eac Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 Apr 2020 09:50:14 +0100 Subject: [PATCH 13/13] C++: Autoformat. --- .../semmle/code/cpp/models/implementations/Allocation.qll | 6 +++--- .../semmle/code/cpp/models/implementations/Deallocation.qll | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll index ad26086998ac..c6766983889c 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll @@ -226,7 +226,8 @@ class OperatorNewAllocationFunction extends AllocationFunction { hasGlobalName(name) and ( // operator new(bytes, ...) - name = "operator new" or + name = "operator new" + or // operator new[](bytes, ...) name = "operator new[]" ) @@ -260,8 +261,7 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall { not ( exists(target.getReallocPtrArg()) and getArgument(target.getSizeArg()).getValue().toInt() = 0 - ) - and + ) and // these are modelled directly (and more accurately), avoid duplication not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this) } diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll index f94800f657dc..d2e4951e436a 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll @@ -90,7 +90,8 @@ class OperatorDeleteDeallocationFunction extends DeallocationFunction { hasGlobalName(name) and ( // operator delete(pointer, ...) - name = "operator delete" or + name = "operator delete" + or // operator delete[](pointer, ...) name = "operator delete[]" )