[[ Bug 20933 ]] Fix unchecked codegen overflow for large handlers #6311
Conversation
|
I did this against develop, but it should be against develop-8.1 as it is a bug there too. |
|
I'm not sure whether you can afford a more extensive refactor here & a jump in the bytecode format at the moment. If you can, consider adopting Self-Delimited Numeric Value representation throughout the bytecode format in order to extend to being able to represent arbitrary-sized everything. |
|
@peter-b : The argument indicies are already SDNVs - they always have been. This patch tries 5 different sizes for jump deltas in a handler - the one chosen being the smallest which allows all deltas to be encoded. Using a fixed chosen size for those indicies means the deltas can be calculated without doing a basic block decomposition (the problem being that the size of a delta depends on the size of the jumps and their deltas it jumps over). |
|
You'd pay a lot of time during compilation to determine the most compact representation, while only saving a few bytes (assuming people only write handlers of a sensible length) |
|
@peter-b : Yes - that's why I think this is a happy medium - small handlers will now be smaller than they were, all handlers which currently work will be the same size and large handlers will be less compact than they could potentially be, but actually work. In terms of doing it 'optimally', then it would be an O(n) algorithm to do so - which is what it is at the moment (imagine the VM had structured control-flow ops rather than jumps and then it is easy to see that the control-flow graph is essentially a tree - which you then compute deltas depth-first). It is worth doing but only when the decomposition is there and being used to actually optimize the bytecode (i.e. eliding jumps etc.). |
947bf89 to
c341920
Compare
This patch changes the way instructions are turned into bytecode for a handler. Previously there was an unreported hard limit which meant that any handler which contained jumps with a delta > 8192 would cause a bad module to be generated. This has been fixed by trying different sizes for jump delta encodings until one is found which works. Very small handlers will be encoded in fewer bytes than they are now, handlers which currently compile and work will be the same size, and large handlers will now compile and work correctly.
c341920 to
d484661
Compare
This patch changes the way instructions are turned into bytecode
for a handler. Previously there was an unreported hard limit which
meant that any handler which contained jumps with a delta > 8192
would cause a bad module to be generated. This has been fixed
by trying different sizes for jump delta encodings until one is
found which works. Very small handlers will be encoded in fewer
bytes than they are now, handlers which currently compile and work
will be the same size, and large handlers will now compile and
work correctly.