Hi Brent! Thanks for taking a look at this! There are no unit tests for most of SM. It's hard to split the interesting parts JS execution up into tidy testable units. Once you have a fix, you can test it using the jit-test suite. [Instructions for building and testing SM are here](https://firefox-source-docs.mozilla.org/js/build.html). (I would recommend testing an optimized debug build.) > Is SimpleLinearSum only looking if it can replace the MDefinition with a constant? Close. `ExtractLinearSum` is trying to replace the MDefinition with the sum of an MDefinition and a constant. So if you had an MDefinition representing `7 + foo() + 5 - 3`, `ExtractLinearSum` would simplify it to `foo() + 9`. `SimpleLinearSum` is the type we use to represent this result. It contains an arbitrary MDefinition (an MCall of `foo` in this example) and a constant (9 in this case). > If SimpleLinearSum.constant == 0 does that mean it was not possible to extract the SimpleLinearSum, so the term will be evaluated? The term will always be evaluated unless the entire expression can be folded to a constant, in which case `term` will be `nullptr`. The problem in this bug happens when we are calling `ExtractLinearSum` on an MDefinition that represents an extremely large sequence of `1 + 1 + 1 + ...`. Because `ExtractLinearSum` calls itself recursively, we end up overflowing the stack and crashing. Normally this kind of expression is cleaned up before we get to this point by another optimization called Global Value Numbering. However, turning off GVN can be useful while (fuzzing)[https://en.wikipedia.org/wiki/Fuzzing] SpiderMonkey. The goal of this bug is to avoid crashing here. There are a few ways we could do this. The easiest might be to keep track of the depth of the recursion and just return `ins + 0` when that limit is exceeded. (The most complete fix would be to change this from a recursive algorithm to an iterative one using a work queue, but that might be overkill for a problem that only occurs when we are tying one hand behind our back.) Does that help? Let me know if you have any additional questions!
Bug 1527839 Comment 26 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Hi Brent! Thanks for taking a look at this! There are no unit tests for most of SM. It's hard to split the interesting parts JS execution up into tidy testable units. Once you have a fix, you can test it using the jit-test suite. [Instructions for building and testing SM are here](https://firefox-source-docs.mozilla.org/js/build.html). (I would recommend testing an optimized debug build.) > Is SimpleLinearSum only looking if it can replace the MDefinition with a constant? Close. `ExtractLinearSum` is trying to replace the MDefinition with the sum of an MDefinition and a constant. So if you had an MDefinition representing `7 + foo() + 5 - 3`, `ExtractLinearSum` would simplify it to `foo() + 9`. `SimpleLinearSum` is the type we use to represent this result. It contains an arbitrary MDefinition (an MCall of `foo` in this example) and a constant (9 in this case). > If SimpleLinearSum.constant == 0 does that mean it was not possible to extract the SimpleLinearSum, so the term will be evaluated? The term will always be evaluated unless the entire expression can be folded to a constant, in which case `term` will be `nullptr`. The problem in this bug happens when we are calling `ExtractLinearSum` on an MDefinition that represents an extremely large sequence of `1 + 1 + 1 + ...`. Because `ExtractLinearSum` calls itself recursively, we end up overflowing the stack and crashing. Normally this kind of expression is cleaned up before we get to this point by another optimization called Global Value Numbering. However, turning off GVN can be useful while [fuzzing](https://en.wikipedia.org/wiki/Fuzzing) SpiderMonkey. The goal of this bug is to avoid crashing here. There are a few ways we could do this. The easiest might be to keep track of the depth of the recursion and just return `ins + 0` when that limit is exceeded. (The most complete fix would be to change this from a recursive algorithm to an iterative one using a work queue, but that might be overkill for a problem that only occurs when we are tying one hand behind our back.) Does that help? Let me know if you have any additional questions!