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

WIP Try to reproduce #1536 #1537

Closed
wants to merge 2 commits into from
Closed

WIP Try to reproduce #1536 #1537

wants to merge 2 commits into from

Conversation

forki
Copy link
Contributor

@forki forki commented Sep 15, 2016

This reproduces the bug from #1536

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

urghs this seems to happen in some COM component ISymUnmanagedWriter2 in method CloseMethod

@forki forki force-pushed the i1536 branch 2 times, most recently from f4bdebc to 7b0c127 Compare September 15, 2016 08:09
@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

Ok this actually happening in ISymUnmanagedWriter2 when we add more then ~475 level deep scopes.

In ce077ea we keep track how deep we are in the scoping level and just skip the rest. This is probably not the the final solution, but at least we don't fail compiler when we write PDBs.

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

@dsyme I just looked into a C# file with over 500 variables in a file. They just put that in the same scope so nobody ever noticed this ;-)

@dsyme
Copy link
Contributor

dsyme commented Sep 15, 2016

This feels like a regression (?), if so we need to check the root cause and test coverage.

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

It's only happens when optimizer is disabled but debug is enabled. I assume methodsplitting stuff worked around this issue.

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

7b0c127 is still green, but when running "build.cmd all" locally I see the stackoverflow happening. It's just that the fsharpQA suite doesn't care and goes on after a while.

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

@enricosada any ideas where we can make fsharpqa stop on compiler throwing stackoverflow!?

@enricosada
Copy link
Contributor

enricosada commented Sep 15, 2016

@forki the perl script should check the exit code of compiler, is not doing that?|
or the exit code is wrong on SO?

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

@enricosada I guess the thing is not exiting at all

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

fun fact: the compiler is emitting the exe for the sample and crashes after that. fsharpqa goes on with it's life.

b62f97c reproduced it indirectly by running the exe (which is broken).

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

Ok I think this a good enough test. Without the fix the CI is red and with fix applied it's green.

Ready for review

@forki
Copy link
Contributor Author

forki commented Sep 15, 2016

I rewrote @estokes sample (including nested scopes) in C# NestedScopes.zip and compiling/debugging works well. So there must exist a real solution.

Does anyone know where the C# pdb writer puts scopes and local variables into methods? /cc @jaredpar

@forki
Copy link
Contributor Author

forki commented Sep 16, 2016

@dsyme something weird is happening here. We have one scope level per variable but the nesting is not ordered like the variables in the source file. I'm not sure if this is the root cause here, but it's definitely weird.

@KevinRansom
Copy link
Member

@forki if you like I will take a look at this ... unless you want to follow up?

@forki
Copy link
Contributor Author

forki commented Sep 16, 2016

Can't look at it before Monday. Have to run hipster F# conference at the
weekend ;-)

Am 16.09.2016 7:09 nachm. schrieb "Kevin Ransom (msft)" <
notifications@github.com>:

@forki https://github.com/forki if you like I will take a look at this
... unless you want to follow up?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1537 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNF539WKvWZFu_jXDcM7iYGZDT3FGks5qqs00gaJpZM4J9i4H
.

@KevinRansom
Copy link
Member

I'm too old to do anything hipster ... so I suggest you have a good time.

@KevinRansom
Copy link
Member

@dsyme @forki

This is not a regression, this is using the oldest preview compiler on codeplex :

c:\temp\foo>"c:\Program Files (x86)\Microsoft SDKs\F#\3.1\Framework\v4.0\Fsc.exe" --debug:full --optimize- foo.fs
Microsoft (R) F# Compiler version 12.0.30623.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

....

Process is terminated due to StackOverflowException.

c:\temp\foo>

Interestingly it only happens when --optimize- is specified.

Kevin

@forki
Copy link
Contributor Author

forki commented Sep 16, 2016

Yeah I'm already convinced optimizer is doing something so that this would
happen much later. One thing I wanted to try out was to use 500 variables
that have same name so that they shadow each other. So with that case the
optimizer could not remove scopes

Am 16.09.2016 21:41 schrieb "Kevin Ransom (msft)" <notifications@github.com

:

@dsyme https://github.com/dsyme @forki https://github.com/forki

This is not a regression, this is using the oldest preview compiler on
codeplex :

c:\temp\foo>"c:\Program Files (x86)\Microsoft SDKs\F#\3.1\Framework\v4.0\Fsc.exe"
--debug:full --optimize- foo.fs
Microsoft (R) F# Compiler version 12.0.30623.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

....

Process is terminated due to StackOverflowException.

c:\temp\foo>

Interestingly it only happens when --optimize- is specified.

Kevin


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1537 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNJeMj2T2eabL8rYP5LIDDg-9kdN_ks5qqvDrgaJpZM4J9i4H
.

@KevinRansom
Copy link
Member

@enricosada @forki @dsyme

Okay I have a PR with a slightly different fix: #1545

The fix merges nested scopes that have the same dimensions.

@forki forki closed this Sep 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants