search

google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.4k stars 1.15k forks source link

Returning loop variables inside a object property getter causes crash #3599

Closed merryman closed 1 year ago

merryman commented 4 years ago

I am able to reproduce this issue with the 20200517.0.0 version of the compiler. I am also able to crash the service running on https://closure-compiler.appspot.com/ with this snippet.

I have not figured out exactly what causes the crash here but it seems like returning any const or let variables defined within the scope of a for loop within a dynamically defined property getter causes a crash:

// ==ClosureCompiler==
// @output_file_name default.js
// @language_out ecmascript5
// @compilation_level SIMPLE_OPTIMIZATIONS
// ==/ClosureCompiler==

for (let i = 0; i < 2; i++) {
   let bar = 42;
   let a = {
     get foo() {
      return bar; // <- this seems to cause the crash
     },
   };
}

I am also able to reproduce this very issue with "older" versions up until 20190618.0.0/. I have not tested further though.

This is the crash dump I receive with the current version / the one hosted on https://closure-compiler.appspot.com/:

23: java.lang.IllegalStateException: Expected FUNCTION but was CALL. Reference node:
CALL 4 [length: 26] [free_call: 1] [source_file: Input_0]
    FUNCTION  4 [length: 26] [source_file: Input_0] [change_time: 3829]
        NAME  4 [length: 26] [source_file: Input_0]
        PARAM_LIST 4 [length: 26] [source_file: Input_0]
            NAME b 4 [length: 26] [source_file: Input_0] [constant_var_flags: 2]
        BLOCK 4 [length: 26] [source_file: Input_0]
            RETURN 4 [length: 26] [source_file: Input_0]
                FUNCTION  4 [length: 26] [source_file: Input_0] [change_time: 3830]
                    NAME  4 [length: 26] [source_file: Input_0]
                    PARAM_LIST 4 [length: 26] [source_file: Input_0]
                    BLOCK 4 [length: 26] [source_file: Input_0]
                        RETURN 5 [length: 11] [source_file: Input_0]
                            GETPROP 5 [length: 3] [source_file: Input_0]
                                NAME b 5 [length: 3] [source_file: Input_0]
                                STRING $jscomp$loop$prop$bar$1 5 [length: 3] [source_file: Input_0]
    NAME $jscomp$loop$0 4 [length: 26] [source_file: Input_0]

 Parent node:
GETTER_DEF foo 4 [length: 3] [source_file: Input_0]
    CALL 4 [length: 26] [free_call: 1] [source_file: Input_0]
        FUNCTION  4 [length: 26] [source_file: Input_0] [change_time: 3829]
            NAME  4 [length: 26] [source_file: Input_0]
            PARAM_LIST 4 [length: 26] [source_file: Input_0]
                NAME b 4 [length: 26] [source_file: Input_0] [constant_var_flags: 2]
            BLOCK 4 [length: 26] [source_file: Input_0]
                RETURN 4 [length: 26] [source_file: Input_0]
                    FUNCTION  4 [length: 26] [source_file: Input_0] [change_time: 3830]
                        NAME  4 [length: 26] [source_file: Input_0]
                        PARAM_LIST 4 [length: 26] [source_file: Input_0]
                        BLOCK 4 [length: 26] [source_file: Input_0]
                            RETURN 5 [length: 11] [source_file: Input_0]
                                GETPROP 5 [length: 3] [source_file: Input_0]
                                    NAME b 5 [length: 3] [source_file: Input_0]
                                    STRING $jscomp$loop$prop$bar$1 5 [length: 3] [source_file: Input_0]
        NAME $jscomp$loop$0 4 [length: 26] [source_file: Input_0]

    at com.google.javascript.jscomp.AstValidator$1.handleViolation(AstValidator.java:82)
    at com.google.javascript.jscomp.AstValidator.violation(AstValidator.java:1903)
    at com.google.javascript.jscomp.AstValidator.validateNodeType(AstValidator.java:1920)
    at com.google.javascript.jscomp.AstValidator.validateFunctionExpressionHelper(AstValidator.java:952)
    at com.google.javascript.jscomp.AstValidator.validateFunctionExpression(AstValidator.java:944)
    at com.google.javascript.jscomp.AstValidator.validateObjectLitGetKey(AstValidator.java:1661)
    at com.google.javascript.jscomp.AstValidator.validateObjectLitKey(AstValidator.java:1630)
    at com.google.javascript.jscomp.AstValidator.validateObjectLit(AstValidator.java:1623)
    at com.google.javascript.jscomp.AstValidator.validateExpression(AstValidator.java:389)
    at com.google.javascript.jscomp.AstValidator.validateNameDeclarationChild(AstValidator.java:1174)
    at com.google.javascript.jscomp.AstValidator.validateNameDeclarationHelper(AstValidator.java:1139)
    at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:202)
    at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:149)
    at com.google.javascript.jscomp.AstValidator.validateBlock(AstValidator.java:856)
    at com.google.javascript.jscomp.AstValidator.validateFor(AstValidator.java:1297)
    at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:176)
    at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:149)
    at com.google.javascript.jscomp.AstValidator.validateStatements(AstValidator.java:143)
    at com.google.javascript.jscomp.AstValidator.validateScript(AstValidator.java:132)
    at com.google.javascript.jscomp.AstValidator.validateCodeRoot(AstValidator.java:119)
    at com.google.javascript.jscomp.AstValidator.process(AstValidator.java:105)
    at com.google.javascript.jscomp.PhaseOptimizer$NamedPass.process(PhaseOptimizer.java:317)
    at com.google.javascript.jscomp.PhaseOptimizer.process(PhaseOptimizer.java:232)
    at com.google.javascript.jscomp.Compiler.performOptimizations(Compiler.java:2419)
    at com.google.javascript.jscomp.Compiler.lambda$stage2Passes$1(Compiler.java:804)
    at com.google.javascript.jscomp.CompilerExecutor.runInCompilerThread(CompilerExecutor.java:129)
    at com.google.javascript.jscomp.Compiler.runInCompilerThread(Compiler.java:831)
    at com.google.javascript.jscomp.Compiler.stage2Passes(Compiler.java:801)
    at com.google.javascript.jscomp.Compiler.compile(Compiler.java:691)
    at com.google.javascript.jscomp.webservice.backend.CompilerInvokerImpl.compile(CompilerInvokerImpl.java:44)
    at com.google.javascript.jscomp.webservice.backend.ServerController.executeRequest(ServerController.java:178)
    at com.google.javascript.jscomp.webservice.backend.CompilationRequestHandler.serviceParsedRequest(CompilationRequestHandler.java:178)
    at com.google.javascript.jscomp.webservice.backend.CompilationRequestHandler.service(CompilationRequestHandler.java:160)
    at com.google.javascript.jscomp.webservice.frontend.CompilationServlet.doPost(CompilationServlet.java:82)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:848)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1772)
    at com.google.apphosting.utils.servlet.JdbcMySqlConnectionCleanupFilter.doFilter(JdbcMySqlConnectionCleanupFilter.java:60)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1759)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:582)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:524)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:226)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
    at com.google.apphosting.runtime.jetty9.ParseBlobUploadHandler.handle(ParseBlobUploadHandler.java:119)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1182)
    at com.google.apphosting.runtime.jetty9.AppEngineWebAppContext.doHandle(AppEngineWebAppContext.java:187)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:512)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1112)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at com.google.apphosting.runtime.jetty9.AppVersionHandlerMap.handle(AppVersionHandlerMap.java:293)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
    at org.eclipse.jetty.server.Server.handle(Server.java:539)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:333)
    at com.google.apphosting.runtime.jetty9.RpcConnection.handle(RpcConnection.java:213)
    at com.google.apphosting.runtime.jetty9.RpcConnector.serviceRequest(RpcConnector.java:81)
    at com.google.apphosting.runtime.jetty9.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:134)
    at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchServletRequest(JavaRuntime.java:757)
    at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchRequest(JavaRuntime.java:720)
    at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:690)
    at com.google.apphosting.runtime.JavaRuntime$NullSandboxRequestRunnable.run(JavaRuntime.java:882)
    at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:270)
    at java.lang.Thread.run(Thread.java:748)
concavelenz commented 4 years ago

Thanks for the report, it looks like the transpilation logic is misfiring.

Repro in the debugger:

https://closure-compiler-debugger.appspot.com/#input0%26input1%3Dfor%2520(let%2520i%2520%253D%25200%253B%2520i%2520%253C%25202%253B%2520i%252B%252B)%2520%257B%250A%2520%2520%2520let%2520bar%2520%253D%252042%253B%250A%2520%2520%2520let%2520a%2520%253D%2520%257B%250A%2520%2520%2520%2520%2520get%2520foo()%2520%257B%250A%2520%2520%2520%2520%2520%2520return%2520bar%253B%2520%252F%252F%2520%253C-%2520this%2520seems%2520to%2520cause%2520the%2520crash%250A%2520%2520%2520%2520%2520%257D%252C%250A%2520%2520%2520%257D%253B%250A%257D%26conformanceConfig%26externs%26refasterjs-template%26includeDefaultExterns%3Dtrue%26TRANSPILE%3Dtrue%26CLOSURE_PASS%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue

t-mangoe commented 1 year ago

I have confirmed that the exception occurs in my local environment as well. It appears that this occurs when the language-out option is ecmascript5 or ecmascript5_strict.

lauraharker commented 1 year ago

This problem seems to start happening after let/const transpilation in https://github.com/google/closure-compiler/blob/394a456cea8b42ed74af668afdc467bf3d5ba56c/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java.

I've reproduced this in a unit test for that pass and am working on a fix.