From b100b6fd6ac8a5f43d6610d99b9564b722d78f52 Mon Sep 17 00:00:00 2001 From: Andrew Hsieh Date: Wed, 29 Feb 2012 13:53:37 -0800 Subject: [PATCH] Fixed crash and 64-bit porting issues 1. "emugen" generates four *dec.cpp files containing code like this to decode offset to pointer in stream tmp = *(T *)(ptr + 8 + 4 + 4 + 4 + *(size_t *)(ptr +8 + 4 + 4)); If *dec.cpp are compiled in 64-bit, size_t is 8-byte and dereferencing of it is likley to get wild offset for dereferencing of *(T *) to crash the code. Solution is to define tsize_t for "target size_t" instead of using host size_t. 2. Cast pointer to "uintptr_t" instead of "unsigned int" for 2nd param of ShareGroup::getGlobalName(NamedObjectType, ObjectLocalName/*64bit*/). 3. Instance of EGLSurface, EGLContext and EGLImageKHR are used as 32-bit key for std::map< unsigned int, * > SurfacesHndlMap, ContextsHndlMap, and ImagesHndlMap, respectively. Cast pointer to uintptr_t and assert upper 32-bit is zero before passing to map::find(). 4. Instance of GLeglImageOES is used to eglAttachEGLImage() which expect "unsigned int". Cast it to uintptr_t and assert upper 32-bit is zero. 5. The 5th param to GLEScontext::setPointer is GLvoid* but contains 32-bit offset to vbo if bufferName exists. Cast it to uintptr_t and assert upper 32-bit is zero. 6. Use %zu instead of %d to print size_t 7. Cast pointer to (uintptr_t) in many other places Change-Id: Iba6e5bda08c43376db5b011e9d781481ee1f5a12 --- .../host/libs/Translator/EGL/EglDisplay.cpp | 49 ++++++++++++++----- .../host/libs/Translator/EGL/EglImp.cpp | 4 +- .../libs/Translator/GLES_CM/GLEScmImp.cpp | 17 +++++-- .../libs/Translator/GLES_V2/GLESv2Imp.cpp | 17 +++++-- .../libs/Translator/GLcommon/GLEScontext.cpp | 5 +- .../host/libs/libOpenglRender/ReadBuffer.cpp | 2 +- .../opengl/host/tools/emugen/ApiGen.cpp | 7 +-- .../shared/OpenglCodecCommon/SocketStream.cpp | 2 +- .../opengl/shared/OpenglOsUtils/osThread.h | 1 + .../shared/OpenglOsUtils/osThreadUnix.cpp | 2 +- 10 files changed, 76 insertions(+), 30 deletions(-) diff --git a/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp b/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp index 46285db47..c2b515e47 100644 --- a/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp +++ b/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp @@ -17,6 +17,7 @@ #include "EglOsApi.h" #include #include +#include EglDisplay::EglDisplay(EGLNativeInternalDisplayType dpy,bool isDefault) : m_dpy(dpy), @@ -141,8 +142,12 @@ EglConfig* EglDisplay::getConfig(EGLConfig conf) { SurfacePtr EglDisplay::getSurface(EGLSurface surface) { android::Mutex::Autolock mutex(m_lock); - - SurfacesHndlMap::iterator it = m_surfaces.find(reinterpret_cast(surface)); + /* surface is "key" in map. + In 64-bit the upper 32-bit should be all zero. Assert for that. */ + uintptr_t hndlptr = (uintptr_t)surface; + unsigned int hndl = (unsigned int)hndlptr; + assert(sizeof(hndl) == sizeof(hndlptr) || hndl == hndlptr); + SurfacesHndlMap::iterator it = m_surfaces.find(hndl); return it != m_surfaces.end() ? (*it).second : SurfacePtr(NULL); @@ -150,8 +155,12 @@ SurfacePtr EglDisplay::getSurface(EGLSurface surface) { ContextPtr EglDisplay::getContext(EGLContext ctx) { android::Mutex::Autolock mutex(m_lock); - - ContextsHndlMap::iterator it = m_contexts.find(reinterpret_cast(ctx)); + /* ctx is "key" in map. + In 64-bit the upper 32-bit should be all zero. Assert for that. */ + uintptr_t hndlptr = (uintptr_t)ctx; + unsigned int hndl = (unsigned int)hndlptr; + assert(sizeof(hndl) == sizeof(hndlptr) || hndl == hndlptr); + ContextsHndlMap::iterator it = m_contexts.find(hndl); return it != m_contexts.end() ? (*it).second : ContextPtr(NULL); @@ -159,8 +168,12 @@ ContextPtr EglDisplay::getContext(EGLContext ctx) { bool EglDisplay::removeSurface(EGLSurface s) { android::Mutex::Autolock mutex(m_lock); - - SurfacesHndlMap::iterator it = m_surfaces.find(reinterpret_cast(s)); + /* s is "key" in map. + In 64-bit the upper 32-bit should be all zero. Assert for that. */ + uintptr_t hndlptr = (uintptr_t)s; + unsigned int hndl = (unsigned int)hndlptr; + assert(sizeof(hndl) == sizeof(hndlptr) || hndl == hndlptr); + SurfacesHndlMap::iterator it = m_surfaces.find(hndl); if(it != m_surfaces.end()) { m_surfaces.erase(it); return true; @@ -187,8 +200,12 @@ bool EglDisplay::removeSurface(SurfacePtr s) { bool EglDisplay::removeContext(EGLContext ctx) { android::Mutex::Autolock mutex(m_lock); - - ContextsHndlMap::iterator it = m_contexts.find(reinterpret_cast(ctx)); + /* ctx is "key" in map. + In 64-bit the upper 32-bit should be all zero. Assert for that. */ + uintptr_t hndlptr = (uintptr_t)ctx; + unsigned int hndl = (unsigned int)hndlptr; + assert(sizeof(hndl) == sizeof(hndlptr) || hndl == hndlptr); + ContextsHndlMap::iterator it = m_contexts.find(hndl); if(it != m_contexts.end()) { m_contexts.erase(it); return true; @@ -254,7 +271,7 @@ int EglDisplay::doChooseConfigs(const EglConfig& dummy,EGLConfig* configs,int co } EGLSurface EglDisplay::addSurface(SurfacePtr s ) { - android::Mutex::Autolock mutex(m_lock); + android::Mutex::Autolock mutex(m_lock); unsigned int hndl = s.Ptr()->getHndl(); EGLSurface ret =reinterpret_cast (hndl); @@ -290,13 +307,23 @@ EGLImageKHR EglDisplay::addImageKHR(ImagePtr img) { ImagePtr EglDisplay::getImage(EGLImageKHR img) { android::Mutex::Autolock mutex(m_lock); - ImagesHndlMap::iterator i( m_eglImages.find((unsigned int)img) ); + /* img is "key" in map. + In 64-bit the upper 32-bit should be all zero. Assert for that. */ + uintptr_t hndlptr = (uintptr_t)img; + unsigned int hndl = (unsigned int)hndlptr; + assert(sizeof(hndl) == sizeof(hndlptr) || hndl == hndlptr); + ImagesHndlMap::iterator i( m_eglImages.find(hndl) ); return (i != m_eglImages.end()) ? (*i).second :ImagePtr(NULL); } bool EglDisplay:: destroyImageKHR(EGLImageKHR img) { android::Mutex::Autolock mutex(m_lock); - ImagesHndlMap::iterator i( m_eglImages.find((unsigned int)img) ); + /* img is "key" in map. + In 64-bit the upper 32-bit should be all zero. Assert for that. */ + uintptr_t hndlptr = (uintptr_t)img; + unsigned int hndl = (unsigned int)hndlptr; + assert(sizeof(hndl) == sizeof(hndlptr) || hndl == hndlptr); + ImagesHndlMap::iterator i( m_eglImages.find(hndl) ); if (i != m_eglImages.end()) { m_eglImages.erase(i); diff --git a/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp b/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp index 92cf065f0..85d1b282a 100644 --- a/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp +++ b/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp @@ -1041,13 +1041,13 @@ EGLImageKHR eglCreateImageKHR(EGLDisplay display, EGLContext context, EGLenum ta ThreadInfo* thread = getThreadInfo(); ShareGroupPtr sg = thread->shareGroup; if (sg.Ptr() != NULL) { - unsigned int globalTexName = sg->getGlobalName(TEXTURE, (unsigned int)buffer); + unsigned int globalTexName = sg->getGlobalName(TEXTURE, (uintptr_t)buffer); if (!globalTexName) return EGL_NO_IMAGE_KHR; ImagePtr img( new EglImage() ); if (img.Ptr() != NULL) { - ObjectDataPtr objData = sg->getObjectData(TEXTURE, (unsigned int)buffer); + ObjectDataPtr objData = sg->getObjectData(TEXTURE, (uintptr_t)buffer); if (!objData.Ptr()) return EGL_NO_IMAGE_KHR; TextureData *texData = (TextureData *)objData.Ptr(); diff --git a/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp b/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp index 8ba7234f6..f1d380165 100644 --- a/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp +++ b/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp @@ -35,6 +35,7 @@ #include #include #include +#include extern "C" { @@ -586,7 +587,7 @@ GL_API void GL_APIENTRY glDrawElements( GLenum mode, GLsizei count, GLenum type GLESConversionArrays tmpArrs; if(ctx->isBindedBuffer(GL_ELEMENT_ARRAY_BUFFER)) { // if vbo is binded take the indices from the vbo const unsigned char* buf = static_cast(ctx->getBindedBuffer(GL_ELEMENT_ARRAY_BUFFER)); - indices = buf+reinterpret_cast(elementsIndices); + indices = buf+reinterpret_cast(elementsIndices); } ctx->setupArraysPointers(tmpArrs,0,count,type,indices,false); @@ -1656,7 +1657,10 @@ GL_API void GL_APIENTRY glEGLImageTargetTexture2DOES(GLenum target, GLeglImageOE { GET_CTX(); SET_ERROR_IF(!GLEScmValidate::textureTargetLimited(target),GL_INVALID_ENUM); - EglImage *img = s_eglIface->eglAttachEGLImage((unsigned int)image); + uintptr_t imagehndlptr = (uintptr_t)image; + unsigned int imagehndl = (unsigned int)imagehndlptr; + assert(sizeof(imagehndl) == sizeof(imagehndlptr) || imagehndl == imagehndlptr); + EglImage *img = s_eglIface->eglAttachEGLImage(imagehndl); if (img) { // Create the texture object in the underlying EGL implementation, // flag to the OpenGL layer to skip the image creation and map the @@ -1680,7 +1684,7 @@ GL_API void GL_APIENTRY glEGLImageTargetTexture2DOES(GLenum target, GLeglImageOE texData->height = img->height; texData->border = img->border; texData->internalFormat = img->internalFormat; - texData->sourceEGLImage = (unsigned int)image; + texData->sourceEGLImage = imagehndl; texData->eglImageDetach = s_eglIface->eglDetachEGLImage; texData->oldGlobal = oldGlobal; } @@ -1691,7 +1695,10 @@ GL_API void GL_APIENTRY glEGLImageTargetRenderbufferStorageOES(GLenum target, GL { GET_CTX(); SET_ERROR_IF(target != GL_RENDERBUFFER_OES,GL_INVALID_ENUM); - EglImage *img = s_eglIface->eglAttachEGLImage((unsigned int)image); + uintptr_t imagehndlptr = (uintptr_t)image; + unsigned int imagehndl = (unsigned int)imagehndlptr; + assert(sizeof(imagehndl) == sizeof(imagehndlptr) || imagehndl == imagehndlptr); + EglImage *img = s_eglIface->eglAttachEGLImage(imagehndl); SET_ERROR_IF(!img,GL_INVALID_VALUE); SET_ERROR_IF(!ctx->shareGroup().Ptr(),GL_INVALID_OPERATION); @@ -1706,7 +1713,7 @@ GL_API void GL_APIENTRY glEGLImageTargetRenderbufferStorageOES(GLenum target, GL // // flag in the renderbufferData that it is an eglImage target // - rbData->sourceEGLImage = (unsigned int)image; + rbData->sourceEGLImage = imagehndl; rbData->eglImageDetach = s_eglIface->eglDetachEGLImage; rbData->eglImageGlobalTexName = img->globalTexName; diff --git a/tools/emulator/opengl/host/libs/Translator/GLES_V2/GLESv2Imp.cpp b/tools/emulator/opengl/host/libs/Translator/GLES_V2/GLESv2Imp.cpp index f5477823b..319930e92 100644 --- a/tools/emulator/opengl/host/libs/Translator/GLES_V2/GLESv2Imp.cpp +++ b/tools/emulator/opengl/host/libs/Translator/GLES_V2/GLESv2Imp.cpp @@ -32,6 +32,7 @@ #include "ProgramData.h" #include #include +#include extern "C" { @@ -593,7 +594,7 @@ GL_APICALL void GL_APIENTRY glDrawElements(GLenum mode, GLsizei count, GLenum t const GLvoid* indices = elementsIndices; if(ctx->isBindedBuffer(GL_ELEMENT_ARRAY_BUFFER)) { // if vbo is binded take the indices from the vbo const unsigned char* buf = static_cast(ctx->getBindedBuffer(GL_ELEMENT_ARRAY_BUFFER)); - indices = buf+reinterpret_cast(elementsIndices); + indices = buf+reinterpret_cast(elementsIndices); } GLESConversionArrays tmpArrs; @@ -2009,7 +2010,10 @@ GL_APICALL void GL_APIENTRY glEGLImageTargetTexture2DOES(GLenum target, GLeglIma { GET_CTX(); SET_ERROR_IF(!GLESv2Validate::textureTargetLimited(target),GL_INVALID_ENUM); - EglImage *img = s_eglIface->eglAttachEGLImage((unsigned int)image); + uintptr_t imagehndlptr = (uintptr_t)image; + unsigned int imagehndl = (unsigned int)imagehndlptr; + assert(sizeof(imagehndl) == sizeof(imagehndlptr) || imagehndl == imagehndlptr); + EglImage *img = s_eglIface->eglAttachEGLImage(imagehndl); if (img) { // Create the texture object in the underlying EGL implementation, // flag to the OpenGL layer to skip the image creation and map the @@ -2033,7 +2037,7 @@ GL_APICALL void GL_APIENTRY glEGLImageTargetTexture2DOES(GLenum target, GLeglIma texData->height = img->height; texData->border = img->border; texData->internalFormat = img->internalFormat; - texData->sourceEGLImage = (unsigned int)image; + texData->sourceEGLImage = imagehndl; texData->eglImageDetach = s_eglIface->eglDetachEGLImage; texData->oldGlobal = oldGlobal; } @@ -2044,7 +2048,10 @@ GL_APICALL void GL_APIENTRY glEGLImageTargetRenderbufferStorageOES(GLenum target { GET_CTX(); SET_ERROR_IF(target != GL_RENDERBUFFER_OES,GL_INVALID_ENUM); - EglImage *img = s_eglIface->eglAttachEGLImage((unsigned int)image); + uintptr_t imagehndlptr = (uintptr_t)image; + unsigned int imagehndl = (unsigned int)imagehndlptr; + assert(sizeof(imagehndl) == sizeof(imagehndlptr) || imagehndl == imagehndlptr); + EglImage *img = s_eglIface->eglAttachEGLImage(imagehndl); SET_ERROR_IF(!img,GL_INVALID_VALUE); SET_ERROR_IF(!ctx->shareGroup().Ptr(),GL_INVALID_OPERATION); @@ -2059,7 +2066,7 @@ GL_APICALL void GL_APIENTRY glEGLImageTargetRenderbufferStorageOES(GLenum target // // flag in the renderbufferData that it is an eglImage target // - rbData->sourceEGLImage = (unsigned int)image; + rbData->sourceEGLImage = imagehndl; rbData->eglImageDetach = s_eglIface->eglDetachEGLImage; rbData->eglImageGlobalTexName = img->globalTexName; diff --git a/tools/emulator/opengl/host/libs/Translator/GLcommon/GLEScontext.cpp b/tools/emulator/opengl/host/libs/Translator/GLcommon/GLEScontext.cpp index 22b4c8b06..90088f3f1 100644 --- a/tools/emulator/opengl/host/libs/Translator/GLcommon/GLEScontext.cpp +++ b/tools/emulator/opengl/host/libs/Translator/GLcommon/GLEScontext.cpp @@ -7,6 +7,7 @@ #include #include #include +#include //decleration static void convertFixedDirectLoop(const char* dataIn,unsigned int strideIn,void* dataOut,unsigned int nBytes,unsigned int strideOut,int attribSize); @@ -172,7 +173,9 @@ GLEScontext::~GLEScontext() { const GLvoid* GLEScontext::setPointer(GLenum arrType,GLint size,GLenum type,GLsizei stride,const GLvoid* data,bool normalize) { GLuint bufferName = m_arrayBuffer; if(bufferName) { - unsigned int offset = reinterpret_cast(data); + uintptr_t offsetptr = (uintptr_t)data; + unsigned int offset = offsetptr; + assert(sizeof(offset) == sizeof(offsetptr) || offset == offsetptr); GLESbuffer* vbo = static_cast(m_shareGroup->getObjectData(VERTEXBUFFER,bufferName).Ptr()); m_map[arrType]->setBuffer(size,type,stride,vbo,bufferName,offset,normalize); return static_cast(vbo->getData()) + offset; diff --git a/tools/emulator/opengl/host/libs/libOpenglRender/ReadBuffer.cpp b/tools/emulator/opengl/host/libs/libOpenglRender/ReadBuffer.cpp index 0949c465d..837b0943b 100644 --- a/tools/emulator/opengl/host/libs/libOpenglRender/ReadBuffer.cpp +++ b/tools/emulator/opengl/host/libs/libOpenglRender/ReadBuffer.cpp @@ -50,7 +50,7 @@ int ReadBuffer::getData() new_buf = (unsigned char*)realloc(m_buf, new_size); if (!new_buf) { - ERR("Failed to alloc %d bytes for ReadBuffer\n", new_size); + ERR("Failed to alloc %zu bytes for ReadBuffer\n", new_size); return -1; } m_size = new_size; diff --git a/tools/emulator/opengl/host/tools/emugen/ApiGen.cpp b/tools/emulator/opengl/host/tools/emugen/ApiGen.cpp index 7c83364ef..bf2d2445d 100644 --- a/tools/emulator/opengl/host/tools/emugen/ApiGen.cpp +++ b/tools/emulator/opengl/host/tools/emugen/ApiGen.cpp @@ -520,7 +520,7 @@ int ApiGen::genEncoderImpl(const std::string &filename) npointers += writeVarEncodingSize(evars[j], fp); } if (npointers > 0) { - fprintf(fp, " + %u*4", npointers); + fprintf(fp, " + %zu*4", npointers); } fprintf(fp, ";\n"); @@ -562,7 +562,7 @@ int ApiGen::genEncoderImpl(const std::string &filename) npointers += writeVarEncodingSize(evars[j], fp); } if (npointers > 0) { - fprintf(fp, "%s%u*4", plus, npointers); plus = " + "; + fprintf(fp, "%s%zu*4", plus, npointers); plus = " + "; } } fprintf(fp,");\n"); @@ -761,6 +761,7 @@ int ApiGen::genDecoderImpl(const std::string &filename) fprintf(fp, "#include \"%s_opcodes.h\"\n\n", m_basename.c_str()); fprintf(fp, "#include \"%s_dec.h\"\n\n\n", m_basename.c_str()); fprintf(fp, "#include \n\n"); + fprintf(fp, "typedef unsigned int tsize_t; // Target \"size_t\", which is 32-bit for now. It may or may not be the same as host's size_t when emugen is compiled.\n\n"); // decoder switch; fprintf(fp, "size_t %s::decode(void *buf, size_t len, IOStream *stream)\n{\n", classname.c_str()); @@ -860,7 +861,7 @@ int ApiGen::genDecoderImpl(const std::string &filename) v->type()->name().c_str(), varoffset.c_str(), varoffset.c_str()); } - varoffset += " + 4 + *(size_t *)(ptr +" + varoffset + ")"; + varoffset += " + 4 + *(tsize_t *)(ptr +" + varoffset + ")"; } else { // out pointer; if (pass == PASS_TmpBuffAlloc) { fprintf(fp, "\t\t\tsize_t tmpPtr%uSize = (size_t)*(unsigned int *)(ptr + %s);\n", diff --git a/tools/emulator/opengl/shared/OpenglCodecCommon/SocketStream.cpp b/tools/emulator/opengl/shared/OpenglCodecCommon/SocketStream.cpp index ddc56d0e3..f7a2314f9 100644 --- a/tools/emulator/opengl/shared/OpenglCodecCommon/SocketStream.cpp +++ b/tools/emulator/opengl/shared/OpenglCodecCommon/SocketStream.cpp @@ -73,7 +73,7 @@ void *SocketStream::allocBuffer(size_t minSize) m_buf = p; m_bufsize = allocSize; } else { - ERR("%s: realloc (%d) failed\n", __FUNCTION__, allocSize); + ERR("%s: realloc (%zu) failed\n", __FUNCTION__, allocSize); free(m_buf); m_buf = NULL; m_bufsize = 0; diff --git a/tools/emulator/opengl/shared/OpenglOsUtils/osThread.h b/tools/emulator/opengl/shared/OpenglOsUtils/osThread.h index 970396dc3..d47342a3c 100644 --- a/tools/emulator/opengl/shared/OpenglOsUtils/osThread.h +++ b/tools/emulator/opengl/shared/OpenglOsUtils/osThread.h @@ -20,6 +20,7 @@ #include #else // !WIN32 #include +#include #endif namespace osUtils { diff --git a/tools/emulator/opengl/shared/OpenglOsUtils/osThreadUnix.cpp b/tools/emulator/opengl/shared/OpenglOsUtils/osThreadUnix.cpp index 66611eee6..0fa7815e7 100644 --- a/tools/emulator/opengl/shared/OpenglOsUtils/osThreadUnix.cpp +++ b/tools/emulator/opengl/shared/OpenglOsUtils/osThreadUnix.cpp @@ -84,7 +84,7 @@ Thread::thread_main(void *p_arg) pthread_mutex_lock(&self->m_lock); self->m_isRunning = false; - self->m_exitStatus = (int)ret; + self->m_exitStatus = (int)(intptr_t)ret; pthread_mutex_unlock(&self->m_lock); return ret;