Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 26cecff3 authored by Jesse Hall's avatar Jesse Hall
Browse files

libvulkan: Fix dEQP-VK.api.object_management.alloc_callback_fail.instance 

The loader was crashing when a std::vector::resize() operation called
the test-provided allocator, which returned failure, and then the
vector blindly started writing into the returned pointer.

Obvious in hindsight, but stdlib containers+strings + user-provided
allocation funcs implies that the loader must be built with exceptions
enabled, and must be exception-safe at least where it uses
containers/strings. We were doing neither.

This change has the minimally invasive fix, which is to (a) throw an
exception from the stdlib Allocator when the app-provided allocation
function fails, and (b) wrap every stdlib operation that might
allocate in a try..catch and turn it into a
VK_ERROR_OUT_OF_HOST_MEMORY error.

This is pretty unsatisfying and I'm not happy with the resulting
mismash of error-handling styles, with having exceptions at all in
code that was not written to be exception-safe, or with the
fine-grained try..catch. We need to decide whether to keep using parts
of stdlib that can allocate, and rewrite a lot of code to be
exception-friendly, or we need to replace the stdlib code with manual
containers and strings. Bug 26732452 filed.

Change-Id: I6f096f25a43a0e3c5f56796c2af19f114d2edac6
(cherry picked from commit ccca46db073dfadc81a68ac1533d8859ed3e109a)
parent 4b62e4fd

vulkan/libvulkan/Android.mk

+2 −1
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ LOCAL_CFLAGS := -DLOG_TAG=\"vulkan\" \ 
	-Wno-undef

#LOCAL_CFLAGS += -DLOG_NDEBUG=0

LOCAL_CPPFLAGS := -std=c++14 \

	-fexceptions \

	-Wno-c++98-compat-pedantic \

	-Wno-exit-time-destructors \

	-Wno-c99-extensions \
@@ -42,7 +43,7 @@ LOCAL_SRC_FILES := \ 
	vulkan_loader_data.cpp

LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk



LOCAL_SHARED_LIBRARIES := libhardware liblog libsync libcutils

LOCAL_SHARED_LIBRARIES := libhardware liblog libsync libutils libcutils



LOCAL_MODULE := libvulkan

include $(BUILD_SHARED_LIBRARY)

vulkan/libvulkan/loader.cpp

+92 −49
Original line number Diff line number Diff line
@@ -14,8 +14,6 @@ 
 * limitations under the License.

 */



// #define LOG_NDEBUG 0



// module header

#include "loader.h"

// standard C headers
@@ -39,6 +37,22 @@ 
#include <log/log.h>

#include <vulkan/vulkan_loader_data.h>



// #define ENABLE_ALLOC_CALLSTACKS 1

#if ENABLE_ALLOC_CALLSTACKS

#include <utils/CallStack.h>

#define ALOGD_CALLSTACK(...)                             \

    do {                                                 \

        ALOGD(__VA_ARGS__);                              \

        android::CallStack callstack;                    \

        callstack.update();                              \

        callstack.log(LOG_TAG, ANDROID_LOG_DEBUG, "  "); \

    } while (false)

#else

#define ALOGD_CALLSTACK(...) \

    do {                     \

    } while (false)

#endif



using namespace vulkan;



static const uint32_t kMaxPhysicalDevices = 4;
@@ -80,10 +94,12 @@ class CallbackAllocator { 
        void* mem =

            alloc->pfnAllocation(alloc->pUserData, n * sizeof(T), alignof(T),

                                 VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);

        if (!mem)

            throw std::bad_alloc();

        return static_cast<T*>(mem);

    }



    void deallocate(T* array, std::size_t /*n*/) {

    void deallocate(T* array, std::size_t /*n*/) noexcept {

        alloc->pfnFree(alloc->pUserData, array);

    }
@@ -101,17 +117,6 @@ bool operator!=(const CallbackAllocator<T>& alloc1, 
    return !(alloc1 == alloc2);

}



template <class Key,

          class T,

          class Hash = std::hash<Key>,

          class Pred = std::equal_to<Key>>

using UnorderedMap =

    std::unordered_map<Key,

                       T,

                       Hash,

                       Pred,

                       CallbackAllocator<std::pair<const Key, T>>>;



template <class T>

using Vector = std::vector<T, CallbackAllocator<T>>;
@@ -127,9 +132,10 @@ VKAPI_ATTR void* DefaultAllocate(void*, 
    void* ptr = nullptr;

    // Vulkan requires 'alignment' to be a power of two, but posix_memalign

    // additionally requires that it be at least sizeof(void*).

    return posix_memalign(&ptr, std::max(alignment, sizeof(void*)), size) == 0

               ? ptr

               : nullptr;

    int ret = posix_memalign(&ptr, std::max(alignment, sizeof(void*)), size);

    ALOGD_CALLSTACK("Allocate: size=%zu align=%zu => (%d) %p", size, alignment,

                    ret, ptr);

    return ret == 0 ? ptr : nullptr;

}



VKAPI_ATTR void* DefaultReallocate(void*,
@@ -162,8 +168,9 @@ VKAPI_ATTR void* DefaultReallocate(void*, 
    return new_ptr;

}



VKAPI_ATTR void DefaultFree(void*, void* pMem) {

    free(pMem);

VKAPI_ATTR void DefaultFree(void*, void* ptr) {

    ALOGD_CALLSTACK("Free: %p", ptr);

    free(ptr);

}



const VkAllocationCallbacks kDefaultAllocCallbacks = {
@@ -360,8 +367,17 @@ bool ActivateLayer(TObject* object, const char* name) { 
    if (!layer)

        return false;

    if (std::find(object->active_layers.begin(), object->active_layers.end(),

                  layer) == object->active_layers.end())

                  layer) == object->active_layers.end()) {

        try {

            object->active_layers.push_back(std::move(layer));

        } catch (std::bad_alloc&) {

            // TODO(jessehall): We should fail with VK_ERROR_OUT_OF_MEMORY

            // if we can't enable a requested layer. Callers currently ignore

            // ActivateLayer's return value.

            ALOGW("failed to activate layer '%s': out of memory", name);

            return false;

        }

    }

    ALOGV("activated layer '%s'", name);

    return true;

}
@@ -374,6 +390,7 @@ struct InstanceNamesPair { 
void SetLayerNamesFromProperty(const char* name,

                               const char* value,

                               void* data) {

    try {

        const char prefix[] = "debug.vulkan.layer.";

        const size_t prefixlen = sizeof(prefix) - 1;

        if (value[0] == '\0' || strncmp(name, prefix, prefixlen) != 0)
@@ -381,8 +398,8 @@ void SetLayerNamesFromProperty(const char* name, 
        const char* number_str = name + prefixlen;

        long layer_number = strtol(number_str, nullptr, 10);

        if (layer_number <= 0 || layer_number == LONG_MAX) {

        ALOGW("Cannot use a layer at number %ld from string %s", layer_number,

              number_str);

            ALOGW("Cannot use a layer at number %ld from string %s",

                  layer_number, number_str);

            return;

        }

        auto instance_names_pair = static_cast<InstanceNamesPair*>(data);
@@ -390,10 +407,15 @@ void SetLayerNamesFromProperty(const char* name, 
        Instance* instance = instance_names_pair->instance;

        size_t layer_size = static_cast<size_t>(layer_number);

        if (layer_size > layer_names->size()) {

        layer_names->resize(layer_size,

                            String(CallbackAllocator<char>(instance->alloc)));

            layer_names->resize(

                layer_size, String(CallbackAllocator<char>(instance->alloc)));

        }

        (*layer_names)[layer_size - 1] = value;

    } catch (std::bad_alloc&) {

        ALOGW("failed to handle property '%s'='%s': out of memory", name,

              value);

        return;

    }

}



template <class TInfo, class TObject>
@@ -408,13 +430,11 @@ VkResult ActivateAllLayers(TInfo create_info, 
    if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) {

        char layer_prop[PROPERTY_VALUE_MAX];

        property_get("debug.vulkan.layers", layer_prop, "");

        String layer_name(string_allocator);

        String layer_prop_str(layer_prop, string_allocator);

        size_t end, start = 0;

        while ((end = layer_prop_str.find(':', start)) != std::string::npos) {

            layer_name = layer_prop_str.substr(start, end - start);

            ActivateLayer(object, layer_name.c_str());

            start = end + 1;

        char* strtok_state;

        char* layer_name = nullptr;

        while ((layer_name = strtok_r(layer_name ? nullptr : layer_prop, ":",

                                      &strtok_state))) {

            ActivateLayer(object, layer_name);

        }

        Vector<String> layer_names(CallbackAllocator<String>(instance->alloc));

        InstanceNamesPair instance_names_pair = {.instance = instance,
@@ -637,7 +657,13 @@ VkResult CreateInstance_Bottom(const VkInstanceCreateInfo* create_info, 
                  result);

            continue;

        }

        try {

            extensions.resize(count);

        } catch (std::bad_alloc&) {

            ALOGE("instance creation failed: out of memory");

            DestroyInstance_Bottom(instance.handle, allocator);

            return VK_ERROR_OUT_OF_HOST_MEMORY;

        }

        if ((result = instance.drv.dispatch.EnumerateDeviceExtensionProperties(

                 instance.physical_devices[i], nullptr, &count,

                 extensions.data())) != VK_SUCCESS) {
@@ -909,8 +935,19 @@ VkResult CreateDevice_Bottom(VkPhysicalDevice gpu, 
    VkLayerLinkedListElem* next_element;

    PFN_vkGetDeviceProcAddr next_get_proc_addr = GetDeviceProcAddr_Bottom;

    Vector<VkLayerLinkedListElem> elem_list(

        device->active_layers.size(),

        CallbackAllocator<VkLayerLinkedListElem>(instance.alloc));

    try {

        elem_list.resize(device->active_layers.size());

    } catch (std::bad_alloc&) {

        ALOGE("device creation failed: out of memory");

        PFN_vkDestroyDevice destroy_device =

            reinterpret_cast<PFN_vkDestroyDevice>(

                instance.drv.dispatch.GetDeviceProcAddr(drv_device,

                                                        "vkDestroyDevice"));

        destroy_device(drv_device, allocator);

        DestroyDevice(device);

        return VK_ERROR_OUT_OF_HOST_MEMORY;

    }



    for (size_t i = elem_list.size(); i > 0; i--) {

        size_t idx = i - 1;
@@ -1090,8 +1127,14 @@ VkResult CreateInstance_Top(const VkInstanceCreateInfo* create_info, 
    VkLayerLinkedListElem* next_element;

    PFN_vkGetInstanceProcAddr next_get_proc_addr = GetInstanceProcAddr_Bottom;

    Vector<VkLayerLinkedListElem> elem_list(

        instance->active_layers.size(),

        CallbackAllocator<VkLayerLinkedListElem>(instance->alloc));

    try {

        elem_list.resize(instance->active_layers.size());

    } catch (std::bad_alloc&) {

        ALOGE("instance creation failed: out of memory");

        DestroyInstance_Bottom(instance->handle, allocator);

        return VK_ERROR_OUT_OF_HOST_MEMORY;

    }



    for (size_t i = elem_list.size(); i > 0; i--) {

        size_t idx = i - 1;

vulkan/libvulkan/swapchain.cpp

+27 −6
Original line number Diff line number Diff line
@@ -77,10 +77,13 @@ class VulkanAllocator { 
        : allocator_(other.allocator_), scope_(other.scope_) {}



    T* allocate(size_t n) const {

        return static_cast<T*>(allocator_.pfnAllocation(

        T* p = static_cast<T*>(allocator_.pfnAllocation(

            allocator_.pUserData, n * sizeof(T), alignof(T), scope_));

        if (!p)

            throw std::bad_alloc();

        return p;

    }

    void deallocate(T* p, size_t) const {

    void deallocate(T* p, size_t) const noexcept {

        return allocator_.pfnFree(allocator_.pUserData, p);

    }
@@ -93,10 +96,15 @@ class VulkanAllocator { 


template <typename T, typename Host>

std::shared_ptr<T> InitSharedPtr(Host host, T* obj) {

    try {

        obj->common.incRef(&obj->common);

        return std::shared_ptr<T>(

            obj, NativeBaseDeleter<T>(),

            VulkanAllocator<T>(*GetAllocator(host), AllocScope<Host>::kScope));

    } catch (std::bad_alloc&) {

        obj->common.decRef(&obj->common);

        return nullptr;

    }

}



// ----------------------------------------------------------------------------
@@ -161,6 +169,12 @@ VkResult CreateAndroidSurfaceKHR_Bottom( 
    Surface* surface = new (mem) Surface;



    surface->window = InitSharedPtr(instance, pCreateInfo->window);

    if (!surface->window) {

        ALOGE("surface creation failed: out of memory");

        surface->~Surface();

        allocator->pfnFree(allocator->pUserData, surface);

        return VK_ERROR_OUT_OF_HOST_MEMORY;

    }



    // TODO(jessehall): Create and use NATIVE_WINDOW_API_VULKAN.

    int err =
@@ -468,6 +482,13 @@ VkResult CreateSwapchainKHR_Bottom(VkDevice device, 
            break;

        }

        img.buffer = InitSharedPtr(device, buffer);

        if (!img.buffer) {

            ALOGE("swapchain creation failed: out of memory");

            surface.window->cancelBuffer(surface.window.get(), buffer,

                                         img.dequeue_fence);

            result = VK_ERROR_OUT_OF_HOST_MEMORY;

            break;

        }

        img.dequeued = true;



        image_create.extent =