Bad shader "#ifdef GL_ES" not correctly validated and translation crashes GL driver

[GoogleCodeExporter]

Hi, this is forwarded from https://bugzilla.mozilla.org/show_bug.cgi?id=597946

Steps:

1. Launch Firefox 4 on any platform EXCEPT Linux 64bit. Check in about:config 
that webgl.enabled_for_all_sites and that webgl.shader_validator are true.
2. Go to http://gyu.que.jp/private/badshader.html

Expected: A messagebox prints the Shader InfoLog.

Actual: Graphics driver crashes (confirmed here on Windows/NVIDIA and by the 
original reported on a Mac).

Disabling the ANGLE translator/validator in Firefox makes this issue disappear.

This is using the copy of ANGLE that we have in Mozilla-central. Let me know if 
you think that it's worth updating it.

Original issue reported on code.google.com by jacob.be...@gmail.com on 20 Sep 2010 at 1:01

[GoogleCodeExporter]

This indicates a bug in the preprocessor. It does not seem to be handling EOF 
properly.

Original comment by alokp@chromium.org on 20 Sep 2010 at 8:09

[GoogleCodeExporter]

Issue 67 has been merged into this issue.

Original comment by alokp@chromium.org on 25 Oct 2010 at 7:42

[GoogleCodeExporter]

Original comment by alokp@chromium.org on 25 Oct 2010 at 7:42

• Changed state: Accepted

[GoogleCodeExporter]

Please give this bug some more attention :) Above-mentioned mozilla bug shows 
that it allows to crash any browser using ANGLE with WebGL. So I don't think 
that any browser (Firefox or Chrome or other) will be able to ship a stable 
version with WebGL enabled until it's fixed!

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 9:04

[GoogleCodeExporter]

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 9:56

Attachments:

• badshader.html

[GoogleCodeExporter]

Hi Benoit/Daniel:
Could anyone on your side take this over? Sorry I am too swamped right now.

The problem is in the preprocessor code. If EOF appears before EOL, it gets 
stuck into an infinite loop looking for an EOL.

I have a replacement for the preprocessor in the works but it is not ready yet.

-Alok

Original comment by alokp@chromium.org on 10 Dec 2010 at 10:22

[GoogleCodeExporter]

> If EOF appears before EOL, it gets stuck into an infinite loop looking for an 
EOL.

Fortunately, it doesn't seem to be quite that general: attached shader (valid 
code without a #ifdef) has only one line, so EOF appears before EOL, yet it 
doesn't trigger this bug.

This really this seems to have something to do with #if not matched by a #endif.

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 10:46

Attachments:

• badshader2.html

[GoogleCodeExporter]

> Could anyone on your side take this over?

I will try, but make no promises as I don't know this code.

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 10:47

[GoogleCodeExporter]

Last time I checked the problem was with many other preprocessor directives - 
#if #ifdef #pragma ... If you cannot reproduce them - good!

Original comment by alokp@chromium.org on 10 Dec 2010 at 10:49

[GoogleCodeExporter]

I didnt try other preprocessor directives -- I was just saying that a 1-line 
program without any preprocessor directive doesn't reproduce the bug.

Call stack when it's frozen (quite old build)

>   xul.dll!CPPifdef(int defined, yystypepp * yylvalpp)  Line 499 + 0x19 bytes  C
    xul.dll!readCPPline(yystypepp * yylvalpp)  Line 760 + 0xb bytes C
    xul.dll!yylex_CPP(char * buf, int maxSize)  Line 696 + 0xc bytes    C
    xul.dll!yy_input(char * buf, int max_size)  Line 2454 + 0xd bytes   C++
    xul.dll!yy_get_next_buffer()  Line 1817 + 0x15 bytes    C++
    xul.dll!yylex(YYSTYPE * pyylval, void * parseContextLocal)  Line 1651 + 0x5 bytes   C++
    xul.dll!yyparse(void * parseContextLocal)  Line 1208 + 0x10 bytes   C++
    xul.dll!PaParseStrings(const char * const * argv, const int * strLen, int argc, TParseContext & parseContextLocal)  Line 2504 + 0x9 bytes   C++
    xul.dll!TCompiler::compile(const char * const * shaderStrings, const int numStrings, int compileOptions)  Line 119 + 0x16 bytes C++
    xul.dll!ShCompile(void * const handle, const char * const * shaderStrings, const int numStrings, int compileOptions)  Line 166 + 0x14 bytes C++
    xul.dll!mozilla::WebGLContext::CompileShader(nsIWebGLShader * sobj)  Line 3185 + 0x11 bytes C++
    xul.dll!nsICanvasRenderingContextWebGL_CompileShader(JSContext * cx, unsigned int argc, jsval_layout * vp)  Line 28645 + 0x12 bytes C++
    mozjs.dll!js::CallJSNative(JSContext * cx, int (JSContext *, unsigned int, js::Value *)* native, unsigned int argc, js::Value * vp)  Line 652 + 0xf bytes   C++

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 10:54

[GoogleCodeExporter]

Here's the source code for this function:

static int CPPifdef(int defined, yystypepp * yylvalpp)
{
    int token = cpp->currentInput->scan(cpp->currentInput, yylvalpp);
    int name = yylvalpp->sc_ident;
    if(++cpp->ifdepth >MAX_IF_NESTING){
        CPPErrorToInfoLog("max #if nesting depth exceeded");
        return 0;
    }
    cpp->elsetracker++;
    cpp->elsedepth[cpp->elsetracker] = 0;
    if (token != CPP_IDENTIFIER) {
            defined ? CPPErrorToInfoLog("ifdef"):CPPErrorToInfoLog("ifndef");
    } else {
        Symbol *s = LookUpSymbol(macros, name);
        token = cpp->currentInput->scan(cpp->currentInput, yylvalpp);
        if (token != '\n') {
            CPPWarningToInfoLog("unexpected tokens following #ifdef preprocessor directive - expected a newline");
            while (token != '\n')
                token = cpp->currentInput->scan(cpp->currentInput, yylvalpp);
        }
        if (((s && !s->details.mac.undef) ? 1 : 0) != defined)
            token = CPPelse(1, yylvalpp);
    }
    return token;
} // CPPifdef

It's stuck in this loop:

            while (token != '\n')
                token = cpp->currentInput->scan(cpp->currentInput, yylvalpp);

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 10:57

[GoogleCodeExporter]

Can't this warning here just be an error, and can't we then just return 0 right 
away?

What is the reason why we need to do this while loop until we find a newline 
(or I guess EOF) ?

Original comment by jacob.be...@gmail.com on 10 Dec 2010 at 11:00

[GoogleCodeExporter]

This patch implements what I proposed in comment 12.

Here it works great: the test case does no longer freeze, instead it now gives 
the expected shader compilation errors.

Original comment by jacob.be...@gmail.com on 11 Dec 2010 at 5:17

Attachments:

• fix-angle-bug-40

[GoogleCodeExporter]

The fix looks good!  Thanks for getting a fix so quickly.

Original comment by z...@google.com on 11 Dec 2010 at 5:32

[GoogleCodeExporter]

I've got a better patch which also fixes the following shaders (all of these 
also cause infinite loops currently).

"#ifdef GL_ES";
"#ifndef GL_ES";
"#if 1";
"#error foo";
"#ifdef GL_ES\n#elif GL_DS";
"#ifdef GL_ES\n#elif GL_DS\n#else";
"#if 1\n#else";
"#define bob 1";
"#else";
"#elif BOB";
"#if defined(BOB)\n#else";

I'll have the fix integrated as soon as I can get someone to review the patch

Original comment by dan...@transgaming.com on 12 Dec 2010 at 8:43

• Changed state: Started

Attachments:

• fix_preprocessor_hangs.patch

[GoogleCodeExporter]

Fixed in r506 (even though the commit message says it fixes Issue 42 …)

Original comment by dan...@transgaming.com on 13 Dec 2010 at 2:50

• Changed state: Fixed

[GoogleCodeExporter]

A user posted a new test case starting in #elif:

https://bugzilla.mozilla.org/show_bug.cgi?id=619455

Please check it's also fixed :-)

Original comment by jacob.be...@gmail.com on 15 Dec 2010 at 9:50

[GoogleCodeExporter]

Yes, it's fixed as well (where "fixed" means it properly fails to compile as 
opposed to going into an infinite loop).

Original comment by dan...@transgaming.com on 20 Dec 2010 at 8:43