Skip to content

Conversation

dklochkov-emb
Copy link
Contributor

@dklochkov-emb dklochkov-emb commented Sep 22, 2025

This patch fixes a downstream tests crash. It was not reproduced in e2e tests.

@dklochkov-emb dklochkov-emb self-assigned this Sep 22, 2025
@dklochkov-emb dklochkov-emb requested a review from a team as a code owner September 22, 2025 17:40
@aelovikov-intel aelovikov-intel changed the title [SYCL] store string copy instead of view [SYCL] Store string copy instead of view Sep 22, 2025
@aelovikov-intel
Copy link
Contributor

aelovikov-intel commented Sep 22, 2025

Store string copy instead of view

Where? Why? What prevents us from changing some other part of the project to extend string lifetime there so that we could keep string_view here?

This patch fixes a downstream tests crash.

What is that? Is that part of this project? Do you have a link to CI logs?

@dklochkov-emb
Copy link
Contributor Author

dklochkov-emb commented Sep 23, 2025

Where? Why? What prevents us from changing some other part of the project to extend string lifetime there so that we could keep string_view here?

It was reported as a crash in downstream tests, please see CMPLRLLVM-70295 for the details. The main problem is invalid pointers to C arrays when some tests run. These arrays are parts of integration header, implemented here.
I failed to reproduce it in sycl e2e test but crash is stable on perflib tests side. Backtrace shows crash when inserting new members into std::unordered_map<std::string_view, unsigned> which will be used father in free function kernels.

What is that? Is that part of this project? Do you have a link to CI logs?

See the aforementioned tracker. Crash happens after free function kernel changes to support num_args functionality when integration header and program manager were changed.

@dklochkov-emb
Copy link
Contributor Author

@aelovikov-intel Please, ping me if you need more info.

@aelovikov-intel
Copy link
Contributor

What prevents us from changing some other part of the project to extend string lifetime there so that we could keep string_view here?

@dklochkov-emb
Copy link
Contributor Author

What prevents us from changing some other part of the project to extend string lifetime there so that we could keep string_view here?

@aelovikov-intel In general, this string is stored in integration header and its view was added to program manager cache. However, downstream tests reproduced scenario when the pointer to string becomes invalid. Originally, header contains string as C-array.

@aelovikov-intel
Copy link
Contributor

this string is stored in integration header

Integration header strings are compile-time constants, how does it end its lifetime? Something in your story isn't right.

@dklochkov-emb
Copy link
Contributor Author

dklochkov-emb commented Oct 1, 2025

this string is stored in integration header

Integration header strings are compile-time constants, how does it end its lifetime? Something in your story isn't right.

I do see this but backtrace shows it:

#0 __memcmp_evex_movbe () at ../sysdeps/x86_64/multiarch/memcmp-evex-movbe.S:123
#1 0x00007f1db5fd630c in std::pair<std::__detail::_Node_iterator<std::pair<std::basic_string_view<char, std::char_traits > const, unsigned int>, false, true>, bool> std::_Hashtable<std::basic_string_view<char, std::char_traits >, std::pair<std::basic_string_view<char, std::char_traits > const, unsigned int>, std::allocator<std::pair<std::basic_string_view<char, std::char_traits > const, unsigned int> >, std::__detail::_Select1st, std::equal_to<std::basic_string_view<char, std::char_traits > >, std::hash<std::basic_string_view<char, std::char_traits > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_emplace<std::pair<std::basic_string_view<char, std::char_traits > const, unsigned int>&>(std::integral_constant<bool, true>, std::pair<std::basic_string_view<char, std::char_traits > const, unsigned int>&) () from compiler/latest/lib/libsycl.so.8
#2 0x00007f1db5fac8f5 in sycl::_V1::detail::ProgramManager::registerKernelGlobalInfo(std::unordered_map<std::basic_string_view<char, std::char_traits >, unsigned int, std::hash<std::basic_string_view<char, std::char_traits > >, std::equal_to<std::basic_string_view<char, std::char_traits > >, std::allocator<std::pair<std::basic_string_view<char, std::char_traits > const, unsigned int> > >&&) ()
from compiler/latest/lib/libsycl.so.8

Also, it is the reason why no one test reproduced issue

@aelovikov-intel
Copy link
Contributor

I don't see how the stacktrace answers my question. It might contain crucial information, but it's too low level and I do expect you to summarize it and present the answer on a higher abstraction level (the one I used in the question).

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.

2 participants