Bug 206972 – Parser needs to restore unary stack state when backtracking

• Component: New Bugs
• Status: RESOLVED FIXED

[Keith Miller]

Parser needs to restore unary stack state when backtracking

[Keith Miller]

Created attachment 389200 [details]
Patch

[Saam Barati]

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

[Saam Barati]

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"

[Keith Miller]

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.

[Keith Miller]

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.

[Keith Miller]

rdar://problem/58862199

[Keith Miller]

Created attachment 389230 [details]
Patch for landing

[WebKit Commit Bot]

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

[Keith Miller]

Created attachment 389248 [details]
Patch for landing

[WebKit Commit Bot]

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

[Keith Miller]

Created attachment 389256 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 389256 [details]
Patch for landing

Clearing flags on attachment: 389256

Committed r255440: <https://trac.webkit.org/changeset/255440>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ross Kirsling]

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?

[Keith Miller]

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...