C++: Flow out of invalid functions#12005
Conversation
…ing a return statement.
| return instanceof ReturnInstruction or | ||
| return instanceof UnreachedInstruction | ||
| | | ||
| block.getInstruction(index) = return and |
There was a problem hiding this comment.
index + 1 has been replaced here by index. Was this incorrect before?
There was a problem hiding this comment.
Oh, good question. It wasn't actually incorrect. The only thing we require is that it's the last use of the parameter in the body of the function. ssa0/SsaInternals.qll did index + 1, and SsaInternals.qll did index. So I just synced them up to be index for consistency. This will hopefully make it easier to merge those two files into one parameterized module eventually.
There was a problem hiding this comment.
index + 1 might actually be problematic since we can generate IR where the UnreachedInstruction is the only thing in the block. So there wouldn't necessarily be any instruction at block.getInstruction(index + 1).
This is different from ReturnInstructions since we know there is always a ExitFunctionInstruction (or whatever it's called) after the ReturnInstructions.
jketema
left a comment
jketema
left a comment
There was a problem hiding this comment.
LGTM provided DCA is happy and we now have the two missing results back in our internal tests.
|
The internal test still isn't recovered, but I think there's an additional step necessary for this. I'll write up my investigation in our internal issue for this. |
MathiasVP
merged commit 9573395
into
github:mathiasvp/replace-ast-with-ir-use-usedataflow
2 participants
We were not getting flow out of functions that were missing
returnstatements since they might not have aReturnInstruction.After this PR, we (ab)use the generated
UnreachedInstructionto assign an index in the final basic block so that SSA will ensure that we have flow out of the (invalid) function.