Fix review findings: harden GPU info, disable by default

- Use RTLD_NOW instead of RTLD_LAZY for immediate symbol validation
- Add dlerror() logging on Unix when Vulkan library fails to load
- Add macOS fallback when MoltenVK portability extensions unavailable
- Guard against UINT64_MAX overflow when summing GPU memory heaps
- Handle string allocation failures in create_gpu_info_from_device
- Log when a GPU device is skipped during enumeration
- Increase context_key buffer from 16 to 32 bytes
- Add ARM (0x13B5) and Samsung (0x144D) vendor IDs
- Improve thread safety documentation with specific lock references
- Document Vulkan API version requirement and driver version encoding
- Disable SENTRY_WITH_GPU_INFO by default (enable with -DSENTRY_WITH_GPU_INFO=ON)
- Update CHANGELOG and README to reflect opt-in behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: mujacica

CHANGELOG.md

@@ -75,7 +75,7 @@
 
 **Features**:
 
-- Implement the GPU Info gathering within the Native SDK ([#1336](https://github.com/getsentry/sentry-native/pull/1336))
+- Implement GPU info context gathering for the Native SDK. Supported on Windows, macOS, and Linux via Vulkan. Disabled by default — enable with `-DSENTRY_WITH_GPU_INFO=ON`. ([#1336](https://github.com/getsentry/sentry-native/pull/1336))
 
 **Fixes**:
 

CMakeLists.txt

@@ -103,22 +103,7 @@ option(SENTRY_PIC "Build sentry (and dependent) libraries as position independen
 
 option(SENTRY_TRANSPORT_COMPRESSION "Enable transport gzip compression" OFF)
 
-# GPU information gathering support - enabled by default on supported platforms only
-set(SENTRY_GPU_INFO_DEFAULT OFF)
-
-# Only enable GPU info on supported platforms
-if(WIN32)
-	set(SENTRY_GPU_INFO_DEFAULT ON)
-elseif(APPLE AND NOT IOS)
-	set(SENTRY_GPU_INFO_DEFAULT ON)
-elseif(LINUX)
-	set(SENTRY_GPU_INFO_DEFAULT ON)
-else()
-	# Disable GPU info on all other platforms (Android, iOS, AIX, etc.)
-	message(STATUS "GPU Info: Platform not supported, GPU information gathering disabled")
-endif()
-
-option(SENTRY_WITH_GPU_INFO "Build with GPU information gathering support" ${SENTRY_GPU_INFO_DEFAULT})
+option(SENTRY_WITH_GPU_INFO "Build with GPU information gathering support (supported on Windows, macOS, Linux)" OFF)
 
 # GPU info enabled - no longer requires Vulkan SDK (uses headers submodule + dynamic linking)
 if(SENTRY_WITH_GPU_INFO)

README.md

@@ -303,12 +303,10 @@ using `cmake -D BUILD_SHARED_LIBS=OFF ..`.
   tuning the thread stack guarantee parameters. Warnings and errors in the process of setting thread stack guarantees
   will always be logged.
 
-- `SENTRY_WITH_GPU_INFO` (Default: `ON` on Windows, macOS, and Linux, otherwise `OFF`):
-  Enables GPU information collection and reporting. When enabled, the SDK will attempt to gather GPU details such as
-  GPU name, vendor, memory size, and driver version, which are included in event contexts. The implementation uses
-  the Vulkan API for cross-platform GPU detection. **Requires the Vulkan SDK to be installed** - if not found,
-  GPU information gathering will be automatically disabled during build. Setting this to `OFF` disables GPU
-  information collection entirely, which can reduce dependencies and binary size.
+- `SENTRY_WITH_GPU_INFO` (Default: `OFF`):
+  Enables GPU information collection and reporting. Supported on Windows, macOS, and Linux. When enabled, the SDK
+  will attempt to gather GPU details such as GPU name, vendor, memory size, and driver version, which are included
+  in event contexts. The implementation uses the Vulkan API with dynamic loading for cross-platform GPU detection.
 
 ### Support Matrix
 

src/gpu/sentry_gpu_common.c

@@ -20,6 +20,10 @@ sentry__gpu_vendor_id_to_name(unsigned int vendor_id)
     case 0x5143:
     case 0x17CB:
         return sentry__string_clone("Qualcomm");
+    case 0x13B5:
+        return sentry__string_clone("ARM");
+    case 0x144D:
+        return sentry__string_clone("Samsung Electronics");
     case 0x1AE0:
         return sentry__string_clone("Google");
     case 0x1010:
@@ -147,7 +151,8 @@ sentry__add_gpu_contexts(sentry_value_t contexts)
         sentry_value_t gpu_context
             = create_gpu_context_from_info(gpu_list->gpus[i]);
         if (!sentry_value_is_null(gpu_context)) {
-            char context_key[16];
+            // "gpu" + up to 10 digits for uint32 + NUL = 14 bytes max
+            char context_key[32];
             if (i == 0) {
                 snprintf(context_key, sizeof(context_key), "gpu");
             } else {

src/gpu/sentry_gpu_vulkan.c

@@ -12,16 +12,10 @@
 #    define SENTRY_LOAD_LIBRARY(name) LoadLibraryA(name)
 #    define SENTRY_GET_PROC_ADDRESS(handle, name) GetProcAddress(handle, name)
 #    define SENTRY_FREE_LIBRARY(handle) FreeLibrary(handle)
-#elif defined(__APPLE__)
-#    include <dlfcn.h>
-#    define SENTRY_LIBRARY_HANDLE void *
-#    define SENTRY_LOAD_LIBRARY(name) dlopen(name, RTLD_LAZY)
-#    define SENTRY_GET_PROC_ADDRESS(handle, name) dlsym(handle, name)
-#    define SENTRY_FREE_LIBRARY(handle) dlclose(handle)
 #else
 #    include <dlfcn.h>
 #    define SENTRY_LIBRARY_HANDLE void *
-#    define SENTRY_LOAD_LIBRARY(name) dlopen(name, RTLD_LAZY)
+#    define SENTRY_LOAD_LIBRARY(name) dlopen(name, RTLD_NOW)
 #    define SENTRY_GET_PROC_ADDRESS(handle, name) dlsym(handle, name)
 #    define SENTRY_FREE_LIBRARY(handle) dlclose(handle)
 #endif
@@ -32,10 +26,10 @@
 #endif
 
 // Dynamic function pointers
-// Note: These are not thread-safe, but this is not a concern for our use case.
-// We are only accessing these during scope initialization, which is explicitly
-// locked, so it's fair to assume that only single-threadded access is happening
-// here.
+// Thread safety: These statics are only accessed from sentry__add_gpu_contexts()
+// which is called during scope initialization in get_scope() (sentry_scope.c).
+// get_scope() is only reachable through sentry__scope_lock() which holds
+// g_lock, so concurrent access cannot happen.
 static PFN_vkCreateInstance pfn_vkCreateInstance = NULL;
 static PFN_vkDestroyInstance pfn_vkDestroyInstance = NULL;
 static PFN_vkEnumeratePhysicalDevices pfn_vkEnumeratePhysicalDevices = NULL;
@@ -57,6 +51,7 @@ load_vulkan_library(void)
 #ifdef _WIN32
     vulkan_library = SENTRY_LOAD_LIBRARY("vulkan-1.dll");
 #elif defined(__APPLE__)
+    // MoltenVK / Vulkan SDK install paths on macOS
     vulkan_library = SENTRY_LOAD_LIBRARY("libvulkan.1.dylib");
     if (!vulkan_library) {
         vulkan_library = SENTRY_LOAD_LIBRARY("libvulkan.dylib");
@@ -73,7 +68,13 @@ load_vulkan_library(void)
 #endif
 
     if (!vulkan_library) {
+#ifndef _WIN32
+        const char *dl_err = dlerror();
+        SENTRY_WARNF("Failed to load Vulkan library: %s",
+            dl_err ? dl_err : "unknown error");
+#else
         SENTRY_WARN("Failed to load Vulkan library");
+#endif
         return false;
     }
 
@@ -128,28 +129,43 @@ create_vulkan_instance(void)
     app_info.applicationVersion = VK_MAKE_VERSION(1, 0, 0);
     app_info.pEngineName = "Sentry";
     app_info.engineVersion = VK_MAKE_VERSION(1, 0, 0);
+    // Vulkan 1.0 is the minimum version we require; all physical device
+    // property and memory queries we use are part of the 1.0 core API.
     app_info.apiVersion = VK_API_VERSION_1_0;
 
     VkInstanceCreateInfo create_info = { 0 };
     create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO;
     create_info.pApplicationInfo = &app_info;
 
+    VkInstance instance = VK_NULL_HANDLE;
+    VkResult result;
+
 #ifdef __APPLE__
-    // Required extensions for MoltenVK on macOS
+    // On macOS, Vulkan is provided through MoltenVK which requires the
+    // portability enumeration extension and flag to expose MoltenVK devices.
+    // We try with extensions first, and fall back to a plain instance if
+    // the extensions are not available (e.g. native Vulkan without MoltenVK).
     const char *extensions[] = { "VK_KHR_portability_enumeration",
         "VK_KHR_get_physical_device_properties2" };
     create_info.enabledExtensionCount = 2;
     create_info.ppEnabledExtensionNames = extensions;
-
-    // Required flag for MoltenVK on macOS
     create_info.flags = VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR;
-
-    // Disable validation layers on macOS as they may not be available
     create_info.enabledLayerCount = 0;
+
+    result = pfn_vkCreateInstance(&create_info, NULL, &instance);
+    if (result == VK_SUCCESS) {
+        return instance;
+    }
+
+    // Fallback: try without portability extensions
+    SENTRY_DEBUG(
+        "MoltenVK extensions not available, retrying without portability");
+    create_info.enabledExtensionCount = 0;
+    create_info.ppEnabledExtensionNames = NULL;
+    create_info.flags = 0;
 #endif
 
-    VkInstance instance = VK_NULL_HANDLE;
-    VkResult result = pfn_vkCreateInstance(&create_info, NULL, &instance);
+    result = pfn_vkCreateInstance(&create_info, NULL, &instance);
     if (result != VK_SUCCESS) {
         SENTRY_DEBUGF("Failed to create Vulkan instance: %d", result);
         return VK_NULL_HANDLE;
@@ -174,11 +190,16 @@ create_gpu_info_from_device(VkPhysicalDevice device)
 
     memset(gpu_info, 0, sizeof(sentry_gpu_info_t));
 
+    // deviceName is a fixed char array in VkPhysicalDeviceProperties,
+    // guaranteed to be null-terminated by the Vulkan spec.
     gpu_info->name = sentry__string_clone(properties.deviceName);
     gpu_info->vendor_id = properties.vendorID;
     gpu_info->device_id = properties.deviceID;
     gpu_info->vendor_name = sentry__gpu_vendor_id_to_name(properties.vendorID);
 
+    // The Vulkan driverVersion encoding is vendor-specific. We use the
+    // standard VK_VERSION macros which work for most drivers; on some
+    // proprietary drivers the output may not match the marketed version.
     char driver_version_str[64];
     uint32_t driver_version = properties.driverVersion;
     snprintf(driver_version_str, sizeof(driver_version_str), "%u.%u.%u",
@@ -187,11 +208,26 @@ create_gpu_info_from_device(VkPhysicalDevice device)
 
     gpu_info->driver_version = sentry__string_clone(driver_version_str);
 
+    // Any NULL string fields (from allocation failure) are handled gracefully
+    // by create_gpu_context_from_info which checks each field before use.
+    if (!gpu_info->name && !gpu_info->vendor_name
+        && !gpu_info->driver_version) {
+        SENTRY_WARN("All string allocations failed for GPU info");
+        sentry_free(gpu_info);
+        return NULL;
+    }
+
     uint64_t total_memory = 0;
     for (uint32_t i = 0; i < memory_properties.memoryHeapCount; i++) {
         if (memory_properties.memoryHeaps[i].flags
             & VK_MEMORY_HEAP_DEVICE_LOCAL_BIT) {
-            total_memory += memory_properties.memoryHeaps[i].size;
+            uint64_t heap_size = memory_properties.memoryHeaps[i].size;
+            // Guard against overflow when summing heap sizes
+            if (total_memory > UINT64_MAX - heap_size) {
+                total_memory = UINT64_MAX;
+                break;
+            }
+            total_memory += heap_size;
         }
     }
 
@@ -265,6 +301,8 @@ sentry__get_gpu_info(void)
         if (gpu_info) {
             gpu_list->gpus[gpu_list->count] = gpu_info;
             gpu_list->count++;
+        } else {
+            SENTRY_DEBUGF("Skipped GPU device %u (info creation failed)", i);
         }
     }
 

tests/unit/test_gpu.c

@@ -81,8 +81,9 @@ SENTRY_TEST(gpu_info_vendor_id_known)
     // Test the common vendor ID to name mapping function with all supported
     // vendors
     unsigned int test_vendor_ids[]
-        = { 0x10DE, 0x1002, 0x8086, 0x106B, 0x1414, 0x5143, 0x1AE0, 0x1010,
-              0x1023, 0x102B, 0x121A, 0x18CA, 0x1039, 0x126F, 0x0000, 0xFFFF };
+        = { 0x10DE, 0x1002, 0x8086, 0x106B, 0x1414, 0x5143, 0x13B5, 0x144D,
+              0x1AE0, 0x1010, 0x1023, 0x102B, 0x121A, 0x18CA, 0x1039, 0x126F,
+              0x0000, 0xFFFF };
 
     for (size_t i = 0; i < sizeof(test_vendor_ids) / sizeof(test_vendor_ids[0]);
         i++) {
@@ -109,6 +110,12 @@ SENTRY_TEST(gpu_info_vendor_id_known)
         case 0x5143:
             TEST_CHECK(strstr(vendor_name, "Qualcomm") != NULL);
             break;
+        case 0x13B5:
+            TEST_CHECK(strstr(vendor_name, "ARM") != NULL);
+            break;
+        case 0x144D:
+            TEST_CHECK(strstr(vendor_name, "Samsung") != NULL);
+            break;
         case 0x1AE0:
             TEST_CHECK(strstr(vendor_name, "Google") != NULL);
             break;
@@ -273,7 +280,7 @@ SENTRY_TEST(gpu_context_scope_integration)
 
         // Check for additional GPUs (gpu2, gpu3, etc.)
         for (int i = 2; i <= 4; i++) {
-            char context_key[16];
+            char context_key[32];
             snprintf(context_key, sizeof(context_key), "gpu%d", i);
             sentry_value_t additional_gpu
                 = sentry_value_get_by_key(contexts, context_key);