Repository navigation
Conversation
The static API wrappers cache function pointers, but only set a local error status during the first cache initialization. If an API is absent from the selected shared loader, the first call returns unsupported and subsequent calls invoke the cached null pointer. Check the cached pointer on each invocation instead of capturing a per-call result in its initializer. Regenerate the core, Sysman, Tools, and runtime wrappers from the updated template. Signed-off-by: Artur Gainullin <artur.gainullin@intel.com>
Level Zero loader ABI checklast-release ( master ( If an ABI change is intended, a reviewer with write access can accept it: leave an approval review on the PR head commit with the keyword |
Contributor
|
The concept seems solid and I am not seeing additional cost in normal good calls, so this is acceptable.. Any chance you may be able to add a unit test for this into the ./test/ ctests? |
Contributor
|
Here is a suggested unit test for this fix. It nulls one entry in the static DDI table after Verified locally (static build):
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 6559267..9dcb3b8 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -1051,6 +1051,11 @@ else()
set_property(TEST init_driver_unit_tests PROPERTY ENVIRONMENT "ZE_ENABLE_LOADER_DEBUG_TRACE=1;ZE_ENABLE_NULL_DRIVER=1;")
endif()
+if(BUILD_STATIC)
+ add_test(NAME tests_static_missing_api_called_twice COMMAND tests --gtest_filter=*StaticLoaderMissingApi*GivenApiMissingFromSharedLoaderWhenCallingApiTwiceThenUnsupportedFeatureIsReturnedBothTimes*)
+ set_property(TEST tests_static_missing_api_called_twice PROPERTY ENVIRONMENT "ZE_ENABLE_LOADER_DEBUG_TRACE=1;ZE_ENABLE_NULL_DRIVER=1")
+endif()
+
# These tests are currently not supported on Windows. The reason is that the std::cerr is not being redirected to a pipe in Windows to be then checked against the expected output.
diff --git a/test/loader_api.cpp b/test/loader_api.cpp
index c04c41b..b51e232 100644
--- a/test/loader_api.cpp
+++ b/test/loader_api.cpp
@@ -4059,4 +4059,33 @@ TEST_F(DriverOrderingTest,
EXPECT_EQ(0, strcmp(errorDesc, "ERROR UNSUPPORTED FEATURE"));
}
+#ifdef L0_STATIC_LOADER_BUILD
+ // The static loader caches each API's function pointer on first call. When the
+ // shared loader does not provide an API, every call must keep returning
+ // ZE_RESULT_ERROR_UNSUPPORTED_FEATURE instead of calling the cached nullptr.
+ // Must run in its own process so zeContextSystemBarrier is not already cached.
+ TEST(
+ StaticLoaderMissingApi,
+ GivenApiMissingFromSharedLoaderWhenCallingApiTwiceThenUnsupportedFeatureIsReturnedBothTimes)
+ {
+ uint32_t pInitDriversCount = 0;
+ ze_init_driver_type_desc_t desc = {ZE_STRUCTURE_TYPE_INIT_DRIVER_TYPE_DESC};
+ desc.flags = UINT32_MAX;
+ desc.pNext = nullptr;
+ EXPECT_EQ(ZE_RESULT_SUCCESS, zeInitDrivers(&pInitDriversCount, nullptr, &desc));
+ EXPECT_GT(pInitDriversCount, 0);
+
+ // Emulate an older shared loader that does not export this API.
+ ze_dditable_t *ddiTable = ze_lib::context->zeDdiTable.load();
+ ASSERT_NE(nullptr, ddiTable);
+ auto savedPfn = ddiTable->Context.pfnSystemBarrier;
+ ddiTable->Context.pfnSystemBarrier = nullptr;
+
+ EXPECT_EQ(ZE_RESULT_ERROR_UNSUPPORTED_FEATURE, zeContextSystemBarrier(nullptr, nullptr));
+ EXPECT_EQ(ZE_RESULT_ERROR_UNSUPPORTED_FEATURE, zeContextSystemBarrier(nullptr, nullptr));
+
+ ddiTable->Context.pfnSystemBarrier = savedPfn;
+ }
+#endif // L0_STATIC_LOADER_BUILD
+
} // namespace |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The static API wrappers cache function pointers, but only set a local error status during the first cache initialization. If an API is absent from the selected shared loader, the first call returns unsupported and subsequent calls invoke the cached null pointer causing a crash.
Check the cached pointer on each invocation instead of capturing a per-call result in its initializer. Regenerate the core, Sysman, Tools, and runtime wrappers from the updated template.