Skip to content

Commit 9179f0b

Browse files
MathiasVPMathiasVP
authored
Merge pull request #15969 from MathiasVP/disable-some-constant-folding
C++: Disable _some_ constant folding in IR
2 parents 1d956e1 + 6bf1611 commit 9179f0b

10 files changed

Lines changed: 931 additions & 1 deletion

File tree

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/constant/ConstantAnalysis.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ private int getBinaryInstructionValue(BinaryInstruction instr) {
3838
or
3939
instr instanceof DivInstruction and result = div(left, right)
4040
or
41+
instr instanceof BitOrInstruction and result = bitOr(left, right)
42+
or
43+
instr instanceof BitAndInstruction and result = bitAnd(left, right)
44+
or
45+
instr instanceof BitXorInstruction and result = bitXor(left, right)
46+
or
4147
instr instanceof CompareEQInstruction and result = compareEQ(left, right)
4248
or
4349
instr instanceof CompareNEInstruction and result = compareNE(left, right)

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/constant/ConstantAnalysis.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ private int getBinaryInstructionValue(BinaryInstruction instr) {
3838
or
3939
instr instanceof DivInstruction and result = div(left, right)
4040
or
41+
instr instanceof BitOrInstruction and result = bitOr(left, right)
42+
or
43+
instr instanceof BitAndInstruction and result = bitAnd(left, right)
44+
or
45+
instr instanceof BitXorInstruction and result = bitXor(left, right)
46+
or
4147
instr instanceof CompareEQInstruction and result = compareEQ(left, right)
4248
or
4349
instr instanceof CompareNEInstruction and result = compareNE(left, right)

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,43 @@ IRTempVariable getIRTempVariable(Locatable ast, TempVariableTag tag) {
4040
result.getTag() = tag
4141
}
4242

43+
/** Gets an operand of `op`. */
44+
private Expr getAnOperand(Operation op) { result = op.getAnOperand() }
45+
46+
/**
47+
* Gets the number of nested operands of `op`. For example,
48+
* `getNumberOfNestedBinaryOperands((1 + 2) + 3))` is `3`.
49+
*/
50+
private int getNumberOfNestedBinaryOperands(Operation op) { result = count(getAnOperand*(op)) }
51+
52+
/**
53+
* Holds if `op` should not be translated to a `ConstantInstruction` as part of
54+
* IR generation, even if the value of `op` is constant.
55+
*/
56+
private predicate ignoreConstantValue(Operation op) {
57+
op instanceof BitwiseAndExpr
58+
or
59+
op instanceof BitwiseOrExpr
60+
or
61+
op instanceof BitwiseXorExpr
62+
}
63+
4364
/**
4465
* Holds if `expr` is a constant of a type that can be replaced directly with
4566
* its value in the IR. This does not include address constants as we have no
4667
* means to express those as QL values.
4768
*/
48-
predicate isIRConstant(Expr expr) { exists(expr.getValue()) }
69+
predicate isIRConstant(Expr expr) {
70+
exists(expr.getValue()) and
71+
// We avoid constant folding certain operations since it's often useful to
72+
// mark one of those as a source in dataflow, and if the operation is
73+
// constant folded it's not possible to mark its operands as a source (or
74+
// sink).
75+
// But to avoid creating an outrageous amount of IR from very large
76+
// constant expressions we fall back to constant folding if the operation
77+
// has more than 50 operands (i.e., 1 + 2 + 3 + 4 + ... + 50)
78+
if ignoreConstantValue(expr) then getNumberOfNestedBinaryOperands(expr) > 50 else any()
79+
}
4980

5081
// Pulled out for performance. See
5182
// https://github.com/github/codeql-coreql-team/issues/1044.

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/constant/ConstantAnalysis.qll‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ private int getBinaryInstructionValue(BinaryInstruction instr) {
3838
or
3939
instr instanceof DivInstruction and result = div(left, right)
4040
or
41+
instr instanceof BitOrInstruction and result = bitOr(left, right)
42+
or
43+
instr instanceof BitAndInstruction and result = bitAnd(left, right)
44+
or
45+
instr instanceof BitXorInstruction and result = bitXor(left, right)
46+
or
4147
instr instanceof CompareEQInstruction and result = compareEQ(left, right)
4248
or
4349
instr instanceof CompareNEInstruction and result = compareNE(left, right)

‎cpp/ql/lib/semmle/code/cpp/ir/internal/IntegerPartial.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,18 @@ int compareLE(int a, int b) { if a <= b then result = 1 else result = 0 }
8989
bindingset[a, b]
9090
int compareGE(int a, int b) { if a >= b then result = 1 else result = 0 }
9191

92+
/** Returns `a | b`. */
93+
bindingset[a, b]
94+
int bitOr(int a, int b) { result = a.bitOr(b) }
95+
96+
/** Returns `a & b`. */
97+
bindingset[a, b]
98+
int bitAnd(int a, int b) { result = a.bitAnd(b) }
99+
100+
/** Returns `a ^ b`. */
101+
bindingset[a, b]
102+
int bitXor(int a, int b) { result = a.bitXor(b) }
103+
92104
/**
93105
* Returns `-a`. If the negation would overflow, there is no result.
94106
*/

‎cpp/ql/test/library-tests/ir/ir/PrintAST.expected‎

Lines changed: 780 additions & 0 deletions
Large diffs are not rendered by default.

‎cpp/ql/test/library-tests/ir/ir/aliased_ir.expected‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15135,6 +15135,36 @@ ir.cpp:
1513515135
# 2374| v2374_7(void) = AliasedUse : m2374_3
1513615136
# 2374| v2374_8(void) = ExitFunction :
1513715137

15138+
# 2381| int small_operation_should_not_be_constant_folded()
15139+
# 2381| Block 0
15140+
# 2381| v2381_1(void) = EnterFunction :
15141+
# 2381| m2381_2(unknown) = AliasedDefinition :
15142+
# 2381| m2381_3(unknown) = InitializeNonLocal :
15143+
# 2381| m2381_4(unknown) = Chi : total:m2381_2, partial:m2381_3
15144+
# 2382| r2382_1(glval<int>) = VariableAddress[#return] :
15145+
# 2382| r2382_2(int) = Constant[1] :
15146+
# 2382| r2382_3(int) = Constant[2] :
15147+
# 2382| r2382_4(int) = BitXor : r2382_2, r2382_3
15148+
# 2382| m2382_5(int) = Store[#return] : &:r2382_1, r2382_4
15149+
# 2381| r2381_5(glval<int>) = VariableAddress[#return] :
15150+
# 2381| v2381_6(void) = ReturnValue : &:r2381_5, m2382_5
15151+
# 2381| v2381_7(void) = AliasedUse : m2381_3
15152+
# 2381| v2381_8(void) = ExitFunction :
15153+
15154+
# 2392| int large_operation_should_be_constant_folded()
15155+
# 2392| Block 0
15156+
# 2392| v2392_1(void) = EnterFunction :
15157+
# 2392| m2392_2(unknown) = AliasedDefinition :
15158+
# 2392| m2392_3(unknown) = InitializeNonLocal :
15159+
# 2392| m2392_4(unknown) = Chi : total:m2392_2, partial:m2392_3
15160+
# 2393| r2393_1(glval<int>) = VariableAddress[#return] :
15161+
# 2393| r2393_2(int) = Constant[0] :
15162+
# 2393| m2393_3(int) = Store[#return] : &:r2393_1, r2393_2
15163+
# 2392| r2392_5(glval<int>) = VariableAddress[#return] :
15164+
# 2392| v2392_6(void) = ReturnValue : &:r2392_5, m2393_3
15165+
# 2392| v2392_7(void) = AliasedUse : m2392_3
15166+
# 2392| v2392_8(void) = ExitFunction :
15167+
1513815168
perf-regression.cpp:
1513915169
# 6| void Big::Big()
1514015170
# 6| Block 0

‎cpp/ql/test/library-tests/ir/ir/ir.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2378,4 +2378,19 @@ namespace return_routine_type {
23782378

23792379
}
23802380

2381+
int small_operation_should_not_be_constant_folded() {
2382+
return 1 ^ 2;
2383+
}
2384+
2385+
#define BINOP2(x) (x ^ x)
2386+
#define BINOP4(x) (BINOP2(x) ^ BINOP2(x))
2387+
#define BINOP8(x) (BINOP4(x) ^ BINOP4(x))
2388+
#define BINOP16(x) (BINOP8(x) ^ BINOP8(x))
2389+
#define BINOP32(x) (BINOP16(x) ^ BINOP16(x))
2390+
#define BINOP64(x) (BINOP32(x) ^ BINOP32(x))
2391+
2392+
int large_operation_should_be_constant_folded() {
2393+
return BINOP64(1);
2394+
}
2395+
23812396
// semmle-extractor-options: -std=c++20 --clang

‎cpp/ql/test/library-tests/ir/ir/operand_locations.expected‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12530,6 +12530,22 @@
1253012530
| ir.cpp:2374:32:2374:47 | SideEffect | m2374_3 |
1253112531
| ir.cpp:2376:9:2376:44 | Address | &:r2376_1 |
1253212532
| ir.cpp:2376:16:2376:43 | StoreValue | r2376_2 |
12533+
| ir.cpp:2381:5:2381:49 | Address | &:r2381_5 |
12534+
| ir.cpp:2381:5:2381:49 | ChiPartial | partial:m2381_3 |
12535+
| ir.cpp:2381:5:2381:49 | ChiTotal | total:m2381_2 |
12536+
| ir.cpp:2381:5:2381:49 | Load | m2382_5 |
12537+
| ir.cpp:2381:5:2381:49 | SideEffect | m2381_3 |
12538+
| ir.cpp:2382:5:2382:17 | Address | &:r2382_1 |
12539+
| ir.cpp:2382:12:2382:12 | Left | r2382_2 |
12540+
| ir.cpp:2382:12:2382:16 | StoreValue | r2382_4 |
12541+
| ir.cpp:2382:16:2382:16 | Right | r2382_3 |
12542+
| ir.cpp:2392:5:2392:45 | Address | &:r2392_5 |
12543+
| ir.cpp:2392:5:2392:45 | ChiPartial | partial:m2392_3 |
12544+
| ir.cpp:2392:5:2392:45 | ChiTotal | total:m2392_2 |
12545+
| ir.cpp:2392:5:2392:45 | Load | m2393_3 |
12546+
| ir.cpp:2392:5:2392:45 | SideEffect | m2392_3 |
12547+
| ir.cpp:2393:5:2393:22 | Address | &:r2393_1 |
12548+
| ir.cpp:2393:12:2393:21 | StoreValue | r2393_2 |
1253312549
| perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
1253412550
| perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
1253512551
| perf-regression.cpp:6:3:6:5 | Address | &:r6_7 |

‎cpp/ql/test/library-tests/ir/ir/raw_ir.expected‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13977,6 +13977,34 @@ ir.cpp:
1397713977
# 2374| v2374_6(void) = AliasedUse : ~m?
1397813978
# 2374| v2374_7(void) = ExitFunction :
1397913979

13980+
# 2381| int small_operation_should_not_be_constant_folded()
13981+
# 2381| Block 0
13982+
# 2381| v2381_1(void) = EnterFunction :
13983+
# 2381| mu2381_2(unknown) = AliasedDefinition :
13984+
# 2381| mu2381_3(unknown) = InitializeNonLocal :
13985+
# 2382| r2382_1(glval<int>) = VariableAddress[#return] :
13986+
# 2382| r2382_2(int) = Constant[1] :
13987+
# 2382| r2382_3(int) = Constant[2] :
13988+
# 2382| r2382_4(int) = BitXor : r2382_2, r2382_3
13989+
# 2382| mu2382_5(int) = Store[#return] : &:r2382_1, r2382_4
13990+
# 2381| r2381_4(glval<int>) = VariableAddress[#return] :
13991+
# 2381| v2381_5(void) = ReturnValue : &:r2381_4, ~m?
13992+
# 2381| v2381_6(void) = AliasedUse : ~m?
13993+
# 2381| v2381_7(void) = ExitFunction :
13994+
13995+
# 2392| int large_operation_should_be_constant_folded()
13996+
# 2392| Block 0
13997+
# 2392| v2392_1(void) = EnterFunction :
13998+
# 2392| mu2392_2(unknown) = AliasedDefinition :
13999+
# 2392| mu2392_3(unknown) = InitializeNonLocal :
14000+
# 2393| r2393_1(glval<int>) = VariableAddress[#return] :
14001+
# 2393| r2393_2(int) = Constant[0] :
14002+
# 2393| mu2393_3(int) = Store[#return] : &:r2393_1, r2393_2
14003+
# 2392| r2392_4(glval<int>) = VariableAddress[#return] :
14004+
# 2392| v2392_5(void) = ReturnValue : &:r2392_4, ~m?
14005+
# 2392| v2392_6(void) = AliasedUse : ~m?
14006+
# 2392| v2392_7(void) = ExitFunction :
14007+
1398014008
perf-regression.cpp:
1398114009
# 6| void Big::Big()
1398214010
# 6| Block 0

0 commit comments

Comments
 (0)