Parser needs to restore unary stack state when backtracking
Created attachment 389200 [details] Patch
Comment on attachment 389200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389200&action=review nice Maybe add some more tests. like test both strict and non strict. And maybe some of the other tests we were dealing with yesterday. Might also be good to try different unary ops, etc. > Source/JavaScriptCore/ChangeLog:8 > + Previously we would try to parse possibly stale stack entries "stack entries" => "unary operator. stack entries" > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:177 > + static constexpr unsigned thisLength = sizeof("this") - 1; // don't count \0 why? why not strlen? > Source/JavaScriptCore/parser/SyntaxChecker.h:340 > + void assignmentStackAppend(int& hasAssignements, int, int, int, int, Operator) { hasAssignements = 1; } hasAssignments => assignmentStackDepth > Source/JavaScriptCore/parser/SyntaxChecker.h:341 > + int createAssignment(const JSTokenLocation&, int& hasAssignements, int, int, int, int) { hasAssignements = 0; return AssignmentExpr; } hasAssignments => assignmentStackDepth
Comment on attachment 389200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389200&action=review > Source/JavaScriptCore/ChangeLog:11 > + reparsing. Additionally, this patch fixes an issue where the "reparsing" => "reparsing after backtracking"
Comment on attachment 389200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389200&action=review >> Source/JavaScriptCore/ChangeLog:8 >> + Previously we would try to parse possibly stale stack entries > > "stack entries" => "unary operator. stack entries" Fixed, I'm assuming the period is a typo? >> Source/JavaScriptCore/ChangeLog:11 >> + reparsing. Additionally, this patch fixes an issue where the > > "reparsing" => "reparsing after backtracking" Fixed. >> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:177 >> + static constexpr unsigned thisLength = sizeof("this") - 1; // don't count \0 > > why? > > why not strlen? This way it can be constexpr thus it has more big brain-ness. :P Fixed. >> Source/JavaScriptCore/parser/SyntaxChecker.h:340 >> + void assignmentStackAppend(int& hasAssignements, int, int, int, int, Operator) { hasAssignements = 1; } > > hasAssignments => assignmentStackDepth That seems inaccurate since it's not incremented. I think I'm going to leave it as is. >> Source/JavaScriptCore/parser/SyntaxChecker.h:341 >> + int createAssignment(const JSTokenLocation&, int& hasAssignements, int, int, int, int) { hasAssignements = 0; return AssignmentExpr; } > > hasAssignments => assignmentStackDepth Ditto.
The function-toString-parentheses.js failures are just incorrect. The crash in the bytecode-cache test is because it asserts there's a codeBlock but since it exception'd the first time there wasn't one. I changed that test to only run in default mode.
rdar://problem/58862199
Created attachment 389230 [details] Patch for landing
Comment on attachment 389230 [details] Patch for landing Rejecting attachment 389230 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 5000 characters of output: RGET -DENABLE_XSLT -DU_HIDE_DEPRECATED_API -DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.14 -g -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -Wno-sign-conversion -Winfinite-recursion -Wmove -Wcomma -Wblock-capture-autoreleasing -Wstrict-prototypes -Wrange-loop-analysis -Wno-semicolon-before-method-body -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-generated-files.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore -I. -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources-normal/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wconditional-uninitialized -Wextra-tokens -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/System.framework/PrivateHeaders --system-header-prefix=unicode/ -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/JavaScriptCorePrefix-focfrejsjmfkulfpzqniyatstmxf/JavaScriptCorePrefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource95.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource95.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource95.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource95.o Failed to run "['Tools/Scripts/build-webkit', '--release']" exit_code: 65 /JavaScriptCore-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore -I. -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources-normal/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wconditional-uninitialized -Wextra-tokens -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/System.framework/PrivateHeaders --system-header-prefix=unicode/ -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/JavaScriptCorePrefix-focfrejsjmfkulfpzqniyatstmxf/JavaScriptCorePrefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource95.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource95.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource95.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource95.o Full output: https://webkit-queues.webkit.org/results/13313829
Created attachment 389248 [details] Patch for landing
Comment on attachment 389248 [details] Patch for landing Rejecting attachment 389248 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 5000 characters of output: TITIONING -DENABLE_CHANNEL_MESSAGING -DENABLE_CONTENT_FILTERING -DENABLE_CSS_BOX_DECORATION_BREAK -DENABLE_CSS_COMPOSITING -DENABLE_CSS_CONIC_GRADIENTS -DENABLE_CSS_PAINTING_API -DENABLE_CSS_SCROLL_SNAP -DENABLE_CSS_SELECTORS_LEVEL4 -DENABLE_CSS_TRAILING_WORD -DENABLE_CSS_TYPED_OM -DENABLE_CURSOR_VISIBILITY -DENABLE_DARK_MODE_CSS -DENABLE_DATACUE_VALUE -DENABLE_DATALIST_ELEMENT -DENABLE_ENCRYPTED_MEDIA -DENABLE_EXPERIMENTAL_FEATURES -DENABLE_FILTERS_LEVEL_2 -DENABLE_FTL_JIT -DENABLE_FULLSCREEN_API -DENABLE_PICTURE_IN_PICTURE_API -DENABLE_GAMEPAD -DENABLE_GEOLOCATION -DENABLE_GPU_PROCESS -DENABLE_INDEXED_DATABASE -DENABLE_INDEXED_DATABASE_IN_WORKERS -DENABLE_INPUT_TYPE_COLOR -DENABLE_INSPECTOR_ALTERNATE_DISPATCHERS -DENABLE_INSPECTOR_TELEMETRY -DENABLE_INTERSECTION_OBSERVER -DENABLE_INTL -DENABLE_LAYOUT_FORMATTING_CONTEXT -DENABLE_LEGACY_CSS_VENDOR_PREFIXES -DENABLE_LEGACY_CUSTOM_PROTOCOL_MANAGER -DENABLE_LEGACY_ENCRYPTED_MEDIA -DENABLE_MATHML -DENABLE_MEDIA_CONTROLS_SCRIPT -DENABLE_MEDIA_SOURCE -DENABLE_MEDIA_STREAM -DENABLE_MEMORY_SAMPLER -DENABLE_METER_ELEMENT -DENABLE_MOUSE_CURSOR_SCALE -DENABLE_NETWORK_CACHE_SPECULATIVE_REVALIDATION -DENABLE_NETWORK_CACHE_STALE_WHILE_REVALIDATE -DENABLE_NOTIFICATIONS -DENABLE_PAYMENT_REQUEST -DENABLE_PDFKIT_PLUGIN -DENABLE_POINTER_EVENTS -DENABLE_POINTER_LOCK -DENABLE_PUBLIC_SUFFIX_LIST -DENABLE_REMOTE_INSPECTOR -DENABLE_RESIZE_OBSERVER -DENABLE_RESOURCE_LOAD_STATISTICS -DENABLE_RESOURCE_USAGE -DENABLE_RUBBER_BANDING -DENABLE_SANDBOX_EXTENSIONS -DENABLE_SERVER_PRECONNECT -DENABLE_SERVICE_CONTROLS -DENABLE_SERVICE_WORKER -DENABLE_SHAREABLE_RESOURCE -DENABLE_SPEECH_SYNTHESIS -DENABLE_STREAMS_API -DENABLE_SVG_FONTS -DENABLE_TELEPHONE_NUMBER_DETECTION -DENABLE_TEXT_AUTOSIZING -DENABLE_USERSELECT_ALL -DENABLE_USER_MESSAGE_HANDLERS -DENABLE_VARIATION_FONTS -DENABLE_VIDEO -DENABLE_VIDEO_PRESENTATION_MODE -DENABLE_VIDEO_TRACK -DENABLE_VIDEO_USES_ELEMENT_FULLSCREEN -DENABLE_WEBDRIVER_MOUSE_INTERACTIONS -DENABLE_WEBDRIVER_KEYBOARD_INTERACTIONS -DENABLE_WEBGL -DENABLE_WEBGL2 -DENABLE_WEBGPU -DENABLE_WEB_AUDIO -DENABLE_WEB_AUTHN -DENABLE_WEB_CRYPTO -DENABLE_WEB_RTC -DENABLE_WIRELESS_PLAYBACK_TARGET -DENABLE_XSLT -DU_HIDE_DEPRECATED_API -DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.14 -g -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -Wno-sign-conversion -Winfinite-recursion -Wmove -Wcomma -Wblock-capture-autoreleasing -Wstrict-prototypes -Wrange-loop-analysis -Wno-semicolon-before-method-body -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-generated-files.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/JavaScriptCore-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore -I. -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources-normal/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wconditional-uninitialized -Wextra-tokens -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/System.framework/PrivateHeaders --system-header-prefix=unicode/ -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/JavaScriptCorePrefix-focfrejsjmfkulfpzqniyatstmxf/JavaScriptCorePrefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource94.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource94.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/JavaScriptCore/unified-sources/UnifiedSource94.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/JavaScriptCore.build/Release/JavaScriptCore.build/Objects-normal/x86_64/UnifiedSource94.o Full output: https://webkit-queues.webkit.org/results/13313979
Created attachment 389256 [details] Patch for landing
Comment on attachment 389256 [details] Patch for landing Clearing flags on attachment: 389256 Committed r255440: <https://trac.webkit.org/changeset/255440>
All reviewed patches have been landed. Closing bug.
Comment on attachment 389256 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=389256&action=review > Source/JavaScriptCore/ChangeLog:13 > + expressions. Intrestingly, this was not tested in test262. Can we make a test262 issue?
Comment on attachment 389256 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=389256&action=review >> Source/JavaScriptCore/ChangeLog:13 >> + expressions. Intrestingly, this was not tested in test262. > > Can we make a test262 issue? I’ll file an issue, but I doubt I’ll have the bandwidth to make the test. It seems like there’s a general problem that test262 doesn’t test early errors inside functions. This means it doesn’t stress out SyntaxChecker...