closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

Peephole pass replaces <array lit>.length with number in places where numbers are syntactically invalid

Open jkukucka opened this issue 2 years ago • 2 comments

This was found during fuzzing research.

Input for $SIMPLE_OPTIMIZATIONS

for(;(d_29);(true)){ ((true) + (-- (([]).length))) }

Stack Trace:

java.lang.IllegalStateException: Invalid child for DEC node. Reference node:
NUMBER 0.0 1:41  [length: 6] [source_file: input0]

 Parent node:
DEC 1:32  [length: 16] [is_parenthesized: 1] [source_file: input0] : Color{id=d081722c, debugInfo=DebugInfo{compositeTypename=number}, prototypes=[], instanceColors=[], invalidating=false, propertiesKeepOriginalName=false, constructor=false, ownProperties=[], boxId=34ba2fb1, closureAssert=false, unionElements=[]}
    NUMBER 0.0 1:41  [length: 6] [source_file: input0]

	at com.google.javascript.jscomp.AstValidator$1.handleViolation(AstValidator.java:86)
	at com.google.javascript.jscomp.AstValidator.violation(AstValidator.java:1993)
	at com.google.javascript.jscomp.AstValidator.validateAssignmentOpTarget(AstValidator.java:1620)
	at com.google.javascript.jscomp.AstValidator.validateIncDecOp(AstValidator.java:1893)
	at com.google.javascript.jscomp.AstValidator.validateExpression(AstValidator.java:293)
	at com.google.javascript.jscomp.AstValidator.validateExprStmt(AstValidator.java:1458)
	at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:209)
	at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:153)
	at com.google.javascript.jscomp.AstValidator.validateBlock(AstValidator.java:939)
	at com.google.javascript.jscomp.AstValidator.validateFor(AstValidator.java:1385)
	at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:180)
	at com.google.javascript.jscomp.AstValidator.validateStatement(AstValidator.java:153)
	at com.google.javascript.jscomp.AstValidator.validateStatements(AstValidator.java:147)
	at com.google.javascript.jscomp.AstValidator.validateScript(AstValidator.java:136)
	at com.google.javascript.jscomp.AstValidator.validateCodeRoot(AstValidator.java:123)
	at com.google.javascript.jscomp.AstValidator.process(AstValidator.java:109)
	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:2583)
	at com.google.javascript.jscomp.Compiler.lambda$stage2Passes$8(Compiler.java:960)
	at com.google.javascript.jscomp.CompilerExecutor.runInCompilerThread(CompilerExecutor.java:127)
	at com.google.javascript.jscomp.Compiler.runInCompilerThread(Compiler.java:1006)
	at com.google.javascript.jscomp.Compiler.stage2Passes(Compiler.java:957)
	at com.google.javascript.jscomp.Compiler.compileModules(Compiler.java:899)
	at com.google.javascript.jscomp.debugger.CompilationServlet.service(CompilationServlet.java:99)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
	at org.eclipse.jetty.servlet.ServletHolder$NotAsync.service(ServletHolder.java:1459)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799)
	at org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1626)
	at com.google.apphosting.utils.servlet.JdbcMySqlConnectionCleanupFilter.doFilter(JdbcMySqlConnectionCleanupFilter.java:62)
	at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)
	at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1601)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:548)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:578)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at com.google.apphosting.runtime.jetty9.ParseBlobUploadHandler.handle(ParseBlobUploadHandler.java:111)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434)
	at com.google.apphosting.runtime.jetty94.AppEngineWebAppContext.doHandle(AppEngineWebAppContext.java:234)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
	at com.google.apphosting.runtime.jetty94.AppVersionHandlerMap.handle(AppVersionHandlerMap.java:256)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
	at org.eclipse.jetty.server.Server.handle(Server.java:516)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380)
	at com.google.apphosting.runtime.jetty94.RpcConnection.handle(RpcConnection.java:243)
	at com.google.apphosting.runtime.jetty94.RpcConnector.serviceRequest(RpcConnector.java:83)
	at com.google.apphosting.runtime.jetty94.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:158)
	at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchServletRequest(JavaRuntime.java:794)
	at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchRequest(JavaRuntime.java:757)
	at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:727)
	at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:261)
	at java.base/java.lang.Thread.run(Thread.java:829)

Reproduce URL: https://closure-compiler-debugger.appspot.com/#input0%3Dfor(%253B(d_29)%253B(true))%257B%2520((true)%2520%252B%2520(--%2520((%255B%255D).length)))%2520%257D%250A%26input1%26conformanceConfig%26externs%26refasterjs-template%26CHECK_TYPES%3Dtrue%26REWRITE_MODULES_BEFORE_TYPECHECKING%3Dtrue%26ALIAS_ALL_STRINGS%3Dtrue%26AMBIGUATE_PROPERTIES%3Dtrue%26COALESCE_VARIABLE_NAMES%3Dtrue%26COLLAPSE_VARIABLE_DECLARATIONS%3Dtrue%26COLLAPSE_ANONYMOUS_FUNCTIONS%3Dtrue%26COLLAPSE_PROPERTIES%3Dtrue%26COLLAPSE_OBJECT_LITERALS%3Dtrue%26COMPUTE_FUNCTION_SIDE_EFFECTS%3Dtrue%26CONVERT_TO_DOTTED_PROPERTIES%3Dtrue%26CROSS_CHUNK_CODE_MOTION%3Dtrue%26CROSS_CHUNK_METHOD_MOTION%3Dtrue%26DEAD_ASSIGNMENT_ELIMINATION%3Dtrue%26DEVIRTUALIZE_METHODS%3Dtrue%26DISAMBIGUATE_PROPERTIES%3Dtrue%26EXTRACT_PROTOTYPE_MEMBER_DECLARATIONS%3Dtrue%26FOLD_CONSTANTS%3Dtrue%26INLINE_CONSTANTS%3Dtrue%26INLINE_FUNCTIONS%3Dtrue%26INLINE_PROPERTIES%3Dtrue%26INLINE_VARIABLES%3Dtrue%26LABEL_RENAMING%3Dtrue%26OPTIMIZE_CALLS%3Dtrue%26OPTIMIZE_CONSTRUCTORS%3Dtrue%26OPTIMIZE_ARGUMENTS_ARRAY%3Dtrue%26REMOVE_ABSTRACT_METHODS%3Dtrue%26REMOVE_DEAD_CODE%3Dtrue%26REMOVE_UNUSED_CLASS_PROPERTIES%3Dtrue%26REMOVE_UNUSED_PROTOTYPE_PROPERTIES%3Dtrue%26REMOVE_UNUSED_VARIABLES%3Dtrue%26REWRITE_FUNCTION_EXPRESSIONS%3Dtrue%26SMART_NAME_REMOVAL%3Dtrue%26USE_TYPES_FOR_LOCAL_OPTIMIZATION%3Dtrue%26VARIABLE_RENAMING%3Dtrue%26PROPERTY_RENAMING%3Dtrue%26MOVE_FUNCTION_DECLARATIONS%3Dtrue%26SYNTHETIC_BLOCK_MARKER%3Dtrue%26CLOSURE_PASS%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue

jkukucka avatar Sep 03 '21 12:09 jkukucka

Smaller repro:

alert(--([2]).length);

https://closure-compiler-debugger.appspot.com/#input0%3Dalert(--(%255B2%255D).length)%253B%26input1%26conformanceConfig%26externs%26refasterjs-template%26CHECK_TYPES%3Dtrue%26REWRITE_MODULES_BEFORE_TYPECHECKING%3Dtrue%26FOLD_CONSTANTS%3Dtrue%26CLOSURE_PASS%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue

lauraharker avatar Sep 03 '21 18:09 lauraharker

As per https://github.com/google/closure-compiler/issues/3859, this issue isn't specific to increment/decrement but applies to any assignment op.

Actual code reassigning array.length in this way seems rare though.

lauraharker avatar Sep 03 '21 19:09 lauraharker