clvk icon indicating copy to clipboard operation
clvk copied to clipboard
verified publisher icon kpet

fix lifetime of cvk_entry_point

Open rjodinchr opened this issue 1 year ago • 1 comments

cvk_entry_point are passed to other structures as a pointer. But it can be deleted at any point when do_build it called.

To avoid invalid access, make entry point a shared_ptr in the program map.

This has been detected running the compiler suite of the CTS on ChromeOS (flaky behaviour).

rjodinchr avatar Sep 09 '24 11:09 rjodinchr

Valgrind log leading to the fix:

==3664815== Invalid read of size 4
==3664815==    at 0x58F7580: pthread_mutex_lock@@GLIBC_2.2.5 (pthread_mutex_lock.c:80)
==3664815==    by 0x4CB13BE: __gthread_mutex_lock(pthread_mutex_t*) (lib/gcc/x86_64-linux-gnu/14/../../../../include/x86_64-linux-gnu/c++/14/bits/gthr-default.h:762)
==3664815==    by 0x4CDB1B4: std::mutex::lock() (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_mutex.h:113)
==3664815==    by 0x4CDB032: std::lock_guard<std::mutex>::lock_guard(std::mutex&) (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_mutex.h:250)
==3664815==    by 0x4CF6505: cvk_entry_point::free_descriptor_set(VkDescriptorSet_T*) (src/program.hpp:457)
==3664815==    by 0x4CF5B9D: cvk_kernel_argument_values::release_resources() (src/kernel.hpp:380)
==3664815==    by 0x4D98D8D: cvk_command_kernel::~cvk_command_kernel() (src/queue.hpp:852)
==3664815==    by 0x4D98DE8: cvk_command_kernel::~cvk_command_kernel() (src/queue.hpp:850)
==3664815==    by 0x4D98FCB: cvk_command_batch::~cvk_command_batch() (src/queue.hpp:903)
==3664815==    by 0x4D99028: cvk_command_batch::~cvk_command_batch() (src/queue.hpp:900)
==3664815==    by 0x4D8B42F: cvk_command_group::execute_cmds(bool) (src/queue.cpp:405)
==3664815==    by 0x4D8C46F: cvk_executor_thread::executor() (src/queue.cpp:522)
==3664815==  Address 0x148a4900 is 224 bytes inside a block of size 392 free'd
==3664815==    at 0x484394A: operator delete(void*, unsigned long) (vg_replace_malloc.c:1072)
==3664815==    by 0x4CEB294: std::default_delete<cvk_entry_point>::operator()(cvk_entry_point*) const (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:93)
==3664815==    by 0x4CEB1FE: std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> >::~unique_ptr() (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unique_ptr.h:398)
==3664815==    by 0x4CEB1AC: std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > >::~pair() (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_iterator.h:2993)
==3664815==    by 0x4CEB0FA: destroy<std::pair<const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > > > (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/new_allocator.h:198)
==3664815==    by 0x4CEB0FA: destroy<std::pair<const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > > > (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/alloc_traits.h:551)
==3664815==    by 0x4CEB0FA: std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > >, true> > >::_M_deallocate_node(std::__detail::_Hash_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > >, true>*) (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/hashtable_policy.h:2040)
==3664815==    by 0x4CEB050: std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > >, true> > >::_M_deallocate_nodes(std::__detail::_Hash_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > >, true>*) (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/hashtable_policy.h:2062)
==3664815==    by 0x4CEAF7B: std::_Hashtable<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > > >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::clear() (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/hashtable.h:2584)
==3664815==    by 0x4D6B004: std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<cvk_entry_point, std::default_delete<cvk_entry_point> > > > >::clear() (lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/unordered_map.h:799)
==3664815==    by 0x4D633E5: cvk_program::do_build() (src/program.cpp:1519)
==3664815==    by 0x4D63D8A: cvk_program::build(build_operation, unsigned int, _cl_device_id* const*, char const*, unsigned int, _cl_program* const*, char const**, void (*)(_cl_program*, void*), void*) (src/program.cpp:1693)
==3664815==    by 0x4C98474: clBuildProgram (src/api.cpp:2354)
==3664815==    by 0x14B42E: test_options_build_macro_existence(_cl_device_id*, _cl_context*, _cl_command_queue*, int) (external/OpenCL-CTS/test_conformance/compiler/test_build_options.cpp:214)
==3664815==  Block was alloc'd at
==3664815==    at 0x4840EE1: operator new(unsigned long) (vg_replace_malloc.c:472)
==3664815==    by 0x4D6448C: cvk_program::get_entry_point(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, int*) (src/program.cpp:1747)
==3664815==    by 0x4D3FA70: cvk_kernel::init() (src/kernel.cpp:37)
==3664815==    by 0x4C9B0C4: cvk_create_kernel(_cl_program*, char const*, int*) (src/api.cpp:2767)
==3664815==    by 0x4C9B2E5: clCreateKernel (src/api.cpp:2799)
==3664815==    by 0x14A65E: get_result_from_program(_cl_context*, _cl_command_queue*, _cl_program*, int*) (external/OpenCL-CTS/test_conformance/compiler/test_build_options.cpp:69)
==3664815==    by 0x14B361: test_options_build_macro_existence(_cl_device_id*, _cl_context*, _cl_command_queue*, int) (external/OpenCL-CTS/test_conformance/compiler/test_build_options.cpp:204)
==3664815==    by 0x18901B: callSingleTestFunction(test_definition, _cl_device_id*, test_harness_config const&) (external/OpenCL-CTS/test_common/harness/testHarness.cpp:951)
==3664815==    by 0x18883C: callTestFunctions(test_definition*, unsigned char*, test_status*, int, _cl_device_id*, test_harness_config const&) (external/OpenCL-CTS/test_common/harness/testHarness.cpp:830)
==3664815==    by 0x1884BC: parseAndCallCommandLineTests(int, char const**, _cl_device_id*, int, test_definition*, test_harness_config const&) (external/OpenCL-CTS/test_common/harness/testHarness.cpp:742)
==3664815==    by 0x187AF1: runTestHarnessWithCheck(int, char const**, int, test_definition*, int, unsigned long, test_status (*)(_cl_device_id*)) (external/OpenCL-CTS/test_common/harness/testHarness.cpp:605)
==3664815==    by 0x1868F2: runTestHarness(int, char const**, int, test_definition*, int, unsigned long) (external/OpenCL-CTS/test_common/harness/testHarness.cpp:117)

rjodinchr avatar Sep 09 '24 11:09 rjodinchr

Created the following issues for gaps in the CTS identified during the review of this PR:

  • https://github.com/KhronosGroup/OpenCL-CTS/issues/2163
  • https://github.com/KhronosGroup/OpenCL-CTS/issues/2164

kpet avatar Nov 26 '24 19:11 kpet