Skip to content

C++: IR: Fix performance of value-init ranges #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 14, 2018
Merged

C++: IR: Fix performance of value-init ranges #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2571c8
C++: IR: Fix performance of value-init ranges
jbj Sep 11, 2018
bb49966
C++: Fixup getEndOfValueInitializedRange
jbj Sep 11, 2018
df948ec
C++: IR: designated initializer test
jbj Sep 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 24 additions & 17 deletions cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ newtype TTranslatedElement =
not ignoreExpr(initList) and
isFirstValueInitializedElementInRange(initList, elementIndex) and
elementCount =
getNextExplicitlyInitializedElementAfter(initList, elementIndex) -
getEndOfValueInitializedRange(initList, elementIndex) -
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need an or getNextExplicitlyInitializedElementAfter too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Fixed.

elementIndex
} or
// The initialization of a base class from within a constructor.
Expand Down Expand Up @@ -322,23 +322,30 @@ 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) {
result = getNextExplicitlyInitializedElementAfter(initList, afterElementIndex)
or
isFirstValueInitializedElementInRange(initList, afterElementIndex) and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be in both halves of the disjunction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getNextExplicitlyInitializedElementAfter calls isFirstValueInitializedElementInRange, so adding another call in this predicate would be redundant.

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)
}

/**
Expand Down
32 changes: 32 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/PrintAST.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
46 changes: 46 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/aliased_ssa_ir.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<int[1000]>) = VariableAddress[a1] :
# 962| r0_3(int) = Constant[0] :
# 962| r0_4(glval<int>) = 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<int>) = 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<int>) = 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<int>) = VariableAddress[#return] :
# 963| r2_1(glval<int[1000]>) = 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<int>) = 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<int>) = 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<int>) = 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
5 changes: 5 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/ir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<int*>(nullptr); // No destructor
Expand Down
46 changes: 46 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/raw_ir.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<int[1000]>) = VariableAddress[a1] :
# 962| r0_3(int) = Constant[0] :
# 962| r0_4(glval<int>) = 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<int>) = 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<int>) = 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<int>) = VariableAddress[#return] :
# 963| r2_1(glval<int[1000]>) = 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<int>) = 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<int>) = 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<int>) = 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
1 change: 1 addition & 0 deletions cpp/ql/test/library-tests/ir/ir/ssa_block_count.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
46 changes: 46 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ir.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<int[1000]>) = VariableAddress[a1] :
# 962| r0_3(int) = Constant[0] :
# 962| r0_4(glval<int>) = 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<int>) = 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<int>) = 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<int>) = VariableAddress[#return] :
# 963| r2_1(glval<int[1000]>) = 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<int>) = 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<int>) = 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<int>) = 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