Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler generates incorrect tempsMap for certain combinations of nested blocks and conditional jumps #304

Open
daniels220 opened this issue Feb 7, 2017 · 2 comments
Labels

Comments

@daniels220
Copy link
Contributor

Debug the following:

| env |
env := 0.
#(#foo #bar #baz)
	withIndexDo: [:argOuter :i | false ifFalse: [#(1 2 3 4 5) do: [:argInner | env := env + 1]]].

Step until env + 1 is selected. Step again at this point and you will hit an "index 2 is out of bounds" error in Debugger>>updateTemporaries. This appears to be caused by the inclusion of argOuter and i in the tempsMap at that point, when in fact they are unavailable.

Minor variations of this code exhibit the same problem—the necessary conditions appear to be as follows:

  • The outer block must have at least two combined args and temps (one arg and one assigned-but-unused temp also causes the problem)
  • The inner block must be inside an optimized conditional expression (#ifNil: also works)
  • The inner block must make use of an env temp, though it need not assign to it (the outer loop can assign to it, causing it to be stored in the Context, while the inner loop only reads from it)
@blairmcg
Copy link
Contributor

This should now be fixed, in VM version 7.0.40. I'm still working on some tests to cover the temps maps better.

@blairmcg
Copy link
Contributor

Creating tests for this is difficult without the test starting to get as complex as the scope analysis code in the compiler itself. Nevertheless I've found another problem, and that the compiler fix I've submitted for this causes another regression. The regression appears somewhat less impactful than the original issue (which the compiler fix does address) because it doesn't actually cause the debugger to fail. However context temps may display a nil or incorrect value in some circumstances that I think should occur rarely in practice. I'm working on fixing that too.

blairmcg added a commit to blairmcg/Dolphin that referenced this issue Jul 9, 2017
blairmcg added a commit to blairmcg/Dolphin that referenced this issue Jul 31, 2017
blairmcg added a commit to blairmcg/Dolphin that referenced this issue Sep 6, 2017
@blairmcg blairmcg added the bug label Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants