669813 - Assertion failed: (ch == '\0'), function ReadToken

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

This bug was discovered during a manual audit for bug: https://bugzilla.mozilla.org/show_bug.cgi?id=668366

Comment 1

[Christoph Diehl [:posidron]]

Attachment: testcase

Comment 2

[Christoph Diehl [:posidron]]

Attachment: testcase - reduced

reduced testcase. looks like something went wrong during the assignment of the int variable and not during the uniform indexing.

Comment 3

[Daniel Veditz [:dveditz]]

Since I can't see the ANGLE bug I'm going to assume they are considering this a security vulnerability -> sg:critical until we know more.

Comment 4

[daniel-bzmz]

This appears to be a simple bug in the preprocessor, which doesn't handle hexadecimal numbers correctly. As far as we can tell, it will only cause a compilation error and is not a security issue.

Comment 5

[Daniel Veditz [:dveditz]]

Does that mean you'll unhide the angleproject bug linked here?

Comment 6

[daniel-bzmz]

(In reply to comment #5)
> Does that mean you'll unhide the angleproject bug linked here?
Done

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

remove sg:critical?

Comment 8

[Kelsey Gilbert [:jgilbert]]

This seems to just result in a link error on non-debug builds. It does crash on debug builds though.

Comment 9

[Kelsey Gilbert [:jgilbert]]

Basically, ANGLE interprets all integer literals as decimal. GLSL accepts octal and hexadecimal, and it seems like these should be required. Further, it appears that we must not be testing for these in the conformance tests.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Attachment: Parse octal/hexadecimal literals properly.

Prerequisite patch is the patch for too-large-tokens at bug 742138.

Comment 11

[Kenneth Russell]

Please file a bug on http://angleproject.googlecode.com/ and attach this patch for review and comment. Someone knowledgeable about the parser in ANGLE's shader translator (alokp at chromium.org in particular) should review this patch.

Comment 12

[Kelsey Gilbert [:jgilbert]]

(In reply to Kenneth Russell from comment #11)
> Please file a bug on http://angleproject.googlecode.com/ and attach this
> patch for review and comment. Someone knowledgeable about the parser in
> ANGLE's shader translator (alokp at chromium.org in particular) should
> review this patch.

Both have respective ANGLE bugs, but I just need to figure out what format the patch should be in. bjacob is more familiar with this process, so I'll sync up with him tomorrow.

Comment 13

[daniel-bzmz]

Either an SVN diff or git diff would be fine.

Comment 14

[Kelsey Gilbert [:jgilbert]]

(In reply to daniel-bzmz from comment #13)
> Either an SVN diff or git diff would be fine.

I posted git diffs, but would hg diffs work, for future reference?
Also, I'm not sure how to trigger review, so the diffs are merely attached for now.

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

Note that hg knows how to generate git diffs:

  hg diff -g
  hg export -g

Comment 16

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 612064 [details] [diff] [review]
Parse octal/hexadecimal literals properly.

Waiting for review by real ANGLE devs.

Comment 17

[Benoit Jacob [:bjacob] (mostly away)]

Fixed by update to ANGLE r1042 (bug 734657) which includes the fix for http://code.google.com/p/angleproject/issues/detail?id=178