Skip to content

C++: Disable _some_ constant folding in IR #15969

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.

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 10 commits into from
Mar 20, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ private int getBinaryInstructionValue(BinaryInstruction instr) {
or
instr instanceof DivInstruction and result = div(left, right)
or
instr instanceof BitOrInstruction and result = bitOr(left, right)
or
instr instanceof BitAndInstruction and result = bitAnd(left, right)
or
instr instanceof BitXorInstruction and result = bitXor(left, right)
or
instr instanceof CompareEQInstruction and result = compareEQ(left, right)
or
instr instanceof CompareNEInstruction and result = compareNE(left, right)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ private int getBinaryInstructionValue(BinaryInstruction instr) {
or
instr instanceof DivInstruction and result = div(left, right)
or
instr instanceof BitOrInstruction and result = bitOr(left, right)
or
instr instanceof BitAndInstruction and result = bitAnd(left, right)
or
instr instanceof BitXorInstruction and result = bitXor(left, right)
or
instr instanceof CompareEQInstruction and result = compareEQ(left, right)
or
instr instanceof CompareNEInstruction and result = compareNE(left, right)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,43 @@ IRTempVariable getIRTempVariable(Locatable ast, TempVariableTag tag) {
result.getTag() = tag
}

/** Gets an operand of `op`. */
private Expr getAnOperand(Operation op) { result = op.getAnOperand() }

/**
* Gets the number of nested operands of `op`. For example,
* `getNumberOfNestedBinaryOperands((1 + 2) + 3))` is `3`.
*/
private int getNumberOfNestedBinaryOperands(Operation op) { result = count(getAnOperand*(op)) }

/**
* Holds if `op` should not be translated to a `ConstantInstruction` as part of
* IR generation, even if the value of `op` is constant.
*/
private predicate ignoreConstantValue(Operation op) {
op instanceof BitwiseAndExpr
or
op instanceof BitwiseOrExpr
or
op instanceof BitwiseXorExpr
}

/**
* Holds if `expr` is a constant of a type that can be replaced directly with
* its value in the IR. This does not include address constants as we have no
* means to express those as QL values.
*/
predicate isIRConstant(Expr expr) { exists(expr.getValue()) }
predicate isIRConstant(Expr expr) {
exists(expr.getValue()) and
// We avoid constant folding certain operations since it's often useful to
// mark one of those as a source in dataflow, and if the operation is
// constant folded it's not possible to mark its operands as a source (or
// sink).
// But to avoid creating an outrageous amount of IR from very large
// constant expressions we fall back to constant folding if the operation
// has more than 50 operands (i.e., 1 + 2 + 3 + 4 + ... + 50)
if ignoreConstantValue(expr) then getNumberOfNestedBinaryOperands(expr) > 50 else any()
}

// Pulled out for performance. See
// https://github.com/github/codeql-coreql-team/issues/1044.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ private int getBinaryInstructionValue(BinaryInstruction instr) {
or
instr instanceof DivInstruction and result = div(left, right)
or
instr instanceof BitOrInstruction and result = bitOr(left, right)
or
instr instanceof BitAndInstruction and result = bitAnd(left, right)
or
instr instanceof BitXorInstruction and result = bitXor(left, right)
or
instr instanceof CompareEQInstruction and result = compareEQ(left, right)
or
instr instanceof CompareNEInstruction and result = compareNE(left, right)
Expand Down
12 changes: 12 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/ir/internal/IntegerPartial.qll
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ int compareLE(int a, int b) { if a <= b then result = 1 else result = 0 }
bindingset[a, b]
int compareGE(int a, int b) { if a >= b then result = 1 else result = 0 }

/** Returns `a | b`. */
bindingset[a, b]
int bitOr(int a, int b) { result = a.bitOr(b) }

/** Returns `a & b`. */
bindingset[a, b]
int bitAnd(int a, int b) { result = a.bitAnd(b) }

/** Returns `a ^ b`. */
bindingset[a, b]
int bitXor(int a, int b) { result = a.bitXor(b) }

/**
* Returns `-a`. If the negation would overflow, there is no result.
*/
Expand Down
780 changes: 780 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/PrintAST.expected

Large diffs are not rendered by default.

30 changes: 30 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/aliased_ir.expected
Original file line number Diff line number Diff line change
Expand Up @@ -15135,6 +15135,36 @@ ir.cpp:
# 2374| v2374_7(void) = AliasedUse : m2374_3
# 2374| v2374_8(void) = ExitFunction :

# 2381| int small_operation_should_not_be_constant_folded()
# 2381| Block 0
# 2381| v2381_1(void) = EnterFunction :
# 2381| m2381_2(unknown) = AliasedDefinition :
# 2381| m2381_3(unknown) = InitializeNonLocal :
# 2381| m2381_4(unknown) = Chi : total:m2381_2, partial:m2381_3
# 2382| r2382_1(glval<int>) = VariableAddress[#return] :
# 2382| r2382_2(int) = Constant[1] :
# 2382| r2382_3(int) = Constant[2] :
# 2382| r2382_4(int) = BitXor : r2382_2, r2382_3
# 2382| m2382_5(int) = Store[#return] : &:r2382_1, r2382_4
# 2381| r2381_5(glval<int>) = VariableAddress[#return] :
# 2381| v2381_6(void) = ReturnValue : &:r2381_5, m2382_5
# 2381| v2381_7(void) = AliasedUse : m2381_3
# 2381| v2381_8(void) = ExitFunction :

# 2392| int large_operation_should_be_constant_folded()
# 2392| Block 0
# 2392| v2392_1(void) = EnterFunction :
# 2392| m2392_2(unknown) = AliasedDefinition :
# 2392| m2392_3(unknown) = InitializeNonLocal :
# 2392| m2392_4(unknown) = Chi : total:m2392_2, partial:m2392_3
# 2393| r2393_1(glval<int>) = VariableAddress[#return] :
# 2393| r2393_2(int) = Constant[0] :
# 2393| m2393_3(int) = Store[#return] : &:r2393_1, r2393_2
# 2392| r2392_5(glval<int>) = VariableAddress[#return] :
# 2392| v2392_6(void) = ReturnValue : &:r2392_5, m2393_3
# 2392| v2392_7(void) = AliasedUse : m2392_3
# 2392| v2392_8(void) = ExitFunction :

perf-regression.cpp:
# 6| void Big::Big()
# 6| Block 0
Expand Down
15 changes: 15 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/ir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2378,4 +2378,19 @@ namespace return_routine_type {

}

int small_operation_should_not_be_constant_folded() {
return 1 ^ 2;
}

#define BINOP2(x) (x ^ x)
#define BINOP4(x) (BINOP2(x) ^ BINOP2(x))
#define BINOP8(x) (BINOP4(x) ^ BINOP4(x))
#define BINOP16(x) (BINOP8(x) ^ BINOP8(x))
#define BINOP32(x) (BINOP16(x) ^ BINOP16(x))
#define BINOP64(x) (BINOP32(x) ^ BINOP32(x))

int large_operation_should_be_constant_folded() {
return BINOP64(1);
}

// semmle-extractor-options: -std=c++20 --clang
16 changes: 16 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/operand_locations.expected
Original file line number Diff line number Diff line change
Expand Up @@ -12530,6 +12530,22 @@
| ir.cpp:2374:32:2374:47 | SideEffect | m2374_3 |
| ir.cpp:2376:9:2376:44 | Address | &:r2376_1 |
| ir.cpp:2376:16:2376:43 | StoreValue | r2376_2 |
| ir.cpp:2381:5:2381:49 | Address | &:r2381_5 |
| ir.cpp:2381:5:2381:49 | ChiPartial | partial:m2381_3 |
| ir.cpp:2381:5:2381:49 | ChiTotal | total:m2381_2 |
| ir.cpp:2381:5:2381:49 | Load | m2382_5 |
| ir.cpp:2381:5:2381:49 | SideEffect | m2381_3 |
| ir.cpp:2382:5:2382:17 | Address | &:r2382_1 |
| ir.cpp:2382:12:2382:12 | Left | r2382_2 |
| ir.cpp:2382:12:2382:16 | StoreValue | r2382_4 |
| ir.cpp:2382:16:2382:16 | Right | r2382_3 |
| ir.cpp:2392:5:2392:45 | Address | &:r2392_5 |
| ir.cpp:2392:5:2392:45 | ChiPartial | partial:m2392_3 |
| ir.cpp:2392:5:2392:45 | ChiTotal | total:m2392_2 |
| ir.cpp:2392:5:2392:45 | Load | m2393_3 |
| ir.cpp:2392:5:2392:45 | SideEffect | m2392_3 |
| ir.cpp:2393:5:2393:22 | Address | &:r2393_1 |
| ir.cpp:2393:12:2393:21 | StoreValue | r2393_2 |
| perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
| perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
| perf-regression.cpp:6:3:6:5 | Address | &:r6_7 |
Expand Down
28 changes: 28 additions & 0 deletions cpp/ql/test/library-tests/ir/ir/raw_ir.expected
Original file line number Diff line number Diff line change
Expand Up @@ -13977,6 +13977,34 @@ ir.cpp:
# 2374| v2374_6(void) = AliasedUse : ~m?
# 2374| v2374_7(void) = ExitFunction :

# 2381| int small_operation_should_not_be_constant_folded()
# 2381| Block 0
# 2381| v2381_1(void) = EnterFunction :
# 2381| mu2381_2(unknown) = AliasedDefinition :
# 2381| mu2381_3(unknown) = InitializeNonLocal :
# 2382| r2382_1(glval<int>) = VariableAddress[#return] :
# 2382| r2382_2(int) = Constant[1] :
# 2382| r2382_3(int) = Constant[2] :
# 2382| r2382_4(int) = BitXor : r2382_2, r2382_3
# 2382| mu2382_5(int) = Store[#return] : &:r2382_1, r2382_4
# 2381| r2381_4(glval<int>) = VariableAddress[#return] :
# 2381| v2381_5(void) = ReturnValue : &:r2381_4, ~m?
# 2381| v2381_6(void) = AliasedUse : ~m?
# 2381| v2381_7(void) = ExitFunction :

# 2392| int large_operation_should_be_constant_folded()
# 2392| Block 0
# 2392| v2392_1(void) = EnterFunction :
# 2392| mu2392_2(unknown) = AliasedDefinition :
# 2392| mu2392_3(unknown) = InitializeNonLocal :
# 2393| r2393_1(glval<int>) = VariableAddress[#return] :
# 2393| r2393_2(int) = Constant[0] :
# 2393| mu2393_3(int) = Store[#return] : &:r2393_1, r2393_2
# 2392| r2392_4(glval<int>) = VariableAddress[#return] :
# 2392| v2392_5(void) = ReturnValue : &:r2392_4, ~m?
# 2392| v2392_6(void) = AliasedUse : ~m?
# 2392| v2392_7(void) = ExitFunction :

perf-regression.cpp:
# 6| void Big::Big()
# 6| Block 0
Expand Down