Darkyenus / glsl4idea

A GLSL language plugin for IntelliJ IDEA
GNU Lesser General Public License v3.0
101 stars 30 forks source link

Stack overflow error in GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67) #142

Closed amaembo closed 5 years ago

amaembo commented 5 years ago

Hello, IntelliJ IDEA developer is here.

The following exception was reported by our customers which is caused by your plugin (GLSL Support 1.16):

java.lang.StackOverflowError
    at com.intellij.psi.impl.PsiElementBase.findChildrenByClass(PsiElementBase.java:293)
    at glslplugin.lang.elements.declarations.GLSLDeclaratorList.getDeclarators(GLSLDeclaratorList.java:44)
    at glslplugin.lang.elements.declarations.GLSLDeclarationImpl.getDeclarators(GLSLDeclarationImpl.java:94)
    at glslplugin.lang.elements.declarations.GLSLDeclarationImpl.processDeclarations(GLSLDeclarationImpl.java:118)
    at glslplugin.lang.elements.statements.GLSLDeclarationStatement.processDeclarations(GLSLDeclarationStatement.java:61)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
    at glslplugin.lang.elements.statements.GLSLCompoundStatement.processDeclarations(GLSLCompoundStatement.java:67)
... hundreds more of lines ...

We don't have any additional information (e.g. steps to reproduce), and reports were submitted anonymously, so we cannot contact the reporting customers and ask for details. Nevertheless the code looks erroneous even without a reproducer:

    public boolean processDeclarations(@NotNull PsiScopeProcessor processor, @NotNull ResolveState state, @Nullable PsiElement lastParent, @NotNull PsiElement place) {
        if (lastParent == null) {
            return true;
        }
        PsiElement child = lastParent.getPrevSibling();
        while (child != null) {
            if (!child.processDeclarations(processor, state, lastParent, place)) return false;
            child = child.getPrevSibling();
        }
        return true;
    }

If child happens to be also GLSLCompoundStatement, then you will enter an infinite recursion as the same lastParent is passed to the processDeclarations, so the same child will be extracted from it.

Darkyenus commented 5 years ago

Thank you for the report! I'll look into it. By the way, is there any supported way to be able to see these reports as a plugin developer as well? In other words, is there any way to hook into the IDE's crash reporting? It seems to work only for Jetbrains plugins and IIRC I've seen Android/Google do it, but no other plugins.

amaembo commented 5 years ago

By the way, is there any supported way to be able to see these reports as a plugin developer as well? In other words, is there any way to hook into the IDE's crash reporting?

Unfortunately we are not still there. There are both technical and legal problems which should be solved. In particular the report may contain bits of sensitive user data (even if exception message contains a single variable name from user code, this still could be considered as a piece of private data), and it should be completely clear for the user where the report goes and who will be able to see it. Also even if report contains some plugin stack frames, it still could be a platform problem. And vice versa: sometimes report has no plugin-specific frames, but it's a plugin problem.

Occasionally we process third-party plugin reports manually and report issues like this removing sensitive data if any.

Darkyenus commented 5 years ago

Ok, that's a shame, but understandable.

Darkyenus commented 5 years ago

Fixed by 1af6f93441f23676a0dab5baed9aa1201a2e7e65