Fixes #1796 by giving OpcodeLabelResets a linenum. #1812
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The problem ultimately was caused by the fact that the ``OpcodeLabelReset`` opcodes that get inserted when compiling to KSM don't have their SourceLine field populated (because they didn't directly come from the user's source code but from the KSM building step). The DebugLineMap stores the mapping of which ranges of opcodes came from which source line of the script, for the sake of restoring line number info when unpacking the KSM file into a runtime program again. This exists so it is still possible to print meaningful error messages when a runtime exception happens. Usually the VM opcodes that come from a line of source code will be one contiguous chunk, so you can store this range as a single pair of begin,end numbers. (i.e. "The opcodes from opcode index 160 through 173 all came from line 32.") But, sometimes they get compiled into two or three non-contiguous sections, especially when doing things that manipulate the stack such that the start of an expression and the end of an expression have to be compiled "out of order". To accommodate that need to store a few noncontiguous sections of code from one line, the DebugLineMap can store a list of multiple ranges for each source line. But under the assumption that things would be *really* messed up in order for there to be more than 3 or 4 different such ranges from the same line, the count of how many such ranges there are is just stored in 1 byte and can't hold more than 255 such ranges. Well, this bug happened because the OpcodeLabelReset's were all being falsely attributed to having come from "line 0", and that's how it was trying to store it in the KSM file in the debugLineMap section (the %D part). Thus it was trying to make the DebugLineMap store the case where there were more than 255 different noncontiguous sections of the program that all allegedly came from the same source line (line 0). The count of how many such sections there are is stored in only 1 byte, so trying to have more than 255 of them overflowed the byte and led to bogus information about how many bytes to read before encountering the next line number's header data in the %D section. Thus it got misaligned and started reading really bogus data out of the KSM file. The fix was to stop claiming all OpcodeLabelReset's "came from line 0". This had been used as a flag to help indicate that they were "internal" errors if an exception happens in them and the error message should give the "see the devs" note. But these don't really ever throw exceptions at runtime because they're stripped out and are gone by the time the program runs so they don't need that. So the fix was to just populate their SourceLine data by inheriting it from whatever opcode they were resetting the label on behalf of.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem ultimately was caused by the fact that the
OpcodeLabelResetopcodes that get inserted when compiling to KSM don't have their SourceLine field populated (because they didn't directly come from the user's source code but from the KSM building step).The OpcodeLabelReset's were all being falsely attributed to having come from "line 0", and that's how it was trying to store it in the KSM file in the debugLineMap section (the %D part). Thus it was trying to make the DebugLineMap store the case where there were more than 255 different noncontiguous sections of the program that all allegedly came from the same source line (line 0). This is a condition that would be really ridiculous to have happen from a real line of code - normally the compiler might make maybe 3 or 4 different sections at max from a single line of code. But with this "fake line 0" information, it was treating it as if this had been exactly what happened. The count of how many such sections there are is stored in only 1 byte, so trying to have more than 255 of them overflowed the byte and led to bogus information about how many bytes to read before encountering the next line number's header data in the %D section. Thus it got misaligned and started reading really bogus data out of the KSM file.
The fix was to stop claiming all OpcodeLabelReset's "came from line 0". This had been used as a flag to help indicate that they were "internal" errors if an exception happens in them and the error message should give the "see the devs" note. But these don't really ever throw exceptions at runtime because they're stripped out and are gone by the time the program runs so they don't need that.
So the fix was to just populate their SourceLine data by inheriting it from whatever opcode they were resetting the label on behalf of.