Skip to content

Commit c5ac357

Browse files
author
Dave Bartolomeo
committed
C++/C#: Fix bad overlap sanity failures
`Instruction.getDefinitionOverlap()` depends on `SSAConstruction::getMemoryOperandDefinition()`, which in turn depends on `SSAConstruction::hasMemoryOperandDefinition()`. When the definition in question came from a `Chi` instruction, `hasMemoryOperandDefinition()` incorrectly bound `overlap` to the overlap relationship between the original (non-`Chi`) instruction and the use. The fix is to make use of the `actualDefLocation` parameter to `getDefinitionOrChiInstruction()`, which specifies the location for the result of the `Chi` in that case.
1 parent a2741da commit c5ac357

File tree

12 files changed

+20
-685
lines changed

12 files changed

+20
-685
lines changed

‎cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,15 @@ private module Cached {
8484
oldOperand instanceof OldIR::NonPhiMemoryOperand and
8585
exists(
8686
OldBlock useBlock, int useRank, Alias::MemoryLocation useLocation,
87-
Alias::MemoryLocation defLocation, OldBlock defBlock, int defRank, int defOffset
87+
Alias::MemoryLocation defLocation, OldBlock defBlock, int defRank, int defOffset,
88+
Alias::MemoryLocation actualDefLocation
8889
|
8990
useLocation = Alias::getOperandMemoryLocation(oldOperand) and
9091
hasUseAtRank(useLocation, useBlock, useRank, oldInstruction) and
9192
definitionReachesUse(useLocation, defBlock, defRank, useBlock, useRank) and
9293
hasDefinitionAtRank(useLocation, defLocation, defBlock, defRank, defOffset) and
93-
instr = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, _) and
94-
overlap = Alias::getOverlap(defLocation, useLocation)
94+
instr = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, actualDefLocation) and
95+
overlap = Alias::getOverlap(actualDefLocation, useLocation)
9596
)
9697
}
9798

‎cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,15 @@ private module Cached {
8484
oldOperand instanceof OldIR::NonPhiMemoryOperand and
8585
exists(
8686
OldBlock useBlock, int useRank, Alias::MemoryLocation useLocation,
87-
Alias::MemoryLocation defLocation, OldBlock defBlock, int defRank, int defOffset
87+
Alias::MemoryLocation defLocation, OldBlock defBlock, int defRank, int defOffset,
88+
Alias::MemoryLocation actualDefLocation
8889
|
8990
useLocation = Alias::getOperandMemoryLocation(oldOperand) and
9091
hasUseAtRank(useLocation, useBlock, useRank, oldInstruction) and
9192
definitionReachesUse(useLocation, defBlock, defRank, useBlock, useRank) and
9293
hasDefinitionAtRank(useLocation, defLocation, defBlock, defRank, defOffset) and
93-
instr = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, _) and
94-
overlap = Alias::getOverlap(defLocation, useLocation)
94+
instr = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, actualDefLocation) and
95+
overlap = Alias::getOverlap(actualDefLocation, useLocation)
9596
)
9697
}
9798

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

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

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

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

‎cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ ssa.cpp:
459459
# 112| m112_12(Point) = Chi : total:m112_7, partial:m112_11
460460
# 113| r113_1(glval<Point>) = VariableAddress[b] :
461461
# 113| r113_2(glval<Point>) = VariableAddress[a] :
462-
# 113| r113_3(Point) = Load : &:r113_2, ~m112_12
462+
# 113| r113_3(Point) = Load : &:r113_2, m112_12
463463
# 113| m113_4(Point) = Store : &:r113_1, r113_3
464464
# 114| v114_1(void) = NoOp :
465465
# 111| v111_10(void) = ReturnVoid :

‎cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ ssa.cpp:
457457
# 112| m112_12(Point) = Chi : total:m112_7, partial:m112_11
458458
# 113| r113_1(glval<Point>) = VariableAddress[b] :
459459
# 113| r113_2(glval<Point>) = VariableAddress[a] :
460-
# 113| r113_3(Point) = Load : &:r113_2, ~m112_12
460+
# 113| r113_3(Point) = Load : &:r113_2, m112_12
461461
# 113| m113_4(Point) = Store : &:r113_1, r113_3
462462
# 114| v114_1(void) = NoOp :
463463
# 111| v111_10(void) = ReturnVoid :
@@ -490,7 +490,7 @@ ssa.cpp:
490490
# 117| m117_12(Point) = Chi : total:m117_7, partial:m117_11
491491
# 118| r118_1(glval<Point>) = VariableAddress[b] :
492492
# 118| r118_2(glval<Point>) = VariableAddress[a] :
493-
# 118| r118_3(Point) = Load : &:r118_2, ~m117_12
493+
# 118| r118_3(Point) = Load : &:r118_2, m117_12
494494
# 118| m118_4(Point) = Store : &:r118_1, r118_3
495495
# 119| r119_1(glval<unknown>) = FunctionAddress[Escape] :
496496
# 119| r119_2(glval<Point>) = VariableAddress[a] :
@@ -941,7 +941,7 @@ ssa.cpp:
941941
# 209| m209_12(int) = Chi : total:m208_2, partial:m209_11
942942
# 210| r210_1(glval<int>) = VariableAddress[#return] :
943943
# 210| r210_2(glval<int>) = VariableAddress[y] :
944-
# 210| r210_3(int) = Load : &:r210_2, ~m209_12
944+
# 210| r210_3(int) = Load : &:r210_2, m209_12
945945
# 210| m210_4(int) = Store : &:r210_1, r210_3
946946
# 207| r207_8(glval<int>) = VariableAddress[#return] :
947947
# 207| v207_9(void) = ReturnValue : &:r207_8, m210_4
@@ -1179,7 +1179,7 @@ ssa.cpp:
11791179
# 251| r251_2(glval<char *>) = VariableAddress[dst] :
11801180
# 251| r251_3(char *) = Load : &:r251_2, m248_12
11811181
# 251| m251_4(char *) = Store : &:r251_1, r251_3
1182-
# 247| v247_12(void) = ReturnIndirection : &:r247_8, ~m249_6
1182+
# 247| v247_12(void) = ReturnIndirection : &:r247_8, m249_6
11831183
# 247| r247_13(glval<char *>) = VariableAddress[#return] :
11841184
# 247| v247_14(void) = ReturnValue : &:r247_13, m251_4
11851185
# 247| v247_15(void) = UnmodeledUse : mu*

‎cpp/ql/test/library-tests/ir/ssa/aliased_ssa_sanity.expected

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,41 +17,6 @@ backEdgeCountMismatch
1717
useNotDominatedByDefinition
1818
switchInstructionWithoutDefaultEdge
1919
invalidOverlap
20-
| ssa.cpp:95:6:95:30 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:95:6:95:30 | IR: MustExactlyOverlapEscaped | void MustExactlyOverlapEscaped(Point) |
21-
| ssa.cpp:97:3:97:8 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:95:6:95:30 | IR: MustExactlyOverlapEscaped | void MustExactlyOverlapEscaped(Point) |
22-
| ssa.cpp:97:10:97:11 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:95:6:95:30 | IR: MustExactlyOverlapEscaped | void MustExactlyOverlapEscaped(Point) |
23-
| ssa.cpp:105:6:105:30 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:105:6:105:30 | IR: MustTotallyOverlapEscaped | void MustTotallyOverlapEscaped(Point) |
24-
| ssa.cpp:108:3:108:8 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:105:6:105:30 | IR: MustTotallyOverlapEscaped | void MustTotallyOverlapEscaped(Point) |
25-
| ssa.cpp:108:10:108:11 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:105:6:105:30 | IR: MustTotallyOverlapEscaped | void MustTotallyOverlapEscaped(Point) |
26-
| ssa.cpp:113:13:113:13 | Load | MemoryOperand 'Load' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:111:6:111:24 | IR: MayPartiallyOverlap | void MayPartiallyOverlap(int, int) |
27-
| ssa.cpp:116:6:116:31 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:116:6:116:31 | IR: MayPartiallyOverlapEscaped | void MayPartiallyOverlapEscaped(int, int) |
28-
| ssa.cpp:118:13:118:13 | Load | MemoryOperand 'Load' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:116:6:116:31 | IR: MayPartiallyOverlapEscaped | void MayPartiallyOverlapEscaped(int, int) |
29-
| ssa.cpp:119:3:119:8 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:116:6:116:31 | IR: MayPartiallyOverlapEscaped | void MayPartiallyOverlapEscaped(int, int) |
30-
| ssa.cpp:119:10:119:11 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:116:6:116:31 | IR: MayPartiallyOverlapEscaped | void MayPartiallyOverlapEscaped(int, int) |
31-
| ssa.cpp:179:5:179:11 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:179:5:179:11 | IR: AsmStmt | int AsmStmt(int*) |
32-
| ssa.cpp:180:3:180:14 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:179:5:179:11 | IR: AsmStmt | int AsmStmt(int*) |
33-
| ssa.cpp:184:13:184:30 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:184:13:184:30 | IR: AsmStmtWithOutputs | void AsmStmtWithOutputs(unsigned int&, unsigned int&, unsigned int&, unsigned int&) |
34-
| ssa.cpp:184:46:184:46 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:184:13:184:30 | IR: AsmStmtWithOutputs | void AsmStmtWithOutputs(unsigned int&, unsigned int&, unsigned int&, unsigned int&) |
35-
| ssa.cpp:184:63:184:63 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:184:13:184:30 | IR: AsmStmtWithOutputs | void AsmStmtWithOutputs(unsigned int&, unsigned int&, unsigned int&, unsigned int&) |
36-
| ssa.cpp:210:10:210:10 | Load | MemoryOperand 'Load' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:207:5:207:21 | IR: ModeledCallTarget | int ModeledCallTarget(int) |
37-
| ssa.cpp:226:6:226:26 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:226:6:226:26 | IR: StringLiteralAliasing | char StringLiteralAliasing() |
38-
| ssa.cpp:227:3:227:14 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:226:6:226:26 | IR: StringLiteralAliasing | char StringLiteralAliasing() |
39-
| ssa.cpp:239:6:239:29 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
40-
| ssa.cpp:240:19:240:20 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
41-
| ssa.cpp:241:3:241:3 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
42-
| ssa.cpp:241:5:241:5 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
43-
| ssa.cpp:242:3:242:3 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
44-
| ssa.cpp:242:5:242:5 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
45-
| ssa.cpp:243:21:243:37 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
46-
| ssa.cpp:244:3:244:4 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
47-
| ssa.cpp:244:6:244:6 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:239:6:239:29 | IR: ExplicitConstructorCalls | void ExplicitConstructorCalls() |
48-
| ssa.cpp:247:7:247:32 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:247:7:247:32 | IR: VoidStarIndirectParameters | char* VoidStarIndirectParameters(char*, int) |
49-
| ssa.cpp:247:40:247:42 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:247:7:247:32 | IR: VoidStarIndirectParameters | char* VoidStarIndirectParameters(char*, int) |
50-
| ssa.cpp:250:15:250:17 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:247:7:247:32 | IR: VoidStarIndirectParameters | char* VoidStarIndirectParameters(char*, int) |
51-
| ssa.cpp:256:5:256:16 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:254:6:254:27 | IR: StringLiteralAliasing2 | char StringLiteralAliasing2(bool) |
52-
| ssa.cpp:259:5:259:16 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:254:6:254:27 | IR: StringLiteralAliasing2 | char StringLiteralAliasing2(bool) |
53-
| ssa.cpp:268:7:268:20 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:268:7:268:20 | IR: MallocAliasing | void* MallocAliasing(void*, int) |
54-
| ssa.cpp:268:28:268:28 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:268:7:268:20 | IR: MallocAliasing | void* MallocAliasing(void*, int) |
5520
missingCanonicalLanguageType
5621
multipleCanonicalLanguageTypes
5722
missingIRType

0 commit comments

Comments
 (0)