mirror of
https://github.com/github/codeql.git
synced 2026-04-29 02:35:15 +02:00
da13dc6442
The `ValueNumbering` library is supposed to propagate value numberings through a `CopyInstruction` only when it's _congruent_, meaning it must have exact overlap with its source. A `CopyInstruction` can be a `LoadInstruction`, a `StoreInstruction`, or a `CopyValueInstruction`. The latter is also a `UnaryInstruction`, and the value numbering rule for `UnaryInstruction` applied to it as well. This meant that value numbering would propagate even through a non-congruent `CopyValueInstruction`. That's semantically wrong but probably only an issue in very rare circumstances, and it should get corrected when we change the definition of `getUnary` to require congruence. What's worse is the performance implications. It meant that the value numbering IPA witness could take two different paths through every `CopyValueInstruction`. If multiple `CopyValueInstruction`s were chained, this would lead to an exponential number of variable numbers for the same `Instruction`, and we would run out of time and space while performing value numbering. This fixes the performance of `ValueNumbering.qll` on https://github.com/asterisk/asterisk, although this project might also require a separate change for fixing an infinite loop in the IR constant analysis.