Skip to content

Commit 53cd5e9

Browse files
C++: Fix bug introduced by earlier commit
1 parent 7e6e6f0 commit 53cd5e9

File tree

2 files changed

+30
-13
lines changed

2 files changed

+30
-13
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ cached private module Cached {
183183
result instanceof UnmodeledDefinitionInstruction and
184184
instruction.getFunction() = result.getFunction()
185185
or
186+
tag instanceof ChiTotalOperandTag and
186187
result = getChiInstructionTotalOperand(instruction)
187188
}
188189

@@ -205,7 +206,7 @@ cached private module Cached {
205206

206207
cached Instruction getChiInstructionTotalOperand(ChiInstruction chiInstr) {
207208
exists(Alias::VirtualVariable vvar, OldIR::Instruction oldInstr, OldIR::IRBlock defBlock,
208-
int defRank, int defIndex, OldIR::IRBlock useBlock, int useRank |
209+
int defRank, int defIndex, OldIR::IRBlock useBlock, int useRank |
209210
ChiTag(oldInstr) = chiInstr.getTag() and
210211
vvar = Alias::getResultMemoryAccess(oldInstr).getVirtualVariable() and
211212
hasDefinitionAtRank(vvar, defBlock, defRank, defIndex) and

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ cached private module Cached {
4343
getOldInstruction(result) = instr
4444
}
4545

46+
/**
47+
* Gets the chi node corresponding to `instr` if one is present, or the new `Instruction`
48+
* corresponding to `instr` if there is no `Chi` node.
49+
*/
4650
private Instruction getNewFinalInstruction(OldIR::Instruction instr) {
4751
result = getChiInstruction(instr)
4852
or
@@ -153,7 +157,7 @@ cached private module Cached {
153157
else
154158
result = getPhiInstruction(instruction.getFunction(), defBlock, vvar)
155159
)
156-
)
160+
)
157161
else (
158162
result = instruction.getFunctionIR().getUnmodeledDefinitionInstruction()
159163
)
@@ -179,7 +183,8 @@ cached private module Cached {
179183
result instanceof UnmodeledDefinitionInstruction and
180184
instruction.getFunction() = result.getFunction()
181185
or
182-
result = getChiInstructionTotalOperand(instruction.(ChiInstruction), tag.(ChiTotalOperandTag))
186+
tag instanceof ChiTotalOperandTag and
187+
result = getChiInstructionTotalOperand(instruction)
183188
}
184189

185190
cached Instruction getPhiInstructionOperandDefinition(PhiInstruction instr,
@@ -199,15 +204,18 @@ cached private module Cached {
199204
)
200205
}
201206

202-
cached Instruction getChiInstructionTotalOperand(ChiInstruction chiInstr, ChiTotalOperandTag tag) {
207+
cached Instruction getChiInstructionTotalOperand(ChiInstruction chiInstr) {
203208
exists(Alias::VirtualVariable vvar, OldIR::Instruction oldInstr, OldIR::IRBlock defBlock,
204-
int defRank, int defIndex, OldIR::IRBlock useBlock, int useRank |
205-
ChiTag(oldInstr) = chiInstr.getTag() and
206-
vvar = Alias::getResultMemoryAccess(oldInstr).getVirtualVariable() and
207-
hasDefinitionAtRank(vvar, defBlock, defRank, defIndex) and
208-
hasUseAtRank(vvar, useBlock, useRank, oldInstr) and
209-
definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and
210-
result = getNewFinalInstruction(defBlock.getInstruction(defIndex))
209+
int defRank, int defIndex, OldIR::IRBlock useBlock, int useRank |
210+
ChiTag(oldInstr) = chiInstr.getTag() and
211+
vvar = Alias::getResultMemoryAccess(oldInstr).getVirtualVariable() and
212+
hasDefinitionAtRank(vvar, defBlock, defRank, defIndex) and
213+
hasUseAtRank(vvar, useBlock, useRank, oldInstr) and
214+
definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and
215+
if defIndex >= 0 then
216+
result = getNewFinalInstruction(defBlock.getInstruction(defIndex))
217+
else
218+
result = getPhiInstruction(chiInstr.getFunction(), defBlock, vvar)
211219
)
212220
}
213221

@@ -226,6 +234,11 @@ cached private module Cached {
226234
result = getOldInstruction(instruction).getUnconvertedResultExpression()
227235
}
228236

237+
/*
238+
* This adds Chi nodes to the instruction successor relation; if an instruction has a Chi node,
239+
* that node is its successor in the new successor relation, and the Chi node's successors are
240+
* the new instructions generated from the successors of the old instruction
241+
*/
229242
cached Instruction getInstructionSuccessor(Instruction instruction, EdgeKind kind) {
230243
if(hasChiNode(_, getOldInstruction(instruction)))
231244
then
@@ -324,6 +337,10 @@ cached private module Cached {
324337
(
325338
access = Alias::getOperandMemoryAccess(use.getAnOperand())
326339
or
340+
/*
341+
* a partial write to a virtual variable is going to generate a use of that variable when
342+
* Chi nodes are inserted, so we need to mark it as a use in the old IR
343+
*/
327344
access = Alias::getResultMemoryAccess(use) and
328345
access.isPartialMemoryAccess()
329346
) and
@@ -464,8 +481,7 @@ cached private module Cached {
464481
ma = Alias::getResultMemoryAccess(def) and
465482
ma.isPartialMemoryAccess() and
466483
ma.getVirtualVariable() = vvar
467-
) and
468-
not def instanceof OldIR::UnmodeledDefinitionInstruction
484+
)
469485
}
470486
}
471487

0 commit comments

Comments
 (0)