Skip to content

Commit b69f889

Browse files
committed
Fix UB related to get_spec_constant_symbolic_ID
Added a wrapper, which is defined in integration footer only after we define all specializations for `get_spec_constant_symbolic_ID`. By using it we ensure that we do not try to instantiate `get_spec_constant_symbolic_ID` until after we made all required specializations.
1 parent 7050d43 commit b69f889

File tree

8 files changed

+25
-14
lines changed

8 files changed

+25
-14
lines changed

clang/lib/Sema/SemaSYCL.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4986,7 +4986,6 @@ bool SYCLIntegrationFooter::emit(raw_ostream &OS) {
49864986
Policy.SuppressUnwrittenScope = true;
49874987

49884988
OS << "#include <CL/sycl/detail/defines_elementary.hpp>\n";
4989-
OS << "#include <CL/sycl/detail/spec_const_integration.hpp>\n";
49904989

49914990
llvm::SmallSet<const VarDecl *, 8> VisitedSpecConstants;
49924991

@@ -5029,6 +5028,9 @@ bool SYCLIntegrationFooter::emit(raw_ostream &OS) {
50295028
OS << "} // namespace sycl\n";
50305029
OS << "} // __SYCL_INLINE_NAMESPACE(cl)\n";
50315030
}
5031+
5032+
OS << "#include <CL/sycl/detail/spec_const_integration.hpp>\n";
5033+
50325034
return true;
50335035
}
50345036

clang/test/CodeGenSYCL/anonymous_integration_footer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ int main() {
1010
}
1111

1212
// CHECK: #include <CL/sycl/detail/defines_elementary.hpp>
13-
// CHECK-NEXT: #include <CL/sycl/detail/spec_const_integration.hpp>
1413

1514
using namespace cl;
1615

@@ -402,3 +401,5 @@ constexpr sycl::specialization_id same_name{17};
402401
// CHECK-NEXT: } // __SYCL_INLINE_NAMESPACE(cl)
403402
}
404403
} // namespace outer
404+
405+
// CHECK: #include <CL/sycl/detail/spec_const_integration.hpp>

clang/test/CodeGenSYCL/anonymous_integration_footer2.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ int main() {
1010
}
1111

1212
// CHECK: #include <CL/sycl/detail/defines_elementary.hpp>
13-
// CHECK-NEXT: #include <CL/sycl/detail/spec_const_integration.hpp>
1413

1514
using namespace cl;
1615

@@ -155,3 +154,5 @@ constexpr sycl::specialization_id same_name{209};
155154
// CHECK-NEXT: } // __SYCL_INLINE_NAMESPACE(cl)
156155
}
157156
}
157+
158+
// CHECK: #include <CL/sycl/detail/spec_const_integration.hpp>

clang/test/CodeGenSYCL/integration_footer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ int main() {
88
}
99

1010
// CHECK: #include <CL/sycl/detail/defines_elementary.hpp>
11-
// CHECK-NEXT: #include <CL/sycl/detail/spec_const_integration.hpp>
1211

1312
using namespace cl::sycl;
1413

@@ -190,3 +189,5 @@ auto x = HasVarTemplate::VarTempl<int, 2>.getDefaultValue();
190189
// CHECK-NEXT: } // namespace detail
191190
// CHECK-NEXT: } // namespace sycl
192191
// CHECK-NEXT: } // __SYCL_INLINE_NAMESPACE(cl)
192+
193+
// CHECK: #include <CL/sycl/detail/spec_const_integration.hpp>

sycl/include/CL/sycl/detail/kernel_desc.hpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,17 @@ template <class Name> struct SpecConstantInfo {
5050
static constexpr const char *getName() { return ""; }
5151
};
5252

53+
// This guard is needed because the libsycl.so can be compiled with C++ <=14
54+
// while the code requires C++17. This code is not supposed to be used by the
55+
// libsycl.so so it should not be a problem.
5356
#if __cplusplus >= 201703L
5457
// Translates SYCL 2020 specialization constant type to its name.
5558
// Definition is in spec_const_integration.hpp
5659
template <auto &SpecName> const char *get_spec_constant_symbolic_ID();
60+
// Wrapper is needed to delay instantiation of 'get_spec_constant_symbolic_ID'
61+
// until after we have encountered all specializations for it generated by the
62+
// compiler in integration footer.
63+
template <auto &SpecName> const char *get_spec_constant_symbolic_ID_wrapper();
5764
#endif
5865

5966
#ifndef __SYCL_UNNAMED_LAMBDA__

sycl/include/CL/sycl/detail/spec_const_integration.hpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,8 @@ namespace detail {
1717

1818
#if __cplusplus >= 201703L
1919
// Translates SYCL 2020 specialization constant type to its name.
20-
template <auto &SpecName> const char *get_spec_constant_symbolic_ID() {
21-
#ifdef SYCL_LANGUAGE_VERSION
22-
return __builtin_sycl_unique_stable_id(SpecName);
23-
#else
24-
return "";
25-
#endif
20+
template <auto &SpecName> const char *get_spec_constant_symbolic_ID_wrapper() {
21+
return get_spec_constant_symbolic_ID<SpecName>();
2622
}
2723
#endif
2824

sycl/include/CL/sycl/kernel_bundle.hpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,8 @@ class kernel_bundle : public detail::kernel_bundle_plain {
263263
/// \returns true if any device image in the kernel_bundle uses specialization
264264
/// constant whose address is SpecName
265265
template <auto &SpecName> bool has_specialization_constant() const noexcept {
266-
const char *SpecSymName = detail::get_spec_constant_symbolic_ID<SpecName>();
266+
const char *SpecSymName =
267+
detail::get_spec_constant_symbolic_ID_wrapper<SpecName>();
267268
return has_specialization_constant_impl(SpecSymName);
268269
}
269270

@@ -274,7 +275,8 @@ class kernel_bundle : public detail::kernel_bundle_plain {
274275
typename = detail::enable_if_t<_State == bundle_state::input>>
275276
void set_specialization_constant(
276277
typename std::remove_reference_t<decltype(SpecName)>::value_type Value) {
277-
const char *SpecSymName = detail::get_spec_constant_symbolic_ID<SpecName>();
278+
const char *SpecSymName =
279+
detail::get_spec_constant_symbolic_ID_wrapper<SpecName>();
278280
set_specialization_constant_impl(SpecSymName, &Value,
279281
sizeof(decltype(Value)));
280282
}
@@ -284,7 +286,8 @@ class kernel_bundle : public detail::kernel_bundle_plain {
284286
template <auto &SpecName>
285287
typename std::remove_reference_t<decltype(SpecName)>::value_type
286288
get_specialization_constant() const {
287-
const char *SpecSymName = detail::get_spec_constant_symbolic_ID<SpecName>();
289+
const char *SpecSymName =
290+
detail::get_spec_constant_symbolic_ID_wrapper<SpecName>();
288291
if (!is_specialization_constant_set(SpecSymName))
289292
return SpecName.getDefaultValue();
290293

sycl/unittests/spec_constants/DefaultValues.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ template <> struct KernelInfo<TestKernel> {
3333
static constexpr bool callsAnyThisFreeFunction() { return false; }
3434
};
3535

36-
template <> const char *get_spec_constant_symbolic_ID<SpecConst1>() {
36+
template <> const char *get_spec_constant_symbolic_ID_wrapper<SpecConst1>() {
3737
return "SC1";
3838
}
3939
} // namespace detail

0 commit comments

Comments
 (0)