Skip to content

Commit da1c320

Browse files
[SYCL] fix for std::locale::global interfering with allowlist check (#18550)
`std::locale::global()` is a very common way for apps to initialize string localization for numeric and monetary values. Unfortunately, it is global. In this very limited fix, we are preventing it from interfering with the allowlist check which uses `std::stringstream`. The insertion of commas into numbers for the locale support messes with the expected values. We'll probably have to audit the other uses of std streams to see if they would be similarly vulnerable. --------- Signed-off-by: Chris Perkins <[email protected]>
1 parent 97fed15 commit da1c320

File tree

2 files changed

+36
-0
lines changed

2 files changed

+36
-0
lines changed

sycl/source/detail/allowlist.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,8 @@ void applyAllowList(std::vector<ur_device_handle_t> &UrDevices,
428428
uint32_t DeviceVendorIdUInt =
429429
DeviceImpl.get_info<info::device::vendor_id>();
430430
std::stringstream DeviceVendorIdHexStringStream;
431+
// To avoid commas or other locale-specific modifications, call imbue().
432+
DeviceVendorIdHexStringStream.imbue(std::locale::classic());
431433
DeviceVendorIdHexStringStream << "0x" << std::hex << DeviceVendorIdUInt;
432434
const auto &DeviceVendorIdValue = DeviceVendorIdHexStringStream.str();
433435
DeviceDesc[DeviceVendorIdKeyName] = DeviceVendorIdValue;

sycl/unittests/allowlist/DeviceIsAllowed.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,14 @@
77
//===----------------------------------------------------------------------===//
88

99
#include <detail/allowlist.hpp>
10+
#include <sycl/platform.hpp>
1011

1112
#include <gtest/gtest.h>
1213

14+
#ifdef _WIN32
15+
#include <windows.h> // SetEnvironmentVariable
16+
#endif
17+
1318
constexpr char SyclDeviceAllowList[] =
1419
"BackendName:opencl,DeviceType:gpu,DeviceVendorId:0x8086,DriverVersion:{{("
1520
"19\\.(4[3-9]|[5-9]\\d)\\..*)|([2-9][0-9]\\.\\d+\\..*)|(\\d+\\.\\d+\\."
@@ -85,6 +90,35 @@ TEST(DeviceIsAllowedTests, CheckSupportedOpenCLGPUDeviceIsAllowed) {
8590
EXPECT_EQ(Actual, true);
8691
}
8792

93+
TEST(DeviceIsAllowedTests, CheckLocalizationDoesNotImpact) {
94+
// The localization can affect std::stringstream output.
95+
// We want to make sure that DeviceVenderId doesn't have a comma
96+
// inserted (ie "0x8,086" ), which will break the platform retrieval.
97+
98+
try {
99+
auto previous = std::locale::global(std::locale("en_US.UTF-8"));
100+
#ifdef _WIN32
101+
SetEnvironmentVariableA("SYCL_DEVICE_ALLOWLIST", SyclDeviceAllowList);
102+
#else
103+
setenv("SYCL_DEVICE_ALLOWLIST", SyclDeviceAllowList, 1);
104+
#endif
105+
106+
auto post_platforms = sycl::platform::get_platforms();
107+
std::locale::global(previous);
108+
#ifdef _WIN32
109+
SetEnvironmentVariableA("SYCL_DEVICE_ALLOWLIST", nullptr);
110+
#else
111+
unsetenv("SYCL_DEVICE_ALLOWLIST");
112+
#endif
113+
114+
EXPECT_NE(size_t{0}, post_platforms.size());
115+
} catch (...) {
116+
// It is possible that the en_US locale is not available.
117+
// In this case, we just skip the test.
118+
GTEST_SKIP() << "Locale en_US.UTF-8 not available.";
119+
}
120+
}
121+
88122
TEST(DeviceIsAllowedTests, CheckSupportedOpenCLCPUDeviceIsAllowed) {
89123
bool Actual = sycl::detail::deviceIsAllowed(
90124
OpenCLCPUDeviceDesc, sycl::detail::parseAllowList(SyclDeviceAllowList));

0 commit comments

Comments
 (0)