Bug 1005658 - Don't pass stack pointers to the GL for buffers, and have GLContext try to guard against it - r=jgilbert

This commit is contained in:
Benoit Jacob 2014-05-08 21:03:37 -04:00
parent 2e1ef0025a
commit 7d4874a620
6 changed files with 108 additions and 15 deletions

View File

@ -1807,7 +1807,38 @@ GLContext::MarkDestroyed()
mSymbols.Zero();
}
#ifdef MOZ_ENABLE_GL_TRACKING
#ifdef DEBUG
/* static */ void
GLContext::AssertNotPassingStackBufferToTheGL(const void* ptr)
{
int somethingOnTheStack;
const void* someStackPtr = &somethingOnTheStack;
const int page_bits = 12;
intptr_t page = reinterpret_cast<uintptr_t>(ptr) >> page_bits;
intptr_t someStackPage = reinterpret_cast<uintptr_t>(someStackPtr) >> page_bits;
uintptr_t pageDistance = std::abs(page - someStackPage);
// Explanation for the "distance <= 1" check here as opposed to just
// an equality check.
//
// Here we assume that pages immediately adjacent to the someStackAddress page,
// are also stack pages. That allows to catch the case where the calling frame put
// a buffer on the stack, and we just crossed the page boundary. That is likely
// to happen, precisely, when using stack arrays. I hit that specifically
// with CompositorOGL::Initialize.
//
// In theory we could be unlucky and wrongly assert here. If that happens,
// it will only affect debug builds, and looking at stacks we'll be able to
// see that this assert is wrong and revert to the conservative and safe
// approach of only asserting when address and someStackAddress are
// on the same page.
bool isStackAddress = pageDistance <= 1;
MOZ_ASSERT(!isStackAddress,
"Please don't pass stack arrays to the GL. "
"Consider using HeapCopyOfStackArray. "
"See bug 1005658.");
}
void
GLContext::CreatedProgram(GLContext *aOrigin, GLuint aName)
{

View File

@ -42,10 +42,6 @@
#include "mozilla/Scoped.h"
#include "gfx2DGlue.h"
#ifdef DEBUG
#define MOZ_ENABLE_GL_TRACKING 1
#endif
class nsIntRegion;
class nsIRunnable;
class nsIThread;
@ -604,7 +600,7 @@ private:
#undef BEFORE_GL_CALL
#undef AFTER_GL_CALL
#ifdef MOZ_ENABLE_GL_TRACKING
#ifdef DEBUG
#ifndef MOZ_FUNCTION_NAME
# ifdef __GNUC__
@ -664,6 +660,8 @@ private:
return tip;
}
static void AssertNotPassingStackBufferToTheGL(const void* ptr);
#define BEFORE_GL_CALL \
do { \
BeforeGLCall(MOZ_FUNCTION_NAME); \
@ -679,11 +677,14 @@ private:
TrackingContext()->a; \
} while (0)
#define ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(ptr) AssertNotPassingStackBufferToTheGL(ptr)
#else // ifdef DEBUG
#define BEFORE_GL_CALL do { } while (0)
#define AFTER_GL_CALL do { } while (0)
#define TRACKING_CONTEXT(a) do {} while (0)
#define ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(ptr) do {} while (0)
#endif // ifdef DEBUG
@ -823,6 +824,7 @@ public:
private:
void raw_fBufferData(GLenum target, GLsizeiptr size, const GLvoid* data, GLenum usage) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(data);
BEFORE_GL_CALL;
mSymbols.fBufferData(target, size, data, usage);
AFTER_GL_CALL;
@ -837,12 +839,14 @@ public:
!data &&
Vendor() == GLVendor::NVIDIA)
{
char c = 0;
fBufferSubData(target, size-1, 1, &c);
ScopedDeleteArray<char> buf(new char[1]);
buf[0] = 0;
fBufferSubData(target, size-1, 1, buf);
}
}
void fBufferSubData(GLenum target, GLintptr offset, GLsizeiptr size, const GLvoid* data) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(data);
BEFORE_GL_CALL;
mSymbols.fBufferSubData(target, offset, size, data);
AFTER_GL_CALL;
@ -887,12 +891,14 @@ public:
}
void fCompressedTexImage2D(GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLsizei imageSize, const GLvoid *pixels) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(pixels);
BEFORE_GL_CALL;
mSymbols.fCompressedTexImage2D(target, level, internalformat, width, height, border, imageSize, pixels);
AFTER_GL_CALL;
}
void fCompressedTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLsizei imageSize, const GLvoid *pixels) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(pixels);
BEFORE_GL_CALL;
mSymbols.fCompressedTexSubImage2D(target, level, xoffset, yoffset, width, height, format, imageSize, pixels);
AFTER_GL_CALL;
@ -1398,6 +1404,7 @@ public:
}
void fTextureRangeAPPLE(GLenum target, GLsizei length, GLvoid *pointer) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(pointer);
BEFORE_GL_CALL;
mSymbols.fTextureRangeAPPLE(target, length, pointer);
AFTER_GL_CALL;
@ -1437,6 +1444,7 @@ public:
private:
void raw_fReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, GLvoid *pixels) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(pixels);
BEFORE_GL_CALL;
mSymbols.fReadPixels(x, y, width, height, format, type, pixels);
AFTER_GL_CALL;
@ -1538,6 +1546,7 @@ public:
private:
void raw_fTexImage2D(GLenum target, GLint level, GLint internalformat, GLsizei width, GLsizei height, GLint border, GLenum format, GLenum type, const GLvoid *pixels) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(pixels);
BEFORE_GL_CALL;
mSymbols.fTexImage2D(target, level, internalformat, width, height, border, format, type, pixels);
AFTER_GL_CALL;
@ -1557,6 +1566,7 @@ public:
}
void fTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width, GLsizei height, GLenum format, GLenum type, const GLvoid* pixels) {
ASSERT_NOT_PASSING_STACK_BUFFER_TO_GL(pixels);
BEFORE_GL_CALL;
mSymbols.fTexSubImage2D(target, level, xoffset, yoffset, width, height, format, type, pixels);
AFTER_GL_CALL;
@ -2510,7 +2520,7 @@ protected:
virtual bool MakeCurrentImpl(bool aForce) = 0;
public:
#ifdef MOZ_ENABLE_GL_TRACKING
#ifdef DEBUG
static void StaticInit() {
PR_NewThreadPrivateIndex(&sCurrentGLContextTLS, nullptr);
}
@ -2520,7 +2530,7 @@ public:
if (IsDestroyed()) {
return false;
}
#ifdef MOZ_ENABLE_GL_TRACKING
#ifdef DEBUG
PR_SetThreadPrivate(sCurrentGLContextTLS, this);
// XXX this assertion is disabled because it's triggering on Mac;
@ -2967,7 +2977,7 @@ public:
#undef ASSERT_SYMBOL_PRESENT
#ifdef MOZ_ENABLE_GL_TRACKING
#ifdef DEBUG
void CreatedProgram(GLContext *aOrigin, GLuint aName);
void CreatedShader(GLContext *aOrigin, GLuint aName);
void CreatedBuffers(GLContext *aOrigin, GLsizei aCount, GLuint *aNames);

View File

@ -0,0 +1,47 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
/* vim: set ts=8 sts=4 et sw=4 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef HEAPCOPYOFSTACKARRAY_H_
#define HEAPCOPYOFSTACKARRAY_H_
#include "mozilla/Attributes.h"
#include "mozilla/Scoped.h"
#include <string.h>
namespace mozilla {
// Takes a stack array and copies it into a heap buffer.
// Useful to retain the convenience of declaring static arrays, while
// avoiding passing stack pointers to the GL (see bug 1005658).
template <typename ElemType>
class HeapCopyOfStackArray
{
public:
template<size_t N>
HeapCopyOfStackArray(ElemType (&array)[N])
: mArrayLength(N)
, mArrayData(new ElemType[N])
{
memcpy(mArrayData, &array[0], N * sizeof(ElemType));
}
ElemType* Data() const { return mArrayData; }
size_t ArrayLength() const { return mArrayLength; }
size_t ByteLength() const { return mArrayLength * sizeof(ElemType); }
private:
HeapCopyOfStackArray() MOZ_DELETE;
HeapCopyOfStackArray(const HeapCopyOfStackArray&) MOZ_DELETE;
const size_t mArrayLength;
ScopedDeletePtr<ElemType> const mArrayData;
};
}
#endif // HEAPCOPYOFSTACKARRAY_H_

View File

@ -142,9 +142,8 @@ SharedSurface_Gralloc::Fence()
// work. glReadPixels seems to, though.
if (gfxPrefs::GrallocFenceWithReadPixels()) {
mGL->MakeCurrent();
// read a 1x1 pixel
unsigned char pixels[4];
mGL->fReadPixels(0, 0, 1, 1, LOCAL_GL_RGBA, LOCAL_GL_UNSIGNED_BYTE, &pixels[0]);
ScopedDeleteArray<char> buf(new char[4]);
mGL->fReadPixels(0, 0, 1, 1, LOCAL_GL_RGBA, LOCAL_GL_UNSIGNED_BYTE, buf);
}
}

View File

@ -46,6 +46,7 @@ EXPORTS += [
'GLTextureImage.h',
'GLTypes.h',
'GLUploadHelpers.h',
'HeapCopyOfStackArray.h',
'ScopedGLHelpers.h',
'SharedSurface.h',
'SharedSurfaceEGL.h',

View File

@ -44,6 +44,7 @@
#include "ScopedGLHelpers.h"
#include "GLReadTexImageHelper.h"
#include "TiledLayerBuffer.h" // for TiledLayerComposer
#include "HeapCopyOfStackArray.h"
#if MOZ_ANDROID_OMTC
#include "TexturePoolOGL.h"
@ -372,7 +373,11 @@ CompositorOGL::Initialize()
/* Then quad texcoords */
0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f,
};
mGLContext->fBufferData(LOCAL_GL_ARRAY_BUFFER, sizeof(vertices), vertices, LOCAL_GL_STATIC_DRAW);
HeapCopyOfStackArray<GLfloat> verticesOnHeap(vertices);
mGLContext->fBufferData(LOCAL_GL_ARRAY_BUFFER,
verticesOnHeap.ByteLength(),
verticesOnHeap.Data(),
LOCAL_GL_STATIC_DRAW);
mGLContext->fBindBuffer(LOCAL_GL_ARRAY_BUFFER, 0);
nsCOMPtr<nsIConsoleService>