Bug 158437 - octal and binary parsing is wrong for some programs
Summary: octal and binary parsing is wrong for some programs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-06 15:28 PDT by Saam Barati
Modified: 2016-06-06 20:00 PDT (History)
11 users (show)

See Also:


Attachments
Patch (2.12 KB, patch)
2016-06-06 17:59 PDT, Michael Saboff
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-06-06 15:28:26 PDT
olliej is the one who found this.

eval(“0o19”)
crashes in a debug build:

ASSERTION FAILED: !isImpureNaN(d)
/Volumes/Untitled/WebKit/WebKit/Source/JavaScriptCore/runtime/JSCJSValueInlines.h(475) : JSC::JSValue::JSValue(JSC::JSValue::EncodeAsDoubleTag, double)
1   0x106323dcd WTFCrash
2   0x105486357 JSC::JSValue::JSValue(JSC::JSValue::EncodeAsDoubleTag, double)
3   0x1054862f5 JSC::JSValue::JSValue(JSC::JSValue::EncodeAsDoubleTag, double)
4   0x10567f0dc JSC::JSValue::JSValue(double)
5   0x10567f03f JSC::JSValue::JSValue(double)
6   0x105ffff9d JSC::NumberNode::NumberNode(JSC::JSTokenLocation const&, double)
7   0x105fffed1 JSC::DoubleNode::DoubleNode(JSC::JSTokenLocation const&, double)
8   0x106000447 JSC::DoubleNode::DoubleNode(JSC::JSTokenLocation const&, double)
9   0x106018f5e JSC::ASTBuilder::createDoubleExpr(JSC::JSTokenLocation const&, double)
10  0x10601327a JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parsePrimaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&)
11  0x1060114b6 JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseMemberExpression<JSC::ASTBuilder>(JSC::ASTBuilder&)
12  0x1060509ea JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseUnaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&)
13  0x10604ffea JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseBinaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder&)
14  0x10604f419 JSC::ASTBuilder::Expression JSC::Parser<JSC::Lexer<unsigned char> >::parseConditionalExpression<JSC::ASTBuilder>(JSC::ASTBuilder&)
Comment 1 Michael Saboff 2016-06-06 15:51:56 PDT
I don't get this crash with a debug build of ToT (r201731):

$ DYLD_FRAMEWORK_PATH=WebKitBuild/Debug WebKitBuild/Debug/jsc 
>>> eval("0o19")
Exception: SyntaxError: Unexpected number '9'. Parse error.
Comment 2 Oliver Hunt 2016-06-06 16:34:59 PDT
It would appear that this is somehow machine dependent. What joy :)
Comment 3 Michael Saboff 2016-06-06 16:57:10 PDT
Turns out that the issue is reading uninitialized memory.  Thus the machine dependent failure.  Bad binary literals have the same issue.  Patch in the works.
Comment 4 Michael Saboff 2016-06-06 17:59:21 PDT
Created attachment 280656 [details]
Patch
Comment 5 Radar WebKit Bug Importer 2016-06-06 17:59:53 PDT
<rdar://problem/26663732>
Comment 6 Darin Adler 2016-06-06 18:48:18 PDT
Comment on attachment 280656 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=280656&action=review

> Source/JavaScriptCore/ChangeLog:9
> +        When there is an error parsing an binary or octal literal, we need to clear the returnValue
> +        of any residual value.

Why?
Comment 7 Michael Saboff 2016-06-06 19:38:13 PDT
(In reply to comment #6)
> Comment on attachment 280656 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=280656&action=review
> 
> > Source/JavaScriptCore/ChangeLog:9
> > +        When there is an error parsing an binary or octal literal, we need to clear the returnValue
> > +        of any residual value.
> 
> Why?

Because returnValue's value is used to determine INTEGER or DOUBLE token type.  If the value is a double and an impure NaN we get the crash.  The syntax checking is based on having leftover characters and is done after the returnValue has been processed.

I'll add some of that detail to the ChangeLog.
Comment 8 Michael Saboff 2016-06-06 20:00:40 PDT
Committed r201737: <http://trac.webkit.org/changeset/201737>