From b2571c8d638ff8fddf6acc887a874c4b675e4e9b Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Sep 2018 11:28:50 +0200 Subject: [PATCH 1/7] C++: IR: Fix performance of value-init ranges On a snapshot of Postgres, evaluation of `getNextExplicitlyInitializedElementAfter#fff#antijoin_rhs#1` took forever, preventing the computation of the IR. I haven't been able to reproduce it with a small test case, but the implementation of `getNextExplicitlyInitializedElementAfter` was fragile because it called the inline predicate `ArrayAggregateLiteral.isInitialized`. It also seemed inefficient that `getNextExplicitlyInitializedElementAfter` was computed for many values of its parameters that were never needed by the caller. This commit replaces `getNextExplicitlyInitializedElementAfter` with a new predicate named `getEndOfValueInitializedRange`, which should have the same behavior but a more efficient implementation. It uses a helper predicate `getNextExplicitlyInitializedElementAfter`, which shares its name with the now-deleted predicate but has behavior that I think matches the name. --- .../raw/internal/TranslatedElement.qll | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll index 380efb6e9247..f0bced00b59c 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll @@ -270,7 +270,7 @@ newtype TTranslatedElement = not ignoreExpr(initList) and isFirstValueInitializedElementInRange(initList, elementIndex) and elementCount = - getNextExplicitlyInitializedElementAfter(initList, elementIndex) - + getEndOfValueInitializedRange(initList, elementIndex) - elementIndex } or // The initialization of a base class from within a constructor. @@ -322,23 +322,28 @@ newtype TTranslatedElement = /** * Gets the index of the first explicitly initialized element in `initList` - * whose index is greater than `afterElementIndex`. If there are no remaining - * explicitly initialized elements in `initList`, the result is the total number - * of elements in the array being initialized. + * whose index is greater than `afterElementIndex`, where `afterElementIndex` + * is a first value-initialized element in a value-initialized range in + * `initList`. If there are no remaining explicitly initialized elements in + * `initList`, the result is the total number of elements in the array being + * initialized. */ - private int getNextExplicitlyInitializedElementAfter( - ArrayAggregateLiteral initList, int afterElementIndex) { - if exists(int x | - x > afterElementIndex and - exists(initList.getElementExpr(x))) - then ( - if exists(initList.getElementExpr(afterElementIndex + 1)) - then result = afterElementIndex + 1 - else result = getNextExplicitlyInitializedElementAfter(initList, afterElementIndex+1)) - else - result = initList.getType().getUnspecifiedType().(ArrayType).getArraySize() and - // required for binding - initList.isInitialized(afterElementIndex) +private int getEndOfValueInitializedRange(ArrayAggregateLiteral initList, int afterElementIndex) { + isFirstValueInitializedElementInRange(initList, afterElementIndex) and + not exists(getNextExplicitlyInitializedElementAfter(initList, afterElementIndex)) and + result = initList.getType().getUnspecifiedType().(ArrayType).getArraySize() +} + +/** + * Gets the index of the first explicitly initialized element in `initList` + * whose index is greater than `afterElementIndex`, where `afterElementIndex` + * is a first value-initialized element in a value-initialized range in + * `initList`. + */ +private int getNextExplicitlyInitializedElementAfter( + ArrayAggregateLiteral initList, int afterElementIndex) { + isFirstValueInitializedElementInRange(initList, afterElementIndex) and + result = min(int i | exists(initList.getElementExpr(i)) and i > afterElementIndex) } /** From bb499663954c54b617bc374c3175fb17f098f378 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Sep 2018 15:19:18 +0200 Subject: [PATCH 2/7] C++: Fixup getEndOfValueInitializedRange --- .../cpp/ir/implementation/raw/internal/TranslatedElement.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll index f0bced00b59c..580dbe0f725e 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll @@ -329,6 +329,8 @@ newtype TTranslatedElement = * initialized. */ private int getEndOfValueInitializedRange(ArrayAggregateLiteral initList, int afterElementIndex) { + result = getNextExplicitlyInitializedElementAfter(initList, afterElementIndex) + or isFirstValueInitializedElementInRange(initList, afterElementIndex) and not exists(getNextExplicitlyInitializedElementAfter(initList, afterElementIndex)) and result = initList.getType().getUnspecifiedType().(ArrayType).getArraySize() From df948ecbbc111d4e9bf7180b671af509b6d46824 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Sep 2018 19:34:20 +0200 Subject: [PATCH 3/7] C++: IR: designated initializer test --- .../library-tests/ir/ir/PrintAST.expected | 32 +++++++++++++ .../ir/ir/aliased_ssa_ir.expected | 46 +++++++++++++++++++ cpp/ql/test/library-tests/ir/ir/ir.cpp | 5 ++ .../test/library-tests/ir/ir/raw_ir.expected | 46 +++++++++++++++++++ .../ir/ir/ssa_block_count.expected | 1 + .../ir/ir/unaliased_ssa_ir.expected | 46 +++++++++++++++++++ 6 files changed, 176 insertions(+) diff --git a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected index a7da38c6d978..884abe554a61 100644 --- a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected +++ b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected @@ -6303,3 +6303,35 @@ ir.cpp: # 958| Type = int # 958| ValueCategory = prvalue(load) # 959| 8: return ... +# 961| designatedInit() -> int +# 961| params: +# 961| body: { ... } +# 962| 0: declaration +# 962| 0: definition of a1 +# 962| Type = int[1000] +# 962| init: initializer for a1 +# 962| expr: {...} +# 962| Type = int[1000] +# 962| ValueCategory = prvalue +# 962| 0: 10002 +# 962| Type = int +# 962| Value = 10002 +# 962| ValueCategory = prvalue +# 962| 1: 10900 +# 962| Type = int +# 962| Value = 10900 +# 962| ValueCategory = prvalue +# 963| 1: return ... +# 963| 0: access to array +# 963| Type = int +# 963| ValueCategory = prvalue(load) +# 963| 0: array to pointer conversion +# 963| Type = int * +# 963| ValueCategory = prvalue +# 963| expr: a1 +# 963| Type = int[1000] +# 963| ValueCategory = lvalue +# 963| 1: 900 +# 963| Type = int +# 963| Value = 900 +# 963| ValueCategory = prvalue diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected index b9a7b4f2403e..5fbbec2afb7b 100644 --- a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected @@ -3906,3 +3906,49 @@ ir.cpp: # 950| v0_65(void) = ReturnVoid : # 950| v0_66(void) = UnmodeledUse : mu* # 950| v0_67(void) = ExitFunction : + +# 961| designatedInit() -> int +# 961| Block 0 +# 961| v0_0(void) = EnterFunction : +# 961| mu0_1(unknown) = UnmodeledDefinition : +# 962| r0_2(glval) = VariableAddress[a1] : +# 962| r0_3(int) = Constant[0] : +# 962| r0_4(glval) = PointerAdd : r0_2, r0_3 +# 962| r0_5(unknown[8]) = Constant[0] : +# 962| mu0_6(unknown[8]) = Store : r0_4, r0_5 +#-----| Goto -> Block 2 + +# 962| Block 1 +# 962| r1_0(int) = Constant[900] : +# 962| r1_1(glval) = PointerAdd : r0_2, r1_0 +# 962| r1_2(int) = Constant[10900] : +# 962| mu1_3(int) = Store : r1_1, r1_2 +# 962| r1_4(int) = Constant[901] : +# 962| r1_5(glval) = PointerAdd : r0_2, r1_4 +# 962| r1_6(unknown[396]) = Constant[0] : +# 962| mu1_7(unknown[396]) = Store : r1_5, r1_6 +#-----| Goto -> Block 2 + +# 963| Block 2 +# 963| r2_0(glval) = VariableAddress[#return] : +# 963| r2_1(glval) = VariableAddress[a1] : +# 963| r2_2(int *) = Convert : r2_1 +# 963| r2_3(int) = Constant[900] : +# 963| r2_4(int *) = PointerAdd[4] : r2_2, r2_3 +# 963| r2_5(int) = Load : r2_4, mu0_1 +# 963| m2_6(int) = Store : r2_0, r2_5 +# 961| r2_7(glval) = VariableAddress[#return] : +# 961| v2_8(void) = ReturnValue : r2_7, m2_6 +# 961| v2_9(void) = UnmodeledUse : mu* +# 961| v2_10(void) = ExitFunction : + +# 962| Block 3 +# 962| r3_0(int) = Constant[2] : +# 962| r3_1(glval) = PointerAdd : r0_2, r3_0 +# 962| r3_2(int) = Constant[10002] : +# 962| mu3_3(int) = Store : r3_1, r3_2 +# 962| r3_4(int) = Constant[3] : +# 962| r3_5(glval) = PointerAdd : r0_2, r3_4 +# 962| r3_6(unknown[3588]) = Constant[0] : +# 962| mu3_7(unknown[3588]) = Store : r3_5, r3_6 +#-----| Goto -> Block 2 diff --git a/cpp/ql/test/library-tests/ir/ir/ir.cpp b/cpp/ql/test/library-tests/ir/ir/ir.cpp index 05682c61db2c..5490cd0f2d97 100644 --- a/cpp/ql/test/library-tests/ir/ir/ir.cpp +++ b/cpp/ql/test/library-tests/ir/ir/ir.cpp @@ -958,6 +958,11 @@ void OperatorNewArray(int n) { new int[n] { 0, 1, 2 }; } +int designatedInit() { + int a1[1000] = { [2] = 10002, [900] = 10900 }; + return a1[900]; +} + #if 0 void OperatorDelete() { delete static_cast(nullptr); // No destructor diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected index 0c361cfb374e..5f5dc4d3f5d2 100644 --- a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected @@ -3885,3 +3885,49 @@ ir.cpp: # 950| v0_65(void) = ReturnVoid : # 950| v0_66(void) = UnmodeledUse : mu* # 950| v0_67(void) = ExitFunction : + +# 961| designatedInit() -> int +# 961| Block 0 +# 961| v0_0(void) = EnterFunction : +# 961| mu0_1(unknown) = UnmodeledDefinition : +# 962| r0_2(glval) = VariableAddress[a1] : +# 962| r0_3(int) = Constant[0] : +# 962| r0_4(glval) = PointerAdd : r0_2, r0_3 +# 962| r0_5(unknown[8]) = Constant[0] : +# 962| mu0_6(unknown[8]) = Store : r0_4, r0_5 +#-----| Goto -> Block 2 + +# 962| Block 1 +# 962| r1_0(int) = Constant[900] : +# 962| r1_1(glval) = PointerAdd : r0_2, r1_0 +# 962| r1_2(int) = Constant[10900] : +# 962| mu1_3(int) = Store : r1_1, r1_2 +# 962| r1_4(int) = Constant[901] : +# 962| r1_5(glval) = PointerAdd : r0_2, r1_4 +# 962| r1_6(unknown[396]) = Constant[0] : +# 962| mu1_7(unknown[396]) = Store : r1_5, r1_6 +#-----| Goto -> Block 2 + +# 963| Block 2 +# 963| r2_0(glval) = VariableAddress[#return] : +# 963| r2_1(glval) = VariableAddress[a1] : +# 963| r2_2(int *) = Convert : r2_1 +# 963| r2_3(int) = Constant[900] : +# 963| r2_4(int *) = PointerAdd[4] : r2_2, r2_3 +# 963| r2_5(int) = Load : r2_4, mu0_1 +# 963| mu2_6(int) = Store : r2_0, r2_5 +# 961| r2_7(glval) = VariableAddress[#return] : +# 961| v2_8(void) = ReturnValue : r2_7, mu0_1 +# 961| v2_9(void) = UnmodeledUse : mu* +# 961| v2_10(void) = ExitFunction : + +# 962| Block 3 +# 962| r3_0(int) = Constant[2] : +# 962| r3_1(glval) = PointerAdd : r0_2, r3_0 +# 962| r3_2(int) = Constant[10002] : +# 962| mu3_3(int) = Store : r3_1, r3_2 +# 962| r3_4(int) = Constant[3] : +# 962| r3_5(glval) = PointerAdd : r0_2, r3_4 +# 962| r3_6(unknown[3588]) = Constant[0] : +# 962| mu3_7(unknown[3588]) = Store : r3_5, r3_6 +#-----| Goto -> Block 2 diff --git a/cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected b/cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected index 4bd31c2a64f0..0ef298fee3de 100644 --- a/cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected +++ b/cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected @@ -92,6 +92,7 @@ | IR: VarArgs | 1 | | IR: VirtualMemberFunction | 1 | | IR: WhileStatements | 4 | +| IR: designatedInit | 4 | | IR: min | 4 | | IR: operator= | 1 | | IR: ~Base | 1 | diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected index c26bcce58103..af2385fb03d4 100644 --- a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected +++ b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected @@ -3906,3 +3906,49 @@ ir.cpp: # 950| v0_65(void) = ReturnVoid : # 950| v0_66(void) = UnmodeledUse : mu* # 950| v0_67(void) = ExitFunction : + +# 961| designatedInit() -> int +# 961| Block 0 +# 961| v0_0(void) = EnterFunction : +# 961| mu0_1(unknown) = UnmodeledDefinition : +# 962| r0_2(glval) = VariableAddress[a1] : +# 962| r0_3(int) = Constant[0] : +# 962| r0_4(glval) = PointerAdd : r0_2, r0_3 +# 962| r0_5(unknown[8]) = Constant[0] : +# 962| mu0_6(unknown[8]) = Store : r0_4, r0_5 +#-----| Goto -> Block 2 + +# 962| Block 1 +# 962| r1_0(int) = Constant[900] : +# 962| r1_1(glval) = PointerAdd : r0_2, r1_0 +# 962| r1_2(int) = Constant[10900] : +# 962| mu1_3(int) = Store : r1_1, r1_2 +# 962| r1_4(int) = Constant[901] : +# 962| r1_5(glval) = PointerAdd : r0_2, r1_4 +# 962| r1_6(unknown[396]) = Constant[0] : +# 962| mu1_7(unknown[396]) = Store : r1_5, r1_6 +#-----| Goto -> Block 2 + +# 963| Block 2 +# 963| r2_0(glval) = VariableAddress[#return] : +# 963| r2_1(glval) = VariableAddress[a1] : +# 963| r2_2(int *) = Convert : r2_1 +# 963| r2_3(int) = Constant[900] : +# 963| r2_4(int *) = PointerAdd[4] : r2_2, r2_3 +# 963| r2_5(int) = Load : r2_4, mu0_1 +# 963| m2_6(int) = Store : r2_0, r2_5 +# 961| r2_7(glval) = VariableAddress[#return] : +# 961| v2_8(void) = ReturnValue : r2_7, m2_6 +# 961| v2_9(void) = UnmodeledUse : mu* +# 961| v2_10(void) = ExitFunction : + +# 962| Block 3 +# 962| r3_0(int) = Constant[2] : +# 962| r3_1(glval) = PointerAdd : r0_2, r3_0 +# 962| r3_2(int) = Constant[10002] : +# 962| mu3_3(int) = Store : r3_1, r3_2 +# 962| r3_4(int) = Constant[3] : +# 962| r3_5(glval) = PointerAdd : r0_2, r3_4 +# 962| r3_6(unknown[3588]) = Constant[0] : +# 962| mu3_7(unknown[3588]) = Store : r3_5, r3_6 +#-----| Goto -> Block 2 From 2b4da8d6a723d0b3a2515b9368e1aba61945c5ce Mon Sep 17 00:00:00 2001 From: Pavel Avgustinov Date: Fri, 14 Sep 2018 12:22:01 +0100 Subject: [PATCH 4/7] Parameter.qll: Tweak how effective declaration entries are computed With the new formulation, we can join on function and index at the same time, leading to significant performance gains on large code bases that use templates extensively. --- cpp/ql/src/semmle/code/cpp/Parameter.qll | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Parameter.qll b/cpp/ql/src/semmle/code/cpp/Parameter.qll index 971a2f33f610..c6fc33b9355a 100644 --- a/cpp/ql/src/semmle/code/cpp/Parameter.qll +++ b/cpp/ql/src/semmle/code/cpp/Parameter.qll @@ -68,10 +68,9 @@ class Parameter extends LocalScopeVariable, @parameter { */ private VariableDeclarationEntry getAnEffectiveDeclarationEntry() { if getFunction().isConstructedFrom(_) - then exists (Parameter prototype - | prototype = result.getVariable() and - prototype.getIndex() = getIndex() and - getFunction().isConstructedFrom(prototype.getFunction())) + then exists (Function prototypeInstantiation + | prototypeInstantiation.getParameter(getIndex()) = result.getVariable() and + getFunction().isConstructedFrom(prototypeInstantiation)) else result = getADeclarationEntry() } From b633ee1bc4f6a4d8fface8e49c0ae30a6ed990e4 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 14 Sep 2018 14:23:31 +0200 Subject: [PATCH 5/7] C++: Add more tests of resolveClass These tests exercise the problematic cases where a variable can appear to have multiple types because of how we fail to account for qualified names when comparing type names. --- .../structs/compatible_cpp/b1.cpp | 6 ++++ .../compatible_cpp/compatible.expected | 3 ++ .../compatible_cpp/compatible_types.expected | 1 + .../pointerbasetype.expected | 9 ++++++ .../structs/incomplete_definition/x.cpp | 28 ++++++++++++++++++ .../structs/qualified_names/c1.cpp | 9 ++++++ .../structs/qualified_names/c2.cpp | 6 ++++ .../structs/qualified_names/decls.cpp | 5 ++++ .../structs/qualified_names/defs.cpp | 29 +++++++++++++++++++ .../structs/qualified_names/header.h | 3 ++ .../qualified_names/pointerBaseType.expected | 6 ++++ .../qualified_names/pointerBaseType.ql | 5 ++++ 12 files changed, 110 insertions(+) create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/c1.cpp create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/c2.cpp create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/decls.cpp create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/defs.cpp create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/header.h create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected create mode 100644 cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.ql diff --git a/cpp/ql/test/library-tests/structs/compatible_cpp/b1.cpp b/cpp/ql/test/library-tests/structs/compatible_cpp/b1.cpp index c7d839b4874d..760147d60ff0 100644 --- a/cpp/ql/test/library-tests/structs/compatible_cpp/b1.cpp +++ b/cpp/ql/test/library-tests/structs/compatible_cpp/b1.cpp @@ -24,3 +24,9 @@ class Damson { int damson_x; void foo(); }; + +namespace unrelated { + class AppleCompatible { + long apple_x; + }; +} diff --git a/cpp/ql/test/library-tests/structs/compatible_cpp/compatible.expected b/cpp/ql/test/library-tests/structs/compatible_cpp/compatible.expected index 4b631dde98b4..326fea4210f8 100644 --- a/cpp/ql/test/library-tests/structs/compatible_cpp/compatible.expected +++ b/cpp/ql/test/library-tests/structs/compatible_cpp/compatible.expected @@ -32,6 +32,9 @@ | b1.cpp:23:7:23:12 | Damson | 5 members | 2 locations | 1 | foo | | b1.cpp:23:7:23:12 | Damson | 5 members | 2 locations | 2 | operator= | | b1.cpp:23:7:23:12 | Damson | 5 members | 2 locations | 3 | operator= | +| b1.cpp:29:9:29:23 | AppleCompatible | 3 members | 1 locations | 0 | apple_x | +| b1.cpp:29:9:29:23 | AppleCompatible | 3 members | 1 locations | 1 | operator= | +| b1.cpp:29:9:29:23 | AppleCompatible | 3 members | 1 locations | 2 | operator= | | b2.cpp:2:7:2:21 | AppleCompatible | 3 members | 2 locations | 0 | apple_x | | b2.cpp:2:7:2:21 | AppleCompatible | 3 members | 2 locations | 1 | operator= | | b2.cpp:2:7:2:21 | AppleCompatible | 3 members | 2 locations | 2 | operator= | diff --git a/cpp/ql/test/library-tests/structs/compatible_cpp/compatible_types.expected b/cpp/ql/test/library-tests/structs/compatible_cpp/compatible_types.expected index df2a178f6674..f9446f1b19a6 100644 --- a/cpp/ql/test/library-tests/structs/compatible_cpp/compatible_types.expected +++ b/cpp/ql/test/library-tests/structs/compatible_cpp/compatible_types.expected @@ -10,6 +10,7 @@ | b1.cpp:11:7:11:22 | BananaCompatible | 0 | file://:0:0:0:0 | int | 1 types | | b1.cpp:16:7:16:12 | Cherry | 0 | file://:0:0:0:0 | int | 1 types | | b1.cpp:23:7:23:12 | Damson | 0 | file://:0:0:0:0 | int | 1 types | +| b1.cpp:29:9:29:23 | AppleCompatible | 0 | file://:0:0:0:0 | long | 1 types | | b2.cpp:2:7:2:21 | AppleCompatible | 0 | file://:0:0:0:0 | int | 1 types | | b2.cpp:9:7:9:22 | BananaCompatible | 0 | file://:0:0:0:0 | int | 1 types | | b2.cpp:14:7:14:12 | Cherry | 0 | file://:0:0:0:0 | short | 1 types | diff --git a/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected b/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected index 98a672ad4353..3efebf1a9278 100644 --- a/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected +++ b/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected @@ -1,2 +1,11 @@ +| a.h:5:8:5:13 | cheese | x.cpp:6:10:6:12 | Foo | 3 | +| a.h:5:8:5:13 | cheese | x.cpp:12:9:12:11 | Foo | 3 | | a.h:5:8:5:13 | cheese | y.cpp:4:8:4:10 | Foo | 3 | | x.cpp:3:6:3:10 | bar_x | a.h:4:8:4:10 | Bar | 3 | +| x.cpp:19:6:19:10 | foo_x | x.cpp:6:10:6:12 | Foo | 3 | +| x.cpp:19:6:19:10 | foo_x | x.cpp:12:9:12:11 | Foo | 3 | +| x.cpp:19:6:19:10 | foo_x | y.cpp:4:8:4:10 | Foo | 3 | +| x.cpp:23:5:23:17 | templateField | x.cpp:6:10:6:12 | Foo | 3 | +| x.cpp:23:5:23:17 | templateField | x.cpp:12:9:12:11 | Foo | 3 | +| x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template | 3 | +| x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template | 3 | diff --git a/cpp/ql/test/library-tests/structs/incomplete_definition/x.cpp b/cpp/ql/test/library-tests/structs/incomplete_definition/x.cpp index 5b2a352e3dd7..ab5eb9474553 100644 --- a/cpp/ql/test/library-tests/structs/incomplete_definition/x.cpp +++ b/cpp/ql/test/library-tests/structs/incomplete_definition/x.cpp @@ -1,3 +1,31 @@ #include "a.h" Bar *bar_x; + +namespace unrelated { + struct Foo { + short val; + }; +} + +struct ContainsAnotherFoo { + class Foo { + long val; + }; +}; + +// The type of `foo_x` should not refer to any of the above classes, none of +// which are named `Foo` in the global scope. +Foo *foo_x; + +template +class Template { + T templateField; +}; + +Template *template_foo; + +// Instantiation of the template with unrelated classes named `Foo` should not +// get mixed up with the instantiation above. +template class Template; +template class Template; diff --git a/cpp/ql/test/library-tests/structs/qualified_names/c1.cpp b/cpp/ql/test/library-tests/structs/qualified_names/c1.cpp new file mode 100644 index 000000000000..f72303701213 --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/c1.cpp @@ -0,0 +1,9 @@ +#include "header.h" + +struct MultipleDefsButSameHeader { + int i; +}; + +struct OneDefInDifferentFile { + int i; +}; diff --git a/cpp/ql/test/library-tests/structs/qualified_names/c2.cpp b/cpp/ql/test/library-tests/structs/qualified_names/c2.cpp new file mode 100644 index 000000000000..be5380566150 --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/c2.cpp @@ -0,0 +1,6 @@ +#include "header.h" + +struct MultipleDefsButSameHeader { + char char1; + char char2; +}; diff --git a/cpp/ql/test/library-tests/structs/qualified_names/decls.cpp b/cpp/ql/test/library-tests/structs/qualified_names/decls.cpp new file mode 100644 index 000000000000..c6cde7b00b90 --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/decls.cpp @@ -0,0 +1,5 @@ +namespace foo { + class C; + + C *x; +} diff --git a/cpp/ql/test/library-tests/structs/qualified_names/defs.cpp b/cpp/ql/test/library-tests/structs/qualified_names/defs.cpp new file mode 100644 index 000000000000..095f84126044 --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/defs.cpp @@ -0,0 +1,29 @@ +namespace foo { + class C { + }; +} + +namespace bar { + class C { + }; +} + +class DefinedAndDeclared { +}; + +// Despite this declaration being present, the variable below is associated +// with the definition above rather than this declaration. +class DefinedAndDeclared; + +DefinedAndDeclared *definedAndDeclared; + +#include "header.h" + +// Because there are multiple definitions of `MultipleDefsButSameHeader`, the +// type of this variable will refer to the declaration in `header.h` rather +// than any of the definitions. +MultipleDefsButSameHeader *mdbsh; + +// Because there is only one definition of `OneDefInDifferentFile`, the type of +// this variable will refer to that definition. +OneDefInDifferentFile *odidf; diff --git a/cpp/ql/test/library-tests/structs/qualified_names/header.h b/cpp/ql/test/library-tests/structs/qualified_names/header.h new file mode 100644 index 000000000000..d410b0d35333 --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/header.h @@ -0,0 +1,3 @@ +struct MultipleDefsButSameHeader; + +struct OneDefInDifferentFile; diff --git a/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected b/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected new file mode 100644 index 000000000000..6e86c7f5f12b --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected @@ -0,0 +1,6 @@ +| decls.cpp:4:6:4:6 | x | defs.cpp:2:9:2:9 | C | +| decls.cpp:4:6:4:6 | x | defs.cpp:7:9:7:9 | C | +| defs.cpp:18:21:18:38 | definedAndDeclared | defs.cpp:11:7:11:24 | DefinedAndDeclared | +| defs.cpp:25:28:25:32 | mdbsh | c1.cpp:3:8:3:32 | MultipleDefsButSameHeader | +| defs.cpp:25:28:25:32 | mdbsh | c2.cpp:3:8:3:32 | MultipleDefsButSameHeader | +| defs.cpp:29:24:29:28 | odidf | c1.cpp:7:8:7:28 | OneDefInDifferentFile | diff --git a/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.ql b/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.ql new file mode 100644 index 000000000000..b2aeaee63488 --- /dev/null +++ b/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.ql @@ -0,0 +1,5 @@ +import cpp + +from Variable x +where exists(x.getFile().getRelativePath()) +select x, x.getType().(PointerType).getBaseType() From d7f442b0428ea119e2e70aa92c9361ab36e2eb8b Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 14 Sep 2018 14:21:44 +0200 Subject: [PATCH 6/7] C++: Force unique resolveClass results --- .../semmle/code/cpp/internal/ResolveClass.qll | 23 +++++++++++-------- .../pointerbasetype.expected | 10 +++----- .../qualified_names/pointerBaseType.expected | 6 ++--- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll b/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll index c3cde4c4346c..76c5b473e43d 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll @@ -1,23 +1,28 @@ import semmle.code.cpp.Type -/** Holds if `d` is a complete class named `name`. */ +pragma[noinline] +private string getTopLevelClassName(@usertype c) { + isClass(c) and + usertypes(c, result, _) +} + +/** Holds if `d` is a unique complete class named `name`. */ pragma[noinline] private predicate existsCompleteWithName(string name, @usertype d) { - isClass(d) and is_complete(d) and - usertypes(d, name, _) + name = getTopLevelClassName(d) and + strictcount(@usertype other | is_complete(other) and getTopLevelClassName(other) = name) = 1 } /** Holds if `c` is an incomplete class named `name`. */ pragma[noinline] private predicate existsIncompleteWithName(string name, @usertype c) { - isClass(c) and not is_complete(c) and - usertypes(c, name, _) + name = getTopLevelClassName(c) } /** - * Holds if `c` is an imcomplete class, and there exists a complete class `d` + * Holds if `c` is an imcomplete class, and there exists a unique complete class `d` * with the same name. */ private predicate hasCompleteTwin(@usertype c, @usertype d) { @@ -30,10 +35,8 @@ private predicate hasCompleteTwin(@usertype c, @usertype d) { import Cached cached private module Cached { /** - * If `c` is incomplete, and there exists a complete class with the same name, - * then the result is that complete class. Otherwise, the result is `c`. If - * multiple complete classes have the same name, this predicate may have - * multiple results. + * If `c` is incomplete, and there exists a unique complete class with the same name, + * then the result is that complete class. Otherwise, the result is `c`. */ cached @usertype resolveClass(@usertype c) { hasCompleteTwin(c, result) diff --git a/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected b/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected index 3efebf1a9278..3e8c8fd34c68 100644 --- a/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected +++ b/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected @@ -1,11 +1,7 @@ -| a.h:5:8:5:13 | cheese | x.cpp:6:10:6:12 | Foo | 3 | -| a.h:5:8:5:13 | cheese | x.cpp:12:9:12:11 | Foo | 3 | +| a.h:5:8:5:13 | cheese | a.h:2:8:2:10 | Foo | 0 | | a.h:5:8:5:13 | cheese | y.cpp:4:8:4:10 | Foo | 3 | | x.cpp:3:6:3:10 | bar_x | a.h:4:8:4:10 | Bar | 3 | -| x.cpp:19:6:19:10 | foo_x | x.cpp:6:10:6:12 | Foo | 3 | -| x.cpp:19:6:19:10 | foo_x | x.cpp:12:9:12:11 | Foo | 3 | -| x.cpp:19:6:19:10 | foo_x | y.cpp:4:8:4:10 | Foo | 3 | +| x.cpp:19:6:19:10 | foo_x | a.h:2:8:2:10 | Foo | 0 | | x.cpp:23:5:23:17 | templateField | x.cpp:6:10:6:12 | Foo | 3 | | x.cpp:23:5:23:17 | templateField | x.cpp:12:9:12:11 | Foo | 3 | -| x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template | 3 | -| x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template | 3 | +| x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template | 0 | diff --git a/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected b/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected index 6e86c7f5f12b..1573f1c446e2 100644 --- a/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected +++ b/cpp/ql/test/library-tests/structs/qualified_names/pointerBaseType.expected @@ -1,6 +1,4 @@ -| decls.cpp:4:6:4:6 | x | defs.cpp:2:9:2:9 | C | -| decls.cpp:4:6:4:6 | x | defs.cpp:7:9:7:9 | C | +| decls.cpp:4:6:4:6 | x | decls.cpp:2:9:2:9 | C | | defs.cpp:18:21:18:38 | definedAndDeclared | defs.cpp:11:7:11:24 | DefinedAndDeclared | -| defs.cpp:25:28:25:32 | mdbsh | c1.cpp:3:8:3:32 | MultipleDefsButSameHeader | -| defs.cpp:25:28:25:32 | mdbsh | c2.cpp:3:8:3:32 | MultipleDefsButSameHeader | +| defs.cpp:25:28:25:32 | mdbsh | header.h:1:8:1:32 | MultipleDefsButSameHeader | | defs.cpp:29:24:29:28 | odidf | c1.cpp:7:8:7:28 | OneDefInDifferentFile | From a7d897108ada5123a6447493085b9f5608fd90db Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Fri, 14 Sep 2018 15:07:25 +0200 Subject: [PATCH 7/7] C++: Exclude non-toplevel items from resolveClass Also exclude templates as their names are not canonical. The test changes in `isfromtemplateinstantiation/` are the inverses of what we got in 34c9892f7, which should be a good thing. --- .../semmle/code/cpp/internal/ResolveClass.qll | 5 ++++- .../pointerbasetype.expected | 3 +-- .../isfromtemplateinstantiation.expected | 19 ------------------- .../isfromuninstantiatedtemplate.expected | 11 ++++++----- 4 files changed, 11 insertions(+), 27 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll b/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll index 76c5b473e43d..eaefc8196cc8 100644 --- a/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll +++ b/cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll @@ -3,7 +3,10 @@ import semmle.code.cpp.Type pragma[noinline] private string getTopLevelClassName(@usertype c) { isClass(c) and - usertypes(c, result, _) + usertypes(c, result, _) and + not namespacembrs(_, c) and // not in a namespace + not member(_, _, c) and // not in some structure + not class_instantiation(c, _) // not a template instantiation } /** Holds if `d` is a unique complete class named `name`. */ diff --git a/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected b/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected index 3e8c8fd34c68..801306a572ab 100644 --- a/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected +++ b/cpp/ql/test/library-tests/structs/incomplete_definition/pointerbasetype.expected @@ -1,7 +1,6 @@ -| a.h:5:8:5:13 | cheese | a.h:2:8:2:10 | Foo | 0 | | a.h:5:8:5:13 | cheese | y.cpp:4:8:4:10 | Foo | 3 | | x.cpp:3:6:3:10 | bar_x | a.h:4:8:4:10 | Bar | 3 | -| x.cpp:19:6:19:10 | foo_x | a.h:2:8:2:10 | Foo | 0 | +| x.cpp:19:6:19:10 | foo_x | y.cpp:4:8:4:10 | Foo | 3 | | x.cpp:23:5:23:17 | templateField | x.cpp:6:10:6:12 | Foo | 3 | | x.cpp:23:5:23:17 | templateField | x.cpp:12:9:12:11 | Foo | 3 | | x.cpp:26:18:26:29 | template_foo | x.cpp:22:7:22:14 | Template | 0 | diff --git a/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromtemplateinstantiation.expected b/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromtemplateinstantiation.expected index 4d2706e2e952..dc73e749a4cd 100644 --- a/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromtemplateinstantiation.expected +++ b/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromtemplateinstantiation.expected @@ -116,32 +116,13 @@ | isfromtemplateinstantiation.cpp:134:29:134:29 | Outer::operator=(const Outer &) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:134:29:134:29 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:134:29:134:29 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(Inner &&) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(Inner &&) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(const Inner &) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(const Inner &) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(Inner &&) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(Inner &&) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(const Inner &) | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:135:31:135:31 | Outer::Inner::operator=(const Inner &) | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | | isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:135:31:135:35 | definition of Inner | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:136:7:136:7 | x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:136:7:136:7 | x | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:136:7:136:7 | x | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | -| isfromtemplateinstantiation.cpp:137:7:137:7 | y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | -| isfromtemplateinstantiation.cpp:137:7:137:7 | y | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | isfromtemplateinstantiation.cpp:137:7:137:7 | y | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | | load.cpp:13:7:13:7 | basic_text_iprimitive::basic_text_iprimitive(basic_text_iprimitive &&) | load.cpp:13:7:13:27 | basic_text_iprimitive | | load.cpp:13:7:13:7 | basic_text_iprimitive::basic_text_iprimitive(const basic_text_iprimitive &) | load.cpp:13:7:13:27 | basic_text_iprimitive | diff --git a/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromuninstantiatedtemplate.expected b/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromuninstantiatedtemplate.expected index 38fda5364eef..74a096dff1aa 100644 --- a/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromuninstantiatedtemplate.expected +++ b/cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromuninstantiatedtemplate.expected @@ -3,6 +3,7 @@ isFromUninstantiatedTemplate | file://:0:0:0:0 | 0 | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass | | file://:0:0:0:0 | (int)... | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass | | file://:0:0:0:0 | (int)... | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass | +| file://:0:0:0:0 | Inner | file://:0:0:0:0 | Inner | | file://:0:0:0:0 | declaration of 1st parameter | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | file://:0:0:0:0 | declaration of 1st parameter | isfromtemplateinstantiation.cpp:134:29:134:33 | Outer | | file://:0:0:0:0 | initializer for MyClassEnumConst | isfromtemplateinstantiation.cpp:77:26:77:45 | AnotherTemplateClass | @@ -433,15 +434,15 @@ isFromUninstantiatedTemplate | isfromtemplateinstantiation.cpp:135:31:135:31 | declaration of operator= | I | T | DeclarationEntry | | | isfromtemplateinstantiation.cpp:135:31:135:31 | operator= | I | T | Declaration | | | isfromtemplateinstantiation.cpp:135:31:135:31 | operator= | I | T | Declaration | | -| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | I | T | Declaration | | +| isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | | T | Declaration | | | isfromtemplateinstantiation.cpp:135:31:135:35 | Inner | I | T | Declaration | | +| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | | T | Definition | | | isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | I | T | Definition | | -| isfromtemplateinstantiation.cpp:136:7:136:7 | definition of x | I | T | Definition | | -| isfromtemplateinstantiation.cpp:136:7:136:7 | x | I | T | Declaration | | +| isfromtemplateinstantiation.cpp:136:7:136:7 | x | | T | Declaration | | | isfromtemplateinstantiation.cpp:136:7:136:7 | x | I | T | Declaration | | +| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | | T | Definition | | | isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | I | T | Definition | | -| isfromtemplateinstantiation.cpp:137:7:137:7 | definition of y | I | T | Definition | | -| isfromtemplateinstantiation.cpp:137:7:137:7 | y | I | T | Declaration | | +| isfromtemplateinstantiation.cpp:137:7:137:7 | y | | T | Declaration | | | isfromtemplateinstantiation.cpp:137:7:137:7 | y | I | T | Declaration | | | isfromtemplateinstantiation.cpp:144:28:144:56 | incomplete_never_instantiated | | T | Declaration | | | isfromtemplateinstantiation.cpp:146:28:146:45 | never_instantiated | | T | Declaration | |