Skip to content

Commit c5da43e

Browse files
MathiasVPMathiasVP
authored
Merge pull request #16981 from MathiasVP/phi-escape-5-follow-up-2
C++: Alias analysis follow-up to #16907
2 parents 032ae9e + b3bffb6 commit c5da43e

File tree

11 files changed

+141
-14
lines changed

11 files changed

+141
-14
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,56 @@ private predicate resultEscapesNonReturn(Instruction instr) {
338338
not instr.isResultModeled()
339339
}
340340

341+
/** Holds if `operand` may (transitively) flow to an `AddressOperand`. */
342+
private predicate consumedAsAddressOperand(Operand operand) {
343+
operand instanceof AddressOperand
344+
or
345+
exists(Operand address |
346+
consumedAsAddressOperand(address) and
347+
operandIsPropagated(operand, _, address.getDef())
348+
)
349+
}
350+
351+
/**
352+
* Holds if `operand` may originate from a base instruction of an allocation,
353+
* and that operand may transitively flow to an `AddressOperand`.
354+
*/
355+
private predicate propagatedFromAllocationBase(Operand operand, Configuration::Allocation allocation) {
356+
consumedAsAddressOperand(operand) and
357+
(
358+
not exists(Configuration::getOldAllocation(allocation)) and
359+
operand.getDef() = allocation.getABaseInstruction()
360+
or
361+
exists(Operand address |
362+
operandIsPropagated(address, _, operand.getDef()) and
363+
propagatedFromAllocationBase(address, allocation)
364+
)
365+
)
366+
}
367+
368+
private predicate propagatedFromNonAllocationBase(Operand operand) {
369+
exists(Instruction def |
370+
def = operand.getDef() and
371+
not operandIsPropagated(_, _, def) and
372+
not def = any(Configuration::Allocation allocation).getABaseInstruction()
373+
)
374+
or
375+
exists(Operand address |
376+
operandIsPropagated(address, _, operand.getDef()) and
377+
propagatedFromNonAllocationBase(address)
378+
)
379+
}
380+
381+
/**
382+
* Holds if we cannot see all producers of an operand for which allocation also flows into.
383+
*/
384+
private predicate operandConsumesEscaped(Configuration::Allocation allocation) {
385+
exists(AddressOperand address |
386+
propagatedFromAllocationBase(address, allocation) and
387+
propagatedFromNonAllocationBase(address)
388+
)
389+
}
390+
341391
/**
342392
* Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur
343393
* either because the allocation's address is taken within the function and escapes, or because the
@@ -346,12 +396,14 @@ private predicate resultEscapesNonReturn(Instruction instr) {
346396
predicate allocationEscapes(Configuration::Allocation allocation) {
347397
allocation.alwaysEscapes()
348398
or
349-
exists(IREscapeAnalysisConfiguration config |
350-
config.useSoundEscapeAnalysis() and resultEscapesNonReturn(allocation.getABaseInstruction())
399+
exists(IREscapeAnalysisConfiguration config | config.useSoundEscapeAnalysis() |
400+
resultEscapesNonReturn(allocation.getABaseInstruction())
401+
or
402+
operandConsumesEscaped(allocation)
351403
)
352404
or
353405
Configuration::phaseNeedsSoundEscapeAnalysis() and
354-
resultEscapesNonReturn(allocation.getABaseInstruction())
406+
(resultEscapesNonReturn(allocation.getABaseInstruction()) or operandConsumesEscaped(allocation))
355407
}
356408

357409
/**

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,8 @@ class DynamicAllocation extends Allocation, TDynamicAllocation {
146146
}
147147

148148
predicate phaseNeedsSoundEscapeAnalysis() { none() }
149+
150+
UnaliasedSsa::Allocation getOldAllocation(VariableAllocation allocation) {
151+
UnaliasedSsa::canReuseSsaForVariable(allocation.getIRVariable()) and
152+
result = allocation.getIRVariable()
153+
}

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ private import semmle.code.cpp.ir.implementation.unaliased_ssa.internal.SSAConst
77
private import semmle.code.cpp.ir.internal.IntegerConstant as Ints
88
private import semmle.code.cpp.ir.internal.IntegerInterval as Interval
99
private import semmle.code.cpp.ir.implementation.internal.OperandTag
10-
private import AliasConfiguration
10+
import AliasConfiguration
1111
private import codeql.util.Boolean
1212

1313
private class IntValue = Ints::IntValue;

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,56 @@ private predicate resultEscapesNonReturn(Instruction instr) {
338338
not instr.isResultModeled()
339339
}
340340

341+
/** Holds if `operand` may (transitively) flow to an `AddressOperand`. */
342+
private predicate consumedAsAddressOperand(Operand operand) {
343+
operand instanceof AddressOperand
344+
or
345+
exists(Operand address |
346+
consumedAsAddressOperand(address) and
347+
operandIsPropagated(operand, _, address.getDef())
348+
)
349+
}
350+
351+
/**
352+
* Holds if `operand` may originate from a base instruction of an allocation,
353+
* and that operand may transitively flow to an `AddressOperand`.
354+
*/
355+
private predicate propagatedFromAllocationBase(Operand operand, Configuration::Allocation allocation) {
356+
consumedAsAddressOperand(operand) and
357+
(
358+
not exists(Configuration::getOldAllocation(allocation)) and
359+
operand.getDef() = allocation.getABaseInstruction()
360+
or
361+
exists(Operand address |
362+
operandIsPropagated(address, _, operand.getDef()) and
363+
propagatedFromAllocationBase(address, allocation)
364+
)
365+
)
366+
}
367+
368+
private predicate propagatedFromNonAllocationBase(Operand operand) {
369+
exists(Instruction def |
370+
def = operand.getDef() and
371+
not operandIsPropagated(_, _, def) and
372+
not def = any(Configuration::Allocation allocation).getABaseInstruction()
373+
)
374+
or
375+
exists(Operand address |
376+
operandIsPropagated(address, _, operand.getDef()) and
377+
propagatedFromNonAllocationBase(address)
378+
)
379+
}
380+
381+
/**
382+
* Holds if we cannot see all producers of an operand for which allocation also flows into.
383+
*/
384+
private predicate operandConsumesEscaped(Configuration::Allocation allocation) {
385+
exists(AddressOperand address |
386+
propagatedFromAllocationBase(address, allocation) and
387+
propagatedFromNonAllocationBase(address)
388+
)
389+
}
390+
341391
/**
342392
* Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur
343393
* either because the allocation's address is taken within the function and escapes, or because the
@@ -346,12 +396,14 @@ private predicate resultEscapesNonReturn(Instruction instr) {
346396
predicate allocationEscapes(Configuration::Allocation allocation) {
347397
allocation.alwaysEscapes()
348398
or
349-
exists(IREscapeAnalysisConfiguration config |
350-
config.useSoundEscapeAnalysis() and resultEscapesNonReturn(allocation.getABaseInstruction())
399+
exists(IREscapeAnalysisConfiguration config | config.useSoundEscapeAnalysis() |
400+
resultEscapesNonReturn(allocation.getABaseInstruction())
401+
or
402+
operandConsumesEscaped(allocation)
351403
)
352404
or
353405
Configuration::phaseNeedsSoundEscapeAnalysis() and
354-
resultEscapesNonReturn(allocation.getABaseInstruction())
406+
(resultEscapesNonReturn(allocation.getABaseInstruction()) or operandConsumesEscaped(allocation))
355407
}
356408

357409
/**

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import AliasConfigurationImports
2+
private import codeql.util.Unit
23

34
/**
45
* A memory allocation that can be tracked by the SimpleSSA alias analysis.
@@ -16,3 +17,5 @@ class Allocation extends IRAutomaticVariable {
1617
}
1718

1819
predicate phaseNeedsSoundEscapeAnalysis() { any() }
20+
21+
Unit getOldAllocation(Allocation allocation) { any() }

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import AliasAnalysis
22
private import SimpleSSAImports
33
import SimpleSSAPublicImports
4-
private import AliasConfiguration
4+
import AliasConfiguration
55
private import codeql.util.Unit
66

77
private predicate isTotalAccess(Allocation var, AddressOperand addrOperand, IRType type) {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18779,22 +18779,23 @@ ir.cpp:
1877918779
# 2667| r2667_1(glval<int>) = VariableAddress[intBuffer] :
1878018780
# 2667| r2667_2(int) = Constant[8] :
1878118781
# 2667| m2667_3(int) = Store[intBuffer] : &:r2667_1, r2667_2
18782+
# 2667| m2667_4(unknown) = Chi : total:m2663_4, partial:m2667_3
1878218783
# 2668| r2668_1(glval<int>) = VariableAddress[intBuffer] :
1878318784
# 2668| r2668_2(int *) = CopyValue : r2668_1
1878418785
# 2668| r2668_3(glval<int *>) = VariableAddress[data] :
1878518786
# 2668| m2668_4(int *) = Store[data] : &:r2668_3, r2668_2
1878618787
#-----| Goto -> Block 2
1878718788

1878818789
# 2670| Block 2
18789-
# 2670| m2670_1(int) = Phi : from 1:m2667_3
18790+
# 2670| m2670_1(unknown) = Phi : from 0:~m2663_4, from 1:~m2667_4
1879018791
# 2670| m2670_2(int *) = Phi : from 0:m2665_3, from 1:m2668_4
1879118792
# 2670| r2670_3(glval<unknown>) = FunctionAddress[use_int] :
1879218793
# 2670| r2670_4(glval<int *>) = VariableAddress[data] :
1879318794
# 2670| r2670_5(int *) = Load[data] : &:r2670_4, m2670_2
1879418795
# 2670| r2670_6(int) = Load[?] : &:r2670_5, ~m2670_1
1879518796
# 2670| v2670_7(void) = Call[use_int] : func:r2670_3, 0:r2670_6
18796-
# 2670| m2670_8(unknown) = ^CallSideEffect : ~m2663_4
18797-
# 2670| m2670_9(unknown) = Chi : total:m2663_4, partial:m2670_8
18797+
# 2670| m2670_8(unknown) = ^CallSideEffect : ~m2670_1
18798+
# 2670| m2670_9(unknown) = Chi : total:m2670_1, partial:m2670_8
1879818799
# 2671| v2671_1(void) = NoOp :
1879918800
# 2663| v2663_7(void) = ReturnVoid :
1880018801
# 2663| v2663_8(void) = AliasedUse : ~m2670_9

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ missingOperand
22
unexpectedOperand
33
duplicateOperand
44
missingPhiOperand
5-
| ir.cpp:2670:3:2670:9 | Phi: call to use_int | Instruction 'Phi: call to use_int' is missing an operand for predecessor block 'EnterFunction: phi_with_single_input_at_merge' in function '$@'. | ir.cpp:2663:13:2663:42 | void phi_with_single_input_at_merge(bool) | void phi_with_single_input_at_merge(bool) |
65
missingOperandType
76
duplicateChiOperand
87
sideEffectWithoutPrimary

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ missingOperand
22
unexpectedOperand
33
duplicateOperand
44
missingPhiOperand
5-
| ir.cpp:2670:3:2670:9 | Phi: call to use_int | Instruction 'Phi: call to use_int' is missing an operand for predecessor block 'EnterFunction: phi_with_single_input_at_merge' in function '$@'. | ir.cpp:2663:13:2663:42 | void phi_with_single_input_at_merge(bool) | void phi_with_single_input_at_merge(bool) |
65
missingOperandType
76
duplicateChiOperand
87
sideEffectWithoutPrimary

‎cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/LoopConditionsConst.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,4 @@
2323
| test.cpp:424:2:425:2 | for(...;...;...) ... | test.cpp:424:18:424:23 | ... < ... | 1 | i | { ... } | i | return ... |
2424
| test.cpp:433:2:434:2 | for(...;...;...) ... | test.cpp:433:18:433:22 | 0 | 0 | | { ... } | 0 | return ... |
2525
| test.cpp:559:3:564:3 | while (...) ... | test.cpp:559:9:559:15 | call to getBool | | call to getBool | { ... } | call to getBool | ExprStmt |
26+
| test.cpp:574:3:579:3 | while (...) ... | test.cpp:574:10:574:16 | call to getBool | | call to getBool | { ... } | call to getBool | ExprStmt |

0 commit comments

Comments
 (0)