Bug 683710: added stripping of comments from shader sources before compiling and check for illegal characters, mostly from webkit code r=bjacob

This code is copied mostly from WebKit. It strips out comments from shader source code before actually compiling it, so that the comments can have illegal characters. It was benchmarked and it was noted that a test attached to the bug ticket took about twice as long with these changes.

This includes the patch from bug 680722 to check for illegal characters.
This commit is contained in:
Doug Sherk 2011-09-07 17:17:44 -04:00
parent 99a7871114
commit f9a5610153
7 changed files with 313 additions and 39 deletions

View File

@ -502,7 +502,9 @@ protected:
PRBool ValidateAttribIndex(WebGLuint index, const char *info);
PRBool ValidateStencilParamsForDrawCall();
bool ValidateGLSLIdentifier(const nsAString& name, const char *info);
bool ValidateGLSLVariableName(const nsAString& name, const char *info);
bool ValidateGLSLCharacter(PRUnichar c);
bool ValidateGLSLString(const nsAString& string, const char *info);
static PRUint32 GetTexelSize(WebGLenum format, WebGLenum type);
@ -1366,12 +1368,12 @@ public:
void IncrementAttachCount() { mAttachCount++; }
void DecrementAttachCount() { mAttachCount--; }
void SetSource(const nsCString& src) {
void SetSource(const nsAString& src) {
// XXX do some quick gzip here maybe -- getting this will be very rare
mSource.Assign(src);
}
const nsCString& Source() const { return mSource; }
const nsString& Source() const { return mSource; }
void SetNeedsTranslation() { mNeedsTranslation = true; }
bool NeedsTranslation() const { return mNeedsTranslation; }
@ -1382,7 +1384,7 @@ public:
}
void SetTranslationFailure(const nsCString& msg) {
mTranslationLog.Assign(msg);
mTranslationLog.Assign(msg);
}
const nsCString& TranslationLog() const { return mTranslationLog; }
@ -1393,7 +1395,7 @@ protected:
WebGLuint mName;
PRBool mDeleted;
WebGLenum mType;
nsCString mSource;
nsString mSource;
nsCString mTranslationLog;
bool mNeedsTranslation;
PRUint32 mAttachCount;

View File

@ -62,6 +62,7 @@
#endif
#include "WebGLTexelConversions.h"
#include "WebGLValidateStrings.h"
using namespace mozilla;
@ -182,8 +183,8 @@ WebGLContext::BindAttribLocation(nsIWebGLProgram *pobj, WebGLuint location, cons
if (!GetGLName<WebGLProgram>("bindAttribLocation: program", pobj, &progname))
return NS_OK;
if (name.IsEmpty())
return ErrorInvalidValue("BindAttribLocation: name can't be null or empty");
if (!ValidateGLSLVariableName(name, "bindAttribLocation"))
return NS_OK;
if (!ValidateAttribIndex(location, "bindAttribLocation"))
return NS_OK;
@ -1856,7 +1857,7 @@ WebGLContext::GetAttribLocation(nsIWebGLProgram *pobj,
if (!GetGLName<WebGLProgram>("getAttribLocation: program", pobj, &progname))
return NS_OK;
if (!ValidateGLSLIdentifier(name, "getAttribLocation"))
if (!ValidateGLSLVariableName(name, "getAttribLocation"))
return NS_OK;
MakeContextCurrent();
@ -2671,7 +2672,7 @@ WebGLContext::GetUniformLocation(nsIWebGLProgram *pobj, const nsAString& name, n
if (!GetConcreteObjectAndGLName("getUniformLocation: program", pobj, &prog, &progname))
return NS_OK;
if (!ValidateGLSLIdentifier(name, "getUniformLocation"))
if (!ValidateGLSLVariableName(name, "getUniformLocation"))
return NS_OK;
MakeContextCurrent();
@ -3970,8 +3971,25 @@ WebGLContext::CompileShader(nsIWebGLShader *sobj)
gl->IsGLES2() ? SH_ESSL_OUTPUT : SH_GLSL_OUTPUT,
&resources);
nsPromiseFlatCString src(shader->Source());
const char *s = src.get();
// We're storing an actual instance of StripComments because, if we don't, the
// cleanSource nsAString instance will be destroyed before the reference is
// actually used.
StripComments stripComments(shader->Source());
const nsAString& cleanSource = nsString(stripComments.result().Elements(), stripComments.length());
if (!ValidateGLSLString(cleanSource, "compileShader"))
return NS_OK;
const nsPromiseFlatString& flatSource = PromiseFlatString(cleanSource);
// shaderSource() already checks that the source stripped of comments is in the
// 7-bit ASCII range, so we can skip the NS_IsAscii() check.
const nsCString& sourceCString = NS_LossyConvertUTF16toASCII(flatSource);
const PRUint32 maxSourceLength = (PRUint32(1)<<18) - 1;
if (sourceCString.Length() > maxSourceLength)
return ErrorInvalidValue("compileShader: source has more than %d characters", maxSourceLength);
const char *s = sourceCString.get();
if (!ShCompile(compiler, &s, 1, SH_OBJECT_CODE)) {
int len = 0;
@ -4012,15 +4030,10 @@ WebGLContext::CompileShader(nsIWebGLShader *sobj)
shader->SetTranslationSuccess();
ShDestruct(compiler);
} else
#endif
{
const char *s = nsDependentCString(shader->Source()).get();
gl->fShaderSource(shadername, 1, &s, NULL);
shader->SetTranslationSuccess();
}
gl->fCompileShader(shadername);
gl->fCompileShader(shadername);
}
#endif
return NS_OK;
}
@ -4119,7 +4132,7 @@ WebGLContext::GetShaderSource(nsIWebGLShader *sobj, nsAString& retval)
if (!GetConcreteObjectAndGLName("getShaderSource: shader", sobj, &shader, &shadername))
return NS_OK;
CopyASCIItoUTF16(shader->Source(), retval);
retval.Assign(shader->Source());
return NS_OK;
}
@ -4131,19 +4144,16 @@ WebGLContext::ShaderSource(nsIWebGLShader *sobj, const nsAString& source)
WebGLuint shadername;
if (!GetConcreteObjectAndGLName("shaderSource: shader", sobj, &shader, &shadername))
return NS_OK;
const nsPromiseFlatString& flatSource = PromiseFlatString(source);
if (!NS_IsAscii(flatSource.get()))
return ErrorInvalidValue("shaderSource: non-ascii characters found in source");
// We're storing an actual instance of StripComments because, if we don't, the
// cleanSource nsAString instance will be destroyed before the reference is
// actually used.
StripComments stripComments(source);
const nsAString& cleanSource = nsString(stripComments.result().Elements(), stripComments.length());
if (!ValidateGLSLString(cleanSource, "compileShader"))
return NS_OK;
const nsCString& sourceCString = NS_LossyConvertUTF16toASCII(flatSource);
const PRUint32 maxSourceLength = (PRUint32(1)<<18) - 1;
if (sourceCString.Length() > maxSourceLength)
return ErrorInvalidValue("shaderSource: source has more than %d characters", maxSourceLength);
shader->SetSource(sourceCString);
shader->SetSource(source);
shader->SetNeedsTranslation();

View File

@ -328,14 +328,31 @@ PRBool WebGLContext::ValidateDrawModeEnum(WebGLenum mode, const char *info)
}
}
bool WebGLContext::ValidateGLSLIdentifier(const nsAString& name, const char *info)
bool WebGLContext::ValidateGLSLVariableName(const nsAString& name, const char *info)
{
const PRUint32 maxSize = 4095;
const PRUint32 maxSize = 255;
if (name.Length() > maxSize) {
ErrorInvalidValue("%s: identifier is %d characters long, exceeds the maximum allowed length of %d characters",
info, name.Length(), maxSize);
return false;
}
if (!ValidateGLSLString(name, info)) {
return false;
}
return true;
}
bool WebGLContext::ValidateGLSLString(const nsAString& string, const char *info)
{
for (PRUint32 i = 0; i < string.Length(); ++i) {
if (!ValidateGLSLCharacter(string.CharAt(i))) {
ErrorInvalidValue("%s: string contains the illegal character '%d'", info, string.CharAt(i));
return false;
}
}
return true;
}

View File

@ -0,0 +1,251 @@
/*
* Copyright (C) 2011 Apple Inc. All rights reserved.
* Copyright (C) 2011 Mozilla Corporation. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
* OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#ifndef WEBGLVALIDATESTRINGS_H_
#define WEBGLVALIDATESTRINGS_H_
#include "WebGLContext.h"
namespace mozilla {
// The following code was taken from the WebKit WebGL implementation,
// which can be found here:
// http://trac.webkit.org/browser/trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp?rev=93625#L121
// Note that some modifications were done to adapt it to Mozilla.
/****** BEGIN CODE TAKEN FROM WEBKIT ******/
bool WebGLContext::ValidateGLSLCharacter(PRUnichar c)
{
// Printing characters are valid except " $ ` @ \ ' DEL.
if (c >= 32 && c <= 126 &&
c != '"' && c != '$' && c != '`' && c != '@' && c != '\\' && c != '\'')
{
return true;
}
// Horizontal tab, line feed, vertical tab, form feed, carriage return are also valid.
if (c >= 9 && c <= 13) {
return true;
}
return false;
}
// Strips comments from shader text. This allows non-ASCII characters
// to be used in comments without potentially breaking OpenGL
// implementations not expecting characters outside the GLSL ES set.
class StripComments {
public:
StripComments(const nsAString& str)
: m_parseState(BeginningOfLine)
, m_end(str.EndReading())
, m_current(str.BeginReading())
, m_position(0)
{
m_result.SetLength(str.Length());
parse();
}
const nsTArray<PRUnichar>& result()
{
return m_result;
}
size_t length()
{
return m_position;
}
private:
bool hasMoreCharacters()
{
return (m_current < m_end);
}
void parse()
{
while (hasMoreCharacters()) {
process(current());
// process() might advance the position.
if (hasMoreCharacters())
advance();
}
}
void process(PRUnichar);
bool peek(PRUnichar& character)
{
if (m_current + 1 >= m_end)
return false;
character = *(m_current + 1);
return true;
}
PRUnichar current()
{
//ASSERT(m_position < m_length);
return *m_current;
}
void advance()
{
++m_current;
}
bool isNewline(PRUnichar character)
{
// Don't attempt to canonicalize newline related characters.
return (character == '\n' || character == '\r');
}
void emit(PRUnichar character)
{
m_result[m_position++] = character;
}
enum ParseState {
// Have not seen an ASCII non-whitespace character yet on
// this line. Possible that we might see a preprocessor
// directive.
BeginningOfLine,
// Have seen at least one ASCII non-whitespace character
// on this line.
MiddleOfLine,
// Handling a preprocessor directive. Passes through all
// characters up to the end of the line. Disables comment
// processing.
InPreprocessorDirective,
// Handling a single-line comment. The comment text is
// replaced with a single space.
InSingleLineComment,
// Handling a multi-line comment. Newlines are passed
// through to preserve line numbers.
InMultiLineComment
};
ParseState m_parseState;
const PRUnichar* m_end;
const PRUnichar* m_current;
size_t m_position;
nsTArray<PRUnichar> m_result;
};
void StripComments::process(PRUnichar c)
{
if (isNewline(c)) {
// No matter what state we are in, pass through newlines
// so we preserve line numbers.
emit(c);
if (m_parseState != InMultiLineComment)
m_parseState = BeginningOfLine;
return;
}
PRUnichar temp = 0;
switch (m_parseState) {
case BeginningOfLine:
// If it's an ASCII space.
if (c <= ' ' && (c == ' ' || (c <= 0xD && c >= 0x9))) {
emit(c);
break;
}
if (c == '#') {
m_parseState = InPreprocessorDirective;
emit(c);
break;
}
// Transition to normal state and re-handle character.
m_parseState = MiddleOfLine;
process(c);
break;
case MiddleOfLine:
if (c == '/' && peek(temp)) {
if (temp == '/') {
m_parseState = InSingleLineComment;
emit(' ');
advance();
break;
}
if (temp == '*') {
m_parseState = InMultiLineComment;
// Emit the comment start in case the user has
// an unclosed comment and we want to later
// signal an error.
emit('/');
emit('*');
advance();
break;
}
}
emit(c);
break;
case InPreprocessorDirective:
// No matter what the character is, just pass it
// through. Do not parse comments in this state. This
// might not be the right thing to do long term, but it
// should handle the #error preprocessor directive.
emit(c);
break;
case InSingleLineComment:
// The newline code at the top of this function takes care
// of resetting our state when we get out of the
// single-line comment. Swallow all other characters.
break;
case InMultiLineComment:
if (c == '*' && peek(temp) && temp == '/') {
emit('*');
emit('/');
m_parseState = MiddleOfLine;
advance();
break;
}
// Swallow all other characters. Unclear whether we may
// want or need to just emit a space per character to try
// to preserve column numbers for debugging purposes.
break;
}
}
/****** END CODE TAKEN FROM WEBKIT ******/
} // end namespace mozilla
#endif // WEBGLVALIDATESTRINGS_H_

View File

@ -8,11 +8,9 @@ conformance/shaders/glsl-features/../../glsl-features.html?feature=abs-vert-vec4
conformance/shaders/glsl-features/../../glsl-features.html?feature=sign-frag-vec4&reffs=shaders/glsl-features/sign-vec4-ref.frag&testfs=shaders/glsl-features/sign-vec4.frag
conformance/shaders/glsl-features/../../glsl-features.html?feature=sign-vert-vec4&refvs=shaders/glsl-features/sign-vec4-ref.vert&testvs=shaders/glsl-features/sign-vec4.vert
conformance/gl-get-active-attribute.html
conformance/gl-getshadersource.html
conformance/gl-uniform-bool.html
conformance/glsl-conformance.html
conformance/glsl-long-variable-names.html
conformance/invalid-passed-params.html
conformance/premultiplyalpha-test.html
conformance/read-pixels-test.html
conformance/uninitialized-test.html

View File

@ -2,11 +2,9 @@ conformance/context-attributes-alpha-depth-stencil-antialias.html
conformance/drawingbuffer-static-canvas-test.html
conformance/drawingbuffer-test.html
conformance/framebuffer-object-attachment.html
conformance/gl-getshadersource.html
conformance/gl-object-get-calls.html
conformance/glsl-conformance.html
conformance/glsl-long-variable-names.html
conformance/invalid-passed-params.html
conformance/premultiplyalpha-test.html
conformance/program-test.html
conformance/read-pixels-test.html

View File

@ -1,10 +1,8 @@
conformance/drawingbuffer-static-canvas-test.html
conformance/drawingbuffer-test.html
conformance/framebuffer-object-attachment.html
conformance/gl-getshadersource.html
conformance/glsl-conformance.html
conformance/glsl-long-variable-names.html
conformance/invalid-passed-params.html
conformance/premultiplyalpha-test.html
conformance/read-pixels-test.html
conformance/more/conformance/quickCheckAPI.html