From b2306edb48e2a63b37a77524bdf8c5262b1783ed Mon Sep 17 00:00:00 2001 From: Cody Northrop Date: Tue, 5 Nov 2019 09:04:33 -0700 Subject: EGL: Fix repeated extension lookups When adding support for GLES layers, we refactored eglGetProcAddress to allow layers to intercept functions unknown to the Loader. During the cleanup, we broke slot tracking if an extension were looked up multiple times. Follow up queries would use an incremented slot that had not been initialized: eglGetProcAddress("foo1") = fptr0:0x77e2c2abd8 eglGetProcAddress("foo2") = fptr1:0x77e2c2abf4 eglGetProcAddress("foo3") = fptr2:0x77e2c2ac10 eglGetProcAddress("foo1") = fptr3:0x77e2c2ac2c <= wrong eglGetProcAddress("foo2") = fptr4:0x77e2c2ac2c <= repeated eglGetProcAddress("foo3") = fptr5:0x77e2c2ac2c <= repeated This CL tracks which slot was used for a given extension, and when it is queried a second time, we look up the correct slot using extensionSlotMap. eglGetProcAddress("foo1") = fptr0:0x77e2c2abd8 eglGetProcAddress("foo2") = fptr1:0x77e2c2abf4 eglGetProcAddress("foo3") = fptr2:0x77e2c2ac10 eglGetProcAddress("foo1") = fptr3:0x77e2c2abd8 <= correct eglGetProcAddress("foo2") = fptr4:0x77e2c2abf4 <= correct eglGetProcAddress("foo3") = fptr5:0x77e2c2ac10 <= correct It also refactors the code a bit to be more readable, and fixes a typo throughout the file. Bug: 143860847 Test: egl_api_test Change-Id: If75c1abdb69a4d82253f28a5a80687f546e33ee0 Merged-In: If75c1abdb69a4d82253f28a5a80687f546e33ee0 --- opengl/libs/EGL/egl_platform_entries.cpp | 91 +++++++++++++++++++------------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/opengl/libs/EGL/egl_platform_entries.cpp b/opengl/libs/EGL/egl_platform_entries.cpp index e996be6853..a3bb6debe9 100644 --- a/opengl/libs/EGL/egl_platform_entries.cpp +++ b/opengl/libs/EGL/egl_platform_entries.cpp @@ -61,7 +61,7 @@ namespace android { using nsecs_t = int64_t; -struct extention_map_t { +struct extension_map_t { const char* name; __eglMustCastToProperFunctionPointerType address; }; @@ -154,7 +154,7 @@ char const * const gClientExtensionString = * (keep in sync with gExtensionString above) * */ -static const extention_map_t sExtensionMap[] = { +static const extension_map_t sExtensionMap[] = { // EGL_KHR_lock_surface { "eglLockSurfaceKHR", (__eglMustCastToProperFunctionPointerType)&eglLockSurfaceKHR }, @@ -257,13 +257,14 @@ static const extention_map_t sExtensionMap[] = { !strcmp((procname), "eglAwakenProcessIMG")) // accesses protected by sExtensionMapMutex -static std::unordered_map sGLExtentionMap; +static std::unordered_map sGLExtensionMap; +static std::unordered_map sGLExtensionSlotMap; -static int sGLExtentionSlot = 0; +static int sGLExtensionSlot = 0; static pthread_mutex_t sExtensionMapMutex = PTHREAD_MUTEX_INITIALIZER; static void(*findProcAddress(const char* name, - const extention_map_t* map, size_t n))() { + const extension_map_t* map, size_t n))() { for (uint32_t i=0 ; isecond : nullptr; - const int slot = sGLExtentionSlot; + // See if we've already looked up this extension + auto pos = extensionMap.find(name); + addr = (pos != extensionMap.end()) ? pos->second : nullptr; - ALOGE_IF(slot >= MAX_NUMBER_OF_GL_EXTENSIONS, - "no more slots for eglGetProcAddress(\"%s\")", - procname); + if (!addr) { + // This is the first time we've looked this function up + // Ensure we have room to track it + const int slot = sGLExtensionSlot; + if (slot < MAX_NUMBER_OF_GL_EXTENSIONS) { - egl_connection_t* const cnx = &gEGLImpl; - LayerLoader& layer_loader(LayerLoader::getInstance()); + if (cnx->dso && cnx->egl.eglGetProcAddress) { - if (!addr && (slot < MAX_NUMBER_OF_GL_EXTENSIONS)) { + // Extensions are independent of the bound context + addr = cnx->egl.eglGetProcAddress(procname); + if (addr) { - if (cnx->dso && cnx->egl.eglGetProcAddress) { + // purposefully track the bottom of the stack in extensionMap + extensionMap[name] = addr; - // Extensions are independent of the bound context - addr = cnx->egl.eglGetProcAddress(procname); - if (addr) { + // Apply layers + addr = layer_loader.ApplyLayers(procname, addr); - // purposefully track the bottom of the stack in extensionMap - extentionMap[name] = addr; + // Track the top most entry point return the extension forwarder + cnx->hooks[egl_connection_t::GLESv1_INDEX]->ext.extensions[slot] = + cnx->hooks[egl_connection_t::GLESv2_INDEX]->ext.extensions[slot] = addr; + addr = gExtensionForwarders[slot]; - // Apply layers - addr = layer_loader.ApplyLayers(procname, addr); + // Remember the slot for this extension + extensionSlotMap[name] = slot; - // Track the top most entry point - cnx->hooks[egl_connection_t::GLESv1_INDEX]->ext.extensions[slot] = - cnx->hooks[egl_connection_t::GLESv2_INDEX]->ext.extensions[slot] = addr; - addr = gExtensionForwarders[slot]; - sGLExtentionSlot++; + // Increment the global extension index + sGLExtensionSlot++; + } } + } else { + // The extension forwarder has a fixed number of slots + ALOGE("no more slots for eglGetProcAddress(\"%s\")", procname); } - } else if (slot < MAX_NUMBER_OF_GL_EXTENSIONS) { + } else { + // We tracked an address, so we've seen this func before + // Look up the slot for this extension + auto slot_pos = extensionSlotMap.find(name); + int ext_slot = (slot_pos != extensionSlotMap.end()) ? slot_pos->second : -1; + if (ext_slot < 0) { + // Something has gone wrong, this should not happen + ALOGE("No extension slot found for %s", procname); + return nullptr; + } - // We've seen this func before, but we tracked the bottom, so re-apply layers - // More layers might have been enabled + // We tracked the bottom of the stack, so re-apply layers since + // more layers might have been enabled addr = layer_loader.ApplyLayers(procname, addr); - // Track the top most entry point - cnx->hooks[egl_connection_t::GLESv1_INDEX]->ext.extensions[slot] = - cnx->hooks[egl_connection_t::GLESv2_INDEX]->ext.extensions[slot] = addr; - addr = gExtensionForwarders[slot]; + // Track the top most entry point and return the extension forwarder + cnx->hooks[egl_connection_t::GLESv1_INDEX]->ext.extensions[ext_slot] = + cnx->hooks[egl_connection_t::GLESv2_INDEX]->ext.extensions[ext_slot] = addr; + addr = gExtensionForwarders[ext_slot]; } pthread_mutex_unlock(&sExtensionMapMutex); -- cgit v1.2.3-59-g8ed1b