take loader_lock when adding unknown functions

[aizu-m]

ThreadSanitizer, test_threading built with -D LOADER_ENABLE_THREAD_SANITIZER=ON, threads querying unknown device extension entry points while others create and destroy devices:

WARNING: ThreadSanitizer: data race
  Write of size 8 by thread T2 (mutexes: write M0):
    #0 loader_add_logical_device loader.c:1736
    #1 terminator_CreateDevice loader.c:6488
    #4 vkCreateDevice trampoline.c:1038

  Previous read of size 8 by thread T1:
    #0 loader_init_dispatch_dev_ext_entry unknown_function_handling.c:91
    #1 loader_dev_ext_gpa_impl unknown_function_handling.c:207
    #2 loader_dev_ext_gpa_tramp unknown_function_handling.c:214
    #4 vkGetInstanceProcAddr trampoline.c:109

M0 is loader_lock and the address is icd_term->logical_device_list. The writer holds the lock, the reader holds nothing. Once vkDestroyDevice has unlinked and freed the loader_device the walk is on, that walk reads freed memory and writes ldev->loader_dispatch.ext_dispatch[idx] into it.

Same shape on the name tables, with neither side holding anything:

WARNING: ThreadSanitizer: data race
  Read of size 8 by thread T2:
    #0 loader_phys_dev_ext_gpa_impl unknown_function_handling.c:290
  Previous write of size 8 by thread T7:
    #0 loader_phys_dev_ext_gpa_impl unknown_function_handling.c:311

Found reading through the unknown function handling after the DestroySurfaceKHR locking change. vkGetInstanceProcAddr takes no lock, and trampoline_get_proc_addr falls through to these handlers once every name table has missed, so a driver reporting an entry point through vk_icdGetInstanceProcAddr or vk_icdGetPhysicalDeviceProcAddr puts the registration on an unlocked path. Registration appends to inst->{dev,phys_dev}_ext_disp_functions, walks inst->icd_terms and each driver's logical device list, and writes icd_term->phys_dev_ext[] and inst->disp->phys_dev_ext[]. Every mutator of that state holds loader_lock: vkCreateDevice/vkDestroyDevice around loader_add_logical_device/loader_remove_logical_device, vkEnumeratePhysicalDevices around unload_drivers_without_physical_devices, instance create and destroy around icd_terms and the free of the tables. None of those require external synchronisation of the VkInstance, and vkGetInstanceProcAddr has no external sync requirement either, so this is a legal call pattern.

So take loader_lock in the four entry points. It is recursive on both platforms, so the instance chain terminators that reach the _term variants while already holding it are unaffected. It only lands on the registration path, which already calls into every driver and down the layer chain per invocation, not on the name lookup the recent vkGetInstanceProcAddr work sped up.

Two tests added. Both abort under ThreadSanitizer before the change; linux-threading is green after it.

[ci-tester-lunarg]

Author aizu-m not on autobuild list. Waiting for curator authorization before starting CI build.

[ci-tester-lunarg]

Author aizu-m not on autobuild list. Waiting for curator authorization before starting CI build.

[ci-tester-lunarg]

Author aizu-m not on autobuild list. Waiting for curator authorization before starting CI build.

[charles-lunarg]

The failing CI jobs were the no-assembly jobs, which need manual disabling of unknown function tests. I went ahead and pushed a commit that adds the correct disables.

[ci-tester-lunarg]

CI Vulkan-Loader build queued with queue ID 141658.

[ci-tester-lunarg]

CI Vulkan-Loader build # 3799 running.

[ci-tester-lunarg]

Author aizu-m not on autobuild list. Waiting for curator authorization before starting CI build.

[ci-tester-lunarg]

Author aizu-m not on autobuild list. Waiting for curator authorization before starting CI build.