Skip to content

gn: Disable function UBsan check to support new vulkan.hpp function pointers#1664

Merged
charles-lunarg merged 1 commit intoKhronosGroup:mainfrom
gnoliyil:ubsan
Mar 4, 2025
Merged

gn: Disable function UBsan check to support new vulkan.hpp function pointers#1664
charles-lunarg merged 1 commit intoKhronosGroup:mainfrom
gnoliyil:ubsan

Conversation

@gnoliyil
Copy link
Contributor

@gnoliyil gnoliyil commented Feb 24, 2025

KhronosGroup/Vulkan-Hpp#2020 added vk::PFN_... function pointer types as the preferred function pointers used in vulkan.hpp structs and functions.

These function pointer types use C++-styled types for the function arguments and return values, so the compiler treats them as types different from the C-styled "PFN_...` function pointers types. Vulkan-Hpp guarantees that they are binary identical during compile time, but UBsan's function sanitizer trigger a "function pointer type different" runtime error when these function pointers are invoked in the Vulkan-Loader (for example, debug_utils.c and allocation.c) -- see KhronosGroup/Vulkan-Hpp#2082 for example.

This change disables the function sanitizer from the Vulkan-Loader (where the function pointers are invoked) so that Vulkan applications created using Vulkan-Hpp can run correctly when UBsan is enabled.

Test: Vulkan example vkcontext with change https://fuchsia-review.googlesource.com/c/fuchsia/+/1208145 didn't crash on Fuchsia core.x64-asan build.

Bug: https://fxbug.dev/378964821

@ci-tester-lunarg
Copy link

Author gnoliyil not on autobuild list. Waiting for curator authorization before starting CI build.

1 similar comment
@ci-tester-lunarg
Copy link

Author gnoliyil not on autobuild list. Waiting for curator authorization before starting CI build.

KhronosGroup/Vulkan-Hpp#2020 added `vk::PFN_...` function pointer
types as the preferred function pointers used in vulkan.hpp structs
and functions.

These function pointer types use C++-styled types for the function
arguments and return values, so the compiler treats them as types
different from the C-styled "PFN_...` function pointers types.
Vulkan-Hpp guarantees that they are binary identical during compile
time, but UBsan's function sanitizer trigger a "function pointer
type different" runtime error when these function pointers are invoked
in the Vulkan-Loader (for example, debug_utils.c and allocation.c).

Thus, we need to disable the function sanitizer from the Vulkan-Loader
so that Vulkan applications created using Vulkan-Hpp can run correctly
when UBsan is enabled.

Test: Vulkan examples (https://fuchsia.googlesource.com/fuchsia/+/
main/src/graphics/tests/common/test_vkcontext.cc) didn't crash on
Fuchsia core.x64-asan build.

Bug: https://fxbug.dev/378964821

Change-Id: I8406daa884e741dbc8ade8c0e402550c450858e0
@ci-tester-lunarg
Copy link

Author gnoliyil not on autobuild list. Waiting for curator authorization before starting CI build.

1 similar comment
@ci-tester-lunarg
Copy link

Author gnoliyil not on autobuild list. Waiting for curator authorization before starting CI build.

Copy link
Collaborator

@charles-lunarg charles-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be applied to any projects using Vulkan-Hpp and UBSAN rather than having to apply it to the Vulkan-Loader?

@jpr42
Copy link
Contributor

jpr42 commented Mar 4, 2025

Shouldn't this be applied to any projects using Vulkan-Hpp and UBSAN rather than having to apply it to the Vulkan-Loader?

Seems similar to KhronosGroup/Vulkan-Headers#379

Copy link
Collaborator

@charles-lunarg charles-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to approve and merge - no sense holding off since its needed to work around sanitizer issues that can't really be 'fixed' by any single repo.

@charles-lunarg charles-lunarg merged commit c0ccad9 into KhronosGroup:main Mar 4, 2025
44 checks passed
@jpr42
Copy link
Contributor

jpr42 commented Mar 4, 2025

Going to approve and merge - no sense holding off since its needed to work around sanitizer issues that can't really be 'fixed' by any single repo.

Couldn't this be fixed by making the change in Vulkan-Headers?

@charles-lunarg
Copy link
Collaborator

Okay, because users are passing in a debug utils callback into the loader, the loader needs to disable this check. The loader calls whatever function has been supplied, which in the error case comes from Vulkan-Hpp.

Maybe there is a way to fix it without modifying the loader, but for the time being it's better to have undefined sanitizer working than it given many many false positives. Since it is the loader calling the Vulkan-Hpp function, the loader may need to apply the __attribute__((no_sanitize("function"))) attribute when compiling with clang. Would be a pretty simple test to setup and see if it reproduces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments