Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix lifetime of cvk_entry_point #721

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rjodinchr
Copy link
Contributor

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).

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
Copy link
Contributor Author

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)

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.

1 participant